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

events: fix bug listenerCount don't compare wrapped listener #48592

Merged
merged 2 commits into from
Jul 10, 2023

Conversation

yuzheng14
Copy link
Contributor

description

When add listener by once, it will be wrapped into another function. And when pass listener and there is just one event listener added by once, it will return 0 even if passed listener equal wrapped event listener.

This bug is included in #46523

Refs: #46523

how to trigger this

const ee = new EventEmitter()
const listener = () => undefined
ee.once('event', listener)
console.log('ee.listenerCount("event",listener)', ee.listenerCount('event', listener))

And it will output like this:

image

When add listener by once, it will be wrapped into another function.
And when pass listener and there is just one event listener added by
once, it will return 0 even if passed listener equal wrapped event
listener.

Refs: nodejs#46523
@nodejs-github-bot nodejs-github-bot added events Issues and PRs related to the events subsystem / EventEmitter. needs-ci PRs that need a full CI run. labels Jun 29, 2023
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 29, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 29, 2023
@nodejs-github-bot
Copy link
Collaborator

@yuzheng14
Copy link
Contributor Author

What do I need to do for this four failing checks?

image

@nodejs-github-bot
Copy link
Collaborator

@ShogunPanda
Copy link
Contributor

Nothing: those changes were unrelated to your PR. I resumed the build to let them succeed.

@yuzheng14
Copy link
Contributor Author

What do I need to do more for this pr? Or just waiting for merge?

@ShogunPanda ShogunPanda added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jul 10, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 10, 2023
@nodejs-github-bot nodejs-github-bot merged commit 0c8539e into nodejs:main Jul 10, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 0c8539e

anonrig pushed a commit to anonrig/node that referenced this pull request Jul 11, 2023
When add listener by once, it will be wrapped into another function.
And when pass listener and there is just one event listener added by
once, it will return 0 even if passed listener equal wrapped event
listener.

Refs: nodejs#46523
PR-URL: nodejs#48592
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
juanarbol pushed a commit that referenced this pull request Jul 13, 2023
When add listener by once, it will be wrapped into another function.
And when pass listener and there is just one event listener added by
once, it will return 0 even if passed listener equal wrapped event
listener.

Refs: #46523
PR-URL: #48592
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
@juanarbol juanarbol mentioned this pull request Jul 13, 2023
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
When add listener by once, it will be wrapped into another function.
And when pass listener and there is just one event listener added by
once, it will return 0 even if passed listener equal wrapped event
listener.

Refs: nodejs#46523
PR-URL: nodejs#48592
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
When add listener by once, it will be wrapped into another function.
And when pass listener and there is just one event listener added by
once, it will return 0 even if passed listener equal wrapped event
listener.

Refs: nodejs#46523
PR-URL: nodejs#48592
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 11, 2023
When add listener by once, it will be wrapped into another function.
And when pass listener and there is just one event listener added by
once, it will return 0 even if passed listener equal wrapped event
listener.

Refs: #46523
PR-URL: #48592
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Sep 11, 2023
ruyadorno pushed a commit that referenced this pull request Sep 13, 2023
When add listener by once, it will be wrapped into another function.
And when pass listener and there is just one event listener added by
once, it will return 0 even if passed listener equal wrapped event
listener.

Refs: #46523
PR-URL: #48592
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 17, 2023
When add listener by once, it will be wrapped into another function.
And when pass listener and there is just one event listener added by
once, it will return 0 even if passed listener equal wrapped event
listener.

Refs: #46523
PR-URL: #48592
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. events Issues and PRs related to the events subsystem / EventEmitter. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants