Skip to content

Commit 9457af9

Browse files
committed
esm: apply code review suggestions
- Refactor getUndefinedCJSGlobalLike to use ArrayPrototypeFind - Change if-else chain to switch statement - Add single quotes around global names in error messages - Revert test file to use --input-type=module flag and original order - Update test regex patterns to expect quoted global names
1 parent aac23bb commit 9457af9

File tree

2 files changed

+29
-24
lines changed

2 files changed

+29
-24
lines changed

lib/internal/modules/esm/module_job.js

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const {
44
Array,
5+
ArrayPrototypeFind,
56
ArrayPrototypeJoin,
67
FunctionPrototype,
78
ObjectSetPrototypeOf,
@@ -60,14 +61,11 @@ const CJSGlobalLike = [
6061
'__filename',
6162
'__dirname',
6263
];
63-
const getUndefinedCJSGlobalLike = (errorMessage) => {
64-
for (const globalLike of CJSGlobalLike) {
65-
if (errorMessage === `${globalLike} is not defined`) {
66-
return globalLike;
67-
}
68-
}
69-
return null;
70-
};
64+
const getUndefinedCJSGlobalLike = (errorMessage) =>
65+
ArrayPrototypeFind(
66+
CJSGlobalLike,
67+
(globalLike) => errorMessage === `${globalLike} is not defined`,
68+
) ?? null;
7169

7270

7371
/**
@@ -81,18 +79,24 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
8179
if (e?.name === 'ReferenceError' && undefinedGlobal !== null) {
8280

8381
if (hasTopLevelAwait) {
84-
let advice = '';
85-
if (undefinedGlobal === 'require') {
86-
advice = 'replace require() with import';
87-
} else if (undefinedGlobal === 'module' || undefinedGlobal === 'exports') {
88-
advice = 'use export instead of module.exports/exports';
89-
} else if (undefinedGlobal === '__filename') {
90-
advice = 'use import.meta.filename instead';
91-
} else if (undefinedGlobal === '__dirname') {
92-
advice = 'use import.meta.dirname instead';
82+
let advice;
83+
switch (undefinedGlobal) {
84+
case 'require':
85+
advice = 'replace require() with import';
86+
break;
87+
case 'module':
88+
case 'exports':
89+
advice = 'use export instead of module.exports/exports';
90+
break;
91+
case '__filename':
92+
advice = 'use import.meta.filename instead';
93+
break;
94+
case '__dirname':
95+
advice = 'use import.meta.dirname instead';
96+
break;
9397
}
9498

95-
e.message = `Cannot determine intended module format because both ${undefinedGlobal} and top-level await are present. If the code is intended to be CommonJS, wrap await in an async function. If the code is intended to be an ES module, ${advice}.`;
99+
e.message = `Cannot determine intended module format because both '${undefinedGlobal}' and top-level await are present. If the code is intended to be CommonJS, wrap await in an async function. If the code is intended to be an ES module, ${advice}.`;
96100
e.code = 'ERR_AMBIGUOUS_MODULE_SYNTAX';
97101
return;
98102
}

test/es-module/test-esm-detect-ambiguous.mjs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ describe('Module syntax detection', { concurrency: !process.env.TEST_PARALLEL },
283283

284284
match(
285285
stderr,
286-
/ReferenceError: Cannot determine intended module format because both require and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, replace require\(\) with import\./
286+
/ReferenceError: Cannot determine intended module format because both 'require' and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, replace require\(\) with import\./
287287
);
288288
strictEqual(stdout, '');
289289
strictEqual(code, 1);
@@ -432,14 +432,15 @@ describe('cjs & esm ambiguous syntax case', () => {
432432
const { stderr, code, signal } = await spawnPromisified(
433433
process.execPath,
434434
[
435+
'--input-type=module',
435436
'--eval',
436-
`const fs = require('fs');\nawait 1;`,
437+
`await 1;\nconst fs = require('fs');`,
437438
]
438439
);
439440

440441
match(
441442
stderr,
442-
/ReferenceError: Cannot determine intended module format because both require and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, replace require\(\) with import\./
443+
/ReferenceError: Cannot determine intended module format because both 'require' and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, replace require\(\) with import\./
443444
);
444445

445446
strictEqual(code, 1);
@@ -457,7 +458,7 @@ describe('cjs & esm ambiguous syntax case', () => {
457458

458459
match(
459460
stderr,
460-
/ReferenceError: Cannot determine intended module format because both exports and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, use export instead of module\.exports\/exports\./
461+
/ReferenceError: Cannot determine intended module format because both 'exports' and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, use export instead of module\.exports\/exports\./
461462
);
462463

463464
strictEqual(code, 1);
@@ -475,7 +476,7 @@ describe('cjs & esm ambiguous syntax case', () => {
475476

476477
match(
477478
stderr,
478-
/ReferenceError: Cannot determine intended module format because both __filename and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, use import\.meta\.filename instead\./
479+
/ReferenceError: Cannot determine intended module format because both '__filename' and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, use import\.meta\.filename instead\./
479480
);
480481

481482
strictEqual(code, 1);
@@ -493,7 +494,7 @@ describe('cjs & esm ambiguous syntax case', () => {
493494

494495
match(
495496
stderr,
496-
/ReferenceError: Cannot determine intended module format because both __dirname and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, use import\.meta\.dirname instead\./
497+
/ReferenceError: Cannot determine intended module format because both '__dirname' and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, use import\.meta\.dirname instead\./
497498
);
498499

499500
strictEqual(code, 1);

0 commit comments

Comments
 (0)