Skip to content

Commit

Permalink
no-unused-modules: support dynamic imports
Browse files Browse the repository at this point in the history
This is still work in progress but I'd like to receive initial thoughts
on this implementation before I proceed.

This introduces support of dynamic imports for a rule
`no-unused-modules`.

So far only "await" form is implemented:

```js
const a = await import("a")
```
is equivalent to default import
```js
import a from "a"
```

```js
const {a,b,c} = await import("a")
```
is equivalent to named imports
```js
import {a,b,c} from "a"
```

Support import('name').then(a) and import('name').then({a,b,c}) to be
addded soon.

TODO/Open questions

- [ ] Existing code is relying on the fact that all imports/reexports
happen at top level of the file while dynamic import can happen anywhere -
that's why I had to implement visitor against visitor keys of the parser
myself not very happy about it - but couldn't figure out quickly how to
use existing visitor (if any?) supplied by a parser.
I also learned that different parsers have visitor keys defined in
different places so this has to be dealt with as well.
  • Loading branch information
maxkomarychev committed Feb 17, 2020
1 parent 2beec94 commit 060ea2c
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 18 deletions.
63 changes: 60 additions & 3 deletions src/ExportMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import debug from 'debug'

import { SourceCode } from 'eslint'

import parse from 'eslint-module-utils/parse'
import parse, { visit } from 'eslint-module-utils/parse'
import resolve from 'eslint-module-utils/resolve'
import isIgnored, { hasValidExtension } from 'eslint-module-utils/ignore'

Expand Down Expand Up @@ -351,9 +351,66 @@ ExportMap.parse = function (path, content, context) {
return m // can't continue
}

if (!unambiguous.isModule(ast)) return null
let hasDynamicImports = false

const docstyle = (context.settings && context.settings['import/docstyle']) || ['jsdoc']
let declarator = null
visit(ast, path, context, {
VariableDeclarator(node) {
declarator = node
if (node.id.type === 'Identifier') {
log('Declarator', node.id.name)
} else if (node.id.type === 'ObjectPattern') {
log('Object pattern')
}
},
'VariableDeclarator:Exit': function() {
declarator = null
},
CallExpression(node) {
log('CALL', node.callee.type)
// log(JSON.stringify(node))
if (node.callee.type === 'Import') {
hasDynamicImports = true
const p = remotePath(node.arguments[0].value)
if (p == null) return null
if (declarator) {
log('IDDDDDDDD', declarator.id.name)
}
const importLiteral = node.arguments[0]
const importedSpecifiers = new Set()
if (declarator) {
if (declarator.id.type === 'Identifier') {
importedSpecifiers.add('ImportDefaultSpecifier')
} else if (declarator.id.type === 'ObjectPattern') {
for (const property of declarator.id.properties) {
log('ADD PROPERTY', property.key.name)
importedSpecifiers.add(property.key.name)
}
}
}
const getter = thunkFor(p, context)
m.imports.set(p, {
getter,
source: {
// capturing actual node reference holds full AST in memory!
value: importLiteral.value,
loc: importLiteral.loc,
},
importedSpecifiers,
})
}
},
})

if (!unambiguous.isModule(ast) && !hasDynamicImports) {
log('is not a module', path, hasDynamicImports)
return null
} else {
log('is a module!', path)
}

const docstyle = (context.settings &&
context.settings['import/docstyle']) || ['jsdoc']
const docStyleParsers = {}
docstyle.forEach(style => {
docStyleParsers[style] = availableDocStyleParsers[style]
Expand Down
3 changes: 3 additions & 0 deletions tests/files/no-unused-modules/dynamic-import-default.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
async function main() {
const value = await import("./exports-for-dynamic")
}
3 changes: 3 additions & 0 deletions tests/files/no-unused-modules/dynamic-import-named.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
async function main() {
const { importMeDynamicallyA, } = await import("./exports-for-dynamic")
}
4 changes: 4 additions & 0 deletions tests/files/no-unused-modules/exports-for-dynamic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const importMeDynamicallyA = 100;
const importMeDynamicallyB = 200;
export const importMeDynamicallyC = 100;
export default importMeDynamicallayB;
55 changes: 42 additions & 13 deletions tests/src/rules/no-unused-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,32 +71,47 @@ ruleTester.run('no-unused-modules', rule, {
// tests for exports
ruleTester.run('no-unused-modules', rule, {
valid: [

test({ options: unusedExportsOptions,
code: 'import { o2 } from "./file-o";export default () => 12',
filename: testFilePath('./no-unused-modules/file-a.js')}),
code: 'import { o2 } from "./file-o";export default () => 12',
filename: testFilePath('./no-unused-modules/file-a.js'),
parser: require.resolve('babel-eslint')}),
test({ options: unusedExportsOptions,
code: 'export const b = 2',
filename: testFilePath('./no-unused-modules/file-b.js')}),
filename: testFilePath('./no-unused-modules/file-b.js'),
parser: require.resolve('babel-eslint')}),
test({ options: unusedExportsOptions,
code: 'const c1 = 3; function c2() { return 3 }; export { c1, c2 }',
filename: testFilePath('./no-unused-modules/file-c.js')}),
filename: testFilePath('./no-unused-modules/file-c.js'),
parser: require.resolve('babel-eslint')}),
test({ options: unusedExportsOptions,
code: 'export function d() { return 4 }',
filename: testFilePath('./no-unused-modules/file-d.js')}),
filename: testFilePath('./no-unused-modules/file-d.js'),
parser: require.resolve('babel-eslint')}),
test({ options: unusedExportsOptions,
code: 'export class q { q0() {} }',
filename: testFilePath('./no-unused-modules/file-q.js')}),
filename: testFilePath('./no-unused-modules/file-q.js'),
parser: require.resolve('babel-eslint')}),
test({ options: unusedExportsOptions,
code: 'const e0 = 5; export { e0 as e }',
filename: testFilePath('./no-unused-modules/file-e.js')}),
filename: testFilePath('./no-unused-modules/file-e.js'),
parser: require.resolve('babel-eslint')}),
test({ options: unusedExportsOptions,
code: 'const l0 = 5; const l = 10; export { l0 as l1, l }; export default () => {}',
filename: testFilePath('./no-unused-modules/file-l.js')}),
filename: testFilePath('./no-unused-modules/file-l.js'),
parser: require.resolve('babel-eslint')}),
test({ options: unusedExportsOptions,
code: 'const o0 = 0; const o1 = 1; export { o0, o1 as o2 }; export default () => {}',
filename: testFilePath('./no-unused-modules/file-o.js')}),
],
filename: testFilePath('./no-unused-modules/file-o.js'),
parser: require.resolve('babel-eslint')}),
test({ options: unusedExportsOptions,
code: `
export const importMeDynamicallyA = 100;
const importMeDynamicallyB = 200;
export default importMeDynamicallayB;
`,
parser: require.resolve('babel-eslint'),
filename: testFilePath('./no-unused-modules/exports-for-dynamic.js')}),
],
invalid: [
test({ options: unusedExportsOptions,
code: `import eslint from 'eslint'
Expand All @@ -117,11 +132,25 @@ ruleTester.run('no-unused-modules', rule, {
error(`exported declaration 'o0' not used within other modules`),
error(`exported declaration 'o3' not used within other modules`),
error(`exported declaration 'p' not used within other modules`),
]}),
],
}),
test({ options: unusedExportsOptions,
code: `const n0 = 'n0'; const n1 = 42; export { n0, n1 }; export default () => {}`,
filename: testFilePath('./no-unused-modules/file-n.js'),
errors: [error(`exported declaration 'default' not used within other modules`)]}),
errors: [
error(`exported declaration 'default' not used within other modules`),
]}),
test({ options: unusedExportsOptions,
code: `
export const importMeDynamicallyC = 100;
`,
parser: require.resolve('babel-eslint'),
filename: testFilePath('./no-unused-modules/exports-for-dynamic.js'),
errors: [
error(
`exported declaration 'importMeDynamicallyC' not used within other modules`,
),
]}),
],
})

Expand Down
33 changes: 33 additions & 0 deletions utils/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,39 @@ exports.default = function parse(path, content, context) {
return parser.parse(content, parserOptions)
}

function __visit(node, keys, visitorSpec) {
if (!node) {
return
}
const type = node.type
if (typeof visitorSpec[type] === 'function') {
visitorSpec[type](node)
}
const childFields = keys[type]
if (!childFields) {
return
}
for (const fieldName of childFields) {
const field = node[fieldName]
if (Array.isArray(field)) {
for (const item of field) {
__visit(item, keys, visitorSpec)
}
} else {
__visit(field, keys, visitorSpec)
}
}
if (typeof visitorSpec[`${type}:Exit`] === 'function') {
visitorSpec[`${type}:Exit`](node)
}
}

exports.visit = function (ast, path, context, visitorSpec) {
const parserPath = getParserPath(path, context)
const keys = moduleRequire(parserPath.replace('index.js', 'visitor-keys.js'))
__visit(ast, keys, visitorSpec)
}

function getParserPath(path, context) {
const parsers = context.settings['import/parsers']
if (parsers != null) {
Expand Down
3 changes: 1 addition & 2 deletions utils/unambiguous.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
'use strict'
exports.__esModule = true


const pattern = /(^|;)\s*(export|import)((\s+\w)|(\s*[{*=]))/m
const pattern = /(^|;)\s*(export|import)((\s+\w)|(\s*[{*=]))|import\(/m
/**
* detect possible imports/exports without a full parse.
*
Expand Down

0 comments on commit 060ea2c

Please sign in to comment.