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

Open
alshdavid opened this issue Oct 31, 2021 · 4 comments
Open

Support CustomEvent #40678

alshdavid opened this issue Oct 31, 2021 · 4 comments
Labels
eventtarget feature request good first issue help wanted

Comments

@alshdavid
Copy link

@alshdavid 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 label Oct 31, 2021
@VoltrexMaster VoltrexMaster added the feature request 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

@github-actions 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

@regseb 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

@bnoordhuis bnoordhuis commented May 17, 2022

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 good first issue 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

@Yokubjon-J Yokubjon-J commented Jun 27, 2022

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

LiviaMedeiros pushed a commit to LiviaMedeiros/done that referenced this issue Jun 28, 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eventtarget feature request good first issue help wanted
Projects
Development

No branches or pull requests

6 participants