From 0dba1cfdfae5c7406d194797ea711f95adc4ec8f Mon Sep 17 00:00:00 2001
From: Ludovico Fischer <livrerie@gmail.com>
Date: Thu, 30 Dec 2021 18:58:46 +0100
Subject: [PATCH] fix: handle named imports from CJS modules that use dynamic
 import

Fix #2294

Mark ambiguous (i.e. not unambiguously ESM) modules that contain dynamic import()
so that they can be ignored like other ambiguous modules when analysing named imports.

In this way, the named rule accepts named imports from CJS modules that contain
dynamic imports.

Also fix the case where the importing module uses a require() call.
---
 CHANGELOG.md                              |  1 +
 src/ExportMap.js                          | 11 ++++++++++-
 src/rules/named.js                        |  3 ++-
 tests/files/dynamic-import-in-commonjs.js |  5 +++++
 tests/src/rules/named.js                  | 11 ++++++++++-
 5 files changed, 28 insertions(+), 3 deletions(-)
 create mode 100644 tests/files/dynamic-import-in-commonjs.js

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 88d7c411f7..894e6c18c4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 - `importType`: avoid crashing on a non-string' ([#2305], thanks [@ljharb])
 - [`first`]: prevent crash when parsing angular templates ([#2210], thanks [@ljharb])
 - `importType`: properly resolve `@/*`-aliased imports as internal ([#2334], thanks [@ombene])
+- `named`: avoid false positives when importing from a CommonJS module that uses `import()` ([#2341], thanks [@ludofischer])
 
 ### Changed
 - [`no-default-import`]: report on the token "default" instead of the entire node ([#2299], thanks [@pmcelhaney])
diff --git a/src/ExportMap.js b/src/ExportMap.js
index d818fa6ca8..9374c4f656 100644
--- a/src/ExportMap.js
+++ b/src/ExportMap.js
@@ -43,6 +43,13 @@ export default class ExportMap {
      */
     this.imports = new Map();
     this.errors = [];
+    /**
+     * true when we are still unsure if
+     * the module is ESM, happens when the module
+     * contains dynamic `import()` but no other
+     * ESM import/export
+     */
+    this.maybeNotEsm = false;
   }
 
   get hasDefault() { return this.get('default') != null; } // stronger than this.has
@@ -406,7 +413,8 @@ ExportMap.parse = function (path, content, context) {
     },
   });
 
-  if (!unambiguous.isModule(ast) && !hasDynamicImports) return null;
+  const maybeNotEsm = !unambiguous.isModule(ast);
+  if (maybeNotEsm && !hasDynamicImports) return null;
 
   const docstyle = (context.settings && context.settings['import/docstyle']) || ['jsdoc'];
   const docStyleParsers = {};
@@ -710,6 +718,7 @@ ExportMap.parse = function (path, content, context) {
     m.namespace.set('default', {}); // add default export
   }
 
+  m.maybeNotEsm = maybeNotEsm;
   return m;
 };
 
diff --git a/src/rules/named.js b/src/rules/named.js
index 24e6bc0ac5..ffc787001e 100644
--- a/src/rules/named.js
+++ b/src/rules/named.js
@@ -40,7 +40,7 @@ module.exports = {
       }
 
       const imports = Exports.get(node.source.value, context);
-      if (imports == null) {
+      if (imports == null || imports.maybeNotEsm) {
         return;
       }
 
@@ -97,6 +97,7 @@ module.exports = {
         // return if it's not a string source
         || source.type !== 'Literal'
         || variableExports == null
+        || variableExports.maybeNotEsm
       ) {
         return;
       }
diff --git a/tests/files/dynamic-import-in-commonjs.js b/tests/files/dynamic-import-in-commonjs.js
new file mode 100644
index 0000000000..259feb4cd9
--- /dev/null
+++ b/tests/files/dynamic-import-in-commonjs.js
@@ -0,0 +1,5 @@
+async function doSomething() {
+  await import('./bar.js');
+}
+
+exports.something = 'hello';
diff --git a/tests/src/rules/named.js b/tests/src/rules/named.js
index 992baa0fd0..10ba76b79d 100644
--- a/tests/src/rules/named.js
+++ b/tests/src/rules/named.js
@@ -190,7 +190,16 @@ ruleTester.run('named', rule, {
         sourceType: 'module',
         ecmaVersion: 2020,
       },
-    })) || []),
+    })),
+
+    testVersion('>=7.8.0', () =>({ code: 'const { something } = require("./dynamic-import-in-commonjs")',
+      parserOptions: { ecmaVersion: 2021 },
+      options: [{ commonjs: true }],
+    })),
+
+    testVersion('>=7.8.0', () => ({ code: 'import { something } from "./dynamic-import-in-commonjs"',
+      parserOptions: { ecmaVersion: 2021 } })),
+    ),
   ],
 
   invalid: [