feat(qwik): add few new lint rules #6887
Conversation
Add Qwik domain support and implement the noReactProps rule to disallow React-specific props in Qwik components.
…of classlist prop over classnames
Add a rule to disallow missing key props in JSX elements inside iterators for Qwik applications.
# Conflicts: # crates/biome_configuration/src/analyzer/linter/rules.rs # crates/biome_diagnostics_categories/src/categories.rs # crates/biome_js_analyze/src/lint/nursery.rs # packages/@biomejs/backend-jsonrpc/src/workspace.ts
🦋 Changeset detectedLatest commit: f37b036 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Ngl this is a decently big PR, and its a bit hard to review. Regardless, I tried my best to give you a comprehensive review, but I almost certainly missed some things. The rules are simple enough that I don't think its worth it to split this up into multiple PRs -- just might take a bit longer to get everything merged in.
Regarding rule names: we recently decided that it would be good for rules specific to a particular framework to call it out in their name with an additional prefix -- so for example useQwikThing or noQwikThing. Though the usefulness of some of these rules don't seem entirely limited to just qwik projects, like useJsxA.
crates/biome_js_analyze/src/lint/nursery/no_use_visible_task.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_use_visible_task.rs
Outdated
Show resolved
Hide resolved
|
Ah one more thing -- since this is adding a new domain, we'll need to have a PR to the website repo as well to document the domain in this file: https://github.com/biomejs/website/blob/c7b801befc8649de03a197aac7ccef41e1dc2c66/codegen/src/domains.rs#L71 We won't be able to merge it until this gets synchronized over to the website repo, i think, but it would be nice to have it queued up. |
CodSpeed Performance ReportMerging #6887 will not alter performanceComparing Summary
Footnotes |
|
Thank you @ptkagori for your great contribution. I want to add a few things to @dyc3 review. If you haven't read our contribution guide, I suggest you do so, because it covers a lot of stuff: https://github.com/biomejs/biome/blob/main/crates/biome_analyze/CONTRIBUTING.md Generally, all rules must follow our rule pillars: https://biomejs.dev/linter/#rule-pillars (this is covered in the contribution guide). |
crates/biome_js_analyze/src/lint/nursery/no_use_visible_task.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_use_visible_task.rs
Outdated
Show resolved
Hide resolved
…issingJsxKey, and replace noReactProps with noQwikReactProps - Renamed the Qwik rule `noUseVisibleTask` to `noQwikUseVisibleTask` across all code, tests, documentation, and configuration. - Removed the obsolete `noMissingJsxKey` rule; its functionality is now unified under `useJsxKeyInIterable`. - Replaced the `noReactProps` rule with `noQwikReactProps` for Qwik-specific JSX attribute enforcement. - Updated all related imports, filenames, test directories, documentation, and .changeset to reflect these changes. - Ensured no unwanted or breaking changes were introduced and all references are consistent.
…issingJsxKey, and replace noReactProps with noQwikReactProps - Renamed the Qwik rule `noUseVisibleTask` to `noQwikUseVisibleTask` across all code, tests, documentation, and configuration. - Removed the obsolete `noMissingJsxKey` rule; its functionality is now unified under `useJsxKeyInIterable`. - Replaced the `noReactProps` rule with `noQwikReactProps` for Qwik-specific JSX attribute enforcement. - Updated all related imports, filenames, test directories, documentation, and .changeset to reflect these changes. - Ensured no unwanted or breaking changes were introduced and all references are consistent.
…, noQwikReactProps)
# Conflicts: # crates/biome_configuration/src/analyzer/linter/rules.rs # crates/biome_rule_options/src/lib.rs # packages/@biomejs/backend-jsonrpc/src/workspace.ts # packages/@biomejs/biome/configuration_schema.json
|
@ptkagori I Ieft 22 comments in my review, and only few of them have been addressed. Please check them, and please check also the snapshots when you update them, some of them are still wrong |
# Conflicts: # packages/@biomejs/backend-jsonrpc/src/workspace.ts
…ing PR requested changes
…rmatter failures on local Windows
|
@dyc3 please review again |
6b772e4 to
fd7490c
Compare
# Conflicts: # crates/biome_configuration/src/analyzer/linter/rules.rs # crates/biome_diagnostics_categories/src/categories.rs # crates/biome_js_analyze/src/lint/nursery.rs # packages/@biomejs/backend-jsonrpc/src/workspace.ts
# Conflicts: # crates/biome_configuration/src/analyzer/linter/rules.rs # crates/biome_diagnostics_categories/src/categories.rs # crates/biome_js_analyze/src/lint/nursery.rs # packages/@biomejs/backend-jsonrpc/src/workspace.ts
|
@ptkagori let me help you with the conflicts. Let's wait for the final review, and then I will take it from there. |
|
@ptkagori I fixed the conflicts. If you intend to do more changes, remember to |
Summary
Added Biome support for the Qwik framework. This is a WIP PR which follows up on Add Qwik Domain and enabled pre-existing rules
noQwikUseVisibleTaskuseQwikClasslistuseImageSizeuseAnchorHrefNote: Changes also include the initial qwik domain setup already covered in the above PR but just so local dev works even before the setup PR is merged
Test Plan
Docs
Added: biomejs/website#2727 to sync addition of Qwik domain with website
Each rule includes insightful documentation with: