Skip to content

Commit

Permalink
module: clarify CJS global-like variables not defined error message
Browse files Browse the repository at this point in the history
Fixes: #33741

PR-URL: #37852
Reviewed-By: Guy Bedford <guybedford@gmail.com>
  • Loading branch information
aduh95 authored and targos committed Jun 11, 2021
1 parent 661e980 commit bd0d964
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 1 deletion.
43 changes: 42 additions & 1 deletion lib/internal/modules/esm/module_job.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@ const {
ArrayPrototypeJoin,
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypeSome,
FunctionPrototype,
ObjectSetPrototypeOf,
PromiseAll,
PromiseResolve,
PromisePrototypeCatch,
ReflectApply,
RegExpPrototypeTest,
SafeSet,
StringPrototypeIncludes,
StringPrototypeMatch,
StringPrototypeReplace,
StringPrototypeSplit,
StringPrototypeStartsWith,
} = primordials;

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

let hasPausedEntry = false;

const CJSGlobalLike = [
'require',
'module',
'exports',
'__filename',
'__dirname',
];
const isCommonJSGlobalLikeNotDefinedError = (errorMessage) =>
ArrayPrototypeSome(
CJSGlobalLike,
(globalLike) => errorMessage === `${globalLike} is not defined`
);

/* A ModuleJob tracks the loading of a single Module, and the ModuleJobs of
* its dependencies, over time. */
class ModuleJob {
Expand Down Expand Up @@ -149,7 +165,32 @@ class ModuleJob {
await this.instantiate();
const timeout = -1;
const breakOnSigint = false;
await this.module.evaluate(timeout, breakOnSigint);
try {
await this.module.evaluate(timeout, breakOnSigint);
} catch (e) {
if (e?.name === 'ReferenceError' &&
isCommonJSGlobalLikeNotDefinedError(e.message)) {
e.message += ' in ES module scope';

if (StringPrototypeStartsWith(e.message, 'require ')) {
e.message += ', you can use import instead';
}

const packageConfig =
StringPrototypeStartsWith(this.module.url, 'file://') &&
RegExpPrototypeTest(/\.js(\?[^#]*)?(#.*)?$/, this.module.url) &&
require('internal/modules/esm/resolve')
.getPackageScopeConfig(this.module.url);
if (packageConfig.type === 'module') {
e.message +=
'\nThis file is being treated as an ES module because it has a ' +
`'.js' file extension and '${packageConfig.pjsonPath}' contains ` +
'"type": "module". To treat it as a CommonJS script, rename it ' +
'to use the \'.cjs\' file extension.';
}
}
throw e;
}
return { module: this.module };
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,7 @@ module.exports = {
DEFAULT_CONDITIONS,
defaultResolve,
encodedSepRegEx,
getPackageScopeConfig,
getPackageType,
packageExportsResolve,
packageImportsResolve
Expand Down
42 changes: 42 additions & 0 deletions test/es-module/test-esm-undefined-cjs-global-like-variables.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'use strict';
const common = require('../common');
const fixtures = require('../common/fixtures');
const assert = require('assert');
const { pathToFileURL } = require('url');

assert.rejects(
import('data:text/javascript,require;'),
/require is not defined in ES module scope, you can use import instead$/
).then(common.mustCall());
assert.rejects(
import('data:text/javascript,exports={};'),
/exports is not defined in ES module scope$/
).then(common.mustCall());

assert.rejects(
import('data:text/javascript,require_custom;'),
/^(?!in ES module scope)(?!use import instead).*$/
).then(common.mustCall());

const pkgUrl = pathToFileURL(fixtures.path('/es-modules/package-type-module/'));
assert.rejects(
import(new URL('./cjs.js', pkgUrl)),
/use the '\.cjs' file extension/
).then(common.mustCall());
assert.rejects(
import(new URL('./cjs.js#target', pkgUrl)),
/use the '\.cjs' file extension/
).then(common.mustCall());
assert.rejects(
import(new URL('./cjs.js?foo=bar', pkgUrl)),
/use the '\.cjs' file extension/
).then(common.mustCall());
assert.rejects(
import(new URL('./cjs.js?foo=bar#target', pkgUrl)),
/use the '\.cjs' file extension/
).then(common.mustCall());

assert.rejects(
import('data:text/javascript,require;//.js'),
/^(?!use the '\.cjs' file extension).*$/
).then(common.mustCall());

0 comments on commit bd0d964

Please sign in to comment.