Knip: remove unused code, remove unused exports#5866
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
Pull Request Overview
This PR removes unused exports from the codebase as identified by Knip, a tool for detecting dead code. The changes streamline the codebase by removing exports that aren't used elsewhere while keeping the functionality intact as local implementations.
Key changes include:
- Converting exported interfaces, types, and functions to non-exported (local) versions
- Removing entire files that only contained unused exports
- Updating Knip configuration to be more strict about detecting unused code
Reviewed Changes
Copilot reviewed 288 out of 893 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Multiple component files | Convert exported interfaces/types to local scope when not used externally |
| Multiple index.ts files | Remove or reduce exports that aren't consumed by other modules |
| .knip.json | Enable stricter rules for types, files, exports, and duplicates detection |
| package.json | Update script references to use .cjs extensions |
Comments suppressed due to low confidence (1)
.knip.json:1
- [nitpick] The Knip configuration has been changed from 'off' to 'error' for multiple rules. This is a good improvement for code quality, but ensure the CI pipeline can handle these stricter checks and consider adding documentation about the new requirements.
{
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
witoszekdev
added
the
skip changeset
Differences FoundExpandLicense Package Apache-2.0 baseline-browser-mapping BSD-2-Clause regjsparser MIT builtin-modules MIT clean-regexp MIT core-js-compat MIT eslint-plugin-unicorn MIT eslint-plugin-unused-imports MIT find-up-simple MIT is-builtin-module MIT pluralize SummaryExpand
|
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 289 out of 906 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 289 out of 909 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 289 out of 916 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5866 +/- ##
==========================================
+ Coverage 38.61% 38.64% +0.02%
==========================================
Files 2753 2745 -8
Lines 43691 43254 -437
Branches 9863 9807 -56
==========================================
- Hits 16873 16714 -159
- Misses 25494 26510 +1016
+ Partials 1324 30 -1294 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 293 out of 912 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
witoszekdev
changed the title
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 299 out of 760 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/collections/components/CollectionListPage/CollectionListPage.tsx:1
- The
currencySymbolparameter is destructured but not used in the component. This should be removed from the destructuring assignment.
// @ts-strict-ignore
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Continuation of #5859
This PR deletes dead-code that wasn't used anywhere and removes exports if they weren't used outside of local file.
Caution
Do not open "Files changed" tab, it will freeze your browser 🙈
Open changes using
github.dev: https://github.dev/saleor/saleor-dashboard/pull/5866/filesOther changes
These changes were needed to make this cleanup possible
ESLint
Added new ESLint rules that were used during this cleanup and will be useful afterwards as well:
no-unused-vars- similar to our existing TypeScript config, but will ignorecatch (error), and vars starting with_eslint-unused-imports-plugin- to auto-delete unused imports (useful in normal development to get rid of annoying errors from TypeScript)eslint-plugin-unicorn with rule
unicorn/no-empty-file- to detect and remove empty files after our cleanup (due to running all rules, file content was completely removed) - we might want to use more rules from this plugin 👀TypeScript
Removed checking unused vars and imports, since this is now done by ESLint. This might improve TypeScript performance.
Other
Added
OrderGiftCardEventBalanceFragmentto fix TypeScript error insrc/orders/components/OrderSummaryCard/utils.tsSteps taken
For future prosperity here were the steps taken to cleanup unused files:
npm run knip:fixnpm run linternpx eslint --quiet -f json | npx remove-unused-varsnpm run check-typesManual fixes
let someVarand then initialize it inbeforeEachblock. If variable wasn't actually used in test, knip will delete it. But it won't delete thebeforeEachinitailization - this needs to be deleted manuallyunicorn/no-empty-filedoesn't support auto-fix, do some bash magic by simply: running eslint, exporting result to json, parse it using jd, pass tormcommand ಠ‿↼remove-unused-vars- these need to be removed manuallyexport {default as something} from "./some-file"line in barrel file. Ideally barrel files should be gone, but we already have 900+ files changed 😄