From d96a97a2b9b14a4f5475129fbfe495588798f36a Mon Sep 17 00:00:00 2001 From: Daniel Clark Date: Tue, 9 Feb 2021 13:23:39 -0800 Subject: [PATCH] module: make synthetic module evaluation steps return a Promise to support top level await MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Top level await expects that all module script evaluation returns a Promise. As such, update ModuleWrap::SyntheticModuleEvaluationStepsCallback to return a resolved Promise now that V8 has enabled top-level await by default. Unfortunately I don't have a spec reference that I can point to here because the Built-in modules proposal isn't yet updated for top level await. The corresponding change for Blink is https://chromium-review.googlesource.com/c/chromium/src/+/2568823. This will allow a workaround for Node in this V8 bugfix to be removed: https://chromium-review.googlesource.com/c/v8/v8/+/2673794. Fixes: https://github.com/nodejs/node/issues/37299 PR-URL: https://github.com/nodejs/node/pull/37300 Reviewed-By: Gus Caplan Reviewed-By: Michaƫl Zasso --- src/module_wrap.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 0ac36d4aa6373f..4885e65f9c5d47 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -665,7 +665,14 @@ MaybeLocal ModuleWrap::SyntheticModuleEvaluationStepsCallback( try_catch.ReThrow(); return MaybeLocal(); } - return Undefined(isolate); + + Local resolver; + if (!Promise::Resolver::New(context).ToLocal(&resolver)) { + return MaybeLocal(); + } + + resolver->Resolve(context, Undefined(isolate)).ToChecked(); + return resolver->GetPromise(); } void ModuleWrap::SetSyntheticExport(const FunctionCallbackInfo& args) {