feat: biome plugin
#1113
feat: biome plugin
#1113
Conversation
|
|
||
| assert(issues.devDependencies['package.json']['@org/unused-config']) | ||
| assert(issues.unresolved['biome.json']['./shared/non-exist-base.json']) | ||
| assert(issues.unlisted['biome.json']['@org/unlisted-configs']) |
There was a problem hiding this comment.
The idea of plugin tests is to have no issues left and counters with only total and processed (see other plugin tests for inspiration please).
There was a problem hiding this comment.
If we expect no issues, how do we test unused and unresolved dependencies which is the spirit of knip in the first place?
There was a problem hiding this comment.
@webpro I'm not sure if I'm misunderstanding something. I've skimed through other tests and see lots of the tests assert on issues which is the same as this test. Can you help me understand this a bit more?
There was a problem hiding this comment.
Most of the plugin tests are pretty much "end to end" tests in that the fixture folder is consumed in its entirety including package.json that should contain the dependencies referenced in the tool configuration, and also executables. E.g. node_modules/@biomejs/biome/package.json contains bin field for biome binary that should actually be referenced and result in zero issues left. This full-circle approach results in well-defined and maintainable tests.
Testing that source code or configuration results in unused or unresolved can happen in additional/other tests. There is definitely room for improvement, but this is the direction I like to go.
There was a problem hiding this comment.
Ahh this is a really cool way of looking at testing. It was counter-intuitive to me at first but then it finally clicked. I updated the test. PTAL.
| const resolveConfig: ResolveConfig<BiomeConfig> = (config, options) => { | ||
| return [ | ||
| // If we're using biome plugin, biome should be ignored in the report | ||
| toIgnore("@biomejs/biome", "dependencies"), |
There was a problem hiding this comment.
Isn't the biome executable expected in e.g. some package.json#scripts? That would add a reference to the bin of the @biomejs/biome package. For instance, Knip itself has this too: https://github.com/thoroc/knip/blob/22739cee31e0845d8e73aeb26c77a0e75a7e1e71/package.json#L23-L25
There was a problem hiding this comment.
yeah that's true. let me fix this
|
Thanks @webpro for the review. I left two questions in the threads and will work on addressing #1113 (comment) as soon as I have time. |
|
Can you please check the changeset carefully, there's a lot of unrelated changes. |
commit: |
Ooops. I must have missed something when syncing with the latest |
Co-authored-by: Lars Kappert <[email protected]>
Co-authored-by: Lars Kappert <[email protected]>
|
🚀 This pull request is included in v5.61.0. See Release 5.61.0 for release notes. Using Knip in a commercial project? Please consider becoming a sponsor. |
This PR adds a plugin for biome.
Since the biome is just tooling on top of existing Typescript projects, we don't need to implement for entry and project files because it will be redundant with at least the Typescript plugin.
Fixes #901 #1071