fix(node/events): allow EventEmitter to be invoked without new

[jeswr]

Linked issue

Closes #552

Problem

In Node.js, EventEmitter is a plain function, so it can be invoked without the new keyword. Libraries inherit from it using the prototypal EventEmitter.call(this) pattern, for example readable-stream and N3.js.

unenv implements EventEmitter as an ES class. ES class constructors cannot be [[Call]]-ed, so EventEmitter.call(this) throws:

TypeError: Class constructor _EventEmitter cannot be invoked without 'new'

This breaks any consumer that uses the function-style inheritance pattern.

Reproduction (before this PR)

import { EventEmitter } from "unenv/runtime/node/events";

function MyStream() {
  EventEmitter.call(this); // ❌ throws: Class constructor cannot be invoked without 'new'
}
Object.setPrototypeOf(MyStream.prototype, EventEmitter.prototype);
Object.setPrototypeOf(MyStream, EventEmitter);

new MyStream();

Fix

The constructor body is extracted into a shared initEventEmitter function (mirroring Node's EventEmitter.init), and the exported EventEmitter is wrapped in a Proxy whose apply trap runs that initialization against the provided this:

const _EventEmitter = new Proxy(_EventEmitterClass, {
  apply(_target, thisArg, args) {
    initEventEmitter.call(thisArg, args[0]);
    return thisArg;
  },
});

This keeps the existing class (and all of its methods, statics, and subclasses such as EventEmitterAsyncResource) untouched, while making the constructor callable as a plain function — matching Node.js behavior:

• new EventEmitter() → unchanged (construct trap is the default).
• EventEmitter.call(this) → initializes this in place.
• class X extends EventEmitter {} → unchanged.
• Static members (EventEmitter.once, defaultMaxListeners, …) and instanceof → unchanged (forwarded by the Proxy).

There is no hot-path overhead: instance method calls (emitter.emit()) never go through the Proxy — only construction and static access do.

Tests

Adds test/node-events.test.ts covering:

• function-style EventEmitter.call(this) inheritance (the regression),
• new EventEmitter(),
• class extends EventEmitter,
• static member access through the wrapper.

pnpm test:types and pnpm build pass. The two failures in test/env.test.ts and test/node-coverage.test.ts on my machine are pre-existing and unrelated (they reproduce on a clean checkout under Node 25 — new builtin modules / changed punycode deprecation-warning format).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor

  • Restructured EventEmitter internals to ensure consistent initialization, preserve callable-without-new semantics, maintain subclassing behavior, and keep options (like capture-rejections) and dynamic defaultMaxListeners in sync across invocation paths.

• Tests

  • Added comprehensive tests validating callable invocation, .call behavior, strict-mode errors, listener registration, event emission, inheritance, static member accessibility, and option propagation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3cf99b83-fe1a-46b0-8461-46f2a1acb290

📥 Commits

Reviewing files that changed from the base of the PR and between ff9ccaa and 2442474.

📒 Files selected for processing (2)

• src/runtime/node/internal/events/events.ts
• test/node-events.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/runtime/node/internal/events/events.ts

---

📝 Walkthrough

Walkthrough

EventEmitter was refactored: initialization moved to a shared initEventEmitter helper, the implementation class became non-exported _EventEmitterClass, and a Proxy-wrapped callable _EventEmitter was exported to allow function-style invocation; tests were added for callable and constructor behaviors.

Changes

EventEmitter Callable Invocation Support

Layer / File(s)	Summary
Class declaration and constructor delegation src/runtime/node/internal/events/events.ts	_EventEmitter renamed to non-exported _EventEmitterClass implements NodeEventEmitter; constructor delegates initialization to initEventEmitter(this, opts) and instance _maxListeners now starts undefined.
Initialization helper and Proxy-wrapped export src/runtime/node/internal/events/events.ts	Added initEventEmitter(this, opts?) and exported _EventEmitter as a Proxy that runs initEventEmitter on function-style calls while preserving constructor flow and static members; added export type _EventEmitter = _EventEmitterClass.
Callable invocation and static member tests test/node-events.test.ts	Vitest suite verifying callable-without-new, .call receiver initialization and return, TypeError on missing receiver, constructor on/emit behavior, subclassing support, options forwarding (captureRejections), defaultMaxListeners propagation, and static member exposure through the callable wrapper.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Proxy__EventEmitter as _EventEmitter (Proxy)
  participant initEventEmitter
  participant _EventEmitterClass
  Caller->>Proxy__EventEmitter: call(thisArg, opts?)
  Proxy__EventEmitter->>initEventEmitter: initEventEmitter.call(thisArg, opts)
  initEventEmitter->>thisArg: initialize internal state (_events, _eventsCount, _maxListeners, kCapture)
  Caller->>Proxy__EventEmitter: new EventEmitter(opts)
  Proxy__EventEmitter->>_EventEmitterClass: constructor(opts) -> delegates to initEventEmitter

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A helper hopped in, neat and spry,
A Proxy let EventEmitter reply,
New or called, it wakes the same,
Subclass, emit — the rules remain,
The rabbit cheers: "Node-like behavior, by name!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(node/events): allow EventEmitter to be invoked without new' directly and clearly describes the main change in the pull request.
Linked Issues check	✅ Passed	The PR implementation fully addresses all coding requirements from issue #552: EventEmitter can now be invoked without new, function-style calls via .call(this) work correctly, prototypal inheritance is supported, and all existing behaviors (subclassing, statics, instanceof) are preserved.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the EventEmitter callable-without-new feature: Proxy-wrapping the emitter class, extracting initialization logic, updating internal helper types, and adding comprehensive tests. No unrelated changes are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds Node-compatible EventEmitter callable behavior (supporting EventEmitter.call(this)) by wrapping the internal class with a Proxy, and introduces Vitest coverage to ensure common inheritance/usage patterns keep working.

Changes:

• Refactors _EventEmitter initialization into a shared initEventEmitter() helper.
• Wraps the internal _EventEmitter class in a Proxy to support function-style invocation.
• Adds a new Vitest suite covering call-without-new, new, subclassing, and static members.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
test/node-events.test.ts	Adds tests validating Node-compatible EventEmitter invocation and inheritance patterns.
src/runtime/node/internal/events/events.ts	Refactors initialization and introduces a Proxy wrapper to allow calling the emitter like a function.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/runtime/node/internal/events/events.ts (1)

79-82: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix _maxListeners parity between new EventEmitter() and EventEmitter.call(this)

_EventEmitterClass initializes _maxListeners from defaultMaxListeners for new, while the Proxy apply path skips class field initializers so initEventEmitter leaves _maxListeners as undefined; _getMaxListeners then falls back to the (possibly updated) defaultMaxListeners. This makes new instances pinned to the old default while call(this) instances track later changes.

Suggested fix

 class _EventEmitterClass implements NodeEventEmitter {
   // Internal state
   _events: any = undefined;
   _eventsCount: number = 0;
-  _maxListeners: number | undefined = defaultMaxListeners;
+  _maxListeners: number | undefined = undefined;
   [kCapture]: boolean = false;
 }

🤖 Prompt for 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.

In `@src/runtime/node/internal/events/events.ts` around lines 79 - 82, The Proxy
apply path (EventEmitter.call(this)) skips class field initializers so instances
created via initEventEmitter may leave _maxListeners undefined and later inherit
a changed defaultMaxListeners, causing parity differences with new
EventEmitter(); modify initEventEmitter to explicitly initialize
this._maxListeners to defaultMaxListeners when it is undefined (or not a number)
so both construction paths behave identically; update initEventEmitter and
mention _maxListeners, defaultMaxListeners, _EventEmitterClass, and
_getMaxListeners as the targeted symbols to locate the change.

🧹 Nitpick comments (1)

test/node-events.test.ts (1)

5-23: ⚡ Quick win

Consider adding a test for options parameter in callable invocation.

The current test validates callable invocation without arguments. Consider adding a test case that verifies options (e.g., captureRejections) are properly handled when passed through the callable path:

EventEmitter.call(this, { captureRejections: true });

This would ensure complete parity between the constructor and callable invocation paths for all initialization parameters.

🤖 Prompt for 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.

In `@test/node-events.test.ts` around lines 5 - 23, Add a new test variant that
verifies callable invocation forwards the options object to EventEmitter: change
the MyStream constructor in a new it-block to call EventEmitter.call(this, {
captureRejections: true }) (instead of EventEmitter.call(this)), create the
instance via new (MyStream as any)(), then assert that the instance has the
option applied (for example expect((stream as
any).captureRejections).toBe(true)) and that normal event wiring still works
(listenerCount/emit/received). This ensures the callable path (MyStream ->
EventEmitter.call) preserves the options parameter.

🤖 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.

Outside diff comments:
In `@src/runtime/node/internal/events/events.ts`:
- Around line 79-82: The Proxy apply path (EventEmitter.call(this)) skips class
field initializers so instances created via initEventEmitter may leave
_maxListeners undefined and later inherit a changed defaultMaxListeners, causing
parity differences with new EventEmitter(); modify initEventEmitter to
explicitly initialize this._maxListeners to defaultMaxListeners when it is
undefined (or not a number) so both construction paths behave identically;
update initEventEmitter and mention _maxListeners, defaultMaxListeners,
_EventEmitterClass, and _getMaxListeners as the targeted symbols to locate the
change.

---

Nitpick comments:
In `@test/node-events.test.ts`:
- Around line 5-23: Add a new test variant that verifies callable invocation
forwards the options object to EventEmitter: change the MyStream constructor in
a new it-block to call EventEmitter.call(this, { captureRejections: true })
(instead of EventEmitter.call(this)), create the instance via new (MyStream as
any)(), then assert that the instance has the option applied (for example
expect((stream as any).captureRejections).toBe(true)) and that normal event
wiring still works (listenerCount/emit/received). This ensures the callable path
(MyStream -> EventEmitter.call) preserves the options parameter.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 12f078ad-1b16-4c3c-8c03-8162696bc47d

📥 Commits

Reviewing files that changed from the base of the PR and between f89b7cc and 6237fef.

📒 Files selected for processing (2)

• src/runtime/node/internal/events/events.ts
• test/node-events.test.ts

[jeswr]

Thanks @coderabbitai — both items from your review are addressed in 2442474:

🟠 _maxListeners parity (major): Confirmed against Node. Native new EventEmitter() leaves _maxListeners unset so getMaxListeners() resolves it dynamically and tracks later changes to defaultMaxListeners; unenv was pinning it to the construction-time value via the class field (and the .call(this) path, which skips field initializers, already left it undefined — so the two paths also diverged from each other). Changed the field initializer to undefined so both paths match Node. Added a regression test asserting both new and .call instances reflect a later defaultMaxListeners change.

🧹 Options forwarding (nitpick): Added a test that invokes EventEmitter.call(this, { captureRejections: true }) and asserts the option is applied (and normal event wiring still works), locking in option parity between the constructor and callable paths.

tsc, eslint, prettier and build are clean; test/node-events.test.ts is now 8/8.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@jeswr: Thanks for the thorough follow-up! Both fixes look exactly right:

• Setting _maxListeners to undefined so getMaxListeners() resolves it dynamically is the correct Node.js-faithful behavior, and having a regression test that mutates defaultMaxListeners after construction guards against future regressions on both paths.
• The captureRejections options-parity test cleanly locks in that the callable path is a true behavioral equivalent of the constructor path.

8/8 with clean tooling — great work! 🎉

(ﾉ◕ヮ◕)ﾉ*:･ﾟ✧

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!