module: better error for named exports from cjs

We do not support importing named exports from a CJS module.
This change decorates the error message for missing named exports in
the case where the module being imported is expected to be CJS by the
ESM loader.

Signed-off-by: Myles Borins <[email protected]>

PR-URL: #33256
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>

Author: MylesBorins

lib/internal/modules/esm/module_job.js

@@ -1,10 +1,14 @@
 'use strict';
 
 const {
+  ArrayPrototypeJoin,
   ObjectSetPrototypeOf,
   PromiseAll,
   SafeSet,
   SafePromise,
+  StringPrototypeIncludes,
+  StringPrototypeMatch,
+  StringPrototypeSplit,
 } = primordials;
 
 const { ModuleWrap } = internalBinding('module_wrap');
@@ -93,6 +97,29 @@ class ModuleJob {
       }
     } catch (e) {
       decorateErrorStack(e);
+      if (StringPrototypeIncludes(e.message,
+                                  ' does not provide an export named')) {
+        const splitStack = StringPrototypeSplit(e.stack, '\n');
+        const parentFileUrl = splitStack[0];
+        const childSpecifier = StringPrototypeMatch(e.message, /module '(.*)' does/)[1];
+        const childFileURL =
+              await this.loader.resolve(childSpecifier, parentFileUrl);
+        const format = await this.loader.getFormat(childFileURL);
+        if (format === 'commonjs') {
+          const importStatement = splitStack[1];
+          const namedImports = StringPrototypeMatch(importStatement, /{.*}/)[0];
+          e.message = `The requested module '${childSpecifier}' is expected ` +
+            'to be of type CommonJS, which does not support named exports. ' +
+            'CommonJS modules can be imported by importing the default ' +
+            'export.\n' +
+            'For example:\n' +
+            `import pkg from '${childSpecifier}';\n` +
+            `const ${namedImports} = pkg;`;
+          const newStack = StringPrototypeSplit(e.stack, '\n');
+          newStack[3] = `SyntaxError: ${e.message}`;
+          e.stack = ArrayPrototypeJoin(newStack, '\n');
+        }
+      }
       throw e;
     }
     for (const dependencyJob of jobsInGraph) {

test/es-module/test-esm-cjs-named-error.mjs

@@ -0,0 +1,64 @@
+import '../common/index.mjs';
+import { rejects } from 'assert';
+
+const fixtureBase = '../fixtures/es-modules/package-cjs-named-error';
+
+const expectedRelative = 'The requested module \'./fail.cjs\' is expected to ' +
+  'be of type CommonJS, which does not support named exports. CommonJS ' +
+  'modules can be imported by importing the default export.\n' +
+  'For example:\n' +
+  'import pkg from \'./fail.cjs\';\n' +
+  'const { comeOn } = pkg;';
+
+const expectedPackageHack = 'The requested module \'./json-hack/fail.js\' is ' +
+  'expected to be of type CommonJS, which does not support named exports. ' +
+  'CommonJS modules can be imported by importing the default export.\n' +
+  'For example:\n' +
+  'import pkg from \'./json-hack/fail.js\';\n' +
+  'const { comeOn } = pkg;';
+
+const expectedBare = 'The requested module \'deep-fail\' is expected to ' +
+  'be of type CommonJS, which does not support named exports. CommonJS ' +
+  'modules can be imported by importing the default export.\n' +
+  'For example:\n' +
+  'import pkg from \'deep-fail\';\n' +
+  'const { comeOn } = pkg;';
+
+rejects(async () => {
+  await import(`${fixtureBase}/single-quote.mjs`);
+}, {
+  name: 'SyntaxError',
+  message: expectedRelative
+}, 'should support relative specifiers with single quotes');
+
+rejects(async () => {
+  await import(`${fixtureBase}/double-quote.mjs`);
+}, {
+  name: 'SyntaxError',
+  message: expectedRelative
+}, 'should support relative specifiers with double quotes');
+
+rejects(async () => {
+  await import(`${fixtureBase}/json-hack.mjs`);
+}, {
+  name: 'SyntaxError',
+  message: expectedPackageHack
+}, 'should respect recursive package.json for module type');
+
+rejects(async () => {
+  await import(`${fixtureBase}/bare-import-single.mjs`);
+}, {
+  name: 'SyntaxError',
+  message: expectedBare
+}, 'should support bare specifiers with single quotes');
+
+rejects(async () => {
+  await import(`${fixtureBase}/bare-import-double.mjs`);
+}, {
+  name: 'SyntaxError',
+  message: expectedBare
+}, 'should support bare specifiers with double quotes');
+
+rejects(async () => {
+  await import(`${fixtureBase}/escaped-single-quote.mjs`);
+}, /import pkg from '\.\/oh'no\.cjs'/, 'should support relative specifiers with escaped single quote');

test/fixtures/es-modules/package-cjs-named-error/bare-import-double.mjs

@@ -0,0 +1 @@
+import { comeOn } from "deep-fail";

test/fixtures/es-modules/package-cjs-named-error/bare-import-single.mjs

@@ -0,0 +1 @@
+import { comeOn } from 'deep-fail';

test/fixtures/es-modules/package-cjs-named-error/double-quote.mjs

@@ -0,0 +1 @@
+import { comeOn } from "./fail.cjs";

test/fixtures/es-modules/package-cjs-named-error/escaped-single-quote.mjs

@@ -0,0 +1 @@
+import { value } from "./oh'no.cjs";

test/fixtures/es-modules/package-cjs-named-error/fail.cjs

@@ -0,0 +1,3 @@
+module.exports = {
+  comeOn: 'fhqwhgads'
+};

test/fixtures/es-modules/package-cjs-named-error/json-hack.mjs

@@ -0,0 +1 @@
+import { comeOn } from './json-hack/fail.js';

test/fixtures/es-modules/package-cjs-named-error/json-hack/fail.js

@@ -0,0 +1,3 @@
+module.exports = {
+  comeOn: 'fhqwhgads'
+};

test/fixtures/es-modules/package-cjs-named-error/json-hack/package.json

@@ -0,0 +1,3 @@
+{
+  "type": "commonjs"
+}

test/fixtures/es-modules/package-cjs-named-error/node_modules/deep-fail/index.js

@@ -0,0 +1,3 @@
+module.exports = {
+  value: 123
+};

test/fixtures/es-modules/package-cjs-named-error/node_modules/deep-fail/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "package-cjs-named-error",
+  "main": "index.mjs",
+  "type": "module"
+}

test/fixtures/es-modules/package-cjs-named-error/oh'no.cjs

@@ -0,0 +1 @@
+import { comeOn } from './fail.cjs';