Skip to content

Commit

Permalink
[Fix] named/ExportMap: handle named imports from CJS modules that…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
ludofischer authored and ljharb committed Dec 30, 2021
1 parent ef980d4 commit e3ca68e
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 3 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion src/ExportMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {};
Expand Down Expand Up @@ -710,6 +715,9 @@ ExportMap.parse = function (path, content, context) {
m.namespace.set('default', {}); // add default export
}

if (unambiguouslyESM) {
m.parseGoal = 'Module';
}
return m;
};

Expand Down
3 changes: 2 additions & 1 deletion src/rules/named.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ module.exports = {
}

const imports = Exports.get(node.source.value, context);
if (imports == null) {
if (imports == null || imports.parseGoal === 'ambiguous') {
return;
}

Expand Down Expand Up @@ -97,6 +97,7 @@ module.exports = {
// return if it's not a string source
|| source.type !== 'Literal'
|| variableExports == null
|| variableExports.parseGoal === 'ambiguous'
) {
return;
}
Expand Down
5 changes: 5 additions & 0 deletions tests/files/dynamic-import-in-commonjs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
async function doSomething() {
await import('./bar.js');
}

exports.something = 'hello';
11 changes: 10 additions & 1 deletion tests/src/rules/named.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down

0 comments on commit e3ca68e

Please sign in to comment.