add ChannelStateChange.hasBacklog

[owenpearson]

This flag is already send as part of the attached protocol message, but should be exposed on ChannelStateChange objects so that users can tell whether to expect rewind messages.

[sync-by-unito]

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3630

[JCB-K]

Hi! Thanks for this issue - this will be very useful for us. I have some thoughts about the proposed API though that I'd like to discuss. I got sent this snippet as an example of the API:

const attached = await channel.subscribe(handler)
if (attached.hasBacklog) {
  // wait for the handler to be called with the rewind message
} else {
  // do whatever they want to do when there is no rewind message
}

With the API above, I’d have to do something like this to implement the behaviour I want:

const subscribe = async (handler) => {
  return new Promise<void>(async (resolve) => {
    const attached = await channel.subscribe((...args) => {
      handler(...args);
      resolve();
    });
    if (attached.hasBacklog) {
      // wait for onSubscribe to be called in the handler
    } else {
      resolve();
    }
  });
};

await subscribe(handler)
allRewindMessagesAppliedCallback();

And when wrapping subscribe in a React hook (as we do), something like this:

const useSubscribe = (handler, allRewindMessagesAppliedCallback) => {
  React.useEffect(() => {
    channel.subscribe((msg) => {
      handler();
      allRewindMessagesAppliedCallback(); // todo: make sure its only called once
    }).then(() => {
      if(!attached.hasBacklog){
        allRewindMessagesAppliedCallback();
      }
    })
    return () => channel.unsubscribe(handler);
  }, [allRewindMessagesAppliedCallback, handler])
}

useSubscribe(handler, allRewindMessagesAppliedCallback)

This feels fairly clunky 🙂 . I have a small suggestion, and that’s to expose the value as a promise instead. That way we can simply await it like this:

const subscribe = async (handler) => {
    const attached = await channel.subscribe(handler);
    return attached.hasBacklog
};

await subscribe()
allRewindMessagesAppliedCallback();

The naming gets a bit strange now, so maybe it would make more sense to keep the hasBacklog flag as is and expose this promise under a different name.

Thoughts?

[owenpearson]

Hey @JCB-K, I've proposed a change which should make this possible (#1347). Unfortunately I don't think the hasBacklog promise from your example would be possible at the moment - the reason being that ChannelParams.rewind may be set to any number of messages (or even a time interval) but the client only knows whether there is a backlog of messages at all (there's currently no way to tell how many messages to expect).rewind=1 is a unique case because there's no possibility of a partial backlog, but since there's no way to extend this to other rewind values I don't think we can add this feature to the lib without a change to the wire protocol.

I hope that makes sense, do let us know if there's anything you'd want to change in the implementation 🙂

[JCB-K]

@owenpearson makes sense, in that case I'd love for it to be part of the write protocol but in the meantime we can implement this in user land. thanks for this change!

[owenpearson]

@JCB-K that's good to know, I'll discuss it internally whether it's feasible to add that to the protocol 👍 for now, we've released version 1.2.41 which includes the ChannelStateChange.hasBacklog flag and makes it so that the channel state change is returned from calling attach/subscribe