lib,src: cache ModuleWrap.hasAsyncGraph

lib/internal/bootstrap/realm.js

@@ -359,7 +359,6 @@ class BuiltinModule {
         this.setExport('default', builtin.exports);
       });
     // Ensure immediate sync execution to capture exports now
-    this.module.link([]);
     this.module.instantiate();
     this.module.evaluate(-1, false);
     return this.module;

lib/internal/modules/esm/loader.js

@@ -387,13 +387,14 @@ class ModuleLoader {
       if (!job.module) {
         assert.fail(getRaceMessage(filename, parentFilename));
       }
-      if (job.module.hasAsyncGraph) {
-        throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
-      }
       const status = job.module.getStatus();
       debug('Module status', job, status);
+      // hasAsyncGraph is available after module been instantiated.
+      if (status >= kInstantiated && job.module.hasAsyncGraph) {
+        throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
+      }
       if (status === kEvaluated) {
-        return { wrap: job.module, namespace: job.module.getNamespaceSync(filename, parentFilename) };
+        return { wrap: job.module, namespace: job.module.getNamespace() };
       } else if (status === kInstantiated) {
         // When it's an async job cached by another import request,
         // which has finished linking but has not started its

lib/internal/modules/esm/module_job.js

@@ -324,17 +324,21 @@ class ModuleJob extends ModuleJobBase {
     let status = this.module.getStatus();
 
     debug('ModuleJob.runSync()', status, this.module);
-    // FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
-    // fully synchronous instead.
     if (status === kUninstantiated) {
-      this.module.hasAsyncGraph = this.module.instantiateSync();
+      // FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
+      // fully synchronous instead.
+      if (this.module.getModuleRequests().length > 0) {
+        const filename = urlToFilename(this.url);
+        const parentFilename = urlToFilename(parent?.filename);
+        throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
+      }
+      this.module.link([]);
+      this.module.instantiate();
       status = this.module.getStatus();
     }
     if (status === kInstantiated || status === kErrored) {
       const filename = urlToFilename(this.url);
       const parentFilename = urlToFilename(parent?.filename);
-      this.module.hasAsyncGraph ??= this.module.isGraphAsync();
-
       if (this.module.hasAsyncGraph && !getOptionValue('--experimental-print-required-tla')) {
         throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
       }
@@ -345,14 +349,19 @@ class ModuleJob extends ModuleJobBase {
       }
       throw this.module.getError();
     } else if (status === kEvaluating || status === kEvaluated) {
+      if (this.module.hasAsyncGraph) {
+        const filename = urlToFilename(this.url);
+        const parentFilename = urlToFilename(parent?.filename);
+        throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
+      }
       // kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
       // Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
       // detected earlier during the linking phase, though the CJS handling in the ESM
       // loader won't be able to emit warnings on pending circular exports like what
       // the CJS loader does.
       // TODO(joyeecheung): remove the re-invented require() in the ESM loader and
       // always handle CJS using the CJS loader to eliminate the quirks.
-      return { __proto__: null, module: this.module, namespace: this.module.getNamespaceSync() };
+      return { __proto__: null, module: this.module, namespace: this.module.getNamespace() };
     }
     assert.fail(`Unexpected module status ${status}.`);
   }
@@ -472,7 +481,7 @@ class ModuleJobSync extends ModuleJobBase {
       }
       return { __proto__: null, module: this.module };
     } else if (status === kInstantiated) {
-      // The evaluation may have been canceled because instantiateSync() detected TLA first.
+      // The evaluation may have been canceled because instantiate() detected TLA first.
       // But when it is imported again, it's fine to re-evaluate it asynchronously.
       const timeout = -1;
       const breakOnSigint = false;
@@ -490,7 +499,7 @@ class ModuleJobSync extends ModuleJobBase {
     debug('ModuleJobSync.runSync()', this.module);
     assert(this.phase === kEvaluationPhase);
     // TODO(joyeecheung): add the error decoration logic from the async instantiate.
-    this.module.hasAsyncGraph = this.module.instantiateSync();
+    this.module.instantiate();
     // If --experimental-print-required-tla is true, proceeds to evaluation even
     // if it's async because we want to search for the TLA and help users locate
     // them.

lib/internal/vm/module.js

@@ -469,7 +469,6 @@ class SyntheticModule extends Module {
       identifier,
     });
     // A synthetic module does not have dependencies.
-    this[kWrap].link([]);
     this[kWrap].instantiate();
   }
 

src/module_wrap.cc

@@ -56,6 +56,7 @@ using v8::ObjectTemplate;
 using v8::PrimitiveArray;
 using v8::Promise;
 using v8::PromiseRejectEvent;
+using v8::PropertyCallbackInfo;
 using v8::ScriptCompiler;
 using v8::ScriptOrigin;
 using v8::String;
@@ -158,6 +159,8 @@ ModuleWrap::ModuleWrap(Realm* realm,
 
   if (!synthetic_evaluation_step->IsUndefined()) {
     synthetic_ = true;
+    // Synthetic modules have no dependencies.
+    linked_ = true;
   }
   MakeWeak();
   module_.SetWeak();
@@ -240,7 +243,7 @@ Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
     return Just(true);
   }
 
-  if (!module->IsGraphAsync()) {  // There is no TLA, no need to check.
+  if (!HasAsyncGraph()) {  // There is no TLA, no need to check.
     return Just(true);
   }
 
@@ -263,6 +266,16 @@ Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
   return Just(false);
 }
 
+bool ModuleWrap::HasAsyncGraph() {
+  if (!has_async_graph_.has_value()) {
+    Isolate* isolate = env()->isolate();
+    HandleScope scope(isolate);
+
+    has_async_graph_ = module_.Get(isolate)->IsGraphAsync();
+  }
+  return has_async_graph_.value();
+}
+
 Local<PrimitiveArray> ModuleWrap::GetHostDefinedOptions(
     Isolate* isolate, Local<Symbol> id_symbol) {
   Local<PrimitiveArray> host_defined_options =
@@ -680,25 +693,28 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
   ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
   Local<Context> context = obj->context();
   Local<Module> module = obj->module_.Get(isolate);
+  Environment* env = realm->env();
 
   if (!obj->IsLinked()) {
-    THROW_ERR_VM_MODULE_LINK_FAILURE(realm->env(), "module is not linked");
+    THROW_ERR_VM_MODULE_LINK_FAILURE(env, "module is not linked");
     return;
   }
 
-  TryCatchScope try_catch(realm->env());
-  USE(module->InstantiateModule(
-      context, ResolveModuleCallback, ResolveSourceCallback));
+  {
+    TryCatchScope try_catch(env);
+    USE(module->InstantiateModule(
+        context, ResolveModuleCallback, ResolveSourceCallback));
 
-  if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
-    CHECK(!try_catch.Message().IsEmpty());
-    CHECK(!try_catch.Exception().IsEmpty());
-    AppendExceptionLine(realm->env(),
-                        try_catch.Exception(),
-                        try_catch.Message(),
-                        ErrorHandlingMode::MODULE_ERROR);
-    try_catch.ReThrow();
-    return;
+    if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
+      CHECK(!try_catch.Message().IsEmpty());
+      CHECK(!try_catch.Exception().IsEmpty());
+      AppendExceptionLine(env,
+                          try_catch.Exception(),
+                          try_catch.Message(),
+                          ErrorHandlingMode::MODULE_ERROR);
+      try_catch.ReThrow();
+      return;
+    }
   }
 }
 
@@ -783,37 +799,6 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
-void ModuleWrap::InstantiateSync(const FunctionCallbackInfo<Value>& args) {
-  Realm* realm = Realm::GetCurrent(args);
-  Isolate* isolate = args.GetIsolate();
-  ModuleWrap* obj;
-  ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
-  Local<Context> context = obj->context();
-  Local<Module> module = obj->module_.Get(isolate);
-  Environment* env = realm->env();
-
-  {
-    TryCatchScope try_catch(env);
-    USE(module->InstantiateModule(
-        context, ResolveModuleCallback, ResolveSourceCallback));
-
-    if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
-      CHECK(!try_catch.Message().IsEmpty());
-      CHECK(!try_catch.Exception().IsEmpty());
-      AppendExceptionLine(env,
-                          try_catch.Exception(),
-                          try_catch.Message(),
-                          ErrorHandlingMode::MODULE_ERROR);
-      try_catch.ReThrow();
-      return;
-    }
-  }
-
-  // TODO(joyeecheung): record Module::HasTopLevelAwait() in every ModuleWrap
-  // and infer the asynchronicity from a module's children during linking.
-  args.GetReturnValue().Set(module->IsGraphAsync());
-}
-
 Maybe<void> ThrowIfPromiseRejected(Realm* realm, Local<Promise> promise) {
   Isolate* isolate = realm->isolate();
   Local<Context> context = realm->context();
@@ -879,7 +864,7 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
-  if (module->IsGraphAsync()) {
+  if (obj->HasAsyncGraph()) {
     CHECK(env->options()->print_required_tla);
     auto stalled_messages =
         std::get<1>(module->GetStalledTopLevelAwaitMessages(isolate));
@@ -901,52 +886,15 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(module->GetModuleNamespace());
 }
 
-void ModuleWrap::GetNamespaceSync(const FunctionCallbackInfo<Value>& args) {
-  Realm* realm = Realm::GetCurrent(args);
-  Isolate* isolate = args.GetIsolate();
-  ModuleWrap* obj;
-  ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
-  Local<Module> module = obj->module_.Get(isolate);
-
-  switch (module->GetStatus()) {
-    case Module::Status::kUninstantiated:
-    case Module::Status::kInstantiating:
-      return realm->env()->ThrowError(
-          "Cannot get namespace, module has not been instantiated");
-    case Module::Status::kInstantiated:
-    case Module::Status::kEvaluating:
-    case Module::Status::kEvaluated:
-    case Module::Status::kErrored:
-      break;
-  }
-
-  if (module->IsGraphAsync()) {
-    return THROW_ERR_REQUIRE_ASYNC_MODULE(realm->env(), args[0], args[1]);
-  }
-  Local<Value> result = module->GetModuleNamespace();
-  args.GetReturnValue().Set(result);
-}
-
 void ModuleWrap::GetNamespace(const FunctionCallbackInfo<Value>& args) {
   Realm* realm = Realm::GetCurrent(args);
   Isolate* isolate = args.GetIsolate();
   ModuleWrap* obj;
   ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
 
   Local<Module> module = obj->module_.Get(isolate);
-
-  switch (module->GetStatus()) {
-    case Module::Status::kUninstantiated:
-    case Module::Status::kInstantiating:
-      return realm->env()->ThrowError(
-          "cannot get namespace, module has not been instantiated");
-    case Module::Status::kInstantiated:
-    case Module::Status::kEvaluating:
-    case Module::Status::kEvaluated:
-    case Module::Status::kErrored:
-      break;
-    default:
-      UNREACHABLE();
+  if (module->GetStatus() < Module::kInstantiated) {
+    return THROW_ERR_MODULE_NOT_INSTANTIATED(realm->env());
   }
 
   Local<Value> result = module->GetModuleNamespace();
@@ -997,23 +945,28 @@ void ModuleWrap::GetStatus(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(module->GetStatus());
 }
 
-void ModuleWrap::IsGraphAsync(const FunctionCallbackInfo<Value>& args) {
+void ModuleWrap::GetError(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());
+  args.GetReturnValue().Set(module->GetException());
 }
 
-void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
+void ModuleWrap::HasAsyncGraph(Local<Name> property,
+                               const PropertyCallbackInfo<Value>& args) {
   Isolate* isolate = args.GetIsolate();
+  Environment* env = Environment::GetCurrent(isolate);
   ModuleWrap* obj;
   ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
 
   Local<Module> module = obj->module_.Get(isolate);
-  args.GetReturnValue().Set(module->GetException());
+  if (module->GetStatus() < Module::kInstantiated) {
+    return THROW_ERR_MODULE_NOT_INSTANTIATED(env);
+  }
+
+  args.GetReturnValue().Set(obj->HasAsyncGraph());
 }
 
 // static
@@ -1417,10 +1370,8 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
 
   SetProtoMethod(isolate, tpl, "link", Link);
   SetProtoMethod(isolate, tpl, "getModuleRequests", GetModuleRequests);
-  SetProtoMethod(isolate, tpl, "instantiateSync", InstantiateSync);
-  SetProtoMethod(isolate, tpl, "evaluateSync", EvaluateSync);
-  SetProtoMethod(isolate, tpl, "getNamespaceSync", GetNamespaceSync);
   SetProtoMethod(isolate, tpl, "instantiate", Instantiate);
+  SetProtoMethod(isolate, tpl, "evaluateSync", EvaluateSync);
   SetProtoMethod(isolate, tpl, "evaluate", Evaluate);
   SetProtoMethod(isolate, tpl, "setExport", SetSyntheticExport);
   SetProtoMethod(isolate, tpl, "setModuleSourceObject", SetModuleSourceObject);
@@ -1429,9 +1380,12 @@ 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);
+
+  tpl->InstanceTemplate()->SetLazyDataProperty(
+      FIXED_ONE_BYTE_STRING(isolate, "hasAsyncGraph"), HasAsyncGraph);
+
   isolate_data->set_module_wrap_constructor_template(tpl);
 
   SetMethod(isolate,
@@ -1479,9 +1433,7 @@ void ModuleWrap::RegisterExternalReferences(
 
   registry->Register(Link);
   registry->Register(GetModuleRequests);
-  registry->Register(InstantiateSync);
   registry->Register(EvaluateSync);
-  registry->Register(GetNamespaceSync);
   registry->Register(Instantiate);
   registry->Register(Evaluate);
   registry->Register(SetSyntheticExport);
@@ -1491,7 +1443,7 @@ void ModuleWrap::RegisterExternalReferences(
   registry->Register(GetNamespace);
   registry->Register(GetStatus);
   registry->Register(GetError);
-  registry->Register(IsGraphAsync);
+  registry->Register(HasAsyncGraph);
 
   registry->Register(CreateRequiredModuleFacade);