chore: add knip and remove dead code#437
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
hii @phoenix-server , could you please take a look at when you get a chance? I’ve added Knip to catch dead code and unused dependencies, cleaned up some unreachable files, and integrated it into the lint pipeline to keep things clean going forward. |
- Add .knip.json with entry points (src/index.ts, src/import-events.ts, knexfile.js) - Add 'npm run knip' script scoped to production files and dependencies - Chain knip into the lint script so CI enforces it automatically - Remove body-parser and dinero.js (no imports found anywhere) - Move rxjs to devDependencies (only used in integration test helpers) - Add Local Quality Checks section to CONTRIBUTING.md
51cfd5a to
2c6b63c
Compare
| "commitlint": false, | ||
| "eslint": false, | ||
| "github-actions": false, | ||
| "husky": false, | ||
| "mocha": false, | ||
| "nyc": false, | ||
| "semantic-release": false |
There was a problem hiding this comment.
@phoenix-server what do each of these lines do when using Knip? Should these be false for this project?
There was a problem hiding this comment.
Hi @cameri , These are Knip "plugins" that auto-detect dependencies by reading each tool's config file. Setting them to false disables that detection.
However, with --production (which our npm run knip script uses), these flags have no effect as --production already skips devDependency analysis entirely. So they're redundant.
They only matter if someone runs npx knip without --production, where the plugins would flag ~17 devDependencies (like chai, sinon, mochawesome, rxjs, ts-node-dev, etc.) as unused even though they're used by tests/tooling.
I can remove them to keep the config minimal since --production handles it, or keep them as a safety net for manual runs. Happy to go either way , what do you prefer?
There was a problem hiding this comment.
Thanks, I just wanted to know what they meant.
There was a problem hiding this comment.
Pull request overview
This PR introduces Knip-based dead-code/dependency checks and removes code/dependencies identified as unused, helping keep the Node/TypeScript codebase smaller and easier to maintain.
Changes:
- Added Knip configuration (
.knip.json) and a newnpm run knipscript; updatednpm run lintto run Knip before ESLint. - Removed unused runtime dependencies and moved
rxjstodevDependencies. - Deleted orphaned/unused “runes” utilities/types/tests and other unused schema/constants files.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.knip.json |
Adds Knip configuration/entry points for dead code + dependency analysis. |
package.json |
Adds Knip script, runs it as part of lint, and adjusts dependencies. |
package-lock.json |
Reflects dependency graph changes (Knip added, deps removed, rxjs moved to dev). |
CONTRIBUTING.md |
Documents running Knip locally and notes lint now includes Knip. |
src/utils/runes/alternative.ts |
Removes unused runes alternative implementation. |
src/utils/runes/restriction.ts |
Removes unused runes restriction implementation. |
src/utils/runes/rune-like.ts |
Removes unused runes rune-like implementation. |
src/@types/runes.ts |
Removes unused runes type definitions. |
test/unit/utils/runes/alternative.spec.ts |
Removes unit tests for deleted runes code. |
test/unit/utils/runes/restriction.spec.ts |
Removes unit tests for deleted runes code. |
test/unit/utils/runes/rune-like.spec.ts |
Removes unit tests for deleted runes code. |
test/unit/utils/runes.spec.ts |
Removes older combined runes unit tests. |
src/schemas/http-request-schemas.ts |
Removes unused request schema module. |
src/constants/payments.ts |
Removes unused payments constant enum. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add Knip checks and remove dead code/dependencies
Description
.knip.json.npm run knipand wirednpm run lintto run Knip before ESLint.rxjsto dev deps.Related Issue
Closes #433
Motivation and Context
This PR adds an automated dead-code/dependency check and removes code/deps that are no longer used, so the codebase stays smaller and easier to maintain.
How Has This Been Tested?
Local verification after rebasing onto latest
main:npm cinpm run build:checknpm run test:unit(383 passing)npm run lint(runs Knip + ESLint)Screenshots (if appropriate):
N/A
Types of changes
Checklist: