Skip to content
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

Ergonomics: Automatic signal argument for switchMap() #134

Open
domfarolino opened this issue Apr 3, 2024 · 1 comment
Open

Ergonomics: Automatic signal argument for switchMap() #134

domfarolino opened this issue Apr 3, 2024 · 1 comment
Labels
possible future enhancement An enhancement that doesn't block standardization or shipping

Comments

@domfarolino
Copy link
Collaborator

The issue #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 #126.

However, there was some discussion (#52 (comment), tc39/proposal-iterator-helpers#162 (comment), and #52 (comment)) 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 #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.

@domfarolino domfarolino added the possible future enhancement An enhancement that doesn't block standardization or shipping label Apr 3, 2024
@Jamesernator
Copy link
Contributor

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();
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
possible future enhancement An enhancement that doesn't block standardization or shipping
Projects
None yet
Development

No branches or pull requests

2 participants