From e3ca68edaf7b34ee17afe13f2117fed87c200007 Mon Sep 17 00:00:00 2001 From: Ludovico Fischer Date: Thu, 30 Dec 2021 18:58:46 +0100 Subject: [PATCH] [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. --- CHANGELOG.md | 3 +++ src/ExportMap.js | 10 +++++++++- src/rules/named.js | 3 ++- tests/files/dynamic-import-in-commonjs.js | 5 +++++ tests/src/rules/named.js | 11 ++++++++++- 5 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 tests/files/dynamic-import-in-commonjs.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 88d7c411f..cf11927c7 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`]/`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 diff --git a/src/ExportMap.js b/src/ExportMap.js index d818fa6ca..564b5d63d 100644 --- a/src/ExportMap.js +++ b/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; }; diff --git a/src/rules/named.js b/src/rules/named.js index 24e6bc0ac..a529c295b 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.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; } diff --git a/tests/files/dynamic-import-in-commonjs.js b/tests/files/dynamic-import-in-commonjs.js new file mode 100644 index 000000000..259feb4cd --- /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 992baa0fd..10ba76b79 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: [