From d9d995ec9bf6bc8f4f9cc4e291c76c6ee21fe02c Mon Sep 17 00:00:00 2001
From: Antoine du Hamel <duhamelantoine1995@gmail.com>
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 <guybedford@gmail.com>
---
 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());