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.

PR-URL: https://github.com/nodejs/node/pull/59396
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/6804466
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

src/module_wrap.cc

@@ -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() {
@@ -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());
@@ -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();
 
@@ -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()));
+  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);
 
-  for (int i = 0; i < requests->Length(); i++) {
+  // 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';
+  //
+  // 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.
+    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));
+    auto it = module_request_map.find(module_cache_key);
-
+    if (it == module_request_map.end()) {
-    Local<Value> module_i;
+      // This is the first request with this identity, record it - any mismatch
-    Local<Value> module_cache_i;
+      // for this would only be found in subsequent requests, so no need to
-    uint32_t coalesced_index = dependent->resolve_cache_[module_cache_key];
+      // check here.
-    if (!modules->Get(context, i).ToLocal(&module_i) ||
+      module_request_map[module_cache_key] = i;
-        !modules->Get(context, coalesced_index).ToLocal(&module_cache_i) ||
+    } else {  // This identity has been seen before, check for mismatch.
-        !module_i->StrictEquals(module_cache_i)) {
+      size_t first_seen_index = it->second;
-      // If the module is different from the one of the same request, throw an
+      // Check that the module is the same as the one resolved by the first
-      // error.
+      // request with this identity.
-      THROW_ERR_MODULE_LINK_MISMATCH(
+      Local<Value> first_seen_value =
-          realm->env(),
+          modules_vector[first_seen_index].Get(isolate);
-          "Module request '%s' at index %d must be linked "
+      if (!module_value->StrictEquals(first_seen_value)) {
-          "to the same module requested at index %d",
+        // If the module is different from the one of the same request, throw an
-          module_cache_key.ToString(),
+        // error.
-          i,
+        THROW_ERR_MODULE_LINK_MISMATCH(
-          coalesced_index);
+            realm->env(),
-      return;
+            "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;
 }
 
@@ -1012,11 +1017,10 @@ void ModuleWrap::HasAsyncGraph(Local<Name> property,
 // static
 MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
     Local<Context> context,
-    Local<String> specifier,
+    size_t module_request_index,
-    Local<FixedArray> import_attributes,
     Local<Module> referrer) {
   ModuleWrap* resolved_module;
-  if (!ResolveModule(context, specifier, import_attributes, referrer)
+  if (!ResolveModule(context, module_request_index, referrer)
            .To(&resolved_module)) {
     return {};
   }
@@ -1027,11 +1031,10 @@ MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
 // static
 MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
     Local<Context> context,
-    Local<String> specifier,
+    size_t module_request_index,
-    Local<FixedArray> import_attributes,
     Local<Module> referrer) {
   ModuleWrap* resolved_module;
-  if (!ResolveModule(context, specifier, import_attributes, referrer)
+  if (!ResolveModule(context, module_request_index, referrer)
            .To(&resolved_module)) {
     return {};
   }
@@ -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(
+Maybe<ModuleWrap*> ModuleWrap::ResolveModule(Local<Context> context,
-    Local<Context> context,
+                                             size_t module_request_index,
-    Local<String> specifier,
+                                             Local<Module> referrer) {
-    Local<FixedArray> import_attributes,
-    Local<Module> referrer) {
   Isolate* isolate = Isolate::GetCurrent();
   Environment* env = Environment::GetCurrent(context);
   if (env == nullptr) {
@@ -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);
+  size_t linked_module_count = dependent->linked_module_wraps_.size();
-  if (it == dependent->resolve_cache_.end()) {
+  if (linked_module_count > 0) {
-    THROW_ERR_VM_MODULE_LINK_FAILURE(
+    CHECK_LT(module_request_index, linked_module_count);
-        env,
+  } else {
-        "request for '%s' is not cached on module '%s'",
+    UNREACHABLE("Module resolution callback invoked for a module"
-        cache_key.specifier,
+                " without linked requests");
-        dependent->url_);
-    return Nothing<ModuleWrap*>();
   }
 
-  ModuleWrap* module_wrap = dependent->GetLinkedRequest(it->second);
+  return Just(dependent->linked_module_wraps_[module_request_index]);
-  CHECK_NOT_NULL(module_wrap);
-  return Just(module_wrap);
 }
 
 static MaybeLocal<Promise> ImportModuleDynamicallyWithPhase(

src/module_wrap.h

@@ -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
   };
 
@@ -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);
 
@@ -196,27 +192,22 @@ class ModuleWrap : public BaseObject {
 
   static v8::MaybeLocal<v8::Module> ResolveModuleCallback(
       v8::Local<v8::Context> context,
-      v8::Local<v8::String> specifier,
+      size_t module_request_index,
-      v8::Local<v8::FixedArray> import_attributes,
       v8::Local<v8::Module> referrer);
   static v8::MaybeLocal<v8::Object> ResolveSourceCallback(
       v8::Local<v8::Context> context,
-      v8::Local<v8::String> specifier,
+      size_t module_request_index,
-      v8::Local<v8::FixedArray> import_attributes,
       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(
+  static v8::Maybe<ModuleWrap*> ResolveModule(v8::Local<v8::Context> context,
-      v8::Local<v8::Context> context,
+                                              size_t module_request_index,
-      v8::Local<v8::String> specifier,
+                                              v8::Local<v8::Module> referrer);
-      v8::Local<v8::FixedArray> import_attributes,
-      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;
@@ -224,6 +215,11 @@ class ModuleWrap : public BaseObject {
   // 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

test/parallel/test-internal-module-wrap.js

@@ -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`
   });
 }