diff --git a/CHANGELOG.md b/CHANGELOG.md index 712bf21a2..03526c700 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ### Fixed - [`no-restricted-paths`]: fix false positive matches ([#2090], thanks [@malykhinvi]) - [`no-cycle`]: ignore imports where imported file only imports types of importing file ([#2083], thanks [@cherryblossom000]) +- [`no-cycle`]: fix false negative when file imports a type after importing a value in Flow ([#2083], thanks [@cherryblossom000]) ### Changed - [Docs] Add `no-relative-packages` to list of to the list of rules ([#2075], thanks [@arvigeus]) diff --git a/src/ExportMap.js b/src/ExportMap.js index 9cc7a089e..76b07f9dc 100644 --- a/src/ExportMap.js +++ b/src/ExportMap.js @@ -495,7 +495,9 @@ ExportMap.parse = function (path, content, context) { if (n.type === 'ImportDeclaration') { // import type { Foo } (TS and Flow) const declarationIsType = n.importKind === 'type'; - let isOnlyImportingTypes = declarationIsType; + // import './foo' or import {} from './foo' (both 0 specifiers) is a side effect and + // shouldn't be considered to be just importing types + let specifiersOnlyImportingTypes = n.specifiers.length; const importedSpecifiers = new Set(); n.specifiers.forEach(specifier => { if (supportedImportTypes.has(specifier.type)) { @@ -506,11 +508,10 @@ ExportMap.parse = function (path, content, context) { } // import { type Foo } (Flow) - if (!declarationIsType) { - isOnlyImportingTypes = specifier.importKind === 'type'; - } + specifiersOnlyImportingTypes = + specifiersOnlyImportingTypes && specifier.importKind === 'type'; }); - captureDependency(n, isOnlyImportingTypes, importedSpecifiers); + captureDependency(n, declarationIsType || specifiersOnlyImportingTypes, importedSpecifiers); const ns = n.specifiers.find(s => s.type === 'ImportNamespaceSpecifier'); if (ns) { diff --git a/tests/files/cycles/flow-types-some-type-imports.js b/tests/files/cycles/flow-types-some-type-imports.js new file mode 100644 index 000000000..9008ba1af --- /dev/null +++ b/tests/files/cycles/flow-types-some-type-imports.js @@ -0,0 +1,3 @@ +// @flow + +import { foo, type BarType } from './depth-zero' diff --git a/tests/src/rules/no-cycle.js b/tests/src/rules/no-cycle.js index 9620eac44..302db8351 100644 --- a/tests/src/rules/no-cycle.js +++ b/tests/src/rules/no-cycle.js @@ -87,6 +87,11 @@ ruleTester.run('no-cycle', rule, { code: 'import { foo } from "./depth-one"', errors: [error(`Dependency cycle detected.`)], }), + test({ + code: 'import { bar } from "./flow-types-some-type-imports"', + parser: require.resolve('babel-eslint'), + errors: [error(`Dependency cycle detected.`)], + }), test({ code: 'import { foo } from "cycles/external/depth-one"', errors: [error(`Dependency cycle detected.`)],