From b5f0fdb248f39688daa9ff46015207b7c086efd3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A1bio=20Santos?= <fabiosantosart@gmail.com>
Date: Sun, 14 Jun 2020 22:01:18 +0100
Subject: [PATCH] module: fix error message about importing names from cjs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When importing specific names from a CJS module, and renaming them using
`as`, the example fix in the error message erroneously contains the
keyword `as` in the destructuring variable declaration.

Example of this issue:

    import { parse as acornParse } from "acorn";
             ^^^^^
    SyntaxError: The requested module 'acorn' is expected to be of type CommonJS, which does not support named exports. CommonJS modules can be imported by importing the default export.
    For example:
    import pkg from 'acorn';
    const { parse as acornParse } = pkg;

PR-URL: https://github.com/nodejs/node/pull/33882
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
---
 lib/internal/modules/esm/module_job.js             |  4 +++-
 test/es-module/test-esm-cjs-named-error.mjs        | 14 ++++++++++++++
 .../package-cjs-named-error/renamed-import.mjs     |  1 +
 3 files changed, 18 insertions(+), 1 deletion(-)
 create mode 100644 test/fixtures/es-modules/package-cjs-named-error/renamed-import.mjs

diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js
index 3b79ac2df5ceef..3f11ffc768eedb 100644
--- a/lib/internal/modules/esm/module_job.js
+++ b/lib/internal/modules/esm/module_job.js
@@ -8,6 +8,7 @@ const {
   SafePromise,
   StringPrototypeIncludes,
   StringPrototypeMatch,
+  StringPrototypeReplace,
   StringPrototypeSplit,
 } = primordials;
 
@@ -108,13 +109,14 @@ class ModuleJob {
         if (format === 'commonjs') {
           const importStatement = splitStack[1];
           const namedImports = StringPrototypeMatch(importStatement, /{.*}/)[0];
+          const destructuringAssignment = StringPrototypeReplace(namedImports, /\s+as\s+/g, ': ');
           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 ${destructuringAssignment} = pkg;`;
           const newStack = StringPrototypeSplit(e.stack, '\n');
           newStack[3] = `SyntaxError: ${e.message}`;
           e.stack = ArrayPrototypeJoin(newStack, '\n');
diff --git a/test/es-module/test-esm-cjs-named-error.mjs b/test/es-module/test-esm-cjs-named-error.mjs
index d34f762dfb4e26..d71dc959e21fb7 100644
--- a/test/es-module/test-esm-cjs-named-error.mjs
+++ b/test/es-module/test-esm-cjs-named-error.mjs
@@ -10,6 +10,13 @@ const expectedRelative = 'The requested module \'./fail.cjs\' is expected to ' +
   'import pkg from \'./fail.cjs\';\n' +
   'const { comeOn } = pkg;';
 
+const expectedRenamed = '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: comeOnRenamed } = 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' +
@@ -38,6 +45,13 @@ rejects(async () => {
   message: expectedRelative
 }, 'should support relative specifiers with double quotes');
 
+rejects(async () => {
+  await import(`${fixtureBase}/renamed-import.mjs`);
+}, {
+  name: 'SyntaxError',
+  message: expectedRenamed
+}, 'should correctly format named imports with renames');
+
 rejects(async () => {
   await import(`${fixtureBase}/json-hack.mjs`);
 }, {
diff --git a/test/fixtures/es-modules/package-cjs-named-error/renamed-import.mjs b/test/fixtures/es-modules/package-cjs-named-error/renamed-import.mjs
new file mode 100644
index 00000000000000..dc601f28fe2f99
--- /dev/null
+++ b/test/fixtures/es-modules/package-cjs-named-error/renamed-import.mjs
@@ -0,0 +1 @@
+import { comeOn as comeOnRenamed } from "./fail.cjs"