Skip to content

Remove dead code from napi/NodeVM C++, zlib, io event-loop, net.ts - #36742

Merged
Jarred-Sumner merged 15 commits into
mainfrom
claude/farm/3e173eb6/dead-code-napi-nodevm-libuv-zlib
Aug 2, 2026
Merged

Remove dead code from napi/NodeVM C++, zlib, io event-loop, net.ts#36742
Jarred-Sumner merged 15 commits into
mainfrom
claude/farm/3e173eb6/dead-code-napi-nodevm-libuv-zlib

JSEnvironmentVariableMap: shrink proxy-var DontEnum comment to two lines

4310491
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Aug 1, 2026 in 22m 17s

Code review found 4 potential issues

Found 5 candidates, confirmed 4. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 4
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/io/windows_event_loop.rs:156-157 FilePoll::ref_/activate removal left three orphans
🟡 Nit src/jsc/bindings/napi.h:722-723 napi.h removals left stale comment and forward-decls

Annotations

Check warning on line 157 in src/io/windows_event_loop.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

FilePoll::ref_/activate removal left three orphans

Removing `FilePoll::ref_`/`activate`/`can_ref` left three orphans behind: (1) `declare_scope!(FilePoll, visible)` at line 16 — the two removed methods held the only `scoped_log!(FilePoll, ...)` calls in the tree; (2) the doc comment "/// Make calling ref() on this poll into a no-op." on `disable_keeping_process_alive` (~line 60) now names a method that no longer exists on Windows `FilePoll`; (3) `posix_event_loop::FilePoll::ref_` (posix_event_loop.rs:562-568) — the PR's own justification ("No ca

Check warning on line 723 in src/jsc/bindings/napi.h

See this annotation in the file changed.

@claude claude / Claude Code Review

napi.h removals left stale comment and forward-decls

Two small orphans from the napi.h deletions: (1) `NapiRef.cpp:17-18` still says "isSet() will return always true after being set once / We cannot rely on isSet() to check if the value is set" — with `NapiWeakValue::isSet()` removed, this now references a nonexistent method (same treatment as the `jsSetterEnvironmentVariable` comment you already updated). (2) The `namespace JSC { class JSGlobalObject; class JSSourceCode; }` block at napi.h:651-654 sat immediately above the removed `generateSource