events: getEventListeners static by benjamingr · Pull Request #35991 · nodejs/node

benjamingr · 2020-11-06T10:10:53Z

This adds a static getEventListeners to events. The major difference between it and .listeners is that it works with EventTargets and not just EventEmitters.

This is useful after talking to users of EventTarget and AbortController for two main reasons:

For diagnostics and digging into why EventTargets are behaving and trying to isolate possible memory leaks.
For creating linked AbortSignals potentially.

cc @bterlson

This has to be a static because we are not allowed to add this method on EventTarget.prototype. I checked with WHATWG and the only way I could come up with that is both spec compliant and addresses the use case.

The web platform itself does not have a way to do this (due to encapsulation) but I double checked we are allowed to expose this functionality as long as it's not on EventTarget itself (from the public #whatwg IRC channel):

Checklist

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

addaleax · 2020-11-06T10:11:56Z

doc/api/events.md

Suggested change

For `EventTargegt`s this is the only way to get the event listeners for the

For `EventTarget`s this is the only way to get the event listeners for the

aduh95

It seems unlikely this will be backported to v12 (going to maintenance this month), so we may as well use optional chaining

lib/events.js

nodejs-github-bot · 2020-11-06T10:42:32Z

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

nodejs-github-bot · 2020-11-07T09:10:27Z

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

aduh95 · 2020-11-07T10:23:49Z

test/parallel/test-events-static-geteventlisteners.js

That seems unnecessary:

Suggested change

// Flags: --expose-internals --no-warnings

This is needed for NodeEventTarget no?

Oh, I ended up not needing it, fun

nodejs-github-bot · 2020-11-08T11:48:59Z

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

doc/api/events.md

Trott · 2020-11-08T13:30:18Z

doc/api/events.md

Suggested change

### `events.getEventListeners(emitterOrTarget, eventName)`

## `events.getEventListeners(emitterOrTarget, eventName)`

nodejs-github-bot · 2020-11-08T13:52:50Z

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

benjamingr · 2020-11-08T16:43:00Z

@addaleax @Trott @mcollina any chance this can get a second LGTM to not wait a week :]?

github-actions · 2020-11-09T07:39:36Z

Landed in ad98cf0...1a6d4dc

PR-URL: #35991 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Notable changes: * events: getEventListeners static (Benjamin Gruenbaum) (#35991) * fs: add support for AbortSignal in readFile (Benjamin Gruenbaum) (#35911) PR URL: #36055

Notable changes: * events: * getEventListeners static (Benjamin Gruenbaum) (#35991) * fs: * support abortsignal in writeFile (Benjamin Gruenbaum) (#35993) * add support for AbortSignal in readFile (Benjamin Gruenbaum) (#35911) * stream: * fix thrown object reference (Gil Pedersen) (#36065) PR URL: #36055

sperez841

PR-URL: nodejs#35991 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

PR-URL: #35991 Backport-PR-URL: #38386 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Use `getEventListeners` instead of `listenerCount` because nodejs#35991 was landed. Refs: nodejs#35991 Refs: nodejs#36006

Use `getEventListeners` instead of `listenerCount` because #35991 was landed. Refs: #35991 Refs: #36006 PR-URL: #47759 Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Mestery <mestery@protonmail.com>

Use `getEventListeners` instead of `listenerCount` because nodejs#35991 was landed. Refs: nodejs#35991 Refs: nodejs#36006 PR-URL: nodejs#47759 Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Mestery <mestery@protonmail.com>

benjamingr added events Issues and PRs related to the events subsystem / EventEmitter. semver-minor PRs that contain new features and should be released in the next minor version. labels Nov 6, 2020

benjamingr requested review from addaleax, jasnell and mcollina November 6, 2020 10:10

addaleax reviewed Nov 6, 2020

View reviewed changes

aduh95 reviewed Nov 6, 2020

View reviewed changes

lib/events.js Outdated Show resolved Hide resolved

lib/events.js Outdated Show resolved Hide resolved

lib/events.js Outdated Show resolved Hide resolved

aduh95 reviewed Nov 6, 2020

View reviewed changes

lib/events.js Outdated Show resolved Hide resolved

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 6, 2020

aduh95 reviewed Nov 6, 2020

View reviewed changes

lib/events.js Outdated Show resolved Hide resolved

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 6, 2020

bterlson mentioned this pull request Nov 6, 2020

Warn on EventTarget maxListeners > THRESHOLD #35990

Closed

benjamingr mentioned this pull request Nov 6, 2020

events: add max listener warning for EventTarget #36001

Closed

4 tasks

aduh95 reviewed Nov 7, 2020

View reviewed changes

jasnell approved these changes Nov 7, 2020

View reviewed changes

benjamingr force-pushed the events-get-listeners branch from 4a25a04 to b104cad Compare November 8, 2020 11:40

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 8, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 8, 2020

benjamingr force-pushed the events-get-listeners branch from b104cad to 6ac986b Compare November 8, 2020 12:37

Trott reviewed Nov 8, 2020

View reviewed changes

doc/api/events.md Outdated Show resolved Hide resolved

Trott reviewed Nov 8, 2020

View reviewed changes

events: getEventListeners static

7bf4ac3

benjamingr force-pushed the events-get-listeners branch from 6ac986b to 7bf4ac3 Compare November 8, 2020 13:46

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 8, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 8, 2020

github-actions bot closed this Nov 9, 2020

nodejs-github-bot pushed a commit that referenced this pull request Nov 9, 2020

events: getEventListeners static

1a6d4dc

PR-URL: #35991 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

todo bot mentioned this pull request Nov 9, 2020

[pull] master from nodejs:master daddyfatstacksBIG/node#973

Merged

danielleadams pushed a commit that referenced this pull request Nov 9, 2020

events: getEventListeners static

567f8d8

PR-URL: #35991 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

danielleadams mentioned this pull request Nov 9, 2020

v15.2.0 proposal #36055

Merged

danielleadams added a commit that referenced this pull request Nov 9, 2020

2020-11-10, Version 15.2.0 (Current)

f24be89

Notable changes: * events: getEventListeners static (Benjamin Gruenbaum) (#35991) * fs: add support for AbortSignal in readFile (Benjamin Gruenbaum) (#35911) PR URL: #36055

danielleadams added a commit that referenced this pull request Nov 10, 2020

2020-11-10, Version 15.2.0 (Current)

5d9154f

Notable changes: * events: getEventListeners static (Benjamin Gruenbaum) (#35991) * fs: add support for AbortSignal in readFile (Benjamin Gruenbaum) (#35911) PR URL: #36055

sperez841 suggested changes Nov 12, 2020

View reviewed changes

todo bot mentioned this pull request Mar 30, 2021

[pull] master from nodejs:master daddyfatstacksBIG/node#1193

Merged

targos added the backport-open-v14.x label Apr 24, 2021

targos pushed a commit to targos/node that referenced this pull request Apr 24, 2021

events: getEventListeners static

2fe65da

PR-URL: nodejs#35991 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

targos mentioned this pull request Apr 24, 2021

[v14.x] Backport AbortController and friends #38386

Closed

targos pushed a commit to targos/node that referenced this pull request Apr 26, 2021

events: getEventListeners static

b02e11c

PR-URL: nodejs#35991 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

targos pushed a commit to targos/node that referenced this pull request Apr 30, 2021

events: getEventListeners static

555a5a2

PR-URL: nodejs#35991 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

targos pushed a commit that referenced this pull request Apr 30, 2021

events: getEventListeners static

6e734a8

PR-URL: #35991 Backport-PR-URL: #38386 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

targos added backported-to-v14.x and removed backport-open-v14.x labels Apr 30, 2021

danielleadams mentioned this pull request May 3, 2021

v14.17.0 proposal #38507

Merged

deokjinkim added a commit to deokjinkim/node that referenced this pull request Apr 28, 2023

test: refactor to use getEventListeners in timers

c8e42f7

Use `getEventListeners` instead of `listenerCount` because nodejs#35991 was landed. Refs: nodejs#35991 Refs: nodejs#36006

deokjinkim mentioned this pull request Apr 28, 2023

test: refactor to use getEventListeners in timers #47759

Merged

	For `EventTargegt`s this is the only way to get the event listeners for the
	For `EventTarget`s this is the only way to get the event listeners for the

	### `events.getEventListeners(emitterOrTarget, eventName)`
	## `events.getEventListeners(emitterOrTarget, eventName)`

Uh oh!

Conversation

benjamingr commented Nov 6, 2020

Checklist

Uh oh!

addaleax Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Nov 6, 2020

Uh oh!

nodejs-github-bot commented Nov 7, 2020

Uh oh!

aduh95 Nov 7, 2020

Choose a reason for hiding this comment

Uh oh!

benjamingr Nov 7, 2020

Choose a reason for hiding this comment

Uh oh!

benjamingr Nov 7, 2020

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Nov 8, 2020

Uh oh!

Uh oh!

Trott Nov 8, 2020

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Nov 8, 2020

Uh oh!

benjamingr commented Nov 8, 2020

Uh oh!

github-actions bot commented Nov 9, 2020

Uh oh!

sperez841 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants