Repository navigation
feat(core): support lazy_loaded_esm modules via import statements - #33873
Conversation
Previously, `lazy_loaded_esm` modules could only be accessed via `Deno.core.createLazyLoader()`. This change makes them resolvable through normal `import` statements by checking `lazy_esm_sources` in both the recursive module loader and V8's module resolve callback. This enables a pattern where `node:*` module ESM wrappers can be stored in the binary as raw text without being snapshotted (since snapshotting ES modules is expensive). The implementation lives in a `lazy_loaded_js` IIFE, and a thin ESM re-export wrapper in `lazy_loaded_esm` provides the `node:*` specifier. As a proof of concept, `node:sys` is converted to this pattern: - `sys.ts`: lazy_loaded_js IIFE that loads node:util - `sys_esm.js`: lazy_loaded_esm thin wrapper with `node:sys` specifier - `01_require.js`: uses lazy getter with core.loadExtScript
- Add fast path in load_dynamic_import to resolve lazy ESM modules synchronously, avoiding the async RecursiveModuleLoad path entirely - Skip prepare_load for lazy ESM sources in RecursiveModuleLoad - Check lazy_esm_sources in the recursive load phase as fallback - Fix 01_require.js to eagerly load sys module (required for assert.strictEqual(sys, util) identity check in Node compat tests) - Add has_lazy_esm_source() helper on ModuleMap
Add a `known_lazy_esm` set to ModuleMapData that records which lazy-loaded ESM specifiers exist. This set is serialized into the snapshot sidecar data, so at runtime the module system knows these modules exist without needing to check lazy_esm_sources directly. This means resolve_callback, load_dynamic_import, and the recursive module loader can all check the snapshotted metadata rather than requiring scattered fallbacks. Also fix 01_require.js to eagerly load sys (required for assert.strictEqual(sys, util) identity check in Node compat tests), and handle dynamic import() for lazy ESM modules.
fibibot
left a comment
fibibot
left a comment
There was a problem hiding this comment.
Substance of the deno_core change is interesting and well-motivated — letting lazy_loaded_esm modules participate in normal import resolution avoids the snapshot cost for thin re-export wrappers. The new known_lazy_esm metadata + the three fast paths in load_dynamic_import / resolve_callback / RecursiveModuleLoad are the right entry points to teach.
But the proof-of-concept conversion fails the proof. parallel/test-sys.js is the exact test that would catch "node:sys works through ESM and CJS" — and it fails on linux-aarch64:
error: Uncaught (in promise) AssertionError: Expected "actual" to be reference-equal to "expected":
+ [Module: null prototype] {
- {
MIMEParams: [class MIMEParams],
MIMEType: [class MIMEType],
...
require("sys") is now returning a Module namespace object ([Module: null prototype] {...}); the test expects it to be === to whatever require("util") returns (a plain object). Node.js's identity invariant require("sys") === require("util") is broken by the conversion.
Why
Looking at the new shape:
ext/node/polyfills/sys.ts(lazy_loaded_js):(function () { return core.createLazyLoader("node:util")(); })()— returns the util Module namespace.ext/node/polyfills/sys_esm.js(lazy_loaded_esm):export * from "node:util"; export { default } from "node:util";— re-exports util's bindings, but the resulting Module namespace is a different object than util's own.01_require.js:155-157:const sys = core.loadExtScript("ext:deno_node/sys.ts");— wires up the CJS-require side, which now returns the lazy-loaded namespace, not the same object asrequire("util").
The PR description claims node:sys was tested via CJS require(), but the identity assertion (which is what this Node test exercises) fails.
Fix directions
Three options, increasing in scope:
-
Don't convert
node:sys— keep it as the existing thin alias and pick a different proof-of-concept module that doesn't have an identity invariant. The deno_core machinery still ships, just without this specific binding. -
Special-case
node:sysin CJSrequire— whenModule._resolveFilename("sys")resolves, return the same object thatrequire("util")returns instead of going through the lazy_loaded_esm path. Brittle but preserves the identity contract. -
Make
lazy_loaded_esmre-export wrappers preserve identity — whenexport * from "X"resolves, return the same Module namespace object asX's, not a fresh wrapper namespace. This is the most general fix but needs careful thought about V8 module-graph semantics (a re-exporting module is, by V8 spec, a distinct module from the source).
My hunch is #1 (or skip the test) is the cleanest path for landing the deno_core feature without getting stuck on node:sys's peculiar identity contract.
CI
61 success, 25 pending, 1 fail (test node_compat (2/3) debug linux-aarch64 on parallel/test-sys.js). The runner explicitly retried at concurrency=1 and the fail is deterministic — not a flake.
Clearing this REQUEST_CHANGES once the test-sys regression is resolved (whichever path you choose).
require("sys") must return the exact same object as require("util")
(Node compat test assert.strictEqual). Map sys to util directly in
01_require.js instead of going through a separate IIFE. The ESM path
(node:sys) still uses the lazy_loaded_esm wrapper (sys_esm.js) which
re-exports from node:util.
Also add snapshot test verifying lazy ESM metadata survives
snapshotting while the module itself is not in the V8 module map.
fibibot
left a comment
fibibot
left a comment
There was a problem hiding this comment.
LGTM — clearing my prior REQUEST_CHANGES at 0e4c059c. 75d4c932 (fix: use util directly for CJS sys, remove unused sys.ts IIFE) takes option (2) from my fix directions: keep the lazy_loaded_esm machinery for ESM import paths, but route CJS require("sys") directly at util's exports so the require("sys") === require("util") identity invariant holds.
What I verified
The previously-failing shard now passes: test node_compat (2/3) debug linux-aarch64 (which runs parallel/test-sys.js) green at this HEAD. All 15 test node_compat × {1/3, 2/3} × {debug, release} × 4 platforms shards I've checked are green.
The fix shape is right:
ext/node/polyfills/sys.ts: deleted (+0/-5) — the IIFE wrapper that returnedcreateLazyLoader("node:util")()is gone.ext/node/lib.rs: removedsys.tsfrom registration (+0/-1).ext/node/polyfills/01_require.js: CJSrequire("sys")now resolves directly to util's object (+1/-2).libs/core/runtime/tests/snapshot.rs: added a deno_core snapshot test for the lazy_loaded_esm flow (+78/-0) — locks in the new resolution paths so future regressions surface here, not via Node-compat tests.
The ESM-side sys_esm.js (export * from "node:util"; export { default } from "node:util";) remains, so static import sys from "node:sys" and await import("node:sys") still go through the new lazy_loaded_esm path — which was the point of this PR.
CI
104 success, 11 pending, 0 fail at this HEAD. The 11 pending are slow test specs / test integration / test unit / test libs debug shards on macos/windows — they don't exercise the node:sys → node:util aliasing. Substance is unambiguously correct.
…noland#33873) - Teach `deno_core` to resolve `lazy_loaded_esm` modules through normal `import` statements (static, dynamic, and CJS require), not just `createLazyLoader()` - Add `known_lazy_esm` metadata set to `ModuleMapData` that persists in the snapshot sidecar, so the module system knows these modules exist without compiling/instantiating them during snapshotting - Add fast paths in `load_dynamic_import`, `resolve_callback`, and `RecursiveModuleLoad` to handle lazy ESM modules - Convert `node:sys` as a proof of concept: implementation in `lazy_loaded_js` IIFE, thin ESM wrapper in `lazy_loaded_esm` with `node:sys` specifier -- neither is snapshotted ## Motivation Snapshotting ES modules is expensive. For `node:*` modules that are thin re-export wrappers, we can avoid the snapshot cost by: 1. Moving the implementation into a `lazy_loaded_js` IIFE (script in binary, not snapshotted) 2. Providing a thin ESM re-export wrapper via `lazy_loaded_esm` (in binary, not snapshotted) 3. Registering the module's existence in the snapshot metadata so all resolution paths work
2 participants
Summary
deno_coreto resolvelazy_loaded_esmmodules through normalimportstatements (static, dynamic, and CJS require), not justcreateLazyLoader()known_lazy_esmmetadata set toModuleMapDatathat persists in the snapshot sidecar, so the module system knows these modules exist without compiling/instantiating them during snapshottingload_dynamic_import,resolve_callback, andRecursiveModuleLoadto handle lazy ESM modulesnode:sysas a proof of concept: implementation inlazy_loaded_jsIIFE, thin ESM wrapper inlazy_loaded_esmwithnode:sysspecifier -- neither is snapshottedMotivation
Snapshotting ES modules is expensive. For
node:*modules that are thin re-export wrappers, we can avoid the snapshot cost by:lazy_loaded_jsIIFE (script in binary, not snapshotted)lazy_loaded_esm(in binary, not snapshotted)Test plan
deno_coreunit tests:test_lazy_loaded_esm_aliased_specifier,test_lazy_loaded_esm_aliased_via_importtest_lazy_loaded_esmtests still passnode:sysworks via static ESM import, dynamicimport(), named import, and CJSrequire()