module: link module with a module request record

When a module is being statically linked with module requests, if two
module requests with a same specifier but different attributes are
resolved to two modules, the module requests should be linked to these
two modules.

Author: legendecas

lib/internal/modules/esm/module_job.js

@@ -190,8 +190,7 @@ class ModuleJob extends ModuleJobBase {
     const evaluationDepJobs = Array(moduleRequests.length);
     ObjectSetPrototypeOf(evaluationDepJobs, null);
 
-    // Specifiers should be aligned with the moduleRequests array in order.
-    const specifiers = Array(moduleRequests.length);
+    // Modules should be aligned with the moduleRequests array in order.
     const modulePromises = Array(moduleRequests.length);
     // Track each loop for whether it is an evaluation phase or source phase request.
     let isEvaluation;
@@ -217,11 +216,10 @@ class ModuleJob extends ModuleJobBase {
         return job.modulePromise;
       });
       modulePromises[idx] = modulePromise;
-      specifiers[idx] = specifier;
     }
 
     const modules = await SafePromiseAllReturnArrayLike(modulePromises);
-    this.module.link(specifiers, modules);
+    this.module.link(modules);
 
     return evaluationDepJobs;
   }
@@ -433,22 +431,20 @@ class ModuleJobSync extends ModuleJobBase {
     this.#loader.loadCache.set(this.url, this.type, this);
     try {
       const moduleRequests = this.module.getModuleRequests();
-      // Specifiers should be aligned with the moduleRequests array in order.
-      const specifiers = Array(moduleRequests.length);
+      // Modules should be aligned with the moduleRequests array in order.
       const modules = Array(moduleRequests.length);
       const evaluationDepJobs = Array(moduleRequests.length);
       let j = 0;
       for (let i = 0; i < moduleRequests.length; ++i) {
         const { specifier, attributes, phase } = moduleRequests[i];
         const job = this.#loader.getModuleJobForRequire(specifier, this.url, attributes, phase);
-        specifiers[i] = specifier;
         modules[i] = job.module;
         if (phase === kEvaluationPhase) {
           evaluationDepJobs[j++] = job;
         }
       }
       evaluationDepJobs.length = j;
-      this.module.link(specifiers, modules);
+      this.module.link(modules);
       this.linked = evaluationDepJobs;
     } finally {
       // Restore it - if it succeeds, we'll reset in the caller; Otherwise it's

lib/internal/vm/module.js

@@ -148,23 +148,10 @@ class Module {
       });
     }
 
-    let registry = { __proto__: null };
     if (sourceText !== undefined) {
       this[kWrap] = new ModuleWrap(identifier, context, sourceText,
                                    options.lineOffset, options.columnOffset,
                                    options.cachedData);
-      registry = {
-        __proto__: null,
-        initializeImportMeta: options.initializeImportMeta,
-        importModuleDynamically: options.importModuleDynamically ?
-          importModuleDynamicallyWrap(options.importModuleDynamically) :
-          undefined,
-      };
-      // This will take precedence over the referrer as the object being
-      // passed into the callbacks.
-      registry.callbackReferrer = this;
-      const { registerModule } = require('internal/modules/esm/utils');
-      registerModule(this[kWrap], registry);
     } else {
       assert(syntheticEvaluationSteps);
       this[kWrap] = new ModuleWrap(identifier, context,
@@ -315,6 +302,19 @@ class SourceTextModule extends Module {
       importModuleDynamically,
     });
 
+    const registry = {
+      __proto__: null,
+      initializeImportMeta: options.initializeImportMeta,
+      importModuleDynamically: options.importModuleDynamically ?
+        importModuleDynamicallyWrap(options.importModuleDynamically) :
+        undefined,
+    };
+    // This will take precedence over the referrer as the object being
+    // passed into the callbacks.
+    registry.callbackReferrer = this;
+    const { registerModule } = require('internal/modules/esm/utils');
+    registerModule(this[kWrap], registry);
+
     this.#moduleRequests = ObjectFreeze(ArrayPrototypeMap(this[kWrap].getModuleRequests(), (request) => {
       return ObjectFreeze({
         __proto__: null,
@@ -329,8 +329,7 @@ class SourceTextModule extends Module {
     this.#statusOverride = 'linking';
 
     // Iterates the module requests and links with the linker.
-    // Specifiers should be aligned with the moduleRequests array in order.
-    const specifiers = Array(this.#moduleRequests.length);
+    // Modules should be aligned with the moduleRequests array in order.
     const modulePromises = Array(this.#moduleRequests.length);
     // Iterates with index to avoid calling into userspace with `Symbol.iterator`.
     for (let idx = 0; idx < this.#moduleRequests.length; idx++) {
@@ -357,12 +356,11 @@ class SourceTextModule extends Module {
           return module[kWrap];
         });
       modulePromises[idx] = modulePromise;
-      specifiers[idx] = specifier;
     }
 
     try {
       const modules = await SafePromiseAllReturnArrayLike(modulePromises);
-      this[kWrap].link(specifiers, modules);
+      this[kWrap].link(modules);
     } catch (e) {
       this.#error = e;
       throw e;

src/module_wrap.cc

@@ -46,7 +46,6 @@ using v8::Message;
 using v8::MicrotaskQueue;
 using v8::Module;
 using v8::ModuleImportPhase;
-using v8::ModuleRequest;
 using v8::Name;
 using v8::Nothing;
 using v8::Null;
@@ -63,6 +62,56 @@ using v8::UnboundModuleScript;
 using v8::Undefined;
 using v8::Value;
 
+ModulePhase to_phase_constant(ModuleImportPhase phase) {
+  switch (phase) {
+    case ModuleImportPhase::kEvaluation:
+      return kEvaluationPhase;
+    case ModuleImportPhase::kSource:
+      return kSourcePhase;
+  }
+  UNREACHABLE();
+}
+
+void ModuleRequest::MemoryInfo(MemoryTracker* tracker) const {
+  tracker->TrackField("specifier", specifier);
+  tracker->TrackField("import_attributes", import_attributes);
+  tracker->TrackField("phase", static_cast<int>(phase));
+}
+
+template <int elements_per_attribute>
+ModuleRequest ModuleRequest::From(Local<Context> context,
+                                  Local<String> specifier,
+                                  Local<FixedArray> import_attributes,
+                                  ModulePhase phase) {
+  CHECK_EQ(import_attributes->Length() % elements_per_attribute, 0);
+  Isolate* isolate = context->GetIsolate();
+  size_t num_attributes = import_attributes->Length() / elements_per_attribute;
+  ImportAttributeVector attributes;
+  attributes.reserve(num_attributes);
+
+  for (int i = 0; i < import_attributes->Length();
+       i += elements_per_attribute) {
+    Local<String> v8_key = import_attributes->Get(context, i).As<String>();
+    Local<String> v8_value =
+        import_attributes->Get(context, i + 1).As<String>();
+    Utf8Value key_utf8(isolate, v8_key);
+    Utf8Value value_utf8(isolate, v8_value);
+
+    attributes.emplace_back(key_utf8.ToString(), value_utf8.ToString());
+  }
+
+  Utf8Value utf8_specifier(isolate, specifier);
+  return ModuleRequest{utf8_specifier.ToString(), attributes, phase};
+}
+
+ModuleRequest ModuleRequest::From(Local<Context> context,
+                                  Local<v8::ModuleRequest> v8_request) {
+  return From(context,
+              v8_request->GetSpecifier(),
+              v8_request->GetImportAttributes(),
+              to_phase_constant(v8_request->GetPhase()));
+}
+
 ModuleWrap::ModuleWrap(Realm* realm,
                        Local<Object> object,
                        Local<Module> module,
@@ -422,16 +471,6 @@ MaybeLocal<Module> ModuleWrap::CompileSourceTextModule(
   return scope.Escape(module);
 }
 
-ModulePhase to_phase_constant(ModuleImportPhase phase) {
-  switch (phase) {
-    case ModuleImportPhase::kEvaluation:
-      return kEvaluationPhase;
-    case ModuleImportPhase::kSource:
-      return kSourcePhase;
-  }
-  UNREACHABLE();
-}
-
 static Local<Object> createImportAttributesContainer(
     Realm* realm,
     Isolate* isolate,
@@ -462,8 +501,8 @@ static Local<Array> createModuleRequestsContainer(
   LocalVector<Value> requests(isolate, raw_requests->Length());
 
   for (int i = 0; i < raw_requests->Length(); i++) {
-    Local<ModuleRequest> module_request =
-        raw_requests->Get(realm->context(), i).As<ModuleRequest>();
+    Local<v8::ModuleRequest> module_request =
+        raw_requests->Get(realm->context(), i).As<v8::ModuleRequest>();
 
     Local<String> specifier = module_request->GetSpecifier();
 
@@ -509,7 +548,7 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo<Value>& args) {
       realm, isolate, module->GetModuleRequests()));
 }
 
-// moduleWrap.link(specifiers, moduleWraps)
+// moduleWrap.link(moduleWraps)
 void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
   Realm* realm = Realm::GetCurrent(args);
   Isolate* isolate = args.GetIsolate();
@@ -518,33 +557,28 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
   ModuleWrap* dependent;
   ASSIGN_OR_RETURN_UNWRAP(&dependent, args.This());
 
-  CHECK_EQ(args.Length(), 2);
+  CHECK_EQ(args.Length(), 1);
 
-  Local<Array> specifiers = args[0].As<Array>();
-  Local<Array> modules = args[1].As<Array>();
-  CHECK_EQ(specifiers->Length(), modules->Length());
+  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>> specifiers_buffer;
-  if (FromV8Array(context, specifiers, &specifiers_buffer).IsNothing()) {
-    return;
-  }
   std::vector<Global<Value>> modules_buffer;
   if (FromV8Array(context, modules, &modules_buffer).IsNothing()) {
     return;
   }
 
-  for (uint32_t i = 0; i < specifiers->Length(); i++) {
-    Local<String> specifier_str =
-        specifiers_buffer[i].Get(isolate).As<String>();
+  for (uint32_t i = 0; i < modules_buffer.size(); i++) {
     Local<Object> module_object = modules_buffer[i].Get(isolate).As<Object>();
 
     CHECK(
         realm->isolate_data()->module_wrap_constructor_template()->HasInstance(
             module_object));
 
-    Utf8Value specifier(isolate, specifier_str);
-    dependent->resolve_cache_[specifier.ToString()].Reset(isolate,
-                                                          module_object);
+    ModuleRequest module_request = ModuleRequest::From(
+        context, requests->Get(context, i).As<v8::ModuleRequest>());
+    dependent->resolve_cache_[module_request].Reset(isolate, module_object);
   }
 }
 
@@ -924,27 +958,28 @@ MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
     return MaybeLocal<Module>();
   }
 
-  Utf8Value specifier_utf8(isolate, specifier);
-  std::string specifier_std(*specifier_utf8, specifier_utf8.length());
+  // ResolveModuleCallback for kEvaluationPhase only.
+  const ModulePhase phase = ModulePhase::kEvaluationPhase;
+  ModuleRequest request =
+      ModuleRequest::From(context, specifier, import_attributes, phase);
 
   ModuleWrap* dependent = GetFromModule(env, referrer);
   if (dependent == nullptr) {
     THROW_ERR_VM_MODULE_LINK_FAILURE(
-        env, "request for '%s' is from invalid module", specifier_std);
+        env, "request for '%s' is from invalid module", request.specifier);
     return MaybeLocal<Module>();
   }
 
-  if (dependent->resolve_cache_.count(specifier_std) != 1) {
+  if (dependent->resolve_cache_.count(request) != 1) {
     THROW_ERR_VM_MODULE_LINK_FAILURE(
-        env, "request for '%s' is not in cache", specifier_std);
+        env, "request for '%s' is not in cache", request.specifier);
     return MaybeLocal<Module>();
   }
 
-  Local<Object> module_object =
-      dependent->resolve_cache_[specifier_std].Get(isolate);
+  Local<Object> module_object = dependent->resolve_cache_[request].Get(isolate);
   if (module_object.IsEmpty() || !module_object->IsObject()) {
     THROW_ERR_VM_MODULE_LINK_FAILURE(
-        env, "request for '%s' did not return an object", specifier_std);
+        env, "request for '%s' did not return an object", request.specifier);
     return MaybeLocal<Module>();
   }
 
@@ -965,27 +1000,28 @@ MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
     return MaybeLocal<Object>();
   }
 
-  Utf8Value specifier_utf8(isolate, specifier);
-  std::string specifier_std(*specifier_utf8, specifier_utf8.length());
+  // ResolveSourceCallback for kSourcePhase only.
+  const ModulePhase phase = ModulePhase::kSourcePhase;
+  ModuleRequest request =
+      ModuleRequest::From(context, specifier, import_attributes, phase);
 
   ModuleWrap* dependent = GetFromModule(env, referrer);
   if (dependent == nullptr) {
     THROW_ERR_VM_MODULE_LINK_FAILURE(
-        env, "request for '%s' is from invalid module", specifier_std);
+        env, "request for '%s' is from invalid module", request.specifier);
     return MaybeLocal<Object>();
   }
 
-  if (dependent->resolve_cache_.count(specifier_std) != 1) {
+  if (dependent->resolve_cache_.count(request) != 1) {
     THROW_ERR_VM_MODULE_LINK_FAILURE(
-        env, "request for '%s' is not in cache", specifier_std);
+        env, "request for '%s' is not in cache", request.specifier);
     return MaybeLocal<Object>();
   }
 
-  Local<Object> module_object =
-      dependent->resolve_cache_[specifier_std].Get(isolate);
+  Local<Object> module_object = dependent->resolve_cache_[request].Get(isolate);
   if (module_object.IsEmpty() || !module_object->IsObject()) {
     THROW_ERR_VM_MODULE_LINK_FAILURE(
-        env, "request for '%s' did not return an object", specifier_std);
+        env, "request for '%s' did not return an object", request.specifier);
     return MaybeLocal<Object>();
   }
 

src/module_wrap.h

@@ -3,9 +3,9 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
+#include <map>
 #include <optional>
 #include <string>
-#include <unordered_map>
 #include <vector>
 #include "base_object.h"
 #include "v8-script.h"
@@ -38,6 +38,43 @@ enum ModulePhase : int {
   kEvaluationPhase = 2,
 };
 
+struct ModuleRequest : public MemoryRetainer {
+  using ImportAttributeVector =
+      std::vector<std::pair<std::string, std::string>>;
+
+  std::string specifier;
+  ImportAttributeVector import_attributes;
+  ModulePhase phase;
+
+  ModuleRequest(std::string specifier,
+                ImportAttributeVector import_attributes,
+                ModulePhase phase)
+      : specifier(specifier),
+        import_attributes(import_attributes),
+        phase(phase) {}
+
+  SET_MEMORY_INFO_NAME(ModuleRequest)
+  SET_SELF_SIZE(ModuleRequest)
+  void MemoryInfo(MemoryTracker* tracker) const override;
+
+  template <int elements_per_attribute = 3>
+  static ModuleRequest From(v8::Local<v8::Context> context,
+                            v8::Local<v8::String> specifier,
+                            v8::Local<v8::FixedArray> import_attributes,
+                            ModulePhase phase);
+  static ModuleRequest From(v8::Local<v8::Context> context,
+                            v8::Local<v8::ModuleRequest> v8_request);
+
+  bool operator<(const ModuleRequest& other) const {
+    if (specifier != other.specifier) {
+      return specifier < other.specifier;
+    } else if (import_attributes != other.import_attributes) {
+      return import_attributes < other.import_attributes;
+    }
+    return phase < other.phase;
+  }
+};
+
 class ModuleWrap : public BaseObject {
  public:
   enum InternalFields {
@@ -149,7 +186,7 @@ class ModuleWrap : public BaseObject {
   static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);
 
   v8::Global<v8::Module> module_;
-  std::unordered_map<std::string, v8::Global<v8::Object>> resolve_cache_;
+  std::map<ModuleRequest, v8::Global<v8::Object>> resolve_cache_;
   contextify::ContextifyContext* contextify_context_ = nullptr;
   bool synthetic_ = false;
   int module_hash_;