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 192667d
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 @@ -52,6 +52,9 @@ ruleTester.run('named', rule, {
test({ code: 'export { foo as bar } from "./bar"' }),
test({ code: 'export { foo } from "./does-not-exist"' }),

// import from CJS module with dynamic import
test({ code: 'import { something } from "./dynamic-import-in-commonjs"',
parserOptions: { ecmaVersion: 2021 } }),
// es7
test({
code: 'export bar, { foo } from "./bar"',
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 192667d

Please sign in to comment.