Skip to content

Commit 30bba6a

Browse files
cherryblossom000ljharb
authored andcommittedMay 17, 2021
[Fix] no-cycle: ignore imports where imported file only imports types of importing file
This fixes this situation: `a.ts`: ```ts import { foo } from './b' ``` `b.ts`: ```ts import type { Bar } from './a' ``` Previously, `no-cycle` would have incorrectly reported a dependency cycle for the import in `a.ts`, even though `b.ts` is only importing types from `a.ts`.
1 parent 3cf913d commit 30bba6a

File tree

5 files changed

+33
-9
lines changed

5 files changed

+33
-9
lines changed
 

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
88

99
### Fixed
1010
- [`no-restricted-paths`]: fix false positive matches ([#2090], thanks [@malykhinvi])
11+
- [`no-cycle`]: ignore imports where imported file only imports types of importing file ([#2083], thanks [@cherryblossom000])
1112

1213
### Changed
1314
- [Docs] Add `no-relative-packages` to list of to the list of rules ([#2075], thanks [@arvigeus])
@@ -790,6 +791,7 @@ for info on changes for earlier releases.
790791
[`memo-parser`]: ./memo-parser/README.md
791792

792793
[#2090]: https://github.com/benmosher/eslint-plugin-import/pull/2090
794+
[#2083]: https://github.com/benmosher/eslint-plugin-import/pull/2083
793795
[#2075]: https://github.com/benmosher/eslint-plugin-import/pull/2075
794796
[#2071]: https://github.com/benmosher/eslint-plugin-import/pull/2071
795797
[#2034]: https://github.com/benmosher/eslint-plugin-import/pull/2034

‎src/rules/no-cycle.js

+17-9
Original file line numberDiff line numberDiff line change
@@ -84,18 +84,26 @@ module.exports = {
8484
traversed.add(m.path);
8585

8686
for (const [path, { getter, declarations }] of m.imports) {
87-
if (path === myPath) return true;
8887
if (traversed.has(path)) continue;
89-
for (const { source, isOnlyImportingTypes } of declarations) {
90-
if (ignoreModule(source.value)) continue;
88+
const toTraverse = [...declarations].filter(({ source, isOnlyImportingTypes }) =>
89+
!ignoreModule(source.value) &&
9190
// Ignore only type imports
92-
if (isOnlyImportingTypes) continue;
91+
!isOnlyImportingTypes
92+
);
93+
/*
94+
Only report as a cycle if there are any import declarations that are considered by
95+
the rule. For example:
9396
94-
if (route.length + 1 < maxDepth) {
95-
untraversed.push({
96-
mget: getter,
97-
route: route.concat(source),
98-
});
97+
a.ts:
98+
import { foo } from './b' // should not be reported as a cycle
99+
100+
b.ts:
101+
import type { Bar } from './a'
102+
*/
103+
if (path === myPath && toTraverse.length > 0) return true;
104+
if (route.length + 1 < maxDepth) {
105+
for (const { source } of toTraverse) {
106+
untraversed.push({ mget: getter, route: route.concat(source) });
99107
}
100108
}
101109
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// @flow
2+
3+
import { type FooType, type BarType } from './depth-zero';
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// @flow
2+
3+
import type { FooType } from './depth-zero';

‎tests/src/rules/no-cycle.js

+8
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ ruleTester.run('no-cycle', rule, {
7373
code: 'import { bar } from "./flow-types"',
7474
parser: require.resolve('babel-eslint'),
7575
}),
76+
test({
77+
code: 'import { bar } from "./flow-types-only-importing-type"',
78+
parser: require.resolve('babel-eslint'),
79+
}),
80+
test({
81+
code: 'import { bar } from "./flow-types-only-importing-multiple-types"',
82+
parser: require.resolve('babel-eslint'),
83+
}),
7684
],
7785
invalid: [
7886
test({

0 commit comments

Comments
 (0)