Repository navigation
Property name "signal" carries no meaning #147
Description
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.
Activity
Jamesernator commentedon Jun 7, 2024
The naming of
signalis mandated for Web APIs by the AbortSignal spec.domfarolino commentedon Jun 7, 2024
Well, sort of. It is mandatory for
AbortSignal-consuming APIs to consume the signal in a dictionary, with the dictionary member namesignal. But I don't think it is mandatory to expose it on an object likeSubscriberwith the interface member namesignal.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 abortreason. That's the place for the intention of the abort to be communicated.mk-pmb commentedon Jun 7, 2024
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.
domfarolino commentedon Jun 10, 2024
What other kind of signals are there? The primitive is literally called
AbortSignal after all. You use it when you intend to.... abort something. Like a fetch, or an event listener, or an Observable, etc.mk-pmb commentedon Jun 10, 2024
I think our misunderstanding perfectly illustrates the argument. From the Readme:
The word
signalappears 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.bakkot commentedon Jun 10, 2024
The AbortController spec says that APIs must
So no, in fact both occurrences of
signalin that code are mandated by the spec.mk-pmb commentedon Jun 10, 2024
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.:
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
signalproperty. With that assurance, they can write concise code without worrying about impeding on garbage collection.domfarolino commentedon Jun 10, 2024
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 consumingAbortSignalthrough asignaldictionary member, and not consuming "the whole AbortController" which feels very wrong), I think we can close this.mk-pmb commentedon Jun 10, 2024
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.
mk-pmb commentedon Jun 10, 2024
Also, fortunately, there was a line missing from the above the quote:
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
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.
mk-pmb commentedon Jun 11, 2024
We may even have an option that can work for both ideals: We can make an option
unsubscribeSignalthat is our default and will unsubscribe at any signal, and then for people who enjoy conformity over readability, we could additionally accept a genericsignalproperty that will unsubscribe only if it is anAbortSignal.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.)