Skip to content

feat(compile): support module.register() and module.registerHooks() in compiled binaries - #33853

Merged
bartlomieju merged 28 commits into
mainfrom
feat/compile-module-hooks
May 6, 2026
Merged

bartlomieju merged 28 commits into
mainfrom
feat/compile-module-hooks

Conversation

@bartlomieju

@bartlomieju bartlomieju commented May 5, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Enable module.registerHooks() and module.register() in deno compile output
  • Wire up LoaderHookRegistry in the standalone module loader (previously None)
  • Route external file imports through resolve/load hook bridges so user-registered hooks can intercept and transform them (e.g., strip TypeScript types at runtime)

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

  • Only external files (not embedded in the VFS) go through hook bridges. Embedded modules load directly, avoiding deadlocks with module.register()'s async hook module loading.
  • External file:// URLs are routed through the resolve bridge (even though they're "already resolved") so that the resolve loop's pendingHookLoads wait ensures hook modules finish loading before the import proceeds.
  • load_inner() provides the fallthrough path when hooks don't intercept.

Example

// main.ts (compiled into binary)
import module from "node:module";

module.registerHooks({
  load(url, context, nextLoad) {
    if (url.endsWith(".ts")) {
      const source = Deno.readTextFileSync(new URL(url));
      const stripped = myTypeStripper(source); // user-provided
      return { source: stripped, format: "module", shortCircuit: true };
    }
    return nextLoad(url, context);
  },
});

const plugin = await import(`file://${Deno.realPathSync("./plugin.ts")}`);
plugin.greet("world"); // works!

Test plan

  • module.registerHooks() load hook transforms TS in compiled binary
  • module.register() async hooks work in compiled binary
  • Plain JS dynamic imports from disk still work (no hooks needed)
  • deno run with module.register() and module.registerHooks() -- no regressions
  • Existing module_register_esm and module_register_hooks_esm spec tests pass
  • CI

bartlomieju and others added 26 commits March 3, 2026 15:38
…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.
Base automatically changed from feat/module-register-api to main May 5, 2026 13:59

@fibibot fibibot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Formatting — lint debug linux-x86_64 and lint debug macos-x86_64 both fail with Found 1 not formatted file. Process completed with exit code 20. Run ./tools/format.js and re-push. (ci status rollup also red because of this.)

  2. [node/module register] is a function failing on all 5 unit_node debug platforms + release linux-x86_64 — same op_module_hooks_poll_resolve leak I diagnosed on #33849. This PR is built on top of #33849's register() implementation but does NOT include #33849's most recent commit b956ce17 (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.

  3. 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 "Enable module.registerHooks() and module.register() in deno compile output" which reads as if those APIs already exist on main. Worth adding Builds 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.

Comment thread cli/rt/run.rs
// 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));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread cli/rt/run.rs
);
}

deno_core::ModuleResolveResponse::Sync(self.resolve_inner(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 fibibot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_64 and lint debug windows-x86_64 now pass; lint debug macos-x86_64 still pending but no failures. ✓
  • Inherited [node/module register] leak — main was merged in (commit c238866c brought in the merged #33849 with sanitizeOps: false). test unit_node now passes on all 8 platforms that have completed (linux/macos × aarch64/x86_64 debug + release; the only pending one is windows-aarch64 debug). ✓

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.

@bartlomieju
bartlomieju merged commit 8595dec into main May 6, 2026
268 of 270 checks passed
@bartlomieju
bartlomieju deleted the feat/compile-module-hooks branch May 6, 2026 08:30
@fibibot fibibot mentioned this pull request May 6, 2026
Comment thread cli/rt/run.rs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be useful to add a test for this one to ensure it doesn't regress in the future.

@bartlomieju bartlomieju May 6, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

bartlomieju added a commit that referenced this pull request May 6, 2026
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.
bartlomieju added a commit that referenced this pull request May 6, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

deno compile output can't dynamically import files that were not present at compilation time

3 participants