Fix a couple of`no-cycle` bugs#2083

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

ljharb merged 2 commits intoimport-js:masterfromcherryblossom000:no-cycle-fixes

May 18, 2021

Merged

Fix a couple of`no-cycle` bugs#2083

ljharb merged 2 commits intoimport-js:masterfromcherryblossom000:no-cycle-fixes

May 18, 2021

Conversation

Copy link

Contributor

cherryblossom000 commentedMay 17, 2021

This fixes these two bugs:

// a.ts// false positive: reports a cycle even though b.ts is only importing typesimport{foo}from'./b'// b.tsimporttype{Bar}from'./a'

// a.js//@flow// false negative: doesn't report a cycle even though b.js is importing the value barimport{foo}from'./b'// b.js//@flowimport{bar,typeBaz}from'./a'

There isn't an open issue for these, but these issues were relatively quick and easy for me to fix so I don't really mind if this PR gets rejected. Also, these bugs were introduced by me in#1974 (oops), so I wanted to fix them.

If you would like me to split these two bug fixes into two PRs, I'm more than happy to do that as well.

cherryblossom000 added2 commits

May 17, 2021 19:28

[Fix]no-cycle: ignore imports where imported file only imports typ…

30bba6a

…es of importing fileThis fixes this situation:`a.ts`:```tsimport { foo } from './b'````b.ts`:```tsimport 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`.

[Fix]no-cycle: fix false negative when file imports a type after i…

72b9c3d

…mporting a value in FlowThis fixes this situation:`a.js`:```jsimport { foo } from './b'````b.js`:```js//@flowimport { bar, type Baz } from './a'```Previously, `no-cycle` would have not reported a dependency cycle for the import in `a.js`, eventhough `b.js` is importing `bar`, which is not a type import, from `a.js`. This commit fixes that.