From 228f5c793ea45fe31c7df624d3e98e536244e19b Mon Sep 17 00:00:00 2001
From: Geoffrey Booth <webadmin@geoffreybooth.com>
Date: Sun, 10 Mar 2024 19:10:27 -0700
Subject: [PATCH] use plain async function wrapper instead of CommonJS module
 wrapper around async function wrapper; add test

---
 doc/api/cli.md                               |  2 +-
 src/node_contextify.cc                       | 37 ++++++++------------
 test/es-module/test-esm-detect-ambiguous.mjs | 13 +++++++
 3 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 9d24741a395660..d7b514694595b0 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -799,7 +799,7 @@ CommonJS. This includes the following:
 * `export` statements.
 * `import.meta` references.
 * `await` at the top level of a module.
-* `const` declarations of the CommonJS wrapper variables (`require`, `module`,
+* Lexical redeclarations of the CommonJS wrapper variables (`require`, `module`,
   `exports`, `__dirname`, `__filename`).
 
 ### `--experimental-import-meta-resolve`
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 36d0f745d42a78..09079e963c7036 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -1510,38 +1510,31 @@ void ContextifyContext::ContainsModuleSyntax(
     if (!should_retry_as_esm) {
       for (const auto& error_message : throws_only_in_cjs_error_messages) {
         if (message.find(error_message) != std::string_view::npos) {
-          // Try parsing again where the user's code is wrapped within an async
-          // function. If the new parse succeeds, then the error was caused by
-          // either a top-level declaration of one of the CommonJS module
-          // variables, or a top-level `await`.
+          // Try parsing again where the CommonJS wrapper is replaced by an
+          // async function wrapper. If the new parse succeeds, then the error
+          // was caused by either a top-level declaration of one of the CommonJS
+          // module variables, or a top-level `await`.
           TryCatchScope second_parse_try_catch(env);
-          Local<String> wrapped_code =
+          code =
               String::Concat(isolate,
                              String::NewFromUtf8(isolate, "(async function() {")
                                  .ToLocalChecked(),
                              code);
-          wrapped_code = String::Concat(
+          code = String::Concat(
               isolate,
-              wrapped_code,
+              code,
               String::NewFromUtf8(isolate, "})();").ToLocalChecked());
-          ScriptCompiler::Source wrapped_source =
-              GetCommonJSSourceInstance(isolate,
-                                        wrapped_code,
-                                        filename,
-                                        0,
-                                        0,
-                                        host_defined_options,
-                                        nullptr);
-          ContextifyContext::CompileFunctionAndCacheResult(
-              env,
+          ScriptCompiler::Source wrapped_source = GetCommonJSSourceInstance(
+              isolate, code, filename, 0, 0, host_defined_options, nullptr);
+          std::ignore = ScriptCompiler::CompileFunction(
               context,
               &wrapped_source,
-              std::move(params),
-              std::vector<Local<Object>>(),
+              params.size(),
+              params.data(),
+              0,
+              nullptr,
               options,
-              true,
-              id_symbol,
-              second_parse_try_catch);
+              v8::ScriptCompiler::NoCacheReason::kNoCacheNoReason);
           if (!second_parse_try_catch.HasTerminated()) {
             if (second_parse_try_catch.HasCaught()) {
               // If on the second parse an error is thrown by ESM syntax, then
diff --git a/test/es-module/test-esm-detect-ambiguous.mjs b/test/es-module/test-esm-detect-ambiguous.mjs
index 5276439118f6d7..2067040114a86b 100644
--- a/test/es-module/test-esm-detect-ambiguous.mjs
+++ b/test/es-module/test-esm-detect-ambiguous.mjs
@@ -276,6 +276,19 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
       strictEqual(signal, null);
     });
 
+    it('throws on undefined `require` when top-level `await` triggers ESM parsing', async () => {
+      const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
+        '--experimental-detect-module',
+        '--eval',
+        'const fs = require("node:fs"); await Promise.resolve();',
+      ]);
+
+      match(stderr, /ReferenceError: require is not defined in ES module scope/);
+      strictEqual(stdout, '');
+      strictEqual(code, 1);
+      strictEqual(signal, null);
+    });
+
     it('permits declaration of CommonJS module variables', async () => {
       const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
         '--experimental-detect-module',