perf(ext/web): convert all ext/web JS sources to lazy-loaded scripts#33760
Conversation
Convert all remaining ext/web JavaScript files from ESM modules to IIFE-wrapped lazy-loaded scripts using `core.loadExtScript()`. This builds on the initial `loadExtScript()` infrastructure from #33739 which converted 3 files. All 24 ext/web JS files are now in `lazy_loaded_js` instead of `esm`, avoiding V8's ES module infrastructure overhead (~31% snapshot size, ~4x serialization cost). External consumers across ext/, runtime/, and cli/ updated from `import` to `core.loadExtScript()`. All `createLazyLoader()` calls referencing ext:deno_web files replaced with `loadExtScript()` since the targets are now plain scripts, not ESM modules.
These files are loaded on-demand at runtime (not during snapshotting), so they need ESM format to access core via import rather than globalThis.__bootstrap which is deleted after bootstrap. Also fixes dlint no-unused-vars in 13_message_port.js.
ops on core.ops are deleted at bootstrap by removeImportedOps(). Destructuring at load time captures the function reference before deletion, matching how other IIFE scripts handle ops.
fibibot
left a comment
There was a problem hiding this comment.
Substance is interesting (converting all 24 ext/web JS files from ESM to lazy-loaded scripts on top of #33739) but two real concerns are blocking merge readiness.
Real V8 fatal in unit::canvas_test
test unit debug linux-x86_64 (and the same shard across all 8 debug platforms × release linux-x86_64) crashes the runtime — not just fails an assertion:
offscreenCanvasConstructor ... FAILED (1ms)
#
# Fatal error in v8::Module::GetModuleNamespace
# v8::Module::GetModuleNamespace must be used on an instantiated module
#
Deno should not have died with a signal follows. This is a hard-crash, not a test-output mismatch. The diagnostic points directly at the conversion: something downstream of the OffscreenCanvas constructor (the recently-landed #29357 surface) is calling v8::Module::GetModuleNamespace on what was a real ESM module before this PR but is now a non-module IIFE script. After the conversion, that V8-side namespace lookup has nothing to bind to.
Worth grepping the ext/canvas and ext/webgpu JS surface for any import.meta, core.getModuleNamespace, prepareLoad-style references to ext:deno_web modules — those are the call sites that need migrating in lockstep with the script conversion. The 9 CI failures all cascade from this one V8 abort (the 8 unit-debug shards plus wpt release linux-x86_64 which probably exercises the same OffscreenCanvas path).
Test plan checkbox is unchecked
The PR body's - [ ] Full test suite box is unchecked, and the smoke-test list in the body ("console, URL, TextEncoder/TextDecoder, ReadableStream, Blob, File, Performance, BroadcastChannel, MessagePort, CompressionStream, DOMException, timers") notably does not include OffscreenCanvas / canvas / webgpu — which is exactly where the regression surfaces. Worth running the full unit suite locally before pushing the next iteration; the V8 abort would have shown up immediately.
Minor: import.meta.log workaround acknowledged
The note in the PR body about import.meta.log in 06_streams.js being replaced with core.print() (because import.meta is module-only) is honest, and the suggestion that "a proper core.log equivalent may be needed" is the right framing. core.print writes to stdout/stderr without log-level routing, which is a regression for anything that relied on the implicit import.meta.log formatting. Worth landing the core.log shim in a precursor PR before this one merges, so the workaround can be undone.
Escalation
This is cross-cutting infrastructure — switching the entire ext/web JS surface from ESM to scripts, plus updating all external consumers across ext/, runtime/, and cli/. 79 files, 542+/394-. The performance benefit is real (lazy scripts skip the ESM module-graph instantiation overhead) but the blast radius is large enough that the V8 abort is unlikely to be the only place this breaks — every import.meta.* reference, every core.createLazyLoader / propNonEnumerableLazyLoaded call in the affected files needs auditing. Worth keeping in mind for the next pass.
Not blocking on substance — happy to re-review once the OffscreenCanvas crash is sorted and the full test suite is green.
09_file.js is now a lazy_loaded_js script, not an ESM module, so it cannot be imported via static import.
fibibot
left a comment
There was a problem hiding this comment.
LGTM at 9062c0d. The OffscreenCanvas V8 abort I'd flagged in my prior review is fixed cleanly:
// ext/canvas/01_canvas.js
-import { Blob } from "ext:deno_web/09_file.js";
+const { Blob } = core.loadExtScript("ext:deno_web/09_file.js");Three-line fix that addresses the exact diagnosis from the prior review — 09_file.js is now a lazy_loaded_js script rather than an ESM module, so the static import was triggering v8::Module::GetModuleNamespace must be used on an instantiated module. Switching to core.loadExtScript with destructuring routes through the script-loading path that does work for the new shape.
Also noted the earlier intermediate fix bb10746 correctly carved out geometry.js and webtransport.js as lazy_loaded_esm rather than scripts, since those are loaded on-demand at runtime (not during snapshot) — that's the right scope decision and worth keeping.
CI 121/123 success at this head, 1 in-progress (canary), 1 skip, 0 fail. All 58 of the relevant lint debug / test unit / test node_compat / wpt shards across 4 platforms × debug+release are SUCCESS, including the previously-crashing unit::canvas_test.
Two follow-up items from my prior review carry forward as non-blocking notes for future iterations:
- The PR body's
- [ ] Full test suitecheckbox is still unchecked, but CI is now in fact running it green — stale doc, not a blocker. - The
import.meta.log→core.print()workaround in06_streams.js:170is still in place. The PR-body note ("A propercore.logequivalent may be needed") remains the right framing — worth landing acore.logshim in a follow-up so this workaround can be undone.
…scripts (#33779) ## Summary Follow-up to #33760. Converts the ESM JS sources in `ext/io`, `ext/os`, and `ext/net` to IIFE-wrapped lazy-loaded scripts (`lazy_loaded_js`), eliminating ESM module resolution overhead. These three extensions have no cross-extension ESM import dependencies (only `core.loadExtScript` calls to already-converted extensions like `ext/web`), so no `internals` bridges are needed. Converting these also unblocks clean conversion of: - `ext/fs` (depends on `ext/io`) - `ext/fetch` (depends on `ext/net`)
Adapt enqueuePerformanceEntry/registerExtraEntryType to lazy-loaded script pattern from denoland#33760: 15_performance.js now returns its exports from an IIFE; consumers (perf_hooks.js, http2.ts) load via core.loadExtScript instead of static ESM import. Co-authored-by: Divy Srivastava <[email protected]>
Follow-up to #33760. Converts `ext/ffi/00_ffi.js` from ESM to an IIFE-wrapped lazy-loaded script (`lazy_loaded_js`). Signed-off-by: Bartek Iwańczuk <[email protected]>
Follow-up to #33760 and #33779. Converts all 8 ESM JS sources in `ext/fetch` to IIFE-wrapped lazy-loaded scripts (`lazy_loaded_js`). - `20_headers.js`, `21_formdata.js`, `22_body.js`, `22_http_client.js`, `23_request.js`, `23_response.js`, `26_fetch.js`, `27_eventsource.js` - `26_fetch.js` imports from `ext/telemetry` (TypeScript ESM, can't be converted yet) -- bridged via `internals.__telemetry` / `internals.__telemetryUtil` set in the telemetry source files - All other cross-extension deps use `core.loadExtScript()` (ext/web, ext/webidl, ext/net are already `lazy_loaded_js`) This also eliminates the `internals.__fetchRequest/Response/Headers` bridges from #33778 since `ext/cache` can now use `core.loadExtScript()` for ext/fetch directly.
…zy-loaded scripts (#33778) Follow-up to #33760. Converts the ESM JS sources in `ext/cache`, `ext/canvas`, and `ext/crypto` to IIFE-wrapped lazy-loaded scripts (`lazy_loaded_js`), eliminating ESM module resolution overhead. - **ext/canvas/02_surface.js** - simple re-export from `core.ops` - **ext/cache/01_cache.js** - ext/fetch dependencies (Request, toInnerRequest, toInnerResponse, getHeader) bridged via `internals` since ext/fetch is still ESM - **ext/crypto/00_crypto.js** - `kKeyObject` symbol bridged via `internals` from `ext/node`; updated `ext/node` consumers to use `core.loadExtScript()`
…34877) ## Summary A `node:worker_threads` Worker that holds a "refed" transferable object such as a `MessagePort` must not be terminated on idle. This covers the use cases from #23169: - a worker pool using a dedicated `MessagePort` for IPC, with a minimum number of workers always kept alive, and - a worker that is set up, idle, and waiting for work. ## Root cause The runtime idle-termination check `hasMessageEventListener()` in `runtime/js/99_main.js` consults `messagePort.refedMessagePortsCount` to decide whether a node worker should stay alive: ```js return (event.listenerCount(globalThis, "message") > 0 && !globalThis[messagePort.unrefParentPort]) || messagePort.refedMessagePortsCount > 0; ``` That counter is a mutable module-level `let` in `ext/web/13_message_port.js`. When ext/web was converted to lazy-loaded IIFE scripts in #33760, the module's real ESM `export { refedMessagePortsCount }` (a **live binding**) became a plain property on the returned object literal, capturing a one-time snapshot of `0`. The refed-port branch of the idle check was therefore dead, and a worker kept alive only by a refed `MessagePort` (with no active "message" listener op promise) was incorrectly terminated on idle. ## Fix Restore the live-binding semantics by exposing `refedMessagePortsCount` as a getter on the module's return object. ## Tests Adds two spec tests under `tests/specs/node/worker_threads/`: - `refed_messageport_idle` — a worker receives delayed work over a refed transferred `MessagePort` after an idle period. - `refed_messageport_holder` — a worker kept alive purely by a refed `MessagePort` it does **not** listen on. This isolates the fix: it fails on `main` (worker terminates on idle) and passes with the change. Closes #23169 Closes denoland/divybot#429 Co-authored-by: divybot <[email protected]> Co-authored-by: Divy Srivastava <[email protected]>
…33780) Follow-up to denoland#33760. Converts `ext/ffi/00_ffi.js` from ESM to an IIFE-wrapped lazy-loaded script (`lazy_loaded_js`). Signed-off-by: Bartek Iwańczuk <[email protected]>
…and#33784) Follow-up to denoland#33760 and denoland#33779. Converts all 8 ESM JS sources in `ext/fetch` to IIFE-wrapped lazy-loaded scripts (`lazy_loaded_js`). - `20_headers.js`, `21_formdata.js`, `22_body.js`, `22_http_client.js`, `23_request.js`, `23_response.js`, `26_fetch.js`, `27_eventsource.js` - `26_fetch.js` imports from `ext/telemetry` (TypeScript ESM, can't be converted yet) -- bridged via `internals.__telemetry` / `internals.__telemetryUtil` set in the telemetry source files - All other cross-extension deps use `core.loadExtScript()` (ext/web, ext/webidl, ext/net are already `lazy_loaded_js`) This also eliminates the `internals.__fetchRequest/Response/Headers` bridges from denoland#33778 since `ext/cache` can now use `core.loadExtScript()` for ext/fetch directly.
…zy-loaded scripts (denoland#33778) Follow-up to denoland#33760. Converts the ESM JS sources in `ext/cache`, `ext/canvas`, and `ext/crypto` to IIFE-wrapped lazy-loaded scripts (`lazy_loaded_js`), eliminating ESM module resolution overhead. - **ext/canvas/02_surface.js** - simple re-export from `core.ops` - **ext/cache/01_cache.js** - ext/fetch dependencies (Request, toInnerRequest, toInnerResponse, getHeader) bridged via `internals` since ext/fetch is still ESM - **ext/crypto/00_crypto.js** - `kKeyObject` symbol bridged via `internals` from `ext/node`; updated `ext/node` consumers to use `core.loadExtScript()`
Summary
core.loadExtScript(), building on the infrastructure from feat(core): addDeno.core.loadExtScript()for lazy-loaded scripts #33739ext/,runtime/, andcli/fromimporttocore.loadExtScript()createLazyLoader()calls targeting ext:deno_web files withloadExtScript()since they are now plain scriptsNotes
import.meta.login06_streams.jsreplaced withcore.print()as a temporary workaround sinceimport.metais not available in non-module scripts. A propercore.logequivalent may be needed.Test plan
cargo buildsucceeds (snapshot creation passes)