Repository navigation
feat(compile): support module.register() and module.registerHooks() in compiled binaries - #33853
Conversation
…onse Add `ModuleResolveResponse` enum (Sync/Async variants) mirroring the existing `ModuleLoadResponse` pattern. This enables Node.js-style loaders where resolve hooks can be async (e.g., querying a package registry or running user-provided JS resolve functions). The change is backward-compatible: all existing loaders wrap their returns in `ModuleResolveResponse::Sync(...)`. The V8 `module_resolve_callback` uses the new `resolve_sync()` method which is safe because by instantiation time all modules are already resolved. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add a loader resolve hook system to the checkin test infrastructure: - `ops_loader.rs`: `op_loader_register_resolve` op that registers specifier-to-URL resolve mappings - `loader.ts`: `checkin:loader` JS module exposing `registerResolveMapping` - `TypescriptModuleLoader`: checks resolve mappings and returns `ModuleResolveResponse::Async` to exercise the async resolution path - `loader_resolve` integration test: registers a virtual specifier, dynamically imports it, and verifies the module loads correctly Co-Authored-By: Claude Opus 4.6 <[email protected]>
Resolve conflicts in recursive_load.rs and map.rs by combining main's refactored resolve caching (resolved_specifier field, accessor methods, resolve_root_from_init) with the async module resolution support (ModuleResolveResponse). Co-Authored-By: Claude Opus 4.6 <[email protected]>
Change ModuleLoader::resolve to return ModuleResolveResponse (Sync/Async) instead of a plain Result, enabling loaders to perform async resolution. Per review feedback, omit the panicking `into_result()` helper. Instead, callers use explicit match or the `resolve_sync`/`resolve_async` methods on ModuleMap. Loaders with async resolve must override import_meta_resolve. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add a message-passing bridge between the Rust module loader and JS hook functions, matching Node.js `module.register()` customization hooks API. Hooks use async functions with `resolve(specifier, context, nextResolve)` and `load(url, context, nextLoad)` signatures. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Remove duplicate `use deno_error::JsErrorBox` in recursive_load.rs that caused build failures (error[E0252]) - Remove `ModuleResolveResponse::into_result()` per review feedback — it panics on async responses which is risky - Change `import_meta_resolve` default to use `resolve_import` directly instead of going through `resolve().into_result()` - Update test to match on `ModuleResolveResponse::Sync` explicitly - Run formatter on core_testing files
- Use #[derive(Default)] instead of manual impl - Remove useless .into() on JsErrorBox
… test - Remove unused import of `ModuleLoaderError` in source_map.rs (clippy) - Restore import_meta_resolve default to call self.resolve() for Sync results, falling back to resolve_import() only for Async. The previous change broke import.meta.resolve() for loaders (like EmbeddedModuleLoader) that don't override import_meta_resolve but do resolve JSR/npm specifiers. - Update loader_resolve test to use the new register() API instead of the removed registerResolveMapping() function.
Node.js invokes registered loader hooks when import.meta.resolve() is called (see test-esm-import-meta-resolve-hooks.mjs). Our current implementation falls back to basic URL resolution for async hooks since import.meta.resolve is synchronous. This test documents the expected behavior and is ignored until a synchronous resolve mechanism (similar to Node's resolveSync) is implemented.
The loader_resolve integration test hung because `op_loader_register()` set a single `active` flag, causing the module loader to route ALL operations (both resolve and load) through the JS hook bridge — even when only a resolve hook was registered. Load requests went to a dead-letter queue with no JS consumer, deadlocking the oneshot channel. Fix: split `active` into `resolve_active` and `load_active` flags so the loader only delegates to JS for hooks that were actually registered. Also address dsherret's review: simplify the default `import_meta_resolve` to just call `resolve_import` directly instead of calling `self.resolve()` and silently falling back when it returns Async. Add explicit `import_meta_resolve` override to `ExtModuleLoader` using its existing `resolve_inner` method.
The existing default import_meta_resolve behavior (try self.resolve(), fall back for Async) is intentional and documented with an ignored test. A proper fix for async hooks in import.meta.resolve is planned via a synchronous resolve mechanism.
Adds a Rust<->JS bridge that allows module.registerHooks() resolve and
load hooks to intercept ESM dynamic import() calls. Uses the same
async channel pattern as the checkin:loader test infrastructure:
- New ops: op_module_hooks_{register,poll_resolve,respond_resolve,
poll_load,respond_load} bridge the CliModuleLoader to JS hooks
- LoaderHookRegistry (shared Rc between module loader and OpState)
queues requests and routes responses via oneshot channels
- CliModuleLoader::resolve() returns ModuleResolveResponse::Async
when hooks are active, allowing JS to intercept resolution
- CliModuleLoader::load() delegates to JS hooks before default loading
- Fallthrough: when hooks call nextResolve/nextLoad without
intercepting, the response signals Rust to use default resolution
- prepare_load skipped when hooks active (virtual modules can't be
fetched by the graph builder)
Resolve conflicts after feature/async-module-resolve landed in main. Keep both CJS hook functions (executeResolveHookChain/executeLoadHookChain) from main and ESM hook functions (executeEsmResolveHookChain/ executeEsmLoadHookChain) plus ESM loop infrastructure from this branch.
Previously, prepare_load was blanket-skipped for ALL dynamic imports when module hooks were active. This meant non-intercepted dynamic imports with non-static specifiers (not already in the module graph) would fail with LoadUnpreparedModuleError. Track which specifiers were actually intercepted by a resolve hook in hook_intercepted_specifiers. Only skip prepare_load for those virtual modules. Fallthrough-resolved specifiers still go through normal prepare_load for graph building and permission checks.
Implements the Node.js module.register() API which loads hook modules and registers their resolve/load hooks for ESM imports. This builds on the existing registerHooks() infrastructure. Key behaviors: - Loads hook module via dynamic import() - Calls initialize(data) with structured clone data and transferList - Registers async resolve/load hooks into the ESM hook chain - Supports LIFO chaining (last registered runs first) - Sync hooks (registerHooks) run before async hooks (register) per spec - Returns undefined immediately (hooks activate asynchronously) Node compat test results: 5/48 async-loader tests pass, 20/62 module-hooks tests pass. Most failures are due to tests using --experimental-loader flag or child process spawning patterns.
When CJS load hooks return format='module', loadESMFromCJS calls op_import_sync which blocks the JS thread. If hooks are active, the Rust module loader would push requests to the hook queue, but the JS loop can't process them since the thread is blocked - deadlock. Fix by: - Skipping resolve hooks for already-resolved specifiers (file://, http://, etc.) since they don't need resolution and avoiding the async bridge prevents deadlocks from op_import_sync - Skipping load hooks when is_synchronous is true (set by deno_core for op_import_sync paths) Also add importAssertions (deprecated alias) to hook context objects for Node.js compatibility.
Enable 30 node_compat tests that now pass: - 25 test-module-hooks tests (registerHooks API) - 5 test-async-loader-hooks tests (register API) Previously only 10 were enabled.
Address review feedback: 1. Fix race between register() and subsequent imports: track pending hook module loads and await them in the ESM resolve loop before processing requests. Pre-activate resolve_active so imports are routed through the bridge and naturally wait for hooks to load. Remove all setTimeout workarounds from tests. 2. Stop swallowing hook module load errors: _loadAndRegisterHookModule no longer catches errors. Failures propagate as unhandled rejections and surface at the import site that triggered the hook bridge. 3. Only pre-activate resolve (not load) to avoid circular deadlock where the hook module's own load would go through the bridge that's waiting for the hook module to load.
…n compiled binaries Enable the Node.js module customization hooks API in deno compile output. This allows compiled binaries to register custom resolve/load hooks that can intercept and transform dynamically imported modules at runtime. Key changes: - Wire up LoaderHookRegistry in StandaloneModuleLoaderFactory (was None) - Add resolve hook bridge support in EmbeddedModuleLoader, routing external file:// imports through the bridge to ensure pending hook module loads complete before proceeding - Add load hook bridge support for non-embedded files, allowing hooks to transform source (e.g., strip TypeScript types) before V8 - Add is_external_file_specifier() to distinguish embedded VFS files from external filesystem files - Add load_inner() for hook fallthrough path This enables use cases like bundling a TypeScript type-stripper into a compiled binary and dynamically importing .ts files from disk at runtime.
fibibot
left a comment
There was a problem hiding this comment.
Substance is well-scoped: only external (non-VFS) files route through hooks, embedded modules go through the existing fast path, and is_external_file_specifier checks vfs.file_entry().is_err() to distinguish. The structure mirrors cli/module_loader.rs so reviewers familiar with that path can map this 1:1.
Three CI blockers to clear before this can land:
-
Formatting —
lint debug linux-x86_64andlint debug macos-x86_64both fail withFound 1 not formatted file. Process completed with exit code 20.Run./tools/format.jsand re-push. (ci statusrollup also red because of this.) -
[node/module register] is a functionfailing on all 5 unit_node debug platforms +release linux-x86_64— sameop_module_hooks_poll_resolveleak I diagnosed on #33849. This PR is built on top of #33849'sregister()implementation but does NOT include #33849's most recent commitb956ce17(fix(test): disable op sanitizer for module.register() test). Once #33849 rebases or merges with that fix, this PR will need a rebase to pick it up. Until then the unit_node failures are inherited noise, not a problem in this PR's diff. -
Dependency on #33849 itself. This PR uses
LoaderHookRegistry,hook_intercepted_specifiers,push_resolve,push_load— all introduced by #33849. The PR description should make the dependency explicit; right now it just says "Enablemodule.registerHooks()andmodule.register()indeno compileoutput" which reads as if those APIs already exist on main. Worth addingBuilds on #33849.to the body so it's clear which has to land first.
Two non-blocking substance observations inline on cli/rt/run.rs:load_inner — corner-case fallthrough behavior that diverges from the regular module loader.
| // are loaded before proceeding to load(). | ||
| let should_use_resolve_hooks = self.hook_registry.resolve_active.get() | ||
| && (!is_already_resolved_specifier(raw_specifier) | ||
| || self.is_external_file_specifier(raw_specifier)); |
There was a problem hiding this comment.
Non-blocking, want this on the radar: a load hook that calls nextLoad(npm_specifier) for an npm-package import that isn't intercepted will hit this branch and error with Module not found: <specifier>, instead of falling through to the default npm-package loading path that the regular load() method (line 670+) handles via node_code_translator.
The comment acknowledges the limitation ("npm modules should not typically go through hooks"), but a hook author who wants to log/observe ALL imports without intercepting them will hit this when the user imports anything from node_modules. Not catastrophic — they can detect npm specifiers and skip the nextLoad call — just a UX paper cut.
If addressing in a follow-up makes sense, the closest fix is to plumb async npm loading through here (mirroring the non-hook path at lines ~670-700). Not blocking this PR.
| ); | ||
| } | ||
|
|
||
| deno_core::ModuleResolveResponse::Sync(self.resolve_inner( |
There was a problem hiding this comment.
Same kind of fallthrough divergence as the npm case above. The comment says:
CJS translation requires async; return as-is and let V8 handle. This path is unlikely in the hook fallthrough since hooks handle the transformation themselves.
The "return as-is" part isn't quite right for V8 — a CJS file (e.g. .cjs or a file with "type": "commonjs" in its package.json) returned as ModuleType matching what v8 thinks it is but with untranslated CJS source will run as raw script with require/module.exports undefined, producing reference errors at runtime rather than the proper CJS-to-ESM wrapper output.
In practice the trigger is narrow: a hook does nextLoad(cjs_url) without intercepting. As long as users follow the example in the PR body (intercept .ts files, fall through everything else), they won't hit this. Worth a // TODO: handle CJS via node_code_translator comment so the limitation isn't forgotten if someone tries to extend the hook surface later.
As with the npm case, this is non-blocking — the typical use case (TS-stripping loader) intercepts the files it cares about and never falls through to CJS.
fibibot
left a comment
There was a problem hiding this comment.
Both CI blockers from my prior comment are addressed at this HEAD:
- Formatting —
e9fca52c(fix: rustfmt formatting in cli/rt/run.rs) cleared the lint failure.lint debug linux-x86_64andlint debug windows-x86_64now pass;lint debug macos-x86_64still pending but no failures. ✓ - Inherited
[node/module register]leak — main was merged in (commitc238866cbrought in the merged #33849 withsanitizeOps: false).test unit_nodenow passes on all 8 platforms that have completed (linux/macos × aarch64/x86_64 debug + release; the only pending one iswindows-aarch64debug). ✓
One remaining fail looks like macos-aarch64 flake: test node_compat (1/3) debug macos-aarch64 is the only failure across the entire matrix. node_compat (1/3) passes on linux-aarch64 debug, windows-x86_64 debug, and on every release variant for all 4 platforms (linux-aarch64, macos-aarch64, macos-x86_64, windows-aarch64, windows-x86_64). A single shard failing on one platform-mode while the same shard passes everywhere else (including the same platform's release build) is the classic flaky-test signal.
I couldn't pull the failing job's logs to confirm the specific test (gh api .../logs returns BlobNotFound) — likely the shard hit one of the known flakes (test-fs-read-stream-pos.js timeout, DNS, etc.). Re-triggering that shard should clear it.
Holding APPROVE pending: 14 shards still building (windows-aarch64 debug variants, macos-x86_64 lint/specs, builds). The substance and CI direction look right; just want the remaining shards to land before clearing. Will re-confirm once they finish.
There was a problem hiding this comment.
Might be useful to add a test for this one to ensure it doesn't regress in the future.
There was a problem hiding this comment.
Added a spec test in #33877 (tests/specs/compile/module_hooks/).
The test registers a registerHooks() load hook in a compiled binary and verifies it can intercept/transform an external (non-embedded) file. It passes — the initial failure I saw was due to a stale denort binary that didn't include the PR changes. After rebuilding denort, the hook bridge works correctly.
Adds a regression test for #33853. The test verifies that registerHooks() load hooks can intercept and transform external (non-embedded) file loads in compiled binaries. Currently fails: the load hook bridge in the EmbeddedModuleLoader does not route external file loads through the JS hook chain.
Adds a regression test for #33853. The test verifies that registerHooks() load hooks can intercept and transform external (non-embedded) file loads in compiled binaries. Currently fails: the load hook bridge in the EmbeddedModuleLoader does not route external file loads through the JS hook chain.
Summary
module.registerHooks()andmodule.register()indeno compileoutputLoaderHookRegistryin the standalone module loader (previouslyNone)This unlocks dynamic imports of TypeScript files from disk in compiled binaries, by allowing users to bundle their own transpiler as a hook module.
Fixes #18327
Design
module.register()'s async hook module loading.file://URLs are routed through the resolve bridge (even though they're "already resolved") so that the resolve loop'spendingHookLoadswait ensures hook modules finish loading before the import proceeds.load_inner()provides the fallthrough path when hooks don't intercept.Example
Test plan
module.registerHooks()load hook transforms TS in compiled binarymodule.register()async hooks work in compiled binarydeno runwithmodule.register()andmodule.registerHooks()-- no regressionsmodule_register_esmandmodule_register_hooks_esmspec tests pass