chore(ui): add knip for dead code detection#10654
Conversation
Install knip and create initial configuration tailored to the Next.js 16 project structure. Adds `lint:knip` and `lint:knip:fix` scripts to surface unused files, dependencies, and exports. Baseline: ~504 issues (37 unused files, 22 unused deps, 399 unused exports, 37 unused types, 5 unused enum members, 2 unresolved imports, 1 duplicate export). These will be addressed in follow-up PRs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
✅ All necessary |
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
🔒 Container Security ScanImage: 📊 Vulnerability Summary
2 package(s) affected
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Alan-TheGentleman
left a comment
There was a problem hiding this comment.
Three things to address:
-
lint:knipexits non-zero with 504 issues. Anyone running it will think something is broken. Add a note in the PR description or a comment inknip.config.tsexplaining the baseline, or consider adding--max-issues 504so it only fails on regressions. The roadmap mentions this for step 5, but even a code comment now would save confusion. -
Missing
instrumentation.tsentry point. Next.js auto-loads this file for Sentry setup. The config already listssentry/sentry.server.config.tsandsentry/sentry.edge.config.tsbut not the rootinstrumentation.tsthat imports them. Knip will likely flag it as unused. -
Missing
middleware.tsentry point. Same idea. Next.js loads it automatically but knip does not know that unless it is listed explicitly. Add it to the entry array to avoid a false positive.
Apply Alan's review feedback so `lint:knip` exits zero on the current inventory and only regressions break CI. Document that knip's Next.js plugin already picks up instrumentation.ts and proxy.ts, so no explicit entries are needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # ui/package.json # ui/pnpm-lock.yaml
# Conflicts: # ui/dependency-log.json
# Conflicts: # ui/CHANGELOG.md
Alan-TheGentleman
left a comment
There was a problem hiding this comment.
All three points from the previous review are addressed — verified against the latest commit.
-
Baseline threshold.
lint:knipandlint:knip:fixnow run with--max-issues 504, andknip.config.tshas a clear header comment explaining the baseline and pointing at step 5 of the roadmap for reducing it. Exactly what I was after. -
instrumentation.ts. Not needed in the explicitentryarray — knip's built-in Next.js plugin already includes{instrumentation,instrumentation-client,middleware,proxy}.{js,jsx,ts,tsx}in its default entry patterns, and since this config only overrides root-levelentry(notplugins.next.entry), the plugin defaults still apply. Good call documenting it inline. -
middleware.ts. My original comment was outdated — this repo is on Next.js 16, wheremiddleware.tswas renamed toproxy.ts. The file exists atui/proxy.tsand is covered by the same Next.js plugin default pattern.
LGTM. Thanks for the clean follow-up and the explanatory comments in the config — they make the intent obvious for the next person who opens the file.
Summary
lint:knipandlint:knip:fixscripts topackage.json@heroui/*sub-package imports (resolved transitively via@heroui/react)Baseline report
Running
pnpm lint:knipsurfaces ~504 issues across the UI codebase:These findings will be addressed in follow-up PRs (dependency cleanup, dead export removal, config refinement).
Roadmap
This is the first step in a series of incremental PRs to reduce dead code in the UI:
--max-issuesin healthcheckDependencies
Test plan
pnpm typecheckpassespnpm lint:checkpassespnpm buildpassespnpm lint:knipruns and produces the baseline report (exits non-zero as expected)