refactor: replace ext/web timer imports with core timers or node:timers#33121
Open
bartlomieju wants to merge 3 commits intomainfrom
Open
refactor: replace ext/web timer imports with core timers or node:timers#33121bartlomieju wants to merge 3 commits intomainfrom
bartlomieju wants to merge 3 commits intomainfrom
Conversation
- ext/node polyfills: use node:timers directly instead of going through ext:deno_web/02_timers.js - ext/websocket, ext/fetch, runtime: use core.createTimer/cancelTimer directly instead of the web timer wrappers - Remove unused webClearTimeout and timerId imports from http.ts - Remove unused FunctionPrototypeCall from 99_main.js Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
core.cancelTimer crashes on null unlike web clearTimeout which was a no-op. Add null guards at all call sites. Also remove the stale clearTimeout(this._timeout[timerId]) line from http.ts and the now unused clearTimeout import. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
System timers (isSystem=true) are excluded from the ops sanitizer's timer leak detection. WebSocket idle timeout, EventSource reconnection, and the window.close exit timer are all internal system timers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
http.ts,pipe_wrap.ts,_util/async.ts): importsetTimeout/clearTimeoutfromnode:timersinstead ofext:deno_web/02_timers.js01_websocket.js): usecore.createTimer/core.cancelTimerdirectly for idle timeout management27_eventsource.js): usecore.createTimer/core.cancelTimerdirectly for SSE reconnection99_main.js): usecore.createTimerfor the exit delay timerwebClearTimeout/timerIdimports fromhttp.tsFunctionPrototypeCallfrom99_main.jsFollow-up to #33118 -- now that Node.js timers are the default, ext/node polyfills should use them directly, and internal Deno plumbing should use core timers instead of the web timer wrappers.
Test plan
./x test-unit timerspassessetTimeout/clearTimeoutwork correctly🤖 Generated with Claude Code