events: combined event target fixes by jasnell · Pull Request #34015 · nodejs/node · GitHub

This PR combines the following separate PRs, with fixups, to make them easier to land (landing them one at a time was starting to get extremely painful)

/cc @benjamingr, @mscdex, @lundibundi, @Zirak

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Remove support for multiple arguments (which don't actually work for EventTarget). Use our EventTarget implementation rather than a mock. Refactor `once` code in preparation of `on` using shared code for supporting `on` with `EventTarget`s. Support EventTarget in the `events.on` static method

On the web, dispatchEvent is finicky about its `this` value. An exception is thrown for `this` values which are not an EventTarget.

Event#cancelBubble is property (and not a function). Change Event#cancelBubble to a property and add a test. PR-URL: nodejs#33613 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>

Co-authored-by: Benjamin Gruenbaum <benjamingr@gmail.com>

Fixes: nodejs#33782

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

Since everything here is already approved in other PRs I think this can/should be fast tracked

Co-authored-by: Benjamin Gruenbaum <benjamingr@gmail.com> PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Fixes: #33782 PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Remove support for multiple arguments (which don't actually work for EventTarget). Use our EventTarget implementation rather than a mock. Refactor `once` code in preparation of `on` using shared code for supporting `on` with `EventTarget`s. Support EventTarget in the `events.on` static method PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

On the web, dispatchEvent is finicky about its `this` value. An exception is thrown for `this` values which are not an EventTarget. PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Event#cancelBubble is property (and not a function). Change Event#cancelBubble to a property and add a test. PR-URL: #33613 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Co-authored-by: Benjamin Gruenbaum <benjamingr@gmail.com> PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Fixes: #33782 PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

lundibundi added 2 commits June 21, 2020 07:35

events: use internal/validators in event_target.js

343bcf7

events: fix depth in customInspectSymbol and clean up

8a220fc

jasnell added the eventtarget Issues and PRs related to the EventTarget implementation. label Jun 22, 2020

nodejs-github-bot added the events Issues and PRs related to the events subsystem / EventEmitter. label Jun 22, 2020

jasnell added the experimental Issues and PRs related to experimental features. label Jun 22, 2020

This comment has been minimized.

Sign in to view

benjamingr and others added 9 commits June 22, 2020 07:54

events: Handle a range of this values for dispatchEvent

5d34ea0

On the web, dispatchEvent is finicky about its `this` value. An exception is thrown for `this` values which are not an EventTarget.

events: expose Event statics

df1a006

events: support event handlers

5893b88

events: add event-target tests

d900f87

events: support dispatching event from event

dbddf3c

Co-authored-by: Benjamin Gruenbaum <benjamingr@gmail.com>

events: improve argument handling, start passive

227258b

Co-authored-by: Benjamin Gruenbaum <benjamingr@gmail.com>

benchmark: fix EventTarget benchmark

65af6f1

Fixes: nodejs#33782

jasnell force-pushed the eventtarget-fixups branch from 532379f to 65af6f1 Compare June 22, 2020 15:05

lundibundi approved these changes Jun 22, 2020

View reviewed changes

jasnell added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jun 22, 2020

github-actions Bot mentioned this pull request Jun 23, 2020

CI Relability 2020-06-23 JaneaSystems/nodejs-reliability#16

Open

29 tasks

benjamingr approved these changes Jun 23, 2020

View reviewed changes

codebytere pushed a commit that referenced this pull request Jun 27, 2020

benchmark: fix EventTarget benchmark

0eed22d

Fixes: #33782 PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

codebytere mentioned this pull request Jun 28, 2020

v14.5.0 proposal #34093

Merged

codebytere pushed a commit that referenced this pull request Jun 30, 2020

events: use internal/validators in event_target.js

eaf5e21

PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

codebytere pushed a commit that referenced this pull request Jun 30, 2020

events: fix depth in customInspectSymbol and clean up

655422a

PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

codebytere pushed a commit that referenced this pull request Jun 30, 2020

events: expose Event statics

a3a6a4d

PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

codebytere pushed a commit that referenced this pull request Jun 30, 2020

events: support event handlers

6ac767c

PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

codebytere pushed a commit that referenced this pull request Jun 30, 2020

events: add event-target tests

79339d7

PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

codebytere pushed a commit that referenced this pull request Jun 30, 2020

benchmark: fix EventTarget benchmark

8c48ef5

Fixes: #33782 PR-URL: #34015 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

lpinca mentioned this pull request Sep 3, 2020

indefinite behavior of nodejs/events.once() on WebSocket websockets/ws#1795

Closed

lpinca mentioned this pull request Sep 22, 2020

V14.12.0 proposal #35285

Merged

4 tasks

addaleax mentioned this pull request Sep 22, 2020

events: initial implementation of experimental EventTarget #33556

Closed

4 tasks

addaleax added the backport-requested-v12.x label Sep 22, 2020

events: combined event target fixes by jasnell · Pull Request #34015 · nodejs/node · GitHub

Uh oh!

Uh oh!

events: combined event target fixes - #34015

events: combined event target fixes#34015
jasnell wants to merge 11 commits into
nodejs:masterfrom
jasnell:eventtarget-fixups

jasnell commented Jun 22, 2020

Uh oh!

This comment has been minimized.

nodejs-github-bot commented Jun 22, 2020

Uh oh!

benjamingr commented Jun 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

events: combined event target fixes by jasnell · Pull Request #34015 · nodejs/node · GitHub

Uh oh!

Uh oh!

events: combined event target fixes - #34015

events: combined event target fixes#34015jasnell wants to merge 11 commits intonodejs:masternodejs/node:masterfrom jasnell:eventtarget-fixupsjasnell/node:eventtarget-fixupsCopy head branch name to clipboard

Conversation

jasnell commented Jun 22, 2020

Checklist

Uh oh!

This comment has been minimized.

nodejs-github-bot commented Jun 22, 2020

Uh oh!

benjamingr commented Jun 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

events: combined event target fixes#34015
jasnell wants to merge 11 commits into
nodejs:masterfrom
jasnell:eventtarget-fixups