EventTarget in Node - should it expose capabilities like removeAllListeners?

[benjamingr]

Work is currently underway to add support for EventTarget in Node.js, this was discussed in several summits in the past and is a stepping stone for AbortController and fetch in Node 🎉

(that PR lands it as experimental and with no open API surface so this issue should not block or delay that PR IMO anyway).

There are (at least) two possible approaches for EventTarget in Node.js:

• We can make EventTarget as close as possible in terms of capabilities to the DOM version.
• We can make EventTarget as compatible with EventEmitter as possible.

Note that to my understanding the API does not prohibit us from exposing additional things (like removeAllListeners) but it would still be a violation of the liskov substitution principle (users might expect event listeners to not be removable or discoverable).

Note the APIs are quite different from each other.

The second approach has advantages of probably easier gradual migration and the first approach has the advantage of more closely being universal. Namely: code using the second approach would have to limit itself to not using these APIs in order to be universal in the first place anyway.

Note as prior art, abort-controller in NPM supports these methods but browsers do not.

cc @jasnell @ljharb @nodejs/events @mcollina

[benjamingr]

Please see the initial discussion in the PR here #33556 (comment)

Personally I see compelling point both ways and I'm not sure what's the better approach.

[strongholdmedia]

Carried my response over to here as the flow suggests. The quotes are from #33556

Note that just exposing these capabilities significantly changes the API surface of EventTarget. For example getting a listener count, event names and who's listening to what event invalidates various guarantees the DOM gives.

Sorry for the intervention but are these guarantees codified anywhere, or, in other words, could theoretically relying on such be ever considered basically some sort of bad habit instead?

Can you help me understand what's the benefit of shipping something called EventTarget that doesn't precisely match the browser's EventTarget?

It may be worthwhile to consider if sticking to a definitely incomplete and asymmetrical, but possibly also crippled and/or irrational implementation for whatever reason is of any actual benefit.
There was some significant (but somewhat inconclusive) discussion on this over at WHATWG as well.
If anything, I'd expect the DOM approach to change.
Actually IIRC some engines, like Blink? already implement stuff like this under the hood, even if not exposing it.

For development, this is immensely helpful (and please let me point out that an implementation ("circumvention") is quite easy simply by mutating the EventTarget's prototype before any event listeners are being added.

[ljharb]

@strongholdmedia they are codified on the web, I believe. The discussion on WHATWG imo was overwhelmingly in favor of not exposing this API.

The primary security JS offers is in first-run code; pretty much anything that requires prior modification is indefensible and thus not a concern.

[benjamingr]

The latest solution in the PR:

• Exposes EventTarget that is as browser compatible as possible.
• Exposes NodeEventTarget that is compatible as possible.

I think that approach makes sense although we would still have to answer what flavor the emitters we ship ourselves are.

[jasnell]

The plan for AbortController.signal is that it would be a NodeEventTarget in order to preserve ecosystem compatibility (there are existing AbortController implementations whose signal implements subsets of both EventTarget and EventEmitter)

[strongholdmedia]

@strongholdmedia they are codified on the web, I believe. The discussion on WHATWG imo was overwhelmingly in favor of not exposing this API.

Indeed, you seemed quite overwhelmed to make it appear so.

Exposes NodeEventTarget that is compatible as possible.

I believe this is an elegant and excellent solution.
Then if enough people gets accustomed to actual usability, perhaps people may (will?) get back to the browser matter.
(I would, however, suggest that you name NodeEventTarget in some more generic way in case any other implementors would "borrow" it later - which, again to me, seems logical.)

[benjamingr]

@strongholdmedia better naming suggestions welcome :]

@jasnell

The plan for AbortController.signal is that it would be a NodeEventTarget in order to preserve ecosystem compatibility

Can you explain this? I just checked really quickly and I don't think abort-controller (on npm) exposes these?

[jasnell]

Ah! I was mistaken about the abort-controller module! There was an impl that I came across that did support both but for the life of me I can't remember the name of it and for some reason had it in my mind that it was abort-controller!

The remove functions in event emitter are absolutely question marks either way. The only ones that I absolutely think we need to have are on/once/addListener. Everything else is optional.

[benjamingr]

@jasnell p

I think Node.js can be helpful (by providing these) as long as we make sure all our internal methods don't rely on this behavior (and work with the regular AbortController API).

It seems to me that the major issue people have with extending these interfaces is we'd be shipping a different API from the DOM's with the same names.

I think we can:

• Ship (Node)EventTarget internally and (Node)AbortController using it.
• As we add APIs with cancellation support through AbortController - all our APIs should not rely on the additional methods on NodeAbortController/NodeEventTarget to make writing universal code easier.

[benjamingr]

There is a PR to only land EventTarget and remove NodeEventTarget #33665

If anyone has strong reasons about whether or not Node should land the extensions or not - now is a good time to bring them up :]

Also - it's worth mentioning that landing EventTarget now without the extensions doesn't mean we can't add the extensions later on.

[jasnell]

Let's keep NodeEventTarget for now but I will open a PR removing the removeAllListeners() functions.

[strongholdmedia]

@strongholdmedia better naming suggestions welcome :]

Well.. If the latest implementation has something along getEventListeners, it could be TransparentEventTarget. :)

If anyone has strong reasons about whether or not Node should land the extensions or not - now is a good time to bring them up :]

After I spent the whole day working with the legacy/browser EventTarget idea I personally find it frustrating that many people can be quite preoccupied with like introducing or deprecating something with little importance (like removal of the commas in the new CSS "functions"), or sizable nuisance (like the notion of abusing the optional SameSite attribute for cookies that are set in the very same manner for more than five years now), but try to stick to irrational legacies in questions that might matter more (like this).

As most of you surely know, the current legacy EventTarget implementation mirrors that of Adobe Flash as in ActionScript 3.
It was dated enough when I got to use it (some 11 years ago) already.
Meanwhile, it is comparable to nothing around - I find even the setTimeout/clearTimeout approach more concise - they are returning handles that can be used for removal, and do that by value, as opposed to the strict reference comparison used here.

But then again, please note that I realize that even this approach does not try to solve the problems (if they even are, judged subjectively).

I would just like to pinpoint that sometimes it may be justifiable to observe the proposed evolution from more than a single angle. If anything it is exactly stuff like this that I consider worthwhile updating/extending/enhancing.

---

Also sorry for the late reply - I mixed up the comments here with those at the PR, and as such, I thought the discussion passed me by long since.