WICG / observable

Observable API proposal
https://wicg.github.io/observable/
Other
543 stars 12 forks source link

Ergonomics: Automatic signal argument for `switchMap()` #134

Open domfarolino opened 3 months ago

domfarolino commented 3 months ago

The issue https://github.com/WICG/observable/issues/52 was originally filed to track the adding of the switchMap() operator. We ultimately decided that it was worth adding this operator both in that issue, and in https://github.com/WICG/observable/issues/126.

However, there was some discussion (https://github.com/WICG/observable/issues/52#issuecomment-1863411008, https://github.com/tc39/proposal-iterator-helpers/issues/162#issuecomment-968929171, and https://github.com/WICG/observable/issues/52#issuecomment-1863575867) about making it easier/ergonomic for code inside of the operator's Mapper callback to cancel work in response to the "inner" Observable being unsubscribed/canceled (as a result of the source Observable pushing a newer value).

Today, as switchMap() currently is proposed https://github.com/WICG/observable/pull/130, in order to get access to the signal associated with the "inner" Observable, you would have to explicitly return an Observable, and utilize subscriber.signal:

input.on('input')
  .switchMap(e => new Observable(async subscriber => {
      try {
        const response = await fetch(`/search?q={e.target.value}`, {signal: subscriber.signal});
        subscriber.next(response);
      } catch (e) {…}
  }))

It has been proposed, however, that switchMap() could have a signal argument that the operator internally provides so that you can continue to return whatever you want (as long as it's convertible to an Observable) from Mapper, and use the auto-provided signal as a means for cancellation. This would look something like:

input.on('input')
  .switchMap(async (e, idx, signal) => {
    try {
      const response = await fetch(`/search?q={e.target.value}`, {signal});
      return response;
    } catch (e) {…}
  })

The difference is being able to (1) use an async function directly and return something like a Promise from Mapper (which gets automatically converted to an Observable via from() semantics) vs. (2) have to return an Observable explicitly in order to get access to the subscriber's signal.

I am not sure the difference in these two is significant enough to justify a new argument and whatever memory overhead that would incur — basically the internal operator would have to create that new signal to unconditionally pass into Mapper and it would just be redundant with the subscriber.signal that the operator creates for the "inner" Observable that the Mapper value gets coerced to anyways. It could be a nice addition though, so I'm filing this bug to track the more targeted discussion about the possible future addition of this Mapper argument, for ergonomics.

Jamesernator commented 2 months ago

It has been proposed, however, that switchMap() could have a signal argument that the operator internally provides so that you can continue to return whatever you want (as long as it's convertible to an Observable) from Mapper, and use the auto-provided signal as a means for cancellation

.flatMap should really do this this as well (and anywhere else where observable converts promises to observables).

e.g.:

urlSource.flatMap(async (url, idx, signal) => {
    const res = await fetch(url, { signal });
    return await res.text();
});