fixup! module: clarify CJS global-like variables not defined error me…

…ssage
nodejs · Mar 22, 2021 · cef31c9 · cef31c9
1 parent 5fb34c7
commit cef31c9
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 37 deletions.
diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js
@@ -11,9 +11,9 @@ const {
   PromiseResolve,
   PromisePrototypeCatch,
   ReflectApply,
+  RegExpPrototypeTest,
   SafeArrayIterator,
   SafeSet,
-  StringPrototypeEndsWith,
   StringPrototypeIncludes,
   StringPrototypeMatch,
   StringPrototypeReplace,
@@ -178,19 +178,18 @@ class ModuleJob {
           e.message += ', you can use import instead';
         }
 
-        if (StringPrototypeEndsWith(this.module.url, '.js') &&
-            StringPrototypeStartsWith(this.module.url, 'file://')) {
-          const { packageJSONUrl, sameFolder } =
+        const packageJSONUrl =
+          StringPrototypeStartsWith(this.module.url, 'file://') &&
+            RegExpPrototypeTest(/\.js(\?[^#]*)?(#.*)?$/, this.module.url) &&
             require('internal/modules/esm/resolve')
               .getCachedPackageScopeURL(this.module.url);
+        if (packageJSONUrl) {
           e.message +=
-            '\nIt seems you are trying to load a file using `.js` extension ' +
-            `and '${packageJSONUrl}' includes ` +
-            '`"type": "module"`; you need to use the `.cjs` extension, or ' +
-            (sameFolder ?
-              `edit '${packageJSONUrl}'` :
-              'add a `package.json` file') +
-            ' with `"type": "commonjs"` to load the file as a CommonJS module.';
+            `\n'${this.module.url}' cannot be parsed as an ES module, but ` +
+            'it is being treated as an ES module because it has a \'.js\' ' +
+            `file extension and '${packageJSONUrl}' contains "type": ` +
+            '"module". To treat it as a CommonJS script, rename it to use ' +
+            'the \'.cjs\' file extension.';
         }
       }
       throw e;

diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -175,15 +175,13 @@ function getPackageConfig(path, specifier, base) {
 
 function getCachedPackageScopeURL(url) {
   let packageJSONUrl = new URL('./package.json', url);
-  let sameFolder = true;
   while (true) {
     const path = fileURLToPath(packageJSONUrl);
     if (packageJSONCache.get(path)?.exists) {
-      return { packageJSONUrl, sameFolder };
+      return packageJSONUrl;
     }
     const previousPathname = packageJSONUrl.pathname;
     packageJSONUrl = new URL('../package.json', packageJSONUrl);
-    sameFolder = false;
 
     // Terminates at root where ../package.json equals ../../package.json
     // (can't just check "/package.json" for Windows support).

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
@@ -2,7 +2,7 @@
 const common = require('../common');
 const fixtures = require('../common/fixtures');
 const assert = require('assert');
-const { pathToFileURL: toFileURL } = require('url');
+const { pathToFileURL } = require('url');
 
 assert.rejects(
   import('data:text/javascript,require;'),
@@ -21,36 +21,22 @@ assert.rejects(
 assert.rejects(
   import('data:text/javascript,require;//.js'),
   // eslint-disable-next-line node-core/no-unescaped-regexp-dot
-  /^(?!It seems you are trying to load a file using .\.js. extension).*$/
+  /^(?!use the .\.cjs. file extension).*$/
 ).then(common.mustCall());
 
+const pkgUrl = pathToFileURL(fixtures.path('/es-modules/package-type-module/'));
 assert.rejects(
-  import(toFileURL(fixtures.path('/es-modules/package-type-module/cjs.js'))),
+  import(new URL('./cjs.js', pkgUrl)),
   // eslint-disable-next-line node-core/no-unescaped-regexp-dot
-  /use the .\.cjs. extension/
+  /use the .\.cjs. file extension/
 ).then(common.mustCall());
 assert.rejects(
-  import(toFileURL(fixtures.path(
-    '/es-modules/package-type-module/nested-inherited-type/cjs.js'))),
+  import(new URL('./nested-inherited-type/cjs.js', pkgUrl)),
   // eslint-disable-next-line node-core/no-unescaped-regexp-dot
-  /use the .\.cjs. extension/
+  /use the .\.cjs. file extension/
 ).then(common.mustCall());
-
-assert.rejects(
-  import(toFileURL(fixtures.path('/es-modules/package-type-module/cjs.js'))),
-  // eslint-disable-next-line node-core/no-unescaped-regexp-dot
-  /edit 'file:[^']+package\.json' with ."type": "commonjs"./
-).then(common.mustCall());
-assert.rejects(
-  import(toFileURL(fixtures.path(
-    '/es-modules/package-type-module/nested-inherited-type/cjs.js'))),
-  // eslint-disable-next-line node-core/no-unescaped-regexp-dot
-  /add a .package.json. file with ."type": "commonjs"./
-).then(common.mustCall());
-
 assert.rejects(
-  import(toFileURL(fixtures.path(
-    '/es-modules/package-type-module/nested-inherited-type/cjs.js'))),
+  import(new URL('./cjs.js?foo=bar#target', pkgUrl)),
   // eslint-disable-next-line node-core/no-unescaped-regexp-dot
-  /ReferenceError: require is not defined in ES module scope, you can use import instead\nIt seems you are trying to load a file using `.js` extension and '[^']+package.json' includes `"type": "module"`; you need to use the `.cjs` extension, or add a `package.json` file with `"type": "commonjs"` to load the file as a CommonJS module./
+  /use the .\.cjs. file extension/
 ).then(common.mustCall());