Skip to content

Commit

Permalink
[Fix] no-cycle: ignore imports where imported file only imports typ…
Browse files Browse the repository at this point in the history
…es 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`.
  • Loading branch information
cherryblossom000 authored and ljharb committed May 17, 2021
1 parent 3cf913d commit 30bba6a
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,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])

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

[#2090]: https://github.com/benmosher/eslint-plugin-import/pull/2090
[#2083]: https://github.com/benmosher/eslint-plugin-import/pull/2083
[#2075]: https://github.com/benmosher/eslint-plugin-import/pull/2075
[#2071]: https://github.com/benmosher/eslint-plugin-import/pull/2071
[#2034]: https://github.com/benmosher/eslint-plugin-import/pull/2034
Expand Down
26 changes: 17 additions & 9 deletions src/rules/no-cycle.js
Expand Up @@ -84,18 +84,26 @@ module.exports = {
traversed.add(m.path);

for (const [path, { getter, declarations }] of m.imports) {
if (path === myPath) return true;
if (traversed.has(path)) continue;
for (const { source, isOnlyImportingTypes } of declarations) {
if (ignoreModule(source.value)) continue;
const toTraverse = [...declarations].filter(({ source, isOnlyImportingTypes }) =>
!ignoreModule(source.value) &&
// Ignore only type imports
if (isOnlyImportingTypes) continue;
!isOnlyImportingTypes
);
/*
Only report as a cycle if there are any import declarations that are considered by
the rule. For example:
if (route.length + 1 < maxDepth) {
untraversed.push({
mget: getter,
route: route.concat(source),
});
a.ts:
import { foo } from './b' // should not be reported as a cycle
b.ts:
import type { Bar } from './a'
*/
if (path === myPath && toTraverse.length > 0) return true;
if (route.length + 1 < maxDepth) {
for (const { source } of toTraverse) {
untraversed.push({ mget: getter, route: route.concat(source) });
}
}
}
Expand Down
@@ -0,0 +1,3 @@
// @flow

import { type FooType, type BarType } from './depth-zero';
3 changes: 3 additions & 0 deletions tests/files/cycles/flow-types-only-importing-type.js
@@ -0,0 +1,3 @@
// @flow

import type { FooType } from './depth-zero';
8 changes: 8 additions & 0 deletions tests/src/rules/no-cycle.js
Expand Up @@ -73,6 +73,14 @@ ruleTester.run('no-cycle', rule, {
code: 'import { bar } from "./flow-types"',
parser: require.resolve('babel-eslint'),
}),
test({
code: 'import { bar } from "./flow-types-only-importing-type"',
parser: require.resolve('babel-eslint'),
}),
test({
code: 'import { bar } from "./flow-types-only-importing-multiple-types"',
parser: require.resolve('babel-eslint'),
}),
],
invalid: [
test({
Expand Down

0 comments on commit 30bba6a

Please sign in to comment.