[Fix] named/ExportMap: 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.

Author: ludofischer

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`]/`ExportMap`: handle named imports from CJS modules that use dynamic import ([#2341], thanks [@ludofischer])
 
 ### Changed
 - [`no-default-import`]: report on the token "default" instead of the entire node ([#2299], thanks [@pmcelhaney])
@@ -951,6 +952,7 @@ for info on changes for earlier releases.
 
 [`memo-parser`]: ./memo-parser/README.md
 
+[#2341]: https://github.com/import-js/eslint-plugin-import/pull/2341
 [#2334]: https://github.com/import-js/eslint-plugin-import/pull/2334
 [#2305]: https://github.com/import-js/eslint-plugin-import/pull/2305
 [#2299]: https://github.com/import-js/eslint-plugin-import/pull/2299
@@ -1549,6 +1551,7 @@ for info on changes for earlier releases.
 [@lo1tuma]: https://github.com/lo1tuma
 [@loganfsmyth]: https://github.com/loganfsmyth
 [@luczsoma]: https://github.com/luczsoma
+[@ludofischer]: https://github.com/ludofischer
 [@lukeapage]: https://github.com/lukeapage
 [@lydell]: https://github.com/lydell
 [@Mairu]: https://github.com/Mairu

src/ExportMap.js

@@ -43,6 +43,10 @@ export default class ExportMap {
      */
     this.imports = new Map();
     this.errors = [];
+    /**
+     * type {'ambiguous' | 'Module' | 'Script'}
+     */
+    this.parseGoal = 'ambiguous';
   }
 
   get hasDefault() { return this.get('default') != null; } // stronger than this.has
@@ -406,7 +410,8 @@ ExportMap.parse = function (path, content, context) {
     },
   });
 
-  if (!unambiguous.isModule(ast) && !hasDynamicImports) return null;
+  const unambiguouslyESM = unambiguous.isModule(ast);
+  if (!unambiguouslyESM && !hasDynamicImports) return null;
 
   const docstyle = (context.settings && context.settings['import/docstyle']) || ['jsdoc'];
   const docStyleParsers = {};
@@ -710,6 +715,9 @@ ExportMap.parse = function (path, content, context) {
     m.namespace.set('default', {}); // add default export
   }
 
+  if (unambiguouslyESM) {
+    m.parseGoal = 'Module';
+  }
   return m;
 };
 

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.parseGoal === 'ambiguous') {
         return;
       }
 
@@ -97,6 +97,7 @@ module.exports = {
         // return if it's not a string source
         || source.type !== 'Literal'
         || variableExports == null
+        || variableExports.parseGoal === 'ambiguous'
       ) {
         return;
       }

tests/files/dynamic-import-in-commonjs.js

@@ -0,0 +1,5 @@
+async function doSomething() {
+  await import('./bar.js');
+}
+
+exports.something = 'hello';

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: [