Skip to content

esm: use index-based resolution callbacks #59396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
joyeecheung wants to merge 3 commits into from
Closed

esm: use index-based resolution callbacks #59396

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7bce637
deps: V8: cherry-pick 3d0f462a17ff
joyeecheung Sep 16, 2025
2e71cf1
benchmark: add vm.SourceTextModule benchmark
joyeecheung Oct 10, 2025
076cdf6
esm: use index-based resolution callbacks
joyeecheung Aug 7, 2025
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
Prev Previous commit
esm: use index-based resolution callbacks 
This makes use of a new module resolution V8 API that passes in
an index to the module request array to identify the module
request, which simplifies the module linking process.
  • Loading branch information
@joyeecheung
joyeecheung committed Oct 13, 2025
commit 076cdf6a62092a8c1e2630e00c67e4578442b353
172 changes: 91 additions & 81 deletions src/module_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,15 +165,6 @@ ModuleWrap::ModuleWrap(Realm* realm,
}
MakeWeak();
module_.SetWeak();

HandleScope scope(realm->isolate());
Local<Context> context = realm->context();
Local<FixedArray> requests = module->GetModuleRequests();
for (int i = 0; i < requests->Length(); i++) {
ModuleCacheKey module_cache_key = ModuleCacheKey::From(
context, requests->Get(context, i).As<ModuleRequest>());
resolve_cache_[module_cache_key] = i;
}
}

ModuleWrap::~ModuleWrap() {
Expand All @@ -194,30 +185,6 @@ Local<Context> ModuleWrap::context() const {
return obj.As<Object>()->GetCreationContextChecked();
}

ModuleWrap* ModuleWrap::GetLinkedRequest(uint32_t index) {
DCHECK(IsLinked());
Isolate* isolate = env()->isolate();
EscapableHandleScope scope(isolate);
Local<Data> linked_requests_data =
object()->GetInternalField(kLinkedRequestsSlot);
DCHECK(linked_requests_data->IsValue() &&
linked_requests_data.As<Value>()->IsArray());
Local<Array> requests = linked_requests_data.As<Array>();

CHECK_LT(index, requests->Length());

Local<Value> module_value;
if (!requests->Get(context(), index).ToLocal(&module_value)) {
return nullptr;
}
CHECK(module_value->IsObject());
Local<Object> module_object = module_value.As<Object>();

ModuleWrap* module_wrap;
ASSIGN_OR_RETURN_UNWRAP(&module_wrap, module_object, nullptr);
return module_wrap;
}

ModuleWrap* ModuleWrap::GetFromModule(Environment* env,
Local<Module> module) {
auto range = env->hash_to_module_map.equal_range(module->GetIdentityHash());
Expand Down Expand Up @@ -653,6 +620,7 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo<Value>& args) {
// moduleWrap.link(moduleWraps)
void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
HandleScope handle_scope(isolate);
Realm* realm = Realm::GetCurrent(args);
Local<Context> context = realm->context();

Expand All @@ -664,33 +632,70 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
Local<FixedArray> requests =
dependent->module_.Get(isolate)->GetModuleRequests();
Local<Array> modules = args[0].As<Array>();
CHECK_EQ(modules->Length(), static_cast<uint32_t>(requests->Length()));

for (int i = 0; i < requests->Length(); i++) {
std::vector<Global<Value>> modules_vector;
if (FromV8Array(context, modules, &modules_vector).IsEmpty()) {
return;
}
size_t request_count = static_cast<size_t>(requests->Length());
CHECK_EQ(modules_vector.size(), request_count);
std::vector<ModuleWrap*> linked_module_wraps(request_count);

// Track the duplicated module requests. For example if a modulelooks like
// this:
//
// import { foo } from 'mod' with { type: 'json' };
// import source ModSource from 'mod' with { type: 'json' };
// import { baz } from 'mod2';
Comment thread
joyeecheung marked this conversation as resolved.
Resolved
//
// The first two module requests are identical. The map would look like
// { mod_key: 0, mod2_key: 2 } in this case, so that module request 0 and
// module request 1 would be mapped to mod_key and both should resolve to the
// module identified by module request 0 (the first one with this identity),
// and module request 2 should resolve the module identified by index 2.
std::unordered_map<ModuleCacheKey, size_t, ModuleCacheKey::Hash>
module_request_map;

for (size_t i = 0; i < request_count; i++) {
// TODO(joyeecheung): merge this with the serializeKey() in module_map.js.
// This currently doesn't sort the import attributes.

@joyeecheung joyeecheung Oct 9, 2025 •
edited
Loading

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.

I left a TODO here since it's a bit out of scope of this PR; I think we can move this check to JS land and for the built-in loader, this check can be omitted (since it can just skip re-initiating loading for the same module request) so we can save a bit of overhead there.

Local<Value> module_value = modules_vector[i].Get(isolate);
ModuleCacheKey module_cache_key = ModuleCacheKey::From(
context, requests->Get(context, i).As<ModuleRequest>());
DCHECK(dependent->resolve_cache_.contains(module_cache_key));

Local<Value> module_i;
Local<Value> module_cache_i;
uint32_t coalesced_index = dependent->resolve_cache_[module_cache_key];
if (!modules->Get(context, i).ToLocal(&module_i) ||
!modules->Get(context, coalesced_index).ToLocal(&module_cache_i) ||
!module_i->StrictEquals(module_cache_i)) {
// If the module is different from the one of the same request, throw an
// error.
THROW_ERR_MODULE_LINK_MISMATCH(
realm->env(),
"Module request '%s' at index %d must be linked "
"to the same module requested at index %d",
module_cache_key.ToString(),
i,
coalesced_index);
return;
auto it = module_request_map.find(module_cache_key);
if (it == module_request_map.end()) {
// This is the first request with this identity, record it - any mismatch
// for this would only be found in subsequent requests, so no need to
// check here.
module_request_map[module_cache_key] = i;
} else { // This identity has been seen before, check for mismatch.
size_t first_seen_index = it->second;
// Check that the module is the same as the one resolved by the first
// request with this identity.
Local<Value> first_seen_value =
modules_vector[first_seen_index].Get(isolate);
if (!module_value->StrictEquals(first_seen_value)) {
// If the module is different from the one of the same request, throw an
// error.
THROW_ERR_MODULE_LINK_MISMATCH(
realm->env(),
"Module request '%s' at index %d must be linked "
"to the same module requested at index %d",
module_cache_key.ToString(),
i,
first_seen_index);
return;
}
}

CHECK(module_value->IsObject()); // Guaranteed by link methods in JS land.
ModuleWrap* resolved =
BaseObject::Unwrap<ModuleWrap>(module_value.As<Object>());
CHECK_NOT_NULL(resolved); // Guaranteed by link methods in JS land.
linked_module_wraps[i] = resolved;
}

args.This()->SetInternalField(kLinkedRequestsSlot, modules);
std::swap(dependent->linked_module_wraps_, linked_module_wraps);
dependent->linked_ = true;
}

Expand Down Expand Up @@ -1012,11 +1017,10 @@ void ModuleWrap::HasAsyncGraph(Local<Name> property,
// static
MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
Local<Context> context,
Local<String> specifier,
Local<FixedArray> import_attributes,
size_t module_request_index,
Local<Module> referrer) {
ModuleWrap* resolved_module;
if (!ResolveModule(context, specifier, import_attributes, referrer)
if (!ResolveModule(context, module_request_index, referrer)
.To(&resolved_module)) {
return {};
}
Expand All @@ -1027,11 +1031,10 @@ MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
// static
MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
Local<Context> context,
Local<String> specifier,
Local<FixedArray> import_attributes,
size_t module_request_index,
Local<Module> referrer) {
ModuleWrap* resolved_module;
if (!ResolveModule(context, specifier, import_attributes, referrer)
if (!ResolveModule(context, module_request_index, referrer)
.To(&resolved_module)) {
return {};
}
Expand All @@ -1050,12 +1053,22 @@ MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
return module_source_object.As<Object>();
}

static std::string GetSpecifierFromModuleRequest(Local<Context> context,
Local<Module> referrer,
size_t module_request_index) {
Local<ModuleRequest> raw_request =
referrer->GetModuleRequests()
->Get(context, static_cast<int>(module_request_index))
.As<ModuleRequest>();
Local<String> specifier = raw_request->GetSpecifier();
Utf8Value specifier_utf8(Isolate::GetCurrent(), specifier);
return specifier_utf8.ToString();
}

// static
Maybe<ModuleWrap*> ModuleWrap::ResolveModule(
Local<Context> context,
Local<String> specifier,
Local<FixedArray> import_attributes,
Local<Module> referrer) {
Maybe<ModuleWrap*> ModuleWrap::ResolveModule(Local<Context> context,
size_t module_request_index,
Local<Module> referrer) {
Isolate* isolate = Isolate::GetCurrent();
Environment* env = Environment::GetCurrent(context);
if (env == nullptr) {
Expand All @@ -1065,37 +1078,34 @@ Maybe<ModuleWrap*> ModuleWrap::ResolveModule(
// Check that the referrer is not yet been instantiated.
DCHECK(referrer->GetStatus() <= Module::kInstantiated);

ModuleCacheKey cache_key =
ModuleCacheKey::From(context, specifier, import_attributes);

ModuleWrap* dependent = ModuleWrap::GetFromModule(env, referrer);
if (dependent == nullptr) {
std::string specifier =
GetSpecifierFromModuleRequest(context, referrer, module_request_index);
THROW_ERR_VM_MODULE_LINK_FAILURE(
env, "request for '%s' is from invalid module", cache_key.specifier);
env, "request for '%s' is from invalid module", specifier);
return Nothing<ModuleWrap*>();
}
if (!dependent->IsLinked()) {
std::string specifier =
GetSpecifierFromModuleRequest(context, referrer, module_request_index);
THROW_ERR_VM_MODULE_LINK_FAILURE(env,
"request for '%s' can not be resolved on "
"module '%s' that is not linked",
cache_key.specifier,
specifier,
dependent->url_);
return Nothing<ModuleWrap*>();
}

auto it = dependent->resolve_cache_.find(cache_key);
if (it == dependent->resolve_cache_.end()) {
THROW_ERR_VM_MODULE_LINK_FAILURE(
env,
"request for '%s' is not cached on module '%s'",
cache_key.specifier,
dependent->url_);
return Nothing<ModuleWrap*>();
size_t linked_module_count = dependent->linked_module_wraps_.size();
if (linked_module_count > 0) {
CHECK_LT(module_request_index, linked_module_count);
} else {
UNREACHABLE("Module resolution callback invoked for a module"
" without linked requests");
}

ModuleWrap* module_wrap = dependent->GetLinkedRequest(it->second);
CHECK_NOT_NULL(module_wrap);
return Just(module_wrap);
return Just(dependent->linked_module_wraps_[module_request_index]);
}

static MaybeLocal<Promise> ImportModuleDynamicallyWithPhase(
Expand Down
28 changes: 12 additions & 16 deletions src/module_wrap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,14 @@ struct ModuleCacheKey : public MemoryRetainer {
};

class ModuleWrap : public BaseObject {
using ResolveCache =
std::unordered_map<ModuleCacheKey, uint32_t, ModuleCacheKey::Hash>;

public:
enum InternalFields {
kModuleSlot = BaseObject::kInternalFieldCount,
kModuleSourceObjectSlot,
kSyntheticEvaluationStepsSlot,
kContextObjectSlot, // Object whose creation context is the target Context
kLinkedRequestsSlot, // Array of linked requests
kLinkedRequestsSlot, // Array of linked requests, each is a ModuleWrap JS
// wrapper object.
kInternalFieldCount
};

Expand Down Expand Up @@ -134,8 +132,6 @@ class ModuleWrap : public BaseObject {

bool IsLinked() const { return linked_; }

ModuleWrap* GetLinkedRequest(uint32_t index);

static v8::Local<v8::PrimitiveArray> GetHostDefinedOptions(
v8::Isolate* isolate, v8::Local<v8::Symbol> symbol);

Expand Down Expand Up @@ -196,34 +192,34 @@ class ModuleWrap : public BaseObject {

static v8::MaybeLocal<v8::Module> ResolveModuleCallback(
v8::Local<v8::Context> context,
v8::Local<v8::String> specifier,
v8::Local<v8::FixedArray> import_attributes,
size_t module_request_index,
v8::Local<v8::Module> referrer);
static v8::MaybeLocal<v8::Object> ResolveSourceCallback(
v8::Local<v8::Context> context,
v8::Local<v8::String> specifier,
v8::Local<v8::FixedArray> import_attributes,
size_t module_request_index,
v8::Local<v8::Module> referrer);
static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);

// This method may throw a JavaScript exception, so the return type is
// wrapped in a Maybe.
static v8::Maybe<ModuleWrap*> ResolveModule(
v8::Local<v8::Context> context,
v8::Local<v8::String> specifier,
v8::Local<v8::FixedArray> import_attributes,
v8::Local<v8::Module> referrer);
static v8::Maybe<ModuleWrap*> ResolveModule(v8::Local<v8::Context> context,
size_t module_request_index,
v8::Local<v8::Module> referrer);

std::string url_;
v8::Global<v8::Module> module_;
ResolveCache resolve_cache_;
contextify::ContextifyContext* contextify_context_ = nullptr;
bool synthetic_ = false;
bool linked_ = false;
// This depends on the module to be instantiated so it begins with a
// nullopt value.
std::optional<bool> has_async_graph_ = std::nullopt;
int module_hash_;
// Corresponds to the ModuleWrap* of the wrappers in kLinkedRequestsSlot.
// These are populated during Link(), and are only valid after that as
// convenient shortcuts, but do not hold the ModuleWraps alive. The actual
// strong references come from the array in kLinkedRequestsSlot.
std::vector<ModuleWrap*> linked_module_wraps_;
};

} // namespace loader
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-internal-module-wrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function testLinkMismatch() {
}, {
code: 'ERR_MODULE_LINK_MISMATCH',
// Test that ModuleCacheKey::ToString() is used in the error message.
message: `Module request 'ModuleCacheKey("bar")' at index 0 must be linked to the same module requested at index 1`
message: `Module request 'ModuleCacheKey("bar")' at index 1 must be linked to the same module requested at index 0`

@joyeecheung joyeecheung Oct 9, 2025 •
edited
Loading

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.

I think the new message is a bit more natural - "a later module request must be resolved to the same module identified by a former identical request", instead of "a former module request must be resolved to the same module identified by a later identical request".

});
}

Expand Down
Loading