Repository: nodejs/node
Pull request: src: separate module.hasAsyncGraph and module.hasTopLevelAwait

Clarify the names - hasAsyncGraph means either the module or its dependencies contains top-level await; hasTopLevelAwait means the module itself contains top-level await. Theoratically the former can be inferred from iterating over the dependencies but for the built-in loader it's currently not fully reliable until we eliminate async linking.

Also remove the hasTopLevelAwait method - we can simply put it on the module wrap for source text modules right after compilation.

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/internal/modules/esm/loader.js
patch: @@ -387,7 +387,7 @@ class ModuleLoader {
       if (!job.module) {
         assert.fail(getRaceMessage(filename, parentFilename));
       }
-      if (job.module.async) {
+      if (job.module.hasAsyncGraph) {
         throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
       }
       const status = job.module.getStatus();

filename: lib/internal/modules/esm/module_job.js
patch: @@ -327,13 +327,13 @@ class ModuleJob extends ModuleJobBase {
     // FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
     // fully synchronous instead.
     if (status === kUninstantiated) {
-      this.module.async = this.module.instantiateSync();
+      this.module.hasAsyncGraph = 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.hasAsyncGraph ??= this.module.isGraphAsync();
 
       if (this.module.async && !getOptionValue('--experimental-print-required-tla')) {
         throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
@@ -370,7 +370,7 @@ class ModuleJob extends ModuleJobBase {
     try {
       await this.module.evaluate(timeout, breakOnSigint);
     } catch (e) {
-      explainCommonJSGlobalLikeNotDefinedError(e, this.module.url, this.module.hasTopLevelAwait());
+      explainCommonJSGlobalLikeNotDefinedError(e, this.module.url, this.module.hasTopLevelAwait);
       throw e;
     }
     return { __proto__: null, module: this.module };
@@ -490,24 +490,25 @@ 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.async = this.module.instantiateSync();
+    this.module.hasAsyncGraph = this.module.instantiateSync();
     // 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.
     // TODO(joyeecheung): track the asynchroniticy using v8::Module::HasTopLevelAwait()
     // and we'll be able to throw right after compilation of the modules, using acron
-    // to find and print the TLA.
+    // to find and print the TLA. This requires the linking to be synchronous in case
+    // it runs into cached asynchronous modules that are not yet fetched.
     const parentFilename = urlToFilename(parent?.filename);
     const filename = urlToFilename(this.url);
-    if (this.module.async && !getOptionValue('--experimental-print-required-tla')) {
+    if (this.module.hasAsyncGraph && !getOptionValue('--experimental-print-required-tla')) {
       throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
     }
     setHasStartedUserESMExecution();
     try {
       const namespace = this.module.evaluateSync(filename, parentFilename);
       return { __proto__: null, module: this.module, namespace };
     } catch (e) {
-      explainCommonJSGlobalLikeNotDefinedError(e, this.module.url, this.module.hasTopLevelAwait());
+      explainCommonJSGlobalLikeNotDefinedError(e, this.module.url, this.module.hasTopLevelAwait);
       throw e;
     }
   }

filename: src/env_properties.h
patch: @@ -208,6 +208,7 @@
   V(gid_string, "gid")                                                         \
   V(groups_string, "groups")                                                   \
   V(has_regexp_groups_string, "hasRegExpGroups")                               \
+  V(has_top_level_await_string, "hasTopLevelAwait")                            \
   V(hash_string, "hash")                                                       \
   V(h2_string, "h2")                                                           \
   V(handle_string, "handle")                                                   \

filename: src/module_wrap.cc
patch: @@ -22,6 +22,7 @@ using errors::TryCatchScope;
 using node::contextify::ContextifyContext;
 using v8::Array;
 using v8::ArrayBufferView;
+using v8::Boolean;
 using v8::Context;
 using v8::Data;
 using v8::EscapableHandleScope;
@@ -414,6 +415,13 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
         return;
       }
 
+      if (that->Set(context,
+                    realm->env()->has_top_level_await_string(),
+                    Boolean::New(isolate, module->HasTopLevelAwait()))
+              .IsNothing()) {
+        return;
+      }
+
       if (that->Set(context,
                     realm->env()->source_url_string(),
                     module->GetUnboundModuleScript()->GetSourceURL())
@@ -999,27 +1007,6 @@ void ModuleWrap::IsGraphAsync(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(module->IsGraphAsync());
 }
 
-void ModuleWrap::HasTopLevelAwait(const FunctionCallbackInfo<Value>& args) {
-  Isolate* isolate = args.GetIsolate();
-  ModuleWrap* obj;
-  ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
-
-  Local<Module> module = obj->module_.Get(isolate);
-
-  // Check if module is valid
-  if (module.IsEmpty()) {
-    args.GetReturnValue().Set(false);
-    return;
-  }
-
-  // For source text modules, check if the graph is async
-  // For synthetic modules, it's always false
-  bool has_top_level_await =
-      module->IsSourceTextModule() && module->IsGraphAsync();
-
-  args.GetReturnValue().Set(has_top_level_await);
-}
-
 void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
   Isolate* isolate = args.GetIsolate();
   ModuleWrap* obj;
@@ -1443,8 +1430,6 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
   SetProtoMethodNoSideEffect(isolate, tpl, "getNamespace", GetNamespace);
   SetProtoMethodNoSideEffect(isolate, tpl, "getStatus", GetStatus);
   SetProtoMethodNoSideEffect(isolate, tpl, "isGraphAsync", IsGraphAsync);
-  SetProtoMethodNoSideEffect(
-      isolate, tpl, "hasTopLevelAwait", HasTopLevelAwait);
   SetProtoMethodNoSideEffect(isolate, tpl, "getError", GetError);
   SetConstructorFunction(isolate, target, "ModuleWrap", tpl);
   isolate_data->set_module_wrap_constructor_template(tpl);
@@ -1507,7 +1492,6 @@ void ModuleWrap::RegisterExternalReferences(
   registry->Register(GetStatus);
   registry->Register(GetError);
   registry->Register(IsGraphAsync);
-  registry->Register(HasTopLevelAwait);
 
   registry->Register(CreateRequiredModuleFacade);
 

filename: src/module_wrap.h
patch: @@ -119,8 +119,6 @@ class ModuleWrap : public BaseObject {
       v8::Local<v8::Module> module,
       v8::Local<v8::Object> meta);
 
-  static void HasTopLevelAwait(const v8::FunctionCallbackInfo<v8::Value>& args);
-
   v8::Local<v8::Context> context() const;
   v8::Maybe<bool> CheckUnsettledTopLevelAwait();