refactor(toolsets): remove unused matchesFilter method#229
Conversation
Remove the private matchesFilter method from StackOneToolSet class as it was never called anywhere in the codebase. The filterTools method implements filtering directly using matchGlob without delegating to matchesFilter. Changes: - Remove matchesFilter method (lines 355-371) - Remove unused import of Arrayable from type-fest - Remove unused import of toArray from utils/array - Remove testMatchesFilter helper from test class - Remove associated test cases for matchesFilter - Rename describe block from 'glob and filter matching' to 'glob matching'
Add typescript to enable vitest type checking during test runs. Previously vitest's typecheck feature would fail with 'spawn tsc ENOENT' because typescript was not installed as a dependency. - Add typescript ^5.8.3 to pnpm-workspace.yaml catalog - Add typescript: catalog:dev to package.json devDependencies
commit: |
There was a problem hiding this comment.
Pull request overview
This PR removes dead code by deleting the unused matchesFilter private method from the StackOneToolSet class. The method was never called anywhere in the codebase as the filterTools method implements filtering directly using matchGlob. The cleanup also removes the now-unnecessary imports (Arrayable from type-fest and toArray from utils/array) from toolsets.ts, along with the corresponding test helper method and test cases. Additionally, TypeScript is added as a devDependency to enable vitest type checking.
Key Changes:
- Removed unused
matchesFiltermethod and its associated imports fromStackOneToolSetclass - Removed
testMatchesFiltertest helper method and related test cases - Added TypeScript as a devDependency for improved type checking support
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/toolsets.ts | Removed unused matchesFilter private method and cleaned up imports (Arrayable, toArray) that were only needed for that method |
| src/toolsets.test.ts | Removed testMatchesFilter test helper method, deleted test cases for the removed functionality, and updated describe block name from "glob and filter matching" to "glob matching" |
| pnpm-workspace.yaml | Added TypeScript version ^5.8.3 to the dev catalog |
| pnpm-lock.yaml | Updated lockfile to reflect TypeScript dependency addition (resolves to 5.9.3) |
| package.json | Added TypeScript as a devDependency using catalog:dev reference |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| publint: ^0.3.12 | ||
| tsdown: ^0.17.2 | ||
| type-fest: ^4.41.0 | ||
| typescript: ^5.8.3 |
There was a problem hiding this comment.
The catalog specifies TypeScript version "^5.8.3" but the lockfile resolves to version "5.9.3". This means 5.9.3 satisfies the semver range ^5.8.3, which is expected behavior for caret ranges in semantic versioning. However, this creates a version drift between what's specified in the catalog and what's actually installed. Consider either:
- Updating the catalog to specify "^5.9.3" to match the installed version
- Or pinning to "~5.8.3" if you specifically want to stay on 5.8.x versions
This ensures the catalog accurately reflects the intended version constraints.
| typescript: ^5.8.3 | |
| typescript: ^5.9.3 |
Summary
matchesFilterprivate method fromStackOneToolSetclass that was never called in the codebasetypescriptas devDependency to enable vitest type checkingDetails
The
matchesFiltermethod was orphaned code - thefilterToolsmethod implements filtering directly usingmatchGlobwithout delegating tomatchesFilter. This cleanup removes dead code and its associated imports (Arrayablefrom type-fest,toArrayfrom utils/array).Test plan
Summary by cubic
Removed the unused matchesFilter method from StackOneToolSet and its related tests to clean up dead code. Added TypeScript as a dev dependency to enable Vitest type checking.
Refactors
Dependencies
Written for commit af110c9. Summary will update automatically on new commits.