Skip to content

node:events: fix ReferenceError in EventEmitter.prototype.listenerCount - #34667

Merged
Jarred-Sumner merged 1 commit into
mainfrom
farm/f0010282/fix-listenercount-referenceerror
Jul 19, 2026
Merged

Jarred-Sumner merged 1 commit into
mainfrom
farm/f0010282/fix-listenercount-referenceerror

Conversation

@robobun

@robobun robobun commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Repro

const { EventEmitter } = require("events");
new EventEmitter().listenerCount("x");
ReferenceError: listenerCountSlow is not defined
      at listenerCount (node:events:450:29)

Every single-argument emitter.listenerCount(type) call hits this, which broke test/js/node/http2/node-http2.test.js (38 fail) via emitSessionCloseNT calling listenerCount("close"), along with anything else that counts listeners.

Cause

#34605 (efab3b2) removed the listenerCountSlow helper when it dropped the fallback path from the static events.listenerCount, but EventEmitterPrototype.listenerCount (added in a227ad9) still called it for the common 1-argument form.

Fix

Inline the lookup into the prototype method, structured the same way as Node's own listenerCount: read _events[type] once, then handle the bare-function and array storage shapes for both the 1-arg (return 1 / handlers.length) and 2-arg (match against method) forms.

Verification

  • bun bd test test/js/node/events/event-emitter.test.ts: 70 pass
  • New EventEmitter.prototype.listenerCount test fails on main with ReferenceError: listenerCountSlow is not defined, passes with the fix
  • All 26 test/js/node/test/parallel/test-event-emitter-*.js pass
  • Test expectations checked against Node.js

efab3b2 removed the listenerCountSlow helper but left a call to it in
EventEmitterPrototype.listenerCount, so any emitter.listenerCount(type)
call (without the second listener argument) threw ReferenceError.

Inline the lookup into the prototype method, structured like Node's own
implementation: read _events[type] once, then handle the bare-function
and array storage shapes for both the 1-arg and 2-arg forms.
@robobun

robobun commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 6:05 PM PT - Jul 18th, 2026

@robobun, your commit 3a53a9a is building: #75562

@robobun

robobun commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator Author

Reproduced with bun bd -e 'new (require("events").EventEmitter)().listenerCount("x")' on main (dd88b5c): ReferenceError: listenerCountSlow is not defined. Fix applied; 70 tests pass in event-emitter.test.ts, all 26 test-event-emitter-*.js parallel tests pass.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

EventEmitterPrototype.listenerCount now directly counts stored handlers for absent, single-function, and array-based listener representations. Tests cover specific listener matching, multiple registrations, once listeners, and null/undefined count-all behavior.

Changes

Listener count behavior

Layer / File(s) Summary
Listener count implementation and validation
src/js/node/events.ts, test/js/node/events/event-emitter.test.ts
listenerCount directly evaluates stored handlers and tests counts for missing listeners, matching listeners, multiple registrations, once listeners, and omitted or null listener arguments.

Possibly related PRs

  • oven-sh/bun#34519: Updates the same listenerCount implementation and related tests.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main fix: preventing a ReferenceError in EventEmitter.prototype.listenerCount.
Description check ✅ Passed The description covers what changed and how it was verified, even though it uses custom headings instead of the template labels.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/node/events/event-emitter.test.ts`:
- Around line 919-944: Extend the EventEmitter.prototype.listenerCount test
immediately after the first ee.on("x", a) call with null and undefined
listener-argument assertions expecting a count of 1, covering count-all behavior
when a single function is stored. Keep the existing array-storage assertions
unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b87db90b-121d-4cda-8cf7-00483452ed18

📥 Commits

Reviewing files that changed from the base of the PR and between dd88b5c and 3a53a9a.

📒 Files selected for processing (2)
  • src/js/node/events.ts
  • test/js/node/events/event-emitter.test.ts

Comment thread test/js/node/events/event-emitter.test.ts

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — straightforward regression fix for a dangling reference to a deleted helper.

What was reviewed:

  • New listenerCount body matches Node's reference implementation structure (bare-function vs array storage, method != null gating both branches).
  • handlers === undefined check covers the shape-mode case where removed listeners are stored as undefined rather than deleted.
  • Test covers all storage shapes (empty / single function / array), the .listener unwrap for once() wrappers, and null/undefined method arg — verified expectations against Node.
Extended reasoning...

Overview

EventEmitterPrototype.listenerCount was calling listenerCountSlow, a helper deleted in #34605, so every 1-arg emitter.listenerCount(type) call threw a ReferenceError on main. This PR inlines the lookup: read this._events?.[type] once, then branch on function-vs-array storage shape for both the 1-arg (count all) and 2-arg (count matching method) forms. A new test in event-emitter.test.ts covers all combinations.

Security risks

None. Pure JS, no untrusted input parsing, no privilege boundaries.

Level of scrutiny

Low-to-medium. This is a hot-path built-in module, but the change is a small, mechanical inline of previously-working logic that mirrors Node's own listenerCount almost line-for-line. The old code's falsy check (!handlers) became a strict === undefined check, which matches Node and correctly handles the kShapeMode path where cleared slots hold undefined. Handlers that aren't undefined or a function are always arrays (per _addListener), so handlers.length is safe.

Other factors

The regression was already breaking 38 tests in node-http2.test.js on main, so this is a build-fixer. The new test would have failed on main with the exact ReferenceError, and the PR reports all 26 test-event-emitter-*.js parallel tests pass. No prior reviewer comments to address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants