Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions src/workerd/jsg/modules-new-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -3474,5 +3474,75 @@ KJ_TEST("Fallback receives REQUIRE source through require() resolution") {
KJ_ASSERT(fallbackCalled);
}

// ======================================================================================

KJ_TEST("Dynamic import from a redirected fallback module works") {
// Reproduces the bug where a module loaded via a fallback redirect fails to
// perform a dynamic import because V8's script origin (the module's canonical
// URL) does not match the import specifier stored in the registry's
// instantiation table.
//
// The fallback simulates a bare-specifier redirect:
// file:///pkg --> 301 to file:///canonical/pkg/index.mjs
// file:///canonical/pkg/index.mjs --> ESM with `import("./dep.mjs")`
// file:///canonical/pkg/dep.mjs --> ESM exporting a value
//
// Without the fix, the dynamic import fails with "Referring module not found
// in the registry: file:///canonical/pkg/index.mjs".

const auto pkg = "file:///pkg"_url;
const auto canonical = "file:///canonical/pkg/index.mjs"_url;
const auto dep = "file:///canonical/pkg/dep.mjs"_url;

// Source strings must outlive the Module objects that reference them
// (the ArrayPtr<const char> overload of newEsm does not take ownership).
auto pkgSource = kj::str("export async function load() { return await import('./dep.mjs'); }");
auto depSource = kj::str("export const value = 'ok';");

auto fallback = ModuleBundle::newFallbackBundle(
[&](const ResolveContext& context) -> kj::Maybe<kj::OneOf<kj::String, kj::Own<Module>>> {
if (context.normalizedSpecifier == pkg) {
// Redirect bare specifier to canonical URL.
return kj::Maybe<kj::OneOf<kj::String, kj::Own<Module>>>(kj::str(canonical.getHref()));
}
if (context.normalizedSpecifier == canonical) {
// The package entry point: dynamically imports a sibling module.
return kj::Maybe<kj::OneOf<kj::String, kj::Own<Module>>>(
Module::newEsm(canonical.clone(), Module::Type::FALLBACK, pkgSource.asPtr()));
}
if (context.normalizedSpecifier == dep) {
return kj::Maybe<kj::OneOf<kj::String, kj::Own<Module>>>(
Module::newEsm(dep.clone(), Module::Type::FALLBACK, depSource.asPtr()));
}
return kj::none;
});

CompilationObserver compilationObserver;

// An entry module that imports the bare specifier and calls load().
ModuleBundle::BundleBuilder bundleBuilder(BASE);
auto entry = kj::str("import { load } from 'pkg';\n"
"const m = await load();\n"
"export default m.value;\n");
bundleBuilder.addEsmModule("entry", entry);

auto registry = ModuleRegistry::Builder(BASE, ModuleRegistry::Builder::Options::ALLOW_FALLBACK)
.add(bundleBuilder.finish())
.add(kj::mv(fallback))
.finish();

PREAMBLE([&](Lock& js) {
auto attached = registry->attachToIsolate(js, compilationObserver);

JSG_TRY(js) {
auto value = ModuleRegistry::resolve(js, "file:///entry", "default"_kjc);
KJ_ASSERT(kj::str(value) == "ok");
}
JSG_CATCH(exception) {
js.throwException(kj::mv(exception));
}
});
}

} // namespace
} // namespace workerd::jsg::test
40 changes: 40 additions & 0 deletions src/workerd/jsg/modules-new.c++
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,22 @@ class IsolateModuleRegistry final {
break;
}
}

// When a module was loaded via a fallback redirect, V8's script origin
// is the module's canonical URL (Module::id()), but the instantiation
// is stored under the original import specifier. Fall back to the
// redirect mapping to find the entry.
if (maybeReferring == kj::none) {
KJ_IF_SOME(originalSpecifier, redirectedCanonicalIds.find(referrer)) {
for (auto type: kReferrerProbeOrder) {
KJ_IF_SOME(found, findResolved(type, originalSpecifier)) {
maybeReferring = found;
break;
}
}
}
}

auto& referring = JSG_REQUIRE_NONNULL(maybeReferring, TypeError,
kj::str("Referring module not found in the registry: ", referrer.getHref()));

Expand Down Expand Up @@ -1209,6 +1225,20 @@ class IsolateModuleRegistry final {
KJ_ASSERT(existing == replacement);
});

// When the module's canonical id differs from the import specifier
// (i.e. a fallback redirect was followed), record a mapping from the
// canonical URL back to the import specifier. V8 sets the compiled
// module's script origin to the canonical URL (Module::id()), so
// dynamicResolve() needs this mapping to find the entry when V8
// reports the canonical URL as the referrer for a dynamic import().
if (context.normalizedSpecifier != found.id()) {
redirectedCanonicalIds.upsert(found.id().clone(), context.normalizedSpecifier.clone(),
[](Url& existing, Url&& replacement) {
// Multiple import aliases can redirect to the same canonical id;
// the first one recorded is sufficient for the referrer lookup.
});
}

return kj::Maybe<Entry&>(entry);
}
return kj::none;
Expand All @@ -1224,6 +1254,16 @@ class IsolateModuleRegistry final {
// the same specifier but different definitions (bundle shadow vs builtin)
// map to distinct instantiations.
kj::HashMap<SpecifierContext, const Module*> resolutions;

// Reverse mapping from a module's canonical URL (Module::id()) to the
// import specifier stored as Entry.id in the instantiations table.
// Populated when a fallback redirect resolves an import specifier to a
// module whose canonical id differs (e.g. "file:///bundle/foo" redirects
// to "file:///project/node_modules/foo/index.mjs"). Used by
// dynamicResolve() as a fallback when the V8 script origin (the canonical
// URL) does not directly appear in the resolutions cache.
kj::HashMap<Url, Url> redirectedCanonicalIds;

friend class SyntheticModule;
};

Expand Down
Loading