Fix events remove listener emission

[Han5991]

Description

This PR fixes a bug where the removeListener event was not being emitted when the last listener was removed from an EventEmitter.

Background

When removeListener() is called and the last listener is removed (_eventsCount === 0), the code path that emits the removeListener event
was being skipped. This was because the emission logic was inside the else block of the event count check.

This issue particularly affects once() listeners, which automatically call removeListener() after execution. If the once listener was the
last listener, the removeListener event would not fire.

Changes

• lib/events.js: Moved the removeListener event emission outside the conditional block to ensure it always executes when a listener is
  removed
• test/parallel/test-event-emitter-remove-listeners.js: Added test coverage for once() listeners with removeListener event monitoring

Test Plan

./node test/parallel/test-event-emitter-remove-listeners.js
make test-ci

All tests pass, including the new test case that verifies removeListener events are emitted when once() listeners are automatically removed.

Checklist

- tests are included
- documentation is not affected
- make -j4 test passes locally

Related Issues

fixes: #59977

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.77%. Comparing base (bfc81ca) to head (598fbb4).
⚠️ Report is 722 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60137      +/-   ##
==========================================
+ Coverage   88.55%   89.77%   +1.22%     
==========================================
  Files         704      672      -32     
  Lines      208087   203908    -4179     
  Branches    40019    39202     -817     
==========================================
- Hits       184266   183058    -1208     
+ Misses      15818    13169    -2649     
+ Partials     8003     7681     -322     

Files with missing lines	Coverage Δ
lib/events.js	99.83% <100.00%> (+<0.01%)	⬆️

... and 332 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[simonkcleung]

Could delete 2 more lines:

del 690-691, 715-716

+718 if (events.removeListener !== undefined)
+719 this.emit('removeListener', type, listener);

[Han5991]

Could delete 2 more lines:

del 690-691, 715-716

+718 if (events.removeListener !== undefined) +719 this.emit('removeListener', type, listener);

Delete lines 690-691, 715-716, and add consolidated emit at 718-719.

The Problem I Found

When I tried this, the test failed because:

• Line 704-705: if (position < 0) return this; - early return when listener
  not found
• If we move emit to line 718, it would never execute after this early return
• But we need it to execute when removal succeeds in the array case

Current State

I kept both emit calls inside their respective branches:

• Line 692-693: emit after single listener removal
• Line 716-717: emit after array listener removal

Your consolidation proposal cannot work due to the early return at line
704-705. The emit must happen before the function can return.

[simonkcleung]

Sorry if I am wrong.
if (position < 0) return this; means - if no listener found in the list, then do nothing.
Because there is no listener being removed, Not emitting 'removeListener' event is fine.
If the test failed, it should also failed before the change.

And I think

if (list === listener || list.listener === listener) {
... ...
if (events.removeListener !== undefined)
 this.emit('removeListener', type, listener);
} else if (typeof list !== 'function') {
... ...
if (events.removeListener !== undefined)
 this.emit('removeListener', type, listener);
}

is same as

if (list === listener || list.listener === listener) {
...
} else if (typeof list !== 'function') {
...
}
if (events.removeListener !== undefined)
 this.emit('removeListener', type, listener);

[Han5991]

Sorry if I am wrong. if (position < 0) return this; means - if no listener found in the list, then do nothing. Because there is no listener being removed, Not emitting 'removeListener' event is fine. If the test failed, it should also failed before the change.

And I think

if (list === listener || list.listener === listener) {
... ...
if (events.removeListener !== undefined)
 this.emit('removeListener', type, listener);
} else if (typeof list !== 'function') {
... ...
if (events.removeListener !== undefined)
 this.emit('removeListener', type, listener);
}

is same as

if (list === listener || list.listener === listener) {
...
} else if (typeof list !== 'function') {
...
}
if (events.removeListener !== undefined)
 this.emit('removeListener', type, listener);

@simonkcleung Thank you for the suggestion! I explored consolidating the emit calls, but found a issue:

Edge Case Problem:
If both conditions fail (listener not found), the code would still emit removeListener event incorrectly.

Test case (line 42-48):

ee.on('hello', listener1);
ee.on('removeListener', common.mustNotCall());
ee.removeListener('hello', listener2);  // listener2 never added - should NOT emit

Performance:
Consolidation would require a tracking variable (let removed or let removedListener), adding overhead to every removeListener() call.

The current approach with duplicate emit calls is actually optimal:
- Zero overhead (no extra variables)
- Handles edge cases correctly
- Properly handles wrapped listeners (list.listener || listener vs listener)

I've kept the duplicate emits while fixing the original bug. Thanks for the detailed review!

[simonkcleung]

@simonkcleung Thank you for the suggestion! I explored consolidating the emit calls, but found a issue:

Edge Case Problem: If both conditions fail (listener not found), the code would still emit removeListener event incorrectly.

Test case (line 42-48):

ee.on('hello', listener1);
ee.on('removeListener', common.mustNotCall());
ee.removeListener('hello', listener2);  // listener2 never added - should NOT emit

Performance:
Consolidation would require a tracking variable (let removed or let removedListener), adding overhead to every removeListener() call.

The current approach with duplicate emit calls is actually optimal:
- Zero overhead (no extra variables)
- Handles edge cases correctly
- Properly handles wrapped listeners (list.listener || listener vs listener)

I've kept the duplicate emits while fixing the original bug. Thanks for the detailed review!

You are right.

[nodejs-github-bot]

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

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/60137
✔  Done loading data for nodejs/node/pull/60137
----------------------------------- PR info ------------------------------------
Title      Fix events remove listener emission (#60137)
Author     sangwook <rewq5991@gmail.com> (@Han5991)
Branch     Han5991:fix-events-remove-listener-emission -> nodejs:main
Labels     events, author ready, needs-ci, commit-queue-rebase
Commits    2
 - test: ensure removeListener event fires for once() listeners
 - fix: emit removeListener event when last listener is removed
Committers 1
 - sangwook <bnbt3@naver.com>
PR-URL: https://github.com/nodejs/node/pull/60137
Fixes: https://github.com/nodejs/node/issues/59977
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/60137
Fixes: https://github.com/nodejs/node/issues/59977
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 07 Oct 2025 05:54:03 GMT
   ✔  Approvals: 2
   ✔  - Aviv Keller (@avivkeller): https://github.com/nodejs/node/pull/60137#pullrequestreview-3668276886
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/60137#pullrequestreview-3702227533
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-01-16T01:17:25Z: https://ci.nodejs.org/job/node-test-pull-request/70824/
- Querying data for job/node-test-pull-request/70824/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 60137
From https://github.com/nodejs/node
 * branch                  refs/pull/60137/merge -> FETCH_HEAD
✔  Fetched commits as 4bc42c0ac85c..d322e4de739c
--------------------------------------------------------------------------------
[main 6b05ddbc1c] test: ensure removeListener event fires for once() listeners
 Author: sangwook <bnbt3@naver.com>
 Date: Tue Oct 7 12:38:02 2025 +0900
 1 file changed, 16 insertions(+)
Auto-merging lib/events.js
[main 43a9d905a5] fix: emit removeListener event when last listener is removed
 Author: sangwook <bnbt3@naver.com>
 Date: Tue Oct 7 13:08:22 2025 +0900
 1 file changed, 3 insertions(+), 2 deletions(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
(node:2355) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated.
(Use `node --trace-deprecation ...` to show where the warning was created)
Rebasing (2/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: ensure removeListener event fires for once() listeners
Adds test coverage for the removeListener event being emitted

when a once() listener is automatically removed after execution.

This verifies that streams and other EventEmitters correctly

emit removeListener events when once() wrappers clean up.
PR-URL: #60137

Fixes: #59977

Reviewed-By: Aviv Keller <me@aviv.sh>

Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
[detached HEAD 470ecd6e2b] test: ensure removeListener event fires for once() listeners

Author: sangwook <bnbt3@naver.com>

Date: Tue Oct 7 12:38:02 2025 +0900

1 file changed, 16 insertions(+)

Rebasing (3/4)

Rebasing (4/4)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

fix: emit removeListener event when last listener is removed
When the last listener is removed and _eventsCount becomes 0,

the removeListener event was not being emitted because the check

was inside the else block. This moves the removeListener emission

outside the conditional to ensure it always fires when a listener

is removed.
PR-URL: #60137

Fixes: #59977

Reviewed-By: Aviv Keller <me@aviv.sh>

Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
[detached HEAD f7c3aaf02d] fix: emit removeListener event when last listener is removed

Author: sangwook <bnbt3@naver.com>

Date: Tue Oct 7 13:08:22 2025 +0900

1 file changed, 3 insertions(+), 2 deletions(-)

Successfully rebased and updated refs/heads/main.
✔  470ecd6e2be4072e433ed660e63b41fc7c9ee499

✔  0:0      no Co-authored-by metadata                co-authored-by-is-trailer

✔  7:7      Valid fixes URL.                          fixes-url

✔  0:0      blank line after title                    line-after-title

✔  0:0      line-lengths are valid                    line-length

✔  0:0      metadata is at end of message             metadata-end

✔  6:8      PR-URL is valid.                          pr-url

✔  0:0      reviewers are valid                       reviewers

✔  0:0      valid subsystems                          subsystem

✔  0:0      Title is formatted correctly.             title-format

⚠  0:50     Title should be <= 50 columns.            title-length

✖  f7c3aaf02d84bfcef92f53411fe3e4ff6d5ecfbb

✔  0:0      no Co-authored-by metadata                co-authored-by-is-trailer

✔  8:7      Valid fixes URL.                          fixes-url

✔  0:0      blank line after title                    line-after-title

✔  0:0      line-lengths are valid                    line-length

✔  0:0      metadata is at end of message             metadata-end

✔  7:8      PR-URL is valid.                          pr-url

✔  0:0      reviewers are valid                       reviewers

✖  0:0      Invalid subsystem: "fix"                  subsystem

✔  0:0      Title is formatted correctly.             title-format

⚠  0:50     Title should be <= 50 columns.            title-length
ℹ  Please fix the commit message and try again.

Please manually ammend the commit message, by running

git commit --amend

Once commit message is fixed, finish the landing command running

git node land --continue

https://github.com/nodejs/node/actions/runs/21318975549

[mertcanaltin]

lgtm