events: add CustomEvent

[daeyeon]

This implements the Web API CustomEvent in internal/event_target.

Refs: #40678

Signed-off-by: Daeyeon Jeong [email protected]

[F3n67u]

Do we need wait #43151 land first before we land this pr? I think it would be better to have wpt test pass.

[F3n67u]

I am wondering if we could replace NodeCustomEvent with CustomEvent. They look the same. NodeCustomEvent is introduced in #35851:

node/lib/internal/event_target.js

Lines 328 to 335 in 3847617

	classNodeCustomEventextendsEvent{
	constructor(type,options){
	super(type,options);
	if(options?.detail){
	this.detail=options.detail;
	}
	}
	}

[benjamingr]

Why?

Like, when EventTarget was added it was to back other APIs like AbortController which was in turn to solve a concrete case (fetch + cancellation in core APIs).

What would Node core do with CustomEvent?

[daeyeon]

@benjamingr Thanks for the question. I'd have mentioned the reason why I propose this.

I think Node can well support use cases like #40678 and libp2p with CustomEvent. Although users technically can assign any property into a new Event object after its creation, it possibly conflicts with other properties. This provides a standardized interface, the detail field, to evade such conflicts. It would make Event and EventTarget more useful.

[daeyeon]

@F3n67u

You can consider this as an initial implementation for CustomEvent. We can update the test/wpt/status/dom/events.json once landed.

IIUC, the WPT doesn't include enough tests specific for CustomeEvent, so we need to make up more tests. Porting tests written in HTML would be also required. We need to review whether to expose this to the global object. However, above all, I'm not sure if CustomEvent itself is acceptable to Node core. :-) So, I think such preparation jobs could start with this implementation in parallel.

Regarding NodeCustomEvent, it looks possible but better in a separate PR.

[jasnell]

I'm torn on this one. I don't see a great immediate use case for it in Node.js but it is a standard API that is used in browsers and is available in other platforms like Deno. As far as the implementation here is concerned, this LGTM and I don't have a reason to block so I'll sign off, but it would be good to get other folks chiming in as well.

[benjamingr]

+0 mostly

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/45481/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/45484/

[nodejs-github-bot]

Landed in cbbd91d