ref(node): Remove two Node <20 leftovers (hrtime.bigint, undici channel refs) - #24183
Merged
Merged
Conversation
…el refs) Both were guarded by a condition that has since passed: the engine floor moved to Node >=20.19.0 in 4f03438. - anr/worker.ts: `createHrTimer()` now uses `process.hrtime.bigint()`; the TODO asking for it was conditioned on dropping Node 8. - node-fetch/undici-instrumentation.ts: `_channelSubs` only existed to keep a reference alive around nodejs/node#42170, which needed Node 18.18.0 support. Replaced with the `_isInstrumented` boolean its own comment proposed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JGK2ap3HHAy2WuDwoS8yTW
mydea
approved these changes
Sep 7, 2026
chargome
approved these changes
Sep 7, 2026
chargome
marked this pull request as ready for review
September 7, 2026 14:53
chargome
requested review from
logaretm and
stephanie-anderson
and removed request for
a team
September 7, 2026 14:53
Member
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 43ea6f7. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both notes named the exact condition for their own removal, and both conditions passed when the engine floor moved to Node
>=20.19.0in 4f03438.createHrTimer()'s TODO was conditioned on dropping Node 8;process.hrtime.bigint()has been available since Node 10.7.getTimeMs()still returns an integernumberof milliseconds — BigInt division truncates toward zero and the elapsed delta is never negative, so it yields the same value the previousMath.floor(seconds * 1e3 + nanoSeconds / 1e6)did._channelSubsexisted only to hold a reference around nodejs/node#42170, which mattered while Node 18.18.0 was supported. It is replaced by the_isInstrumentedboolean its own comment proposed. Nothing else in the repo read the array — the only uses were the.lengthguard and the.push(). The flag is now set before subscribing rather than after, so a re-entrant call cannot double-subscribe; the old.lengthcheck only flipped after the first push, making this equal-or-stricter.Fixes #24182
🤖 Generated with Claude Code
https://claude.ai/code/session_01JGK2ap3HHAy2WuDwoS8yTW