chore(ci): add dead-code and unused-export detection to CI (closes #297) - #324
Open
Cyber-Mitch wants to merge 1 commit into
Open
chore(ci): add dead-code and unused-export detection to CI (closes #297)#324Cyber-Mitch wants to merge 1 commit into
Cyber-Mitch wants to merge 1 commit into
Conversation
✅ Deploy Preview for spiffy-melomakarona-eb1e8a ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@Cyber-Mitch Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
✅ Deploy Preview for smart-drop ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Author
|
@ritaifeoluwa @prodbycorne Please Review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #297
No CI step detected unused exports or dead code, so it accumulated silently over time. Adds
knipas a dead-code/unused-export gate, wired into the existing PR-gate workflow.Tool selection — knip, not the two the issue also named
ts-prune— rejected. Effectively unmaintained upstream, and has no Next.js App Router awareness: everypage.tsx/layout.tsx/route.tsdefault export, plusmetadata/generateStaticParams, would need hand-maintained entry config it barely supports. On a 104-file App Router codebase that's a false-positive storm, not a usable gate.eslint-plugin-unused-imports— rejected as the answer to this specific issue. It only finds unused imports/variables within a file — it does not detect cross-file unused exports, which is literally the issue title. ESLint here already reports unused vars; this plugin wouldn't add the capability actually being asked for.knip— chosen. Built-in Next.js plugin correctly resolves App Router's implicit entry points — verified empirically, not assumed: 0 page/layout/route false positives across all 104.ts/.tsxfiles. Reads the repo's one path alias (@/* → ./src/*) fromtsconfig.jsonautomatically, with zero manual config. Single-package repo means none of knip's monorepo configuration curve applies. Covers unused exports, unused exported types, and unused dependencies in one pass — actively maintained (v6.33.0).Scope discipline — a detection gate, not a cleanup PR
First zero-config run: 36 findings (2 unused deps, 1 unused devDep, 24 unused exports, 9 unused exported types, 0 unused files). That's too many to verify-and-delete safely inside a "wire in the detector" PR — several are genuinely ambiguous (framework re-export idioms, a config gap producing a false positive, and an 11-symbol block in
src/lib/error-handler.tsthat looks like a complete module built ahead of use, which needs real investigation before anyone decides it's safe to remove).Decision: scoped-ignore baseline. All 36 pre-existing findings are individually grandfathered in
knip.tswith per-file comments naming each symbol, so future contributors know exactly why they're allowlisted rather than wondering. Zero code deleted or modified — this PR is the gate, not the cleanup.The one real config gap was fixed properly, not ignored around
e2e/mocks/freighter.tsinitially flagged as an unreferenced file — but that's a config gap, not a real dead-code finding: this repo has two Playwright configs (rootplaywright.config.tsfor./tests,e2e/playwright.config.tsfor./e2e), and knip's Playwright plugin only auto-detects one. Fixed by pointing the plugin at both config files, rather than ignoring the false positive — this keeps future dead code undere2e/andtests/genuinely catchable, instead of permanently blind to that directory.Proof the gate actually works — a canary, not just a clean run
A clean
pnpm run deadcodeexit-0 against the current tree doesn't prove the gate would catch anything new — it could just as easily mean the tool is misconfigured to find nothing. Verified instead: added a strayexport const __x = 42in a newsrc/lib/file →pnpm run deadcodecorrectly failed (Unused files (1), exit 1). Removed after confirming. The gate demonstrably fails the build on a genuine new finding.Changed files
.github/workflows/e2e.yml | 3 + (new step, after "Dependency vulnerability audit")
knip.ts | 92 + (new config: Next.js + Vitest + dual-Playwright plugins, plus the commented 36-item allowlist)
package.json | 2 + (deadcode script, knip devDependency)
pnpm-lock.yaml | 551 +- (knip + transitive deps only — no existing package's pinned version changed or removed)
CI step (
.github/workflows/e2e.yml), placed as a cheap static check before the expensive build/Lighthouse/Playwright steps:Verification
pnpm run typecheckpnpm run lintpnpm run buildpnpm run audit:highpnpm run bundle:budgetpnpm run deadcode"How to test" — satisfied
package.jsonnow has a dead-code detection tool configured —knip@^6.33.0,deadcodescript,knip.tsconfig.github/workflows/e2e.yml→ "Dead code and unused exports"Follow-up cleanup — a precise checklist, not a vague pointer
Every one of the 36 baseline findings, individually, so whoever picks this up doesn't need to re-run
knipand re-derive the list:Unused dependencies (3)
@chakra-ui/next-js— not imported anywhere insrc/@stellar/stellar-base— transitively provided by@stellar/stellar-sdk; no direct importautoprefixer(devDep) — Tailwind v3 leftover; Tailwind v4 (@tailwindcss/postcss) autoprefixes internallysrc/lib/error-handler.ts— highest-value investigation (11 findings):RPCError,ContractError,ValidationError,ErrorLogger,ErrorHandler,withErrorHandler,useErrorHandler,retryWithBackoff,enum ErrorType,interface AppError. Looks like a complete error-handling module built ahead of use — needs a dynamic-import / planned-feature check before anyone removes it.Everything else, by file:
src/lib/soroban.ts(5) —buildUnlockAssetsTransaction,parseSimulationError,LeaderboardRow,BoostConfig,ContractCallOptionssrc/lib/backend.ts(3) —getAirdrop,listAirdropRecipients,Recipientsrc/hooks/useSorobanQuery.ts(3) —useLockAssets,useOptimisticUpdate,useTransactionStatessrc/i18n/routing.ts(5) —Link,redirect,usePathname,useRouter,Locale— next-intl idiom; either wire up consumers or drop the re-exportssrc/hooks/useSorobanEvents.ts(1) —classifyGetEventsErrorsrc/lib/feeBumpGuard.ts(1) —SPONSORABLE_FUNCTIONSsrc/types/farm.ts(1) —toStroopssrc/config/index.ts(1) —StellarNetworkPresetsrc/hooks/useToast.tsx(1) —NotificationTypesrc/components/ErrorBoundary/ErrorBoundary.tsx(1) —ErrorBoundarySectione2e/mocks/freighter.ts(1) —injectFreighterMock: trivial fix, just drop theexportkeyword (only used within its own file)Also unrelated, worth its own small issue: 15 pre-existing
vitestfailures (matchMedia is not a function, a jsdom/Chakra environment gap) — vitest isn't run in CI today, so this has been invisible. Needs awindow.matchMediapolyfill in the vitest setup.