From d9d995ec9bf6bc8f4f9cc4e291c76c6ee21fe02c Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 21 Mar 2021 15:41:24 +0100 Subject: [PATCH] module: clarify CJS global-like variables not defined error message Fixes: https://github.com/nodejs/node/issues/33741 PR-URL: https://github.com/nodejs/node/pull/37852 Reviewed-By: Guy Bedford --- lib/internal/modules/esm/module_job.js | 43 ++++++++++++++++++- lib/internal/modules/esm/resolve.js | 1 + ...esm-undefined-cjs-global-like-variables.js | 42 ++++++++++++++++++ 3 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 test/es-module/test-esm-undefined-cjs-global-like-variables.js diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index ba97a0e31784c2..b899c233d45a09 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -4,18 +4,21 @@ const { ArrayPrototypeJoin, ArrayPrototypeMap, ArrayPrototypePush, + ArrayPrototypeSome, FunctionPrototype, ObjectSetPrototypeOf, PromiseAll, PromiseResolve, PromisePrototypeCatch, ReflectApply, + RegExpPrototypeTest, SafeArrayIterator, SafeSet, StringPrototypeIncludes, StringPrototypeMatch, StringPrototypeReplace, StringPrototypeSplit, + StringPrototypeStartsWith, } = primordials; const { ModuleWrap } = internalBinding('module_wrap'); @@ -28,6 +31,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 { @@ -155,7 +171,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 }; } } diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 96d03a9c369e9d..7b503376af0950 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -893,6 +893,7 @@ module.exports = { DEFAULT_CONDITIONS, defaultResolve, encodedSepRegEx, + getPackageScopeConfig, getPackageType, packageExportsResolve, packageImportsResolve diff --git a/test/es-module/test-esm-undefined-cjs-global-like-variables.js b/test/es-module/test-esm-undefined-cjs-global-like-variables.js new file mode 100644 index 00000000000000..10867919f6f036 --- /dev/null +++ b/test/es-module/test-esm-undefined-cjs-global-like-variables.js @@ -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());