From a8ef8b8aa42a58e2c97bd9ecdef80104da093bcf Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 29 Aug 2025 16:33:20 +0200 Subject: [PATCH] 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. --- lib/internal/modules/esm/loader.js | 2 +- lib/internal/modules/esm/module_job.js | 15 ++++++------ src/env_properties.h | 1 + src/module_wrap.cc | 32 +++++++------------------- src/module_wrap.h | 2 -- 5 files changed, 18 insertions(+), 34 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 0443838c8a4ba5..03fe91a3d75109 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -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(); diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index a3b36add4fbe6a..3b48233d636c32 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -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,16 +490,17 @@ 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(); @@ -507,7 +508,7 @@ class ModuleJobSync extends ModuleJobBase { 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; } } diff --git a/src/env_properties.h b/src/env_properties.h index 945f4ffae4d97a..93800478c576c6 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -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") \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 72d910fa4a8144..e5f9b81a994bbf 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -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; @@ -395,6 +396,13 @@ void ModuleWrap::New(const FunctionCallbackInfo& 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()) @@ -948,27 +956,6 @@ void ModuleWrap::IsGraphAsync(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(module->IsGraphAsync()); } -void ModuleWrap::HasTopLevelAwait(const FunctionCallbackInfo& args) { - Isolate* isolate = args.GetIsolate(); - ModuleWrap* obj; - ASSIGN_OR_RETURN_UNWRAP(&obj, args.This()); - - Local 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& args) { Isolate* isolate = args.GetIsolate(); ModuleWrap* obj; @@ -1392,8 +1379,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); @@ -1456,7 +1441,6 @@ void ModuleWrap::RegisterExternalReferences( registry->Register(GetStatus); registry->Register(GetError); registry->Register(IsGraphAsync); - registry->Register(HasTopLevelAwait); registry->Register(CreateRequiredModuleFacade); diff --git a/src/module_wrap.h b/src/module_wrap.h index 467a9af1177b0f..ec8bf5b38e87a9 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -116,8 +116,6 @@ class ModuleWrap : public BaseObject { v8::Local module, v8::Local meta); - static void HasTopLevelAwait(const v8::FunctionCallbackInfo& args); - v8::Local context() const; v8::Maybe CheckUnsettledTopLevelAwait();