refactor(core): pre-resolve child imports before V8 module registration#34050
Merged
Conversation
…ooks When module.register() flips on resolve_active, every bare-specifier resolution is routed through the loader-hook bridge as an async response. deno_core's async-resolve path computes a placeholder URL by joining the referrer with the raw specifier; for cannot-be-a-base referrers (blob:, data:) the join fails and surfaces as "Cannot resolve module \"X\": relative URL with a cannot-be-a-base base". This broke e.g. lume's MDX plugin, which compiles MDX to a blob: module that statically imports bare-mapped specifiers like "lume/jsx-runtime". Bypasses the hook bridge in both CliModuleLoader and EmbeddedModuleLoader when the referrer is cannot-be-a-base, and adds a synthetic placeholder fallback in deno_core so any loader that returns Async in this shape no longer panics.
…alled Drop the CLI-side bypass: with the synthetic async-resolve placeholder in place, the hook bridge now drives bare-specifier resolution from cannot-be-a-base referrers the same way Node does (verified: Node invokes the user resolve hook for `bare-spec` with parentURL = the data: URL). Narrow the placeholder branch in map.rs to trigger only when the parsed referrer is `cannot_be_a_base()`, so other unforeseen join failures still surface as typed `Cannot resolve module ...` errors. Expand the spec test so the hook prints to stderr when it sees the bare specifier from a blob: parent; the expected output asserts on it, ruling out a silent bypass regression.
Removes the discard-and-redo pattern at `find_imports_in_compiled_module`: when a child import returned `ModuleResolveResponse::Async`, deno_core used to throw the future away, place a placeholder URL on the request with `needs_resolve = true`, and let `RecursiveModuleLoad` issue a second resolve later. That doubled every async resolve round-trip and required the synth-placeholder fallback added in #34050. Now the resolve future is stored alongside the (compiled) module as a `PendingModule`. `register_and_recurse` returns `RegisterOutcome::Done` when every child import resolved synchronously and `RegisterOutcome::PendingFinalize` when any returned `Async`. The driver loops -- `run_to_completion` for main/side loads and `drain_dyn_imports` for dynamic imports -- drop the V8 scope, `await ModuleMap::finalize_pending_module`, then re-enter scope to register the module with its real child URLs and recurse. Single round-trip per resolve, no placeholders observed downstream. Drops `ModuleRequest.needs_resolve`, the duplicate async-resolve in `recursive_load::register_and_recurse_inner`, `ModuleMap::resolve_async`, and the `synth_async_resolve_placeholder` added in #34050 -- all subsumed by the new flow. Sync call sites (V8 `resolve_callback`, `lazy_load_esm_module`, `do_load_job`) continue to use `new_module_from_js_source`, which now errors explicitly if a sync context ever hits an async-resolve loader instead of quietly producing a placeholder. The lume MDX repro and the rollup+hooks regression both pass.
…-bare-specifier # Conflicts: # libs/core/modules/recursive_load.rs # tests/specs/node/module_register_esm/__test__.jsonc
nathanwhit
approved these changes
May 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a child import returned
ModuleResolveResponse::Asyncfrom the loader,find_imports_in_compiled_moduleused to discard the future, place aplaceholder URL on the request with
needs_resolve = true, and letRecursiveModuleLoadissue a second resolve later. That doubled every asyncresolve round-trip (the first reply was sent to a dropped
oneshot::Receiverand silently lost) and required the synth-placeholder fallback that was
necessary because
base.join(spec)fails forcannot-be-a-basereferrerslike
blob:anddata:— the original bug this PR was opened for: lume'sMDX plugin compiling MDX into a blob: module that imports
lume/jsx-runtime, plus the related vite/rollup case where any transitivemodule.register()(e.g. tailwindcss) flipped onresolve_activeand brokenode:pathimports from inside npm packages.The fix is to pre-resolve child imports before V8 registers the module.
new_module_from_js_source_with_pendingcompiles via V8 in scope, tries eachchild resolve synchronously, and either:
NewModuleResult::Ready(id)— fast path when noasync hooks are active, no behaviour change;
PendingModule { handle, requests, pending_resolves, … }and returnsPending.finalize_pending_moduleawaits the pending futures, patches the URLs intothe request list, and calls
create_module_info. Single worker round-tripper resolve.
register_and_recursenow returnsRegisterOutcome::{Done, PendingFinalize}; bothrun_to_completion(main /side loads) and
drain_dyn_imports(dynamic imports viaop_import) dropthe V8 scope on
PendingFinalize, awaitfinalize_pending_module, re-enterscope, and call
finalize_after_pendingto register and recurse.ModuleRequest.needs_resolve, the duplicateresolve_asyncatrecursive_load::register_and_recurse_innerline 495,ModuleMap::resolve_async, and the synth-placeholder fallback are all gone— subsumed by the new flow. Placeholder slots on
PendingModule.requestsare never observed by anything downstream. Sync call sites (V8
module_resolve_callback,lazy_load_esm_module,do_load_job) keep usingnew_module_from_js_sourcewhich now errors clearly if a synchronouscontext ever encounters an async-resolve loader, instead of papering over it
with a placeholder.
Adds a spec test under
tests/specs/node/module_register_esm/blob_bare_specifiercovering theoriginal lume case: a passthrough
module.register()hook, a blob: moduleimporting a bare specifier resolved via the import map. The hook writes to
stderr when it sees the bare specifier from a blob: parent and the expected
output asserts on it, ruling out a silent bypass regression. The lume MDX
repro and the rollup-with-hooks
node:pathregression both pass.Closes bartlomieju/orchid-inbox#57