Closed mk-pmb closed 3 months ago
The naming of signal
is mandated for Web APIs by the AbortSignal spec.
Well, sort of. It is mandatory for AbortSignal
-consuming APIs to consume the signal in a dictionary, with the dictionary member name signal
. But I don't think it is mandatory to expose it on an object like Subscriber
with the interface member name signal
.
A property should only be named "signal" in contexts where there is really only one thing imaginable that could be signaled
Still, I am unconvinced by this. The signal is there for the consumer (of the Observable) to express that they are no longer interested in receiving the values that the Observable is producing. Whatever their reason, the Observable-side result of "shutting down" will be the same. Whatever their reason, this is meant to be communicated through AbortController#abort()
as the abort reason
. That's the place for the intention of the abort to be communicated.
That's the place for the intention of the abort to be communicated.
Agreed. My point was about other kinds of signals, that are not about aborting the subscription. Even if we were omniscient enough to 100% know that the Observable will never have any other kind of signal, it may still trip up readers of code that uses the Observable. Idealistic programmers may feel a need to add a clarifying comment anywhere they use the signal property. Other programmers will omit that comment and someone fixing that part of the code years later will think this line is about the other signal they intended to debug. Thousands of life hours wasted on clarification and having to remember.
My point was about other kinds of signals, that are not about aborting the subscription.
What other kind of signals are there? The primitive is literally called Abort
Signal after all. You use it when you intend to.... abort something. Like a fetch, or an event listener, or an Observable, etc.
I think our misunderstanding perfectly illustrates the argument. From the Readme:
let controller = new AbortController();
observable.subscribe({
next: (data) => {
if (data > 100) controller.abort();
}}, {signal: controller.signal},
});
The word signal
appears twice in that code. I'm rather sure that only the 2nd is mandated by the AbortController spec, and there, it makes sense. The first one however, is the one I meant should carry more meaning.
A first step towards clarity at least in this discussion may be {unsubscribeWhen: controller.signal}
, and I hope we can come up with even better names.
The AbortController spec says that APIs must
Accept AbortSignal objects through a
signal
dictionary member.
So no, in fact both occurrences of signal
in that code are mandated by the spec.
Wow, that seems to be a surprisingly hostile spec. Still, we can work around it by containing the madness in something that clarifies the meaning, e.g.:
observable.subscribe({
next: (data) => {
if (data > 100) controller.abort();
},
unsubscribe: { signal: controller.signal },
});
Edit: With that, people could even choose to pass the entire controller instead of that signal-property-extraction object. In the API docs, we can assure them that from that object we won't keep around references to anything besides the value of the signal
property. With that assurance, they can write concise code without worrying about impeding on garbage collection.
I just don't think this needs any clarification. AbortSignals are there to abort. For event listeners, that means unregister the event listener. For fetch()
, that means stop fetching. For Observables, that means stop producing values & teardown. I am not convinced that there is any more work to do here, and since the current spec is exactly inline what DOM requirements and how other APIs use AbortSignals (i.e., by consuming AbortSignal
through a signal
dictionary member, and not consuming "the whole AbortController" which feels very wrong), I think we can close this.
AbortSignals are there to abort.
Yes. My problem was, and still is, that from our side of the API there is nothing that shows that we expect an AbortSignal. Just some generic signal that the reader has to guess what it may be meant for.
Also, fortunately, there was a line missing from the above the quote:
Any web platform API using promises to represent operations that can be aborted must adhere to the following: • Accept AbortSignal objects through a signal dictionary member. — DOM spec, chapter 3.3 "Using AbortController and AbortSignal objects in APIs"
Are we really "using promises to represent operations that can be aborted"? The act of subscribing seems to be immediate/synchronous, so its result (undefined/throw) doesn't need to be a promise. The resulting subscription itself is not an operation.
Edit: Below the algorithm box it says
APIs not using promises should still adhere to the above as much as possible.
but I still suggest we try and fix the upstream requirement rather than infecting our own spec with such a hostile-to-readers stance. The only positive argument I could find in the other thread is that other APIs use that property name already – which is a non-starter because their contexts may be entirely different.
We may even have an option that can work for both ideals: We can make an option unsubscribeSignal
that is our default and will unsubscribe at any signal, and then for people who enjoy conformity over readability, we could additionally accept a generic signal
property that will unsubscribe only if it is an AbortSignal
.
I have no hopes of convincing you, I just want to bust all possible excuses for perpetuating the (potentially, if understood as some seem to have) reader-hostile upstream approach. (Still waiting for actual readability/ mistake-avoidance/ ergonomics arguments there.)
A property should only be named "signal" in contexts where there is really only one thing imaginable that could be signaled, and this is not such a case. (E.g. a subscriber might want to provide a signal for when the first event was successfully handled, e.g. for monitoring in cases where the first event handling requires substantial effort like breaking the seal on a new output tape.)
My first spontaneous idea for a better name would be noLongerInterested. It might not be the best name either, but a step towards a better name.