Skip to content

Commit

Permalink
fix: handle named imports from CJS modules that use dynamic import
Browse files Browse the repository at this point in the history
Fix import-js#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 committed Dec 30, 2021
1 parent ef980d4 commit 8821127
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 2 deletions.
11 changes: 10 additions & 1 deletion src/ExportMap.js
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {};
Expand Down Expand Up @@ -710,6 +718,7 @@ ExportMap.parse = function (path, content, context) {
m.namespace.set('default', {}); // add default export
}

m.maybeNotEsm = maybeNotEsm;
return m;
};

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

const imports = Exports.get(node.source.value, context);
if (imports == null) {
if (imports == null || imports.maybeNotEsm) {
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.maybeNotEsm
) {
return;
}
Expand Down
5 changes: 5 additions & 0 deletions tests/files/dynamic-import-in-commonjs.js
@@ -0,0 +1,5 @@
async function doSomething() {
await import('./bar.js');
}

exports.something = 'hello';
8 changes: 8 additions & 0 deletions tests/src/rules/named.js
Expand Up @@ -32,6 +32,9 @@ ruleTester.run('named', rule, {
test({ code: 'import { jsxFoo } from "./jsx/AnotherComponent"',
settings: { 'import/resolve': { 'extensions': ['.js', '.jsx'] } } }),

test({ code: 'import { something } from "./dynamic-import-in-commonjs"',
parserOptions: { ecmaVersion: 2021 } }),

// validate that eslint-disable-line silences this properly
test({ code: 'import {a, b, d} from "./common"; ' +
'// eslint-disable-line named' }),
Expand Down Expand Up @@ -165,6 +168,11 @@ ruleTester.run('named', rule, {
options: [{ commonjs: true }],
}),

test({ code: 'const { something } = require("./dynamic-import-in-commonjs")',
parserOptions: { ecmaVersion: 2021 },
options: [{ commonjs: true }],
}),

test({
code: 'const { baz } = require("./bar")',
errors: [error('baz', './bar')],
Expand Down

0 comments on commit 8821127

Please sign in to comment.