-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SDK-3630] feat: add ChannelStateChange.hasBacklog
and return state change to attach promise/callback
#1347
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No objection to the idea, seems like a reasonable feature.
There's nothing ably-js specific about this; if we think it's worth doing we should have a spec PR for this so it can eventually be done in all sdks (and added to api docs).
which resolves once a channel is attached and, if applicable, the rewind message has been received.
not sure I understand what the last part of that sentence means -- it resolves once the channel is attached full stop, no?
@SimonWoolf Yeah we did consider this, my thinking was that this is unlikely to be needed in other SDKs so adding it to the spec would likely be futile. I'm kinda tempted to add it regardless though, will ask what the team thinks.
I'm not talking about any of the promises returned by the ably-js API here - the feature request was to make it possible to wait for a channel with |
👍
Shrug, I think there's still some value in formalising the API in the spec even if we're not imminently intending to implement in other sdks. there's other bits of the spec that are not-mandatory ("Libraries may offer a ClientOptions#agents property...", "Client libraries can optionally save a round-trip request to the Ably service for expired tokens by detecting when a token has expired...", etc)
yeah, really for that feature you'd need realtime to tell you when the backlog is finished, and tbh I'm not sure that that'd be a good idea. (also most sensible ways of returning another promise would involve a breaking api change, and would make it a very prominent part of the api, and istm it's not worth either of those) |
I wasn't aware of the rewind feature, popping a link to the documentation here for future reference. I think that exposing the As for the change to the signature of |
Makes it so that `RealtimeChannel.attach` exposes the `ChannelStateChange` via whatever async api (invoke callback or fulfill promise). This makes it easy for users to access flags on the `ChannelStateChange` to access information about the attachment.
Exposes the `hasBacklog` flag on `ChannelStateChange`. This may be used in combination with rewind to check whether to expect a backlog of messages upon attachment.
ChannelStateChange.hasBacklog
and return state change to attach promise/callbackChannelStateChange.hasBacklog
and return state change to attach promise/callback
Resolves #1328
See corresponding spec PR
These features in combination will allow users to check whether to expect a backlog of messages once attached to a realtime channel. In particular, when using
rewind=1
, this will allow users to create a promise which resolves once a channel is attached and, if applicable, the rewind message has been received.