Add circular dependencies as opt-in cycles issue type (resolve #1734)#1812
Conversation
Detect circular dependencies across the whole module graph, reported as the
opt-in `cycles` issue type. Enable with `--cycles`, `--include cycles` or
`{"include": ["cycles"]}`; defaults to the `warn` rule (set `rules.cycles` to
`error` to fail on them).
Detection is a single-pass back-edge DFS (`findAllCycles`) over runtime
(non-type-only) edges, emitting one cycle per back-edge. Reuses the issue-type
machinery — filters, rules, every reporter, counters, exit code.
In the default report each cycle is one row: the route from the anchor file,
paths relative to the root. The dedicated `cycles` reporter prints a verbose
multi-line tree per cycle.
commit: |
|
👋 @webpro, great stuff! Excited to try this new feature shortly! Just to make sure I've understood it: would |
|
Collect the require.context directory glob during static analysis and resolve + glob it in the build phase, so aliased directories (tsconfig `paths`) are picked up as entry files. Register the visitor for rspack, not only webpack.
Resolve tsconfig `paths` and Vite `resolve.alias` at the head of glob patterns - wildcard, non-wildcard, and multi-target - while keeping relative and negated patterns working, and resolve root-relative patterns against the Vite root.
Storybook processes imports in `.mdx` docs via `@storybook/addon-docs` (bundled in `@storybook/addon-essentials` before v8). Register the shared MDX compiler when either is present, so helpers imported only from MDX docs are no longer reported as unused exports/files.
Both options reference local asset files relative to the config file. Mark them as used so they are not reported as unused files.
A `babel-jest` transformer may carry an inline Babel config (presets/plugins) in its options. Resolve those the same way the Babel plugin does, so they are not reported as unused dependencies.
# Conflicts: # packages/knip/src/constants.ts
Dynamic `import()` edges are excluded from cycle detection by default: a deferred import doesn't cause the synchronous initialization hazard cycles are meant to catch, and making an edge dynamic is the standard way to break a cycle. Set `cycles.dynamicImports` to include them; they render with the `dynamic import` edge kind in the `cycles` reporter. Consolidate cycle configuration under a single `cycles` key holding `dynamicImports` and `allow` (accept known cycles by exact path, replacing `ignoreCycles.paths`). `allow` signals an accepted-cycles allowlist and avoids collision with the glob-taking `ignore*` options.
|
Interesting feature! Historically, the ecosystem has used the Also, I think it would be nice to add to the docs a discussion of the |
|
Knip has already created a module-graph as part of, well, existing. eslint-plugin-import is slow, incompatible with ESLint's caching (though so is TSESLint), is heavy, and generally doesn't support as much of of the ecosystem (e.g., it's not going to include MDX in the module graph). (That's not meant as a criticism of eslint-plugin-import(-x), it's fundamentally a limitation of how ESLint processes files) |
|
Next to what Eli lays out, I can add that it was fun to build and it was already one of the existing operations on the module graph (i.e. before this PR, available in the Editor Extension). The heavy lifting is done building the module graph, such ops are then relatively cheap. By design such graph-based ops spanning multiple files fit Knip better than any file-based linter. Integrated into Knip also means affordances like support for monorepos/workspaces (including workspace filtering), non-standard file support, etc. etc. as Btw we can add the ESLint rule to https://knip.dev/reference/related-tooling#circular-dependencies when we're at it. |
# Conflicts: # packages/knip/fixtures/plugins/typedoc/custom.js # packages/knip/src/typescript/resolve-module-names.ts
Deploying knip with
|
| Latest commit: |
faff8b7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8e18cbe7.knip.pages.dev |
| Branch Preview URL: | https://circular-dependencies.knip.pages.dev |
Is there a way to have this reporter while running other checks too? I am trying to have only one run of knip in CI which does what it used to plus checks for cycles. I'm a bit confused by what changes to make in |
I think you can use multiple reporters if you want. Try it out. |
|
🚀 This pull request is included in v6.25.0. See Release 6.25.0 for release notes. Using Knip in a commercial project? Please consider becoming a sponsor. |
4 participants
tl/dr; Try with
knip --cycles --reporter cycles:Detect circular dependencies across the whole module graph, reported as the opt-in
cyclesissue type. Enable with--cycles,--include cyclesor{"include": ["cycles"]}; defaults to thewarnrule. Setrules.cyclestoerrorto fail on them.Detection is a single-pass back-edge DFS (
findAllCycles) over runtime (non-type-only) edges, emitting one cycle per back-edge. Reuses the issue-type machinery: filters, rules, every reporter, counters, exit code.In the default report each cycle is one row: the route from the anchor file, paths relative to the root. The dedicated
cyclesreporter prints a verbose multi-line tree per cycle.Updated docs at https://circular-dependencies.knip.pages.dev/features/reporters#cycles