module: handle instantiated async module jobs in require(esm)

When require(esm) encounters a cached module job that is instantiated but not yet evaluated, run the evaluation. This catches an edge case previously missed in #57187. PR-URL: #58067 Fixes: #58061 Reviewed-By: Jacob Smith <jacob@frende.me>
nodejs · joyeecheung · Oct 31, 2024 · Feb 18, 2025 · Feb 17, 2025 · Mar 4, 2025
commit 08ff294e801aec940b79fe2c10517ff7f4a337d0
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -336,7 +336,7 @@ class ModuleLoader {
         throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
       }
       const status = job.module.getStatus();
-      debug('Module status', filename, status);
+      debug('Module status', job, status);
       if (status === kEvaluated) {
         return { wrap: job.module, namespace: job.module.getNamespaceSync(filename, parentFilename) };
       } else if (status === kInstantiated) {

diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js
@@ -21,8 +21,13 @@ let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
   debug = fn;
 });
 
-const { ModuleWrap, kInstantiated } = internalBinding('module_wrap');
-
+const {
+  ModuleWrap,
+  kErrored,
+  kEvaluated,
+  kInstantiated,
+  kUninstantiated,
+} = internalBinding('module_wrap');
 const { decorateErrorStack, kEmptyObject } = require('internal/util');
 const {
   getSourceMapsEnabled,
@@ -242,17 +247,34 @@ class ModuleJob extends ModuleJobBase {
 
   runSync(parent) {
     assert(this.module instanceof ModuleWrap);
-    if (this.instantiated !== undefined) {
-      return { __proto__: null, module: this.module };
+    let status = this.module.getStatus();
+
+    debug('ModuleJob.runSync', this.module);
+    // FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
+    // fully synchronous instead.
+    if (status === kUninstantiated) {
+      this.module.async = this.module.instantiateSync();
+      status = this.module.getStatus();
     }
+    if (status === kInstantiated || status === kErrored) {
+      const filename = urlToFilename(this.url);
+      const parentFilename = urlToFilename(parent?.filename);
+      this.module.async ??= this.module.isGraphAsync();
 
-    this.module.instantiate();
-    this.instantiated = PromiseResolve();
-    setHasStartedUserESMExecution();
-    const filename = urlToFilename(this.url);
-    const parentFilename = urlToFilename(parent?.filename);
-    const namespace = this.module.evaluateSync(filename, parentFilename);
-    return { __proto__: null, module: this.module, namespace };
+      if (this.module.async && !getOptionValue('--experimental-print-required-tla')) {
+        throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
+      }
+      if (status === kInstantiated) {
+        setHasStartedUserESMExecution();
+        const namespace = this.module.evaluateSync(filename, parentFilename);
+        return { __proto__: null, module: this.module, namespace };
+      }
+      throw this.module.getError();
+
+    } else if (status === kEvaluated) {
+      return { __proto__: null, module: this.module, namespace: this.module.getNamespaceSync() };
+    }
+    assert.fail(`Unexpected module status ${status}.`);
   }
 
   async run() {

diff --git a/src/debug_utils.h b/src/debug_utils.h
@@ -51,6 +51,7 @@ void NODE_EXTERN_PRIVATE FWrite(FILE* file, const std::string& str);
   V(NGTCP2_DEBUG)                                                              \
   V(SEA)                                                                       \
   V(WASI)                                                                      \
+  V(MODULE)                                                                    \
   V(MKSNAPSHOT)                                                                \
   V(SNAPSHOT_SERDES)                                                           \
   V(PERMISSION_MODEL)

diff --git a/src/module_wrap.cc b/src/module_wrap.cc
@@ -743,6 +743,16 @@ void ModuleWrap::GetStatus(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(module->GetStatus());
 }
 
+void ModuleWrap::IsGraphAsync(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  ModuleWrap* obj;
+  ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
+
+  Local<Module> module = obj->module_.Get(isolate);
+
+  args.GetReturnValue().Set(module->IsGraphAsync());
+}
+
 void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
   Isolate* isolate = args.GetIsolate();
   ModuleWrap* obj;
@@ -1090,6 +1100,7 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
       isolate, tpl, "createCachedData", CreateCachedData);
   SetProtoMethodNoSideEffect(isolate, tpl, "getNamespace", GetNamespace);
   SetProtoMethodNoSideEffect(isolate, tpl, "getStatus", GetStatus);
+  SetProtoMethodNoSideEffect(isolate, tpl, "isGraphAsync", IsGraphAsync);
   SetProtoMethodNoSideEffect(isolate, tpl, "getError", GetError);
   SetConstructorFunction(isolate, target, "ModuleWrap", tpl);
   isolate_data->set_module_wrap_constructor_template(tpl);
@@ -1146,6 +1157,7 @@ void ModuleWrap::RegisterExternalReferences(
   registry->Register(GetNamespace);
   registry->Register(GetStatus);
   registry->Register(GetError);
+  registry->Register(IsGraphAsync);
 
   registry->Register(CreateRequiredModuleFacade);
 

diff --git a/src/module_wrap.h b/src/module_wrap.h
@@ -110,6 +110,7 @@ class ModuleWrap : public BaseObject {
   static void Instantiate(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Evaluate(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetNamespace(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void IsGraphAsync(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetStatus(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetError(const v8::FunctionCallbackInfo<v8::Value>& args);
 

diff --git a/test/es-module/test-require-module-instantiated.mjs b/test/es-module/test-require-module-instantiated.mjs
@@ -0,0 +1,4 @@
+import '../common/index.mjs';
+import assert from 'node:assert';
+import { b, c } from '../fixtures/es-modules/require-module-instantiated/a.mjs';
+assert.strictEqual(b, c);
diff --git a/test/fixtures/es-modules/require-module-instantiated/a.mjs b/test/fixtures/es-modules/require-module-instantiated/a.mjs
@@ -0,0 +1,2 @@
+export { default as b } from './b.cjs';
+export { default as c } from './c.mjs';
diff --git a/test/fixtures/es-modules/require-module-instantiated/b.cjs b/test/fixtures/es-modules/require-module-instantiated/b.cjs
@@ -0,0 +1 @@
+module.exports = require('./c.mjs');
diff --git a/test/fixtures/es-modules/require-module-instantiated/c.mjs b/test/fixtures/es-modules/require-module-instantiated/c.mjs
@@ -0,0 +1,3 @@
+const foo = 1;
+export default foo;
+export { foo as 'module.exports' };
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		export { default as b } from './b.cjs';
		export { default as c } from './c.mjs';