search

nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.28k stars 29.45k forks source link

Add on/off to EventSignal? #37399

Open ronag opened 3 years ago

ronag commented 3 years ago

Would if make sense to add on/off as aliases to addEventListener/removeEventListener?

I'm ending up writing quite a bit of code that checks for either since usually I like to implement api's to support both EventEmitter and AbortSignal. Having the aliases would make code much easier and less error prone.

ronag commented 3 years ago

@benjamingr @jasnell

benjamingr commented 3 years ago

we are not allowed to do this, WebIDL is very specific about that fact and we're not going to be able to push such a huge change the EventTarget spec (almost everything in the web basically is an EventTarget from a div to an XMLHttpRequest).

Use the static helpers - they already support both EventTarget and EventEmitter explicitly for this purpuse:

events.once(emitterOrTarget, 'eventName'); // does the right thing, even with regards to error handling
events.on(emitterOrTarget, 'eventName'); // async iterator, ditto

They both even take a signal parameter.

benjamingr commented 3 years ago

I like to implement api's to support both EventEmitter and AbortSignal.

Also - we have in the past supported both in some cancellation APIs (made it intentionally possible to pass an EventEmitter and avoided branding checks) - but I think AbortSignal is probably the future in terms of what we're going with.

(There is also NodeEventTarget by the way :D )

benjamingr commented 3 years ago

I will ask in whatwg/dom just in case

ronag commented 3 years ago

I'm happy with the answer if you wish to close.

benjamingr commented 3 years ago

Do we want to add a section to the docs discussing this (code supporting both EventTarget and EventEmitter) and add a recommendation to use on/once for that case?

Linkgoron commented 3 years ago

Do we want to add a section to the docs discussing this (code supporting both EventTarget and EventEmitter) and add a recommendation to use on/once for that case?

I'd be happy to try and help writing some docs, although i'll only have time in a few days, in the weekend.

jasnell commented 3 years ago

@Linkgoron ... are you still interested in working on the doc updates for this?

Linkgoron commented 3 years ago

@jasnell whoops, totally forgot about this. I'll take care of this.