Skip to content

node:events: mutate listener arrays in place; copy only when emit() has iterated them - #35823

Open
robobun wants to merge 4 commits into
mainfrom
farm/843594f7/events-inplace-mutate
Open

node:events: mutate listener arrays in place; copy only when emit() has iterated them#35823
robobun wants to merge 4 commits into
mainfrom
farm/843594f7/events-inplace-mutate

tighten kIterated comments: one invariant doc at the symbol, drop the…

ac05716
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 26, 2026 in 22m 2s

Code review found 1 potential issue

Found 2 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/js/node/events.ts:475-478 spliceOne uses non-primordial list.pop()

Annotations

Check warning on line 478 in src/js/node/events.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

spliceOne uses non-primordial list.pop()

nit: `spliceOne` calls `list.pop()` directly, which routes through user-overridable `Array.prototype.pop` — inconsistent with `_addListener` in this same PR, which uses the captured primordial `ArrayPrototypeUnshift.$call` and intrinsic `$arrayPush`. Consider capturing `const ArrayPrototypePop = Array.prototype.pop;` alongside `ArrayPrototypeUnshift` at the top of the file and calling `ArrayPrototypePop.$call(list)`. (Node's own `spliceOne` also uses raw `.pop()`, so this is consistency within t