Skip to content

Commit

Permalink
[fix] Fix overwriting of dynamic import() CallExpression
Browse files Browse the repository at this point in the history
 - fix import() to work with no-cycle
 - add tests using multiple imports in no-cycle

Fixes #1035. Fixes #1166.
  • Loading branch information
vikr01 authored and ljharb committed Oct 21, 2018
1 parent e4850df commit 70a59fe
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 6 deletions.
6 changes: 1 addition & 5 deletions src/rules/no-cycle.js
Expand Up @@ -31,14 +31,10 @@ module.exports = {
function checkSourceValue(sourceNode, importer) {
const imported = Exports.get(sourceNode.value, context)

if (sourceNode.parent && sourceNode.parent.importKind === 'type') {
if (importer.importKind === 'type') {
return // no Flow import resolution
}

if (sourceNode._babelType === 'Literal') {
return // no Flow import resolution, workaround for ESLint < 5.x
}

if (imported == null) {
return // no-unresolved territory
}
Expand Down
32 changes: 32 additions & 0 deletions tests/src/rules/no-cycle.js
Expand Up @@ -36,10 +36,23 @@ ruleTester.run('no-cycle', rule, {
code: 'import { foo } from "./depth-two"',
options: [{ maxDepth: 1 }],
}),
test({
code: 'import { foo, bar } from "./depth-two"',
options: [{ maxDepth: 1 }],
}),
test({
code: 'import("./depth-two").then(function({ foo }){})',
options: [{ maxDepth: 1 }],
parser: 'babel-eslint',
}),
test({
code: 'import type { FooType } from "./depth-one"',
parser: 'babel-eslint',
}),
test({
code: 'import type { FooType, BarType } from "./depth-one"',
parser: 'babel-eslint',
}),
],
invalid: [
test({
Expand Down Expand Up @@ -84,10 +97,29 @@ ruleTester.run('no-cycle', rule, {
code: 'import { two } from "./depth-three-star"',
errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
}),
test({
code: 'import one, { two, three } from "./depth-three-star"',
errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
}),
test({
code: 'import { bar } from "./depth-three-indirect"',
errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
}),
test({
code: 'import { bar } from "./depth-three-indirect"',
errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
parser: 'babel-eslint',
}),
test({
code: 'import("./depth-three-star")',
errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
parser: 'babel-eslint',
}),
test({
code: 'import("./depth-three-indirect")',
errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
parser: 'babel-eslint',
}),
],
})
// })
8 changes: 8 additions & 0 deletions tests/src/rules/no-relative-parent-imports.js
Expand Up @@ -93,6 +93,14 @@ ruleTester.run('no-relative-parent-imports', rule, {
line: 1,
column: 17
}]
}),
test({
code: 'import("../../api/service")',
errors: [ {
message: 'Relative imports from parent directories are not allowed. Please either pass what you\'re importing through at runtime (dependency injection), move `index.js` to same directory as `../../api/service` or consider making `../../api/service` a package.',
line: 1,
column: 8
}],
})
],
})
9 changes: 9 additions & 0 deletions tests/src/rules/no-unresolved.js
Expand Up @@ -29,6 +29,8 @@ function runResolverTests(resolver) {
rest({ code: "import bar from './bar.js';" }),
rest({ code: "import {someThing} from './test-module';" }),
rest({ code: "import fs from 'fs';" }),
rest({ code: "import('fs');"
, parser: 'babel-eslint' }),

rest({ code: 'import * as foo from "a"' }),

Expand Down Expand Up @@ -114,6 +116,13 @@ function runResolverTests(resolver) {
"module 'in-alternate-root'."
, type: 'Literal',
}]}),
rest({
code: "import('in-alternate-root').then(function({DEEP}){});",
errors: [{ message: 'Unable to resolve path to ' +
"module 'in-alternate-root'."
, type: 'Literal',
}],
parser: 'babel-eslint'}),

rest({ code: 'export { foo } from "./does-not-exist"'
, errors: ["Unable to resolve path to module './does-not-exist'."] }),
Expand Down
23 changes: 22 additions & 1 deletion tests/src/rules/no-useless-path-segments.js
Expand Up @@ -17,7 +17,6 @@ function runResolverTests(resolver) {
test({ code: 'import "."' }),
test({ code: 'import ".."' }),
test({ code: 'import fs from "fs"' }),
test({ code: 'import fs from "fs"' }),

// ES modules + noUselessIndex
test({ code: 'import "../index"' }), // noUselessIndex is false by default
Expand All @@ -28,6 +27,13 @@ function runResolverTests(resolver) {
test({ code: 'import "./malformed.js"', options: [{ noUselessIndex: true }] }), // ./malformed directory does not exist
test({ code: 'import "./malformed"', options: [{ noUselessIndex: true }] }), // ./malformed directory does not exist
test({ code: 'import "./importType"', options: [{ noUselessIndex: true }] }), // ./importType.js does not exist

test({ code: 'import(".")'
, parser: 'babel-eslint' }),
test({ code: 'import("..")'
, parser: 'babel-eslint' }),
test({ code: 'import("fs").then(function(fs){})'
, parser: 'babel-eslint' }),
],

invalid: [
Expand Down Expand Up @@ -190,6 +196,21 @@ function runResolverTests(resolver) {
options: [{ noUselessIndex: true }],
errors: ['Useless path segments for "../index.js", should be ".."'],
}),
test({
code: 'import("./")',
errors: [ 'Useless path segments for "./", should be "."'],
parser: 'babel-eslint',
}),
test({
code: 'import("../")',
errors: [ 'Useless path segments for "../", should be ".."'],
parser: 'babel-eslint',
}),
test({
code: 'import("./deep//a")',
errors: [ 'Useless path segments for "./deep//a", should be "./deep/a"'],
parser: 'babel-eslint',
}),
],
})
}
Expand Down
2 changes: 2 additions & 0 deletions utils/moduleVisitor.js
Expand Up @@ -91,7 +91,9 @@ exports.default = function visitModules(visitor, options) {
}

if (options.commonjs || options.amd) {
const currentCallExpression = visitors['CallExpression']
visitors['CallExpression'] = function (call) {
if (currentCallExpression) currentCallExpression(call)
if (options.commonjs) checkCommon(call)
if (options.amd) checkAMD(call)
}
Expand Down

0 comments on commit 70a59fe

Please sign in to comment.