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

std/node "EventTarget" is incompatible with global "EventTarget" #1455

Closed
bartlomieju opened this issue Oct 24, 2021 · 3 comments
Closed

std/node "EventTarget" is incompatible with global "EventTarget" #1455

bartlomieju opened this issue Oct 24, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@bartlomieju
Copy link
Member

Currently in node/events_test.ts there are some tests that use EventTarget from the global scope, however Node's implementation of EventTarget is different that one coming from DOM: https://nodejs.org/api/events.html#eventtarget-and-event-api

$ node
Welcome to Node.js v16.11.1.
Type ".help" for more information.
> EventTarget
[class EventTarget] { [Symbol(nodejs.event_target)]: true }
>
@bartlomieju
Copy link
Member Author

So "events" implementation was reworked in #1558; we still need to fix situation with EventTarget - that said I think it will be quite hard as Deno's and Node's global EventTarget are incompatible.

@bartlomieju bartlomieju changed the title std/node event implementation might be broken std/node "EventTarget" is incompatible with global "EventTarget" Nov 17, 2021
@lucacasonato
Copy link
Member

As for:

  1. Whereas DOM EventTarget instances may be hierarchical, there is no concept of hierarchy and event propagation in Node.js. That is, an event dispatched to an EventTarget does not propagate through a hierarchy of nested target objects that may each have their own set of handlers for the event.

We have the same behaviour as node, because we also don't have a hierarchy. This should not be an issue.

In the Node.js EventTarget, if an event listener is an async function or returns a Promise, and the returned Promise rejects, the rejection is automatically captured and handled the same way as a listener that throws synchronously (see EventTarget error handling for details).

This is just a dumb deviation from Web APIs. Why did anyone think this was ok? It breaks the entire model for event handlers... we should not implement this. Any Node code that uses this behaviour should be considered buggy (as it won't work in browsers), and should fix their code to not rely on this. It breaks the entire internal propagation and preventDefault handling if event targets handle errors asynchronously... we only need to get 80% there, right?

@piscisaureus
Copy link
Member

Discussed off-github: although this issue exists, it is unlikely to cause any problems and we won't address it for now.
If anyone discovers an real world bug that is caused by this issue, feel free to open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants