Skip to content

Commit

Permalink
use plain async function wrapper instead of CommonJS module wrapper a…
Browse files Browse the repository at this point in the history
…round async function wrapper; add test

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
GeoffreyBooth and aduh95 committed Mar 11, 2024
1 parent c117865 commit 3fbeb43
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 15 deletions.
29 changes: 14 additions & 15 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1510,38 +1510,37 @@ 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,
code,
filename,
0,
0,
host_defined_options,
nullptr);
ContextifyContext::CompileFunctionAndCacheResult(
env,
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
Expand Down
13 changes: 13 additions & 0 deletions test/es-module/test-esm-detect-ambiguous.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 3fbeb43

Please sign in to comment.