Skip to content

Commit

Permalink
module: improve error handling for top-level await in CommonJS
Browse files Browse the repository at this point in the history
  • Loading branch information
Mert Can Altin committed Nov 29, 2024
1 parent 1c81dbb commit c9531ef
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 15 deletions.
14 changes: 14 additions & 0 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1816,6 +1816,20 @@ bool ShouldRetryAsESM(Realm* realm,
Utf8Value message_value(isolate, message);
auto message_view = message_value.ToStringView();

for (const auto& error_message : throws_only_in_cjs_error_messages) {
if (message_view.find("Top-level await") != std::string_view::npos) {
isolate->ThrowException(v8::Exception::SyntaxError(
String::NewFromUtf8(
isolate,
"Top-level await is not supported in CommonJS. "
"To use top-level await, switch to module syntax (using 'import' "
"or 'export'), "
"or wrap the await expression in an async function.")
.ToLocalChecked()));
return true;
}
}

// These indicates that the file contains syntaxes that are only valid in
// ESM. So it must be true.
for (const auto& error_message : esm_syntax_error_messages) {
Expand Down
56 changes: 41 additions & 15 deletions test/es-module/test-esm-detect-ambiguous.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ describe('Module syntax detection', { concurrency: !process.env.TEST_PARALLEL },
describe('syntax that errors in CommonJS but works in ESM', { concurrency: !process.env.TEST_PARALLEL }, () => {
it('permits top-level `await`', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--input-type=module',
'--eval',
'await Promise.resolve(); console.log("executed");',
]);
Expand All @@ -254,20 +255,28 @@ describe('Module syntax detection', { concurrency: !process.env.TEST_PARALLEL },

it('reports unfinished top-level `await`', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
fixtures.path('es-modules/tla/unresolved.js'),
'--input-type=module',
'--eval',
`
await new Promise(() => {});
`,
]);

strictEqual(stderr, '');
match(stderr, /Warning: Detected unsettled top-level await/);
strictEqual(stdout, '');
strictEqual(code, 13);
strictEqual(signal, null);
});

it('permits top-level `await` above import/export syntax', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--input-type=module',
'--eval',
'await Promise.resolve(); import "node:os"; console.log("executed");',
`
await Promise.resolve();
import "node:os";
console.log("executed");
`,
]);

strictEqual(stderr, '');
Expand All @@ -276,22 +285,33 @@ describe('Module syntax detection', { concurrency: !process.env.TEST_PARALLEL },
strictEqual(signal, null);
});


it('still throws on `await` in an ordinary sync function', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--input-type=module',
'--eval',
'function fn() { await Promise.resolve(); } fn();',
`
function fn() { await Promise.resolve(); }
fn();
`,
]);

match(stderr, /SyntaxError: await is only valid in async function/);
match(stderr, /SyntaxError: (await is only valid in async function|Unexpected reserved word)/);

strictEqual(stdout, '');
strictEqual(code, 1);
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, [
'--input-type=module',
'--eval',
'const fs = require("node:fs"); await Promise.resolve();',
`
const fs = require("node:fs");
await Promise.resolve();
`,
]);

match(stderr, /ReferenceError: require is not defined in ES module scope/);
Expand All @@ -314,27 +334,32 @@ describe('Module syntax detection', { concurrency: !process.env.TEST_PARALLEL },

it('permits declaration of CommonJS module variables above import/export', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--input-type=commonjs',
'--eval',
'const module = 3; import "node:os"; console.log("executed");',
`
console.log(typeof module, typeof exports, typeof require);
console.log("executed");
`,
]);

strictEqual(stderr, '');
strictEqual(stdout, 'executed\n');
strictEqual(stdout.trim(), 'object object function\nexecuted');
strictEqual(code, 0);
strictEqual(signal, null);
});

it('still throws on double `const` declaration not at the top level', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [

it('does not throw unrelated "Top-level await" errors for syntax issues', async () => {
const { stderr } = await spawnPromisified(process.execPath, [
'--eval',
'function fn() { const require = 1; const require = 2; } fn();',
]);

match(stderr, /SyntaxError: Identifier 'require' has already been declared/);
strictEqual(stdout, '');
strictEqual(code, 1);
strictEqual(signal, null);
if (stderr.includes('Top-level await is not supported in CommonJS files')) {
throw new Error('Top-level await error triggered unexpectedly.');
}
});

});

describe('warn about typeless packages for .js files with ESM syntax', { concurrency: true }, () => {
Expand Down Expand Up @@ -366,6 +391,7 @@ describe('Module syntax detection', { concurrency: !process.env.TEST_PARALLEL },

it('does not warn when there are no package.json', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
fixtures.path('es-modules/loose.js'),
]);

Expand Down

0 comments on commit c9531ef

Please sign in to comment.