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
  • Loading branch information
GeoffreyBooth committed Mar 11, 2024
1 parent 026d6c1 commit 228f5c7
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 23 deletions.
2 changes: 1 addition & 1 deletion doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
37 changes: 15 additions & 22 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
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 228f5c7

Please sign in to comment.