Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: import/no-cycle triggering errors for Flow imports #1494

Merged
merged 1 commit into from Dec 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -21,6 +21,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`prefer-default-export`]: fix false positive with type export ([#1506], thanks [@golopot])
- [`extensions`]: Fix `ignorePackages` to produce errors ([#1521], thanks [@saschanaz])
- [`no-unused-modules`]: fix crash due to `export *` ([#1496], thanks [@Taranys])
- [`no-cycle`]: should not warn for Flow imports ([#1494], thanks [@maxmalov])

### Docs
- [`no-useless-path-segments`]: add docs for option `commonjs` ([#1507], thanks [@golopot])
Expand Down Expand Up @@ -629,6 +630,7 @@ for info on changes for earlier releases.
[#1506]: https://github.com/benmosher/eslint-plugin-import/pull/1506
[#1496]: https://github.com/benmosher/eslint-plugin-import/pull/1496
[#1495]: https://github.com/benmosher/eslint-plugin-import/pull/1495
[#1494]: https://github.com/benmosher/eslint-plugin-import/pull/1494
[#1472]: https://github.com/benmosher/eslint-plugin-import/pull/1472
[#1470]: https://github.com/benmosher/eslint-plugin-import/pull/1470
[#1436]: https://github.com/benmosher/eslint-plugin-import/pull/1436
Expand Down Expand Up @@ -1019,3 +1021,4 @@ for info on changes for earlier releases.
[@saschanaz]: https://github.com/saschanaz
[@brettz9]: https://github.com/brettz9
[@Taranys]: https://github.com/Taranys
[@maxmalov]: https://github.com/maxmalov
12 changes: 10 additions & 2 deletions src/ExportMap.js
Expand Up @@ -404,19 +404,27 @@ ExportMap.parse = function (path, content, context) {

function captureDependency(declaration) {
if (declaration.source == null) return null
if (declaration.importKind === 'type') return null // skip Flow type imports
const importedSpecifiers = new Set()
const supportedTypes = new Set(['ImportDefaultSpecifier', 'ImportNamespaceSpecifier'])
let hasImportedType = false
if (declaration.specifiers) {
declaration.specifiers.forEach(specifier => {
if (supportedTypes.has(specifier.type)) {
const isType = specifier.importKind === 'type'
hasImportedType = hasImportedType || isType

if (supportedTypes.has(specifier.type) && !isType) {
importedSpecifiers.add(specifier.type)
}
if (specifier.type === 'ImportSpecifier') {
if (specifier.type === 'ImportSpecifier' && !isType) {
importedSpecifiers.add(specifier.imported.name)
}
})
}

// only Flow types were imported
if (hasImportedType && importedSpecifiers.size === 0) return null

const p = remotePath(declaration.source.value)
if (p == null) return null
const existing = m.imports.get(p)
Expand Down
6 changes: 6 additions & 0 deletions tests/files/cycles/flow-types-depth-one.js
@@ -0,0 +1,6 @@
// @flow

import type { FooType } from './flow-types-depth-two';
import { type BarType, bar } from './flow-types-depth-two';

export { bar }
1 change: 1 addition & 0 deletions tests/files/cycles/flow-types-depth-two.js
@@ -0,0 +1 @@
import { foo } from './depth-one'
6 changes: 6 additions & 0 deletions tests/files/cycles/flow-types.js
@@ -0,0 +1,6 @@
// @flow

import type { FooType } from './flow-types-depth-two';
maxmalov marked this conversation as resolved.
Show resolved Hide resolved
import { type BarType } from './flow-types-depth-two';

export const bar = 1;
9 changes: 9 additions & 0 deletions tests/src/rules/no-cycle.js
Expand Up @@ -53,6 +53,10 @@ ruleTester.run('no-cycle', rule, {
code: 'import type { FooType, BarType } from "./depth-one"',
parser: require.resolve('babel-eslint'),
}),
test({
code: 'import { bar } from "./flow-types"',
parser: require.resolve('babel-eslint'),
}),
],
invalid: [
test({
Expand Down Expand Up @@ -120,6 +124,11 @@ ruleTester.run('no-cycle', rule, {
errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)],
parser: require.resolve('babel-eslint'),
}),
test({
code: 'import { bar } from "./flow-types-depth-one"',
parser: require.resolve('babel-eslint'),
errors: [error(`Dependency cycle via ./flow-types-depth-two:4=>./depth-one:1`)],
}),
],
})
// })