module: clarify CJS global-like variables not defined error message

Fixes: #33741 PR-URL: #37852 Reviewed-By: Guy Bedford <[email protected]>
nodejs · Jun 5, 2021 · 41722ee · 41722ee
1 parent 607aa9a
commit 41722ee
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 1 deletion.
diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js
@@ -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');
@@ -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 {
@@ -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 };
   }
 }

diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -845,6 +845,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
@@ -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());