perf(ext): convert ext/io, ext/os, ext/net JS sources to lazy-loaded scripts#33779
Conversation
fibibot
left a comment
There was a problem hiding this comment.
Substance follows the same lazy_loaded_js pattern I approved in #33760 and commented on in #33778, and the body's claim that ext/io/ext/os/ext/net have no cross-extension ESM imports (only core.loadExtScript calls into already-converted extensions) checks out — no internals symbol bridges needed.
Real regression: IIFE wrapper shifts line numbers in stack traces
integration::run::permissions_trace fails on both debug macos-aarch64 and debug linux-aarch64 (so this is deterministic, not a single-platform flake). The expected output asserts:
┃ ├─ Object.hostname (ext:deno_os/30_os.js:43:10)
but actual output after the conversion is:
┃ ├─ Object.hostname (ext:deno_os/30_os.js:45:10)
Two-line shift — the IIFE wrapper ((function () { + setup) adds two lines at the top of 30_os.js, moving Object.hostname from line 43 to line 45. Stack-trace-pinning tests like permissions_trace see this directly.
The permissions_trace regression is the kind of thing #33760 / #33778 didn't trip on because their converted files (16_image_data.js, 00_crypto.js, etc.) weren't on any permissions_trace-style stack. ext/os/30_os.js's Object.hostname is — it's on the canonical "sys access requested" trace path.
Fix: update the expected output in tests/testdata/run/permissions_trace.out (the file the macros line tests/integration/run_tests.rs:430:1 points at) to the new line numbers. Or — cleaner long-term — switch the line numbers to [WILDLINE] so future IIFE/format changes don't cascade. The non-regenerated line-number assertion is going to keep biting on every script-conversion follow-up otherwise.
Other failure
test node_compat (2/3) debug windows-x86_64 fails on parallel/test-dns-resolver-max-timeout.js: timeout1: 505, timeout2: 515 — same Windows DNS-timing flake that's hit ~half the daily PRs. Pre-existing, unrelated.
Substance walk
ext/io/lib.rs,ext/os/lib.rs,ext/net/lib.rsall switch the relevantesm = [...]entries tolazy_loaded_js = [...]. Right shape for the now-IIFE files.- 23 files, +67/-47 net: most of the diff is the JS-side
import→core.loadExtScriptmigration in consumers, plus the IIFE wrap on the converted files themselves. Smaller than #33778 because there's no equivalent of the giant00_crypto.jspatch. - No
internalsshims — the body's statement that these extensions have no cross-ESM dependencies is verifiable from the diff: every JS-side import either staysimport(for files still in ESM form) or migrates tocore.loadExtScript(for already-converted extensions likeext/web).
CI
83/106 success at this head, 18 in flight, 3 failures (2 real permissions_trace, 1 unrelated DNS flake). Once the .out fixture is updated for the new line numbers, the substance side is clean and ready for re-CI.
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.
…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.
Summary
Follow-up to #33760. Converts the ESM JS sources in
ext/io,ext/os, andext/netto 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.loadExtScriptcalls to already-converted extensions likeext/web), so nointernalsbridges are needed.Converting these also unblocks clean conversion of:
ext/fs(depends onext/io)ext/fetch(depends onext/net)Test plan
cargo checkpassescargo build --bin denosucceeds (snapshot builds)tools/format.jsandtools/lint.js --jspassDeno.hostname(),Deno.stdin,Deno.connect,Deno.connectTlsall work