Skip to content

Commit bd0d964

Browse files
aduh95targos
authored andcommitted
module: clarify CJS global-like variables not defined error message
Fixes: #33741 PR-URL: #37852 Reviewed-By: Guy Bedford <guybedford@gmail.com>
1 parent 661e980 commit bd0d964

File tree

3 files changed

+85
-1
lines changed

3 files changed

+85
-1
lines changed

lib/internal/modules/esm/module_job.js

+42-1
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,20 @@ const {
44
ArrayPrototypeJoin,
55
ArrayPrototypeMap,
66
ArrayPrototypePush,
7+
ArrayPrototypeSome,
78
FunctionPrototype,
89
ObjectSetPrototypeOf,
910
PromiseAll,
1011
PromiseResolve,
1112
PromisePrototypeCatch,
1213
ReflectApply,
14+
RegExpPrototypeTest,
1315
SafeSet,
1416
StringPrototypeIncludes,
1517
StringPrototypeMatch,
1618
StringPrototypeReplace,
1719
StringPrototypeSplit,
20+
StringPrototypeStartsWith,
1821
} = primordials;
1922

2023
const { ModuleWrap } = internalBinding('module_wrap');
@@ -27,6 +30,19 @@ const noop = FunctionPrototype;
2730

2831
let hasPausedEntry = false;
2932

33+
const CJSGlobalLike = [
34+
'require',
35+
'module',
36+
'exports',
37+
'__filename',
38+
'__dirname',
39+
];
40+
const isCommonJSGlobalLikeNotDefinedError = (errorMessage) =>
41+
ArrayPrototypeSome(
42+
CJSGlobalLike,
43+
(globalLike) => errorMessage === `${globalLike} is not defined`
44+
);
45+
3046
/* A ModuleJob tracks the loading of a single Module, and the ModuleJobs of
3147
* its dependencies, over time. */
3248
class ModuleJob {
@@ -149,7 +165,32 @@ class ModuleJob {
149165
await this.instantiate();
150166
const timeout = -1;
151167
const breakOnSigint = false;
152-
await this.module.evaluate(timeout, breakOnSigint);
168+
try {
169+
await this.module.evaluate(timeout, breakOnSigint);
170+
} catch (e) {
171+
if (e?.name === 'ReferenceError' &&
172+
isCommonJSGlobalLikeNotDefinedError(e.message)) {
173+
e.message += ' in ES module scope';
174+
175+
if (StringPrototypeStartsWith(e.message, 'require ')) {
176+
e.message += ', you can use import instead';
177+
}
178+
179+
const packageConfig =
180+
StringPrototypeStartsWith(this.module.url, 'file://') &&
181+
RegExpPrototypeTest(/\.js(\?[^#]*)?(#.*)?$/, this.module.url) &&
182+
require('internal/modules/esm/resolve')
183+
.getPackageScopeConfig(this.module.url);
184+
if (packageConfig.type === 'module') {
185+
e.message +=
186+
'\nThis file is being treated as an ES module because it has a ' +
187+
`'.js' file extension and '${packageConfig.pjsonPath}' contains ` +
188+
'"type": "module". To treat it as a CommonJS script, rename it ' +
189+
'to use the \'.cjs\' file extension.';
190+
}
191+
}
192+
throw e;
193+
}
153194
return { module: this.module };
154195
}
155196
}

lib/internal/modules/esm/resolve.js

+1
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,7 @@ module.exports = {
845845
DEFAULT_CONDITIONS,
846846
defaultResolve,
847847
encodedSepRegEx,
848+
getPackageScopeConfig,
848849
getPackageType,
849850
packageExportsResolve,
850851
packageImportsResolve
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
'use strict';
2+
const common = require('../common');
3+
const fixtures = require('../common/fixtures');
4+
const assert = require('assert');
5+
const { pathToFileURL } = require('url');
6+
7+
assert.rejects(
8+
import('data:text/javascript,require;'),
9+
/require is not defined in ES module scope, you can use import instead$/
10+
).then(common.mustCall());
11+
assert.rejects(
12+
import('data:text/javascript,exports={};'),
13+
/exports is not defined in ES module scope$/
14+
).then(common.mustCall());
15+
16+
assert.rejects(
17+
import('data:text/javascript,require_custom;'),
18+
/^(?!in ES module scope)(?!use import instead).*$/
19+
).then(common.mustCall());
20+
21+
const pkgUrl = pathToFileURL(fixtures.path('/es-modules/package-type-module/'));
22+
assert.rejects(
23+
import(new URL('./cjs.js', pkgUrl)),
24+
/use the '\.cjs' file extension/
25+
).then(common.mustCall());
26+
assert.rejects(
27+
import(new URL('./cjs.js#target', pkgUrl)),
28+
/use the '\.cjs' file extension/
29+
).then(common.mustCall());
30+
assert.rejects(
31+
import(new URL('./cjs.js?foo=bar', pkgUrl)),
32+
/use the '\.cjs' file extension/
33+
).then(common.mustCall());
34+
assert.rejects(
35+
import(new URL('./cjs.js?foo=bar#target', pkgUrl)),
36+
/use the '\.cjs' file extension/
37+
).then(common.mustCall());
38+
39+
assert.rejects(
40+
import('data:text/javascript,require;//.js'),
41+
/^(?!use the '\.cjs' file extension).*$/
42+
).then(common.mustCall());

0 commit comments

Comments
 (0)