WICG / observable

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

How to removeEventListener? #75

Closed esprehn closed 6 months ago

esprehn commented 1 year ago

The word removeEventListener is never mentioned on the explainer or in the examples.

domfarolino commented 1 year ago

I guess it's not super explicitly enunciated, but the idea is that the teardown for a natively-constructed Observable returned via EventTarget#on() would remove the event listener. So if you passed an AbortSignal to subscribe(), when called one of of these EventTarget-observables, then when you abort the signal, it would teardown the Observable and remove the event listener, just like addEventListener() does today.

esprehn commented 1 year ago

Does that mean if I don't pass an AbortSignal that there's no way to remove an event listener? That seems like a critical flaw compared to addEventListener where as long as you have the function you can always remove it.

for example if I do:

// imagine this is actually inside a component we instantiate a thousand times, not literally a loop.
for (let i = 0; i < 1000; ++i) {
  document.on('mousedown')
     .takeUntil(document.on('mouseup'))
     .subscribe({next: e => ...});
}

after I click how many event listeners remain on document?

domfarolino commented 1 year ago

Does that mean if I don't pass an AbortSignal that there's no way to remove an event listener?

No, so the real story is that when an observable subscription "closes" in any way (there are three ways: via AbortSignal, or the observable itself firing error() or complete() handlers), then the platform-internal "teardown" will be to remove the listener.

So with a normal subscription like element.on('click').subscribe({/*whoops, I forgot to pass in AbortSignal*/}) this observable will never "close" the subscription itself (nothing in C++ land will be prompted to call Subscriber#complete()), since you subscribed to an observable that was not defined to terminate (and you forgot to pass in your consumer-initiated termination mechanism).

But in your example, you're subscribing specifically to an observable that's designed to terminate, where the termination condition is defined by the operator you called. So when you call subscribe() above, internally here's what would happen:

  1. Install an event listener on document for mousedown, forwarding events outward to your next()
    • Set up a teardown to remove the event listener once the subscription closes
    • (So far, nothing here is specific to this operator in particular)
  2. Subscribe to the input document.on('mouseup') observable
    • Set up a teardown to cancel our "inner" subscription to this observable once the "outer" subscription closes
  3. Run the logic that's specific to the takeUntil() operator: This forwards mousedown events along, but calls complete() (thus closing the "outer" subscription to the observable returned by takeUntil()) when mouseup fires. As a part of closing that "outer" subscription, we run the two teardowns above.

after I click how many event listeners remain on document?

Given the above explanation, I believe the answer should be 0.

esprehn commented 1 year ago

I see, thanks for the detailed explanation that's really helpful!

In rxjs subscribe returns a Subscription that has an unsubscribe callback, but that's been removed from this API. What was the reason for that? With addEventListener you can't lose the ability to unsubscribe as long as you keep the function reference around (which most folks do). It seems really easy with the on() API to subscribe deep down in a library and lose the ability to unsubscribe.

Jamesernator commented 12 months ago

In rxjs subscribe returns a Subscription that has an unsubscribe callback, but that's been removed from this API. What was the reason for that?

This has been discussed in these issues:

The TLDR is that abort signals both are the existing mechanism for cancellation in browsers, and various problems like the synchronous firehose are directly resolvable by having the cancellation mechanism being available before the subscription is available.

It seems really easy with the on() API to subscribe deep down in a library and lose the ability to unsubscribe.

If a library didn't store the Subscription in a reference you'd lose the ability to unsubscribe anyway. There is no difference in any of the approaches in this regard, to be able to unsubscribe you need a reference to one of:

benlesh commented 10 months ago

When you're registering resources (like callbacks or subscriptions) with something, and you need to provide a means of removing the resource, there are a few patterns to follow:

  1. Return a means of removing the resource, (RxJS does this with subscribe(): Subscription, other APIs might return a removal function like subscribe(cb: Callback): UnsubscribeFunction, etc.
  2. Provide a separate unregistration function. This is what addEventListener and removeEventListener are. Or think of like the Observer pattern's Subject: addObserver/removeObserver, etc.
  3. Allow the consumer to pass in a token-based cancellation mechanism. This is AbortSignal, or in something like .NET, CancellationToken, etc.

Every single one of the above is just a different way to do the same thing. They all have advantages and disadvantages.

Using method 3 above, with AbortSignal has the most advantages for Observable because it allows the cancellation mechanism to be created before the subscription starts, when the subscription could emit values synchronously (as is required of anything that is going to model EventTarget). It's also nice because there's no need to keep the EventTarget instance itself on-hand to unregister the listener, like you would if you had to use removeEventListener. So to this point:

compared to addEventListener where as long as you have the function you can always remove it

...Sort of: You not only have to have a handle to your function, but you must also have a handle to the actual target as well, AND you have to know the "magic string" it was registered under. So you need 3 components: target.removeEventListener('type', func). With an AbortSignal, you only need to have a reference to the AbortController to unregister the listener.

domfarolino commented 6 months ago

I think the concerns originally raised in this issue have been suitably resolved with the design of the Observable API (see comments https://github.com/WICG/observable/issues/75#issuecomment-1743224438 and https://github.com/WICG/observable/issues/75#issuecomment-1832698540), and I think all questions have been answered up to this point. So I'm going to close this issue, but feel free to re-open to keep the discussion going if you feel the need to!