Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support CustomEvent #40678

Closed
alshdavid opened this issue Oct 31, 2021 · 6 comments · Fixed by #43885
Closed

Support CustomEvent #40678

alshdavid opened this issue Oct 31, 2021 · 6 comments · Fixed by #43885
Labels
eventtarget Issues and PRs related to the EventTarget implementation. feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors. help wanted Issues that need assistance from volunteers or PRs that need help to proceed.

Comments

@alshdavid
Copy link

alshdavid commented Oct 31, 2021

Is your feature request related to a problem? Please describe.
When trying to mimic browser APIs surrounding EventTarget, we frequently use CustomEvent which is currently unavailable in Node.

It's useful in that it allows us to dispatch events through an EventTarget and include a custom data payload.

In a some way, this is the closest we have to a standardised Observable API (specifically Subject) and it's very useful.

Describe the solution you'd like
CustomEvent exists

Describe alternatives you've considered
Writing a polyfill

class CustomEvent extends Event { 
  constructor(message, data) {
    super(message, data)
    this.detail = data.detail
  }
}

const et = new EventTarget()
et.addEventListener('message', ev => console.log(ev.detail))
et.dispatchEvent(new CustomEvent('message', { detail: 'foo' }))
@Mesteery Mesteery added the eventtarget Issues and PRs related to the EventTarget implementation. label Oct 31, 2021
@VoltrexKeyva VoltrexKeyva added the feature request Issues that request new features to be added to Node.js. label Nov 1, 2021
achingbrain added a commit to libp2p/js-libp2p-interfaces that referenced this issue Feb 10, 2022
Replaces the node `EventEmitter` with the pure-js `EventTarget` class.

All events are now instances of [Event](https://developer.mozilla.org/en-US/docs/Web/API/Event).

For typing [CustomEvent](https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent) can be used which gives us a typed `.detail` field.

Tediously `EventTarget` itself [doesn't support typed events](microsoft/TypeScript#28357) (yet?) and node.js [doesn't support](nodejs/node#40678) `CustomEvent` globally so bit of trickery was required but hopefully this can be removed in future:

```js
import { EventEmitter, CustomEvent } from '@libp2p/interfaces'

interface EventMap {
  'foo': CustomEvent<number>
}

class MyEmitter extends EventEmitter<EventMap> {
  // ... other code here
} 

const emitter = new MyEmitter()

emitter.addEventListener('foo', (evt) => {
  const n = evt.detail // n is a 'number'
})
```
@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2022

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label May 1, 2022
@regseb
Copy link
Contributor

regseb commented May 15, 2022

Yet another polyfill proposal that:

  • handles no options;
  • uses null for the default value of detail;
  • define detail readonly.
class CustomEvent extends Event {
    #detail;

    constructor(type, options) {
        super(type, options);
        this.#detail = options?.detail ?? null;
    }

    get detail() {
        return this.#detail;
    }
}

@github-actions github-actions bot removed the stale label May 16, 2022
@bnoordhuis
Copy link
Member

For anyone wanting to work on this, the steps to take are:

  1. Enable the dom/events WPT tests in test/wpt
  2. Make them pass for regular Events/EventTargets
  3. Then start working on CustomEvent

W.r.t. (1):

diff --git a/test/wpt/status/dom/events.json b/test/wpt/status/dom/events.json
new file mode 100644
index 00000000000..0967ef424bc
--- /dev/null
+++ b/test/wpt/status/dom/events.json
@@ -0,0 +1 @@
+{}
diff --git a/test/wpt/test-events.js b/test/wpt/test-events.js
new file mode 100644
index 00000000000..5040d56d6a2
--- /dev/null
+++ b/test/wpt/test-events.js
@@ -0,0 +1,7 @@
+'use strict';
+require('../common');
+const { WPTRunner } = require('../common/wpt');
+
+const runner = new WPTRunner('dom/events');
+
+runner.runJsTests();

Where dom/events is https://github.com/web-platform-tests/wpt/tree/master/dom/events

@bnoordhuis bnoordhuis added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. good first issue Issues that are suitable for first-time contributors. labels May 17, 2022
nodejs-github-bot pushed a commit that referenced this issue Jun 22, 2022
PR-URL: #43151
Refs: #40678
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
F3n67u pushed a commit to F3n67u/node that referenced this issue Jun 24, 2022
PR-URL: nodejs#43151
Refs: nodejs#40678
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Yokubjon-J
Copy link

Is this issue still open? It looks like someone has opened a PR on it.

targos pushed a commit that referenced this issue Jul 12, 2022
PR-URL: #43151
Refs: #40678
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs-github-bot pushed a commit that referenced this issue Jul 17, 2022
This implements the Web API `CustomEvent` in `internal/event_target`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43514
Refs: #40678
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@F3n67u
Copy link
Member

F3n67u commented Jul 17, 2022

Is this issue still open? It looks like someone has opened a PR on it.

@daeyeon CustomEvent has been implemented by #43514 in events module, could we close this issue?

@daeyeon
Copy link
Member

daeyeon commented Jul 17, 2022

@F3n67u We have a bit more. It needs to be exposed as global. Working on it. :-)

targos pushed a commit that referenced this issue Jul 20, 2022
PR-URL: #43151
Refs: #40678
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs-github-bot pushed a commit that referenced this issue Jul 23, 2022
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43885
Fixes: #40678
Refs: #43514
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
danielleadams pushed a commit that referenced this issue Jul 26, 2022
This implements the Web API `CustomEvent` in `internal/event_target`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43514
Refs: #40678
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
danielleadams pushed a commit that referenced this issue Jul 26, 2022
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43885
Fixes: #40678
Refs: #43514
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
targos pushed a commit that referenced this issue Jul 31, 2022
PR-URL: #43151
Refs: #40678
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit to aduh95/node that referenced this issue Aug 1, 2022
This implements the Web API `CustomEvent` in `internal/event_target`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs#43514
Refs: nodejs#40678
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
aduh95 pushed a commit to aduh95/node that referenced this issue Aug 1, 2022
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs#43885
Fixes: nodejs#40678
Refs: nodejs#43514
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
aduh95 pushed a commit to aduh95/node that referenced this issue Aug 1, 2022
This implements the Web API `CustomEvent` in `internal/event_target`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs#43514
Refs: nodejs#40678
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
aduh95 pushed a commit to aduh95/node that referenced this issue Aug 1, 2022
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs#43885
Fixes: nodejs#40678
Refs: nodejs#43514
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
targos pushed a commit that referenced this issue Aug 2, 2022
This implements the Web API `CustomEvent` in `internal/event_target`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43514
Backport-PR-URL: #44082
Refs: #40678
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
targos pushed a commit that referenced this issue Aug 2, 2022
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43885
Backport-PR-URL: #44082
Fixes: #40678
Refs: #43514
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Fyko pushed a commit to Fyko/node that referenced this issue Sep 15, 2022
This implements the Web API `CustomEvent` in `internal/event_target`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs#43514
Refs: nodejs#40678
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Fyko pushed a commit to Fyko/node that referenced this issue Sep 15, 2022
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs#43885
Fixes: nodejs#40678
Refs: nodejs#43514
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
PR-URL: nodejs/node#43151
Refs: nodejs/node#40678
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
This implements the Web API `CustomEvent` in `internal/event_target`.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs/node#43514
Backport-PR-URL: nodejs/node#44082
Refs: nodejs/node#40678
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs/node#43885
Backport-PR-URL: nodejs/node#44082
Fixes: nodejs/node#40678
Refs: nodejs/node#43514
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
achingbrain pushed a commit to libp2p/js-libp2p that referenced this issue Aug 9, 2024
The EventTarget implementation of libp2p contained workarounds for incomplete runtime support for the feature. One of the workarounds was specifically marked to be removed once CustomEvent was implemented in NodeJS and the [upstream ticket](nodejs/node#40678) was closed. This has happened two years ago. The implementation of the standard event in Node18 and the subsequent removal of the experimental flag in Node19 broke the current workaround and causes tsc to error. Some of these were suppressed with @ts-ignore, others were not. This fix closes #2420.

The fix removes the workaround as instructed by the source code and restores compatibility with recent versions of Node types.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eventtarget Issues and PRs related to the EventTarget implementation. feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors. help wanted Issues that need assistance from volunteers or PRs that need help to proceed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants