feat(lint): implement vue/noSetupPropsReactivityLoss #7513
Conversation
🦋 Changeset detectedLatest commit: 56869c2 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 |
github-actions
bot
added
A-Project
WalkthroughAdds a new Vue nursery lint rule Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (4)crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
crates/biome_*/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{rs,toml}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
🧬 Code graph analysis (1)crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (1)
🔇 Additional comments (13)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
.changeset/yellow-parts-appear.md (1)
5-7: Changeset polish: add docs link, fix punctuation, and include a valid snippet.
- Please include a link to the rule’s docs, per our changeset guidelines.
- The “Invalid code example:” line should end with a full stop.
- Consider adding a minimal “Valid code example.” using toRefs to show the safe pattern.
Apply this diff:
@@ -Added the `vue/noSetupPropsReactivityLoss` rule to the nursery. +Added the `vue/noSetupPropsReactivityLoss` rule to the nursery. @@ -This new rule disallows usages that cause the reactivity of `props` passed to the `setup` function to be lost. +This new rule disallows usages that cause the reactivity of `props` passed to the `setup` function to be lost. +See documentation: https://biomejs.dev/linter/rules/vue/noSetupPropsReactivityLoss. @@ -Invalid code example: +Invalid code example. @@export default {
setup({ count }) {
//countis no longer reactive here.
return () => h("div", count);
},
};+ +Valid code example. + +```jsx +export default { + setup(props) { + const { count } = toRefs(props); + return () => h("div", count.value); + }, +}; +```Also applies to: 9-9, 18-18
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js (1)
1-7: Good invalid fixture; optional expectation hint.Add a short top comment to indicate this should produce a diagnostic (helps skim large suites).
-// Destructuring assignment +// Destructuring assignment +/* should generate diagnostics */crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.js (1)
1-6: Covers parameter destructuring nicely; add hint comment.Same tiny note as above for consistency across fixtures.
-// Destructuring in setup parameters +// Destructuring in setup parameters +/* should generate diagnostics */crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRefs.js (1)
4-9: Nice positive case; consider adding toRef/alias variants.To harden coverage, add a companion fixture exercising
toRef(props, "count")and aliasing (const { count: c } = toRefs(props)).Example new file:
// crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRef.js export default { setup(props) { const c = toRef(props, "count"); const { count: alias } = toRefs(props); return () => h('div', c.value + alias.value); } }crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/array_destructuring.js (1)
1-6: Drop array-destructuring test — upstream rule only flags object-pattern/member accessVerified: eslint-plugin-vue's no-setup-props-reactivity-loss flags object-pattern destructuring and member expressions of props, not array destructuring. Remove or skip this test to keep parity and avoid drift.
Location: crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/array_destructuring.js
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs (2)
89-92: Consider documenting theRuleStateenum variants.While the variant names are descriptive, adding doc comments would clarify when each state is triggered for future maintainers.
+/// Represents different ways props reactivity can be lost in Vue setup pub enum RuleState { + /// Props were destructured directly in the setup function parameters DestructuredParameter(TextRange), + /// Props were destructured via assignment in the setup body root scope DestructuredAssignment(TextRange), }
236-268: Consider extracting the reactive helper check for clarity.The nested logic for checking reactive helpers could be clearer as a separate function.
+fn is_reactive_helper_call(call_expr: &JsCallExpression) -> bool { + call_expr + .callee() + .ok() + .and_then(|callee| callee.as_js_identifier_expression().cloned()) + .and_then(|id_expr| id_expr.name().ok()) + .and_then(|name| name.value_token().ok()) + .is_some_and(|token| { + let text = token.text_trimmed(); + matches!(text, "toRefs" | "toRef" | "reactive" | "ref") + }) +} let should_report = match init_expr { AnyJsExpression::JsIdentifierExpression(id_expr) => id_expr .name() .ok() .and_then(|name| name.value_token().ok()) .is_some_and(|token| token.text_trimmed() == props_name), AnyJsExpression::JsCallExpression(call_expr) => { - let is_reactive_helper = call_expr - .callee() - .ok() - .and_then(|callee| callee.as_js_identifier_expression().cloned()) - .and_then(|id_expr| id_expr.name().ok()) - .and_then(|name| name.value_token().ok()) - .is_some_and(|token| { - let text = token.text_trimmed(); - matches!(text, "toRefs" | "toRef" | "reactive" | "ref") - }); - - !is_reactive_helper + !is_reactive_helper_call(call_expr) && call_expr.arguments().ok().is_some_and(|args| { args.args().into_iter().flatten().any(|arg| {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/array_destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_callbacks.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRefs.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (13)
.changeset/yellow-parts-appear.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/array_destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_callbacks.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRefs.js(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/no_setup_props_reactivity_loss.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_rule_options/src/no_setup_props_reactivity_loss.rscrates/biome_rule_options/src/lib.rscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRefs.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/array_destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_callbacks.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.jscrates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_rule_options/src/no_setup_props_reactivity_loss.rscrates/biome_rule_options/src/lib.rscrates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRefs.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/array_destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_callbacks.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.jscrates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRefs.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/array_destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_callbacks.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.jscrates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changeset files, only use #### or ##### headers; avoid other header levels
Changeset descriptions should use past tense for what you did (e.g., "Added...")
Describe current Biome behavior in present tense within changesets (e.g., "Biome now supports...")
For bug fixes in changesets, start with a link to the issue (e.g., "Fixed #1234: ...")
When referencing rules or assists in changesets, include links to their documentation pages
Include a minimal code block in the changeset when applicable to demonstrate the change
End every sentence in the changeset description with a period
Files:
.changeset/yellow-parts-appear.md
🧠 Learnings (3)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Options types must implement serialization/deserialization and schema support using derives: `Serialize`, `Deserialize`, `Deserializable`, and `#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]`
Applied to files:
crates/biome_rule_options/src/no_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options types in `biome_rule_options` crate (one file per rule, e.g., `lib/use_my_rule.rs`)
Applied to files:
crates/biome_rule_options/src/lib.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : In declare_lint_rule! macros, set `version: "next"` for new or updated rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
🧬 Code graph analysis (10)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRefs.js (5)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js (1)
props(4-4)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js (4)
props(12-12)props(57-57)props(58-58)props(74-74)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js (2)
props(4-4)props(5-5)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js (3)
toRefs(69-69)count(54-54)count(86-86)crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
h(131-133)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.js (1)
crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
h(131-133)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js (3)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js (1)
props(4-4)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js (4)
props(12-12)props(57-57)props(58-58)props(74-74)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js (2)
count(54-54)count(86-86)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/array_destructuring.js (1)
crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
h(131-133)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.js (2)
crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
h(131-133)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js (2)
count(54-54)count(86-86)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_callbacks.js (4)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js (1)
props(4-4)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js (4)
props(12-12)props(57-57)props(58-58)props(74-74)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js (2)
props(4-4)props(5-5)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js (2)
count(54-54)count(86-86)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js (3)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js (4)
props(12-12)props(57-57)props(58-58)props(74-74)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js (2)
props(4-4)props(5-5)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js (2)
count(54-54)count(86-86)
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs (2)
crates/biome_js_syntax/src/function_ext.rs (1)
statements(94-111)crates/biome_js_syntax/src/expr_ext.rs (2)
callee(32-37)value_token(1547-1556)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js (3)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js (1)
props(4-4)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js (4)
props(12-12)props(57-57)props(58-58)props(74-74)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js (2)
props(4-4)props(5-5)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js (4)
crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
h(131-133)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js (2)
count(54-54)count(86-86)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js (1)
props(4-4)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js (2)
props(4-4)props(5-5)
🔇 Additional comments (8)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.js (1)
1-6: LGTM — nested parameter destructuring covered.This neatly captures the nested case the rule should flag.
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js (1)
1-8: LGTM — multiple assignments case is covered.Captures repeated root-scope destructuring within setup.
crates/biome_rule_options/src/lib.rs (1)
165-165: LGTM — module exposed and options file present.
Options file: crates/biome_rule_options/src/no_setup_props_reactivity_loss.rs — containspub struct NoSetupPropsReactivityLossOptions {}.crates/biome_rule_options/src/no_setup_props_reactivity_loss.rs (1)
1-6: LGTM! The options struct follows the established patterns.The empty options struct is correctly configured with all required derives and attributes for serialisation/deserialisation support, following the codebase conventions.
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_callbacks.js (1)
1-15: LGTM! Test case correctly validates nested scope destructuring.The test properly demonstrates that destructuring within callbacks and render functions is permitted, as these maintain the reactive context.
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js (1)
1-77: Excellent test coverage for invalid patterns!The test suite comprehensively covers all major destructuring patterns that break reactivity: parameter destructuring, root-scope assignments, array destructuring, nested patterns, and various declaration types (const/let/var).
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs (1)
11-87: Comprehensive documentation with clear examples!The rule documentation excellently explains the reactivity issue and provides both invalid and valid examples, making it easy for users to understand the problem and solutions.
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js (1)
1-103: Thorough test coverage for valid patterns!The test suite covers all the key valid scenarios: direct props usage, nested destructuring, reactive helpers (toRefs), computed properties, and edge cases like arrow functions and context parameters.
AsherDe
force-pushed
the
feat/vue-reactivity-rule
branch
from
61a2780 to
f925549
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js (2)
34-38: Array pattern insetupparams is dubious for Vue props.The rule’s contract is about destructuring the “props object”. Array destructuring here isn’t a realistic Vue usage and may not be flagged depending on implementation. Either drop this case or add a clarifying comment asserting parity with
eslint-plugin-vueif it intentionally warns on any non‑Identifier first param.If parity is intended, please confirm the upstream rule’s behaviour and that our snapshot shows a diagnostic for this block.
72-77: Consider one more “invalid” for assignment without declaration.Add a case for bare assignment destructuring, which also breaks reactivity:
({ count } = props)- And optionally default-in-assignment:
const { count = 0 } = propsI can spin up a separate spec file (e.g.,
destructuring_assignment_no_decl.js) with these examples if you’d like.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/array_destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_callbacks.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRefs.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (13)
.changeset/yellow-parts-appear.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_callbacks.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRef.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRefs.js(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/no_setup_props_reactivity_loss.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- crates/biome_rule_options/src/lib.rs
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js
- crates/biome_rule_options/src/no_setup_props_reactivity_loss.rs
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.js
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.js
- .changeset/yellow-parts-appear.md
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_callbacks.js
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRef.js
- crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRefs.js
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js
🧬 Code graph analysis (1)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js (3)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid.js (2)
count(54-54)count(86-86)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js (1)
props(5-5)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js (2)
props(5-5)props(6-6)
🔇 Additional comments (2)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js (2)
2-7: Good coverage of the core “invalid” patterns.These cases accurately exercise parameter destructuring, root-scope destructuring (including renames), nested properties, defaults, and multiple declarations with var/let/const. Keep them—just not all in the same file.
Also applies to: 10-16, 19-23, 26-31, 41-45, 48-52, 55-61, 64-69, 72-77
55-61: Ensure both destructurings emit separate diagnostics.Two root-scope destructurings from
propsshould yield two lint messages (no dedup). Worth double‑checking the snapshot to avoid accidental suppression.
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid.js
Outdated
Show resolved
Hide resolved
AsherDe
force-pushed
the
feat/vue-reactivity-rule
branch
from
f925549 to
5aa1e66
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs (2)
261-272: Reactive helper detection could be more robust.The function correctly identifies Vue's reactivity helpers, but consider whether it should also handle namespaced imports (e.g.,
Vue.toRefs) or renamed imports (e.g.,import { toRefs as tr }).For enhanced coverage, consider extending the helper detection:
fn is_reactive_helper_call(call_expr: &JsCallExpression) -> bool { - call_expr - .callee() - .ok() - .and_then(|callee| callee.as_js_identifier_expression().cloned()) - .and_then(|id_expr| id_expr.name().ok()) - .and_then(|name| name.value_token().ok()) - .is_some_and(|token| { - let text = token.text_trimmed(); - matches!(text, "toRefs" | "toRef" | "reactive" | "ref") - }) + let callee = call_expr.callee().ok()?; + + // Handle direct calls (toRefs, toRef, etc.) + if let Some(id_expr) = callee.as_js_identifier_expression() { + return id_expr.name().ok() + .and_then(|name| name.value_token().ok()) + .is_some_and(|token| { + let text = token.text_trimmed(); + matches!(text, "toRefs" | "toRef" | "reactive" | "ref") + }); + } + + // Handle member expressions (Vue.toRefs, etc.) + if let Some(member_expr) = callee.as_js_static_member_expression() { + return member_expr.member().ok() + .and_then(|member| member.as_js_name()) + .and_then(|name| name.value_token().ok()) + .is_some_and(|token| { + let text = token.text_trimmed(); + matches!(text, "toRefs" | "toRef" | "reactive" | "ref") + }); + } + + false }
162-169: Handle quoted and computed 'setup' property namesfind_setup_method currently only matches literal names via name_token.text() == "setup"; this will miss quoted property keys and computed member names — normalise/strip quotes for string-literal keys and also check as_js_computed_member_name().
Location: crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs:162–169
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid_assignment_no_decl.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRef.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (7)
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid_assignment_no_decl.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRef.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_params.js
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/nested_destructuring.js
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/valid_toRef.js
- crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid_assignment_no_decl.jscrates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid_assignment_no_decl.jscrates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid_assignment_no_decl.js
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
🧠 Learnings (1)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : In declare_lint_rule! macros, set `version: "next"` for new or updated rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
🧬 Code graph analysis (2)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid_assignment_no_decl.js (2)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/destructuring_assignment.js (1)
props(5-5)crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/multiple_destructuring.js (2)
props(5-5)props(6-6)
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs (2)
crates/biome_js_syntax/src/function_ext.rs (1)
statements(94-111)crates/biome_js_syntax/src/expr_ext.rs (2)
callee(32-37)value_token(1547-1556)
🔇 Additional comments (5)
crates/biome_js_analyze/tests/specs/nursery/noSetupPropsReactivityLoss/invalid_assignment_no_decl.js (1)
1-9: Well-crafted test case for destructuring assignment pattern.This fixture properly tests the
DestructuredAssignmentpattern where a variable is declared separately (let count;) and then assigned via destructuring (({ count } = props)). The test structure aligns perfectly with the rule's detection logic and complements the existing test coverage.crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs (4)
82-88: Rule declaration follows expected conventions.The rule metadata is properly configured with
version: "next"as required for new rules, and the naming follows Vue.js conventions consistently.
203-218: Props parameter extraction handles complex cases well.The parameter name extraction correctly navigates through the AST layers to identify the props parameter name. The chain of optional unwrapping is appropriate for handling various parameter declaration patterns.
241-250: Parenthesised assignment handling is thorough.Good attention to detail in handling both direct assignment expressions and parenthesised ones (
({ count } = props)). This covers the test case ininvalid_assignment_no_decl.jsproperly.
310-325: Verify call-expression argument matching for propsFile: crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs:310–325 — the matcher only accepts a JsIdentifierExpression named "props" and relies on is_reactive_helper_call to exclude toRef(s)/toRefs (tests already cover those). Ensure it still flags plain calls like someFunction(props) and add/verify tests for these edge cases:
- parenthesised args: someFunction((props))
- member access / property use: someFunction(props.count)
- spread/object wrappers: someFunction({...props}) or someFunction({ ...props })
- aliasing/variable pass-through: const p = props; someFunction(p)
- non-identifier patterns (object/array destructuring as arg)
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs (5)
82-87: Add Vue domain and rule source metadataThis rule should be scoped to the Vue domain and attribute its provenance (ported from eslint-plugin-vue). Please add the appropriate
domainsandsourcemetadata indeclare_lint_rule!.To mirror existing conventions, search for how other framework‑scoped rules declare these fields and follow the same pattern:
#!/bin/bash # Find examples of 'domains' and 'source' usage in rules rg -nP '(?s)declare_lint_rule!\s*\{[^}]*\bdomains\b' crates | sed -n '1,120p' rg -nP '(?s)declare_lint_rule!\s*\{[^}]*\bsource\b' crates | sed -n '1,120p'Apply something along these lines (adjust to match the repo’s canonical types):
pub NoSetupPropsReactivityLoss { version: "next", name: "noSetupPropsReactivityLoss", language: "js", recommended: false, + domains: ["vue"], + source: RuleSource::Eslint { plugin: "eslint-plugin-vue", rule: "no-setup-props-reactivity-loss" }, }
13-26: Tighten docs and remove unsupported claim
- Clarify the first sentence with Vue context.
- The rule doesn’t implement “member access patterns” checks; drop that bullet to avoid over‑promising.
- /// Disallow usages that lose the reactivity of `props` passed to `setup`. + /// Disallow usages that lose the reactivity of `props` passed to `setup` in Vue projects. @@ - /// This rule reports: - /// - Direct destructuring of props in setup function parameters - /// - Destructuring assignment of props in the root scope of setup - /// - Member access patterns that could break reactivity + /// This rule reports: + /// - Direct destructuring of props in setup function parameters. + /// - Destructuring assignment of props in the root scope of `setup`. @@ - /// Note: Destructuring is allowed inside nested functions, callbacks, and + /// Note: destructuring is allowed inside nested functions, callbacks, andAlso applies to: 20-23
1-1: Use Ast query (no semantic model needed here)The rule doesn’t use symbol resolution; prefer
Ast<...>overSemantic<...>.- use crate::services::semantic::Semantic; + use crate::services::node::Ast; @@ - type Query = Semantic<JsExportDefaultExpressionClause>; + type Query = Ast<JsExportDefaultExpressionClause>;Also applies to: 98-103
209-211: Prefer borrowingOption<&T>and avoid string allocations
- Pass
Option<&JsParameters>instead of&Option<JsParameters>.- Pass
Option<&str>for the props name and derive it viaas_deref(); avoidStringallocations.- check_setup_body_statements(body.statements().iter(), signals, ¶ms.ok()); + check_setup_body_statements(body.statements().iter(), signals, params.ok().as_ref()); @@ -fn check_setup_body_statements( - statements: impl Iterator<Item = AnyJsStatement>, - signals: &mut Vec<RuleState>, - params: &Option<biome_js_syntax::JsParameters>, -) { +fn check_setup_body_statements( + statements: impl Iterator<Item = AnyJsStatement>, + signals: &mut Vec<RuleState>, + params: Option<&biome_js_syntax::JsParameters>, +) { @@ - .map(|n| n.text().to_string()); + .map(|n| n.text_trimmed().to_string()); // optionally: refactor to avoid allocation entirely @@ -fn check_variable_declaration_for_props_destructuring( +fn check_variable_declaration_for_props_destructuring( declaration: &JsVariableDeclaration, signals: &mut Vec<RuleState>, - props_param_name: &Option<String>, + props_param_name: Option<&str>, ) { - let Some(props_name) = props_param_name else { + let Some(props_name) = props_param_name else { return; }; @@ -fn check_assignment_expression_for_props_destructuring( +fn check_assignment_expression_for_props_destructuring( assign_expr: &JsAssignmentExpression, signals: &mut Vec<RuleState>, - props_param_name: &Option<String>, + props_param_name: Option<&str>, ) { - let Some(props_name) = props_param_name.as_ref() else { + let Some(props_name) = props_param_name else { return; };And at the call sites:
- &props_param_name, + props_param_name.as_deref(),Same for the assignment helper call.
Also applies to: 198-201, 306-311, 368-371, 227-227
129-152: Sharper diagnostic phrasing (error first, then why)Follow Biome’s “pillars”: crisp error, brief why, actionable note. Current text is okay but can be stronger.
- "Destructuring props in setup function parameters causes reactivity loss." + "Do not destructure `props` in `setup` parameters." ) .note(markup! { - "Use the props object directly to maintain reactivity: `props.propertyName`" + "Why: destructuring breaks Vue’s reactivity link to `props`. Use `props.propertyName` or destructure inside callbacks/render functions." }), @@ - "Destructuring props in the root scope of setup causes reactivity loss." + "Avoid destructuring `props` in the root scope of `setup`." ) .note(markup! { - "Use the props object directly or destructure inside callbacks/render functions." + "Why: destructuring at the root of `setup` breaks reactivity. Access via `props.x` or destructure inside callbacks/render functions." }),
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs (1)
90-97: MakeRuleStateprivateNo external consumers need this; keeping it private avoids surface‑area creep.
-pub enum RuleState { +enum RuleState {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
🧠 Learnings (9)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : In declare_lint_rule! macros, set `version: "next"` for new or updated rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Follow rule naming conventions: use `no<Concept>` to forbid and `use<Concept>` to mandate; prefer consistent prefixes (e.g., `noDuplicate<Concept>`, `useConsistent<Concept>`)
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options types in `biome_rule_options` crate (one file per rule, e.g., `lib/use_my_rule.rs`)
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/**/nursery/**/*.rs : Place all new rules in the nursery group (implement rule files under a `.../src/*/nursery/` directory)
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Recommended rules with domains are enabled only when users enable the matching domains; use `domains` metadata judiciously
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Code blocks in rule docs must specify language; invalid snippets require `expect_diagnostic`; use `options`/`full_options`/`use_options` markers as appropriate
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Documentation for rules: first paragraph must be a single line; include `## Examples` with `### Invalid` first then `### Valid`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Quick test: edit `tests/quick_test.rs`, remove or comment `#[ignore]`, set `SOURCE`, and adjust `RuleFilter`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
📚 Learning: 2025-08-11T11:46:05.836Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Diagnostics should explain why something went wrong, using log advices and links to relevant documentation
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs (2)
crates/biome_js_syntax/src/function_ext.rs (1)
statements(94-111)crates/biome_js_syntax/src/expr_ext.rs (2)
callee(32-37)value_token(1547-1556)
🔇 Additional comments (1)
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs (1)
266-300: Helper detection looks good
is_reactive_helper_callhandles identifiers and static members (Vue.toRefsetc.). Nice and tidy.
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
CodSpeed Performance ReportMerging #7513 will not alter performanceComparing Summary
Footnotes
|
|
By the way, you might have noticed that using a "top to bottom" strategy might be very challenging, because you need to track all places where props are used. The current implementation doesn't cut it. You have two options:
|
|
@ematipico Thanks so much for this detailed and insightful feedback. To be honest, I was actually struggling with the top-down approach, wondering how I could reliably cover all the different ways I will go with Option 1 and refactor the rule using the bottom-up strategy. The Thanks again for the great guidance! I'll push the updated implementation once it's ready. |
AsherDe
force-pushed
the
feat/vue-reactivity-rule
branch
from
4e92f8f to
7fb5a8d
Compare
AsherDe
force-pushed
the
feat/vue-reactivity-rule
branch
from
7fb5a8d to
0fafd9b
Compare
|
Hi @ematipico , Thanks again for the great feedback and guidance. Your suggestion to switch to a bottom-up strategy was spot on, and your I've pushed the new implementation. Here’s a quick overview of the changes:
I believe this new implementation is much more robust and correctly handles the edge cases. It's ready for another look whenever you have a chance. Thanks again for your help! |
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
.../tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js.snap
Outdated
Show resolved
Hide resolved
...ze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js.snap
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (12)
.changeset/grumpy-pigs-chew.md (1)
7-8: Add present‑tense behaviour line and a docs link.Changesets should include the “what Biome does now” sentence and a link to the rule docs.
Apply this diff:
This new rule disallows usages that cause the reactivity of `props` passed to the `setup` function to be lost. + +Biome now reports destructuring in Vue setup that causes props reactivity loss. + +See documentation: https://biomejs.dev/linter/rules/vue/no-setup-props-reactivity-loss/.crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (1)
1-50: Add a few more variable destructuring variants (rest, rename, defaults, nested arrays).These strengthen coverage for root‑scope destructuring patterns.
Append examples like:
+ +// Variable destructuring with rest + renaming +export default { + setup(props) { + const { foo: f, ...rest } = props; + return () => h('div', f + rest.bar) + } +} + +// Variable destructuring with defaults +export default { + setup(props) { + const { foo = 'x', bar = 1 } = props; + return () => h('div', foo + bar) + } +} + +// Nested object+array destructuring from props +export default { + setup(props) { + const { items: [first, second] } = props; + return () => h('div', first + second) + } +}crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts (1)
1-38: Add a generic defineComponent case.Covers TS generic inference at the component factory boundary.
Append:
+ +// defineComponent with generic Props +interface Props2 { foo: string; bar?: number } +export default defineComponent<Props2>({ + setup(props) { + const { foo, bar } = props; + return () => h('div', foo + bar) + } +});crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js (1)
24-30: Also test property initialiser and quoted key forms of setup.Covers
setup: ({ foo }) => {}and["setup"]({ foo }) {}which are common and mentioned in the PR.Append:
+ +// Property initialiser (arrow) with parameter destructuring +export default { + setup: ({ foo, bar }) => { + return () => h('div', foo + bar) + } +} + +// Quoted setup key +export default { + ["setup"]({ foo, bar }) { + return () => h('div', foo + bar) + } +}crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (1)
40-49: Add more control‑flow variants (for‑of, while).Ensures we still report outside nested functions.
Append:
+ +// Destructuring in for-of +export default { + setup(props) { + for (const x of [1]) { + const { foo } = props; + console.log(foo, x); + } + return () => h('div', props.bar) + } +} + +// Destructuring in while loop +export default { + setup(props) { + let i = 0; + while (i++ < 1) { + const { foo } = props; + console.log(foo); + } + return () => h('div', props.bar) + } +}crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.js (2)
19-24: Usecomputedhere for clarity (currently usingref(() => ...)).Purely readability; still parses either way.
Apply this diff:
- const computed = ref(() => { + const value = computed(() => { const { foo, bar } = props; // OK in computed return foo + bar; }); - return () => h('div', computed.value) + return () => h('div', value.value)
37-55: Optional: add anonMounted(or similar lifecycle) callback example.Strengthens “nested function” coverage with a canonical lifecycle.
Append:
+ +// Destructuring in lifecycle callback +export default { + setup(props) { + onMounted(() => { + const { foo } = props; // OK in lifecycle callback + console.log(foo); + }); + return () => h('div', props.foo) + } +}crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (3)
29-35: Minor:reactive(props)is unconventional onpropsWrapping
propsinreactiveis unnecessary in Vue 3 (props are already reactive but readonly). It’s fine here (no destructuring), but the comment “preserve reactivity” may mislead newcomers.Apply if you want to tighten wording:
-// Using reactive wrapper +// Using a reactive wrapper (not generally needed for `props`; shown for rule coverage)
37-42: “ref(props.foo)” doesn’t preserve the reactive link
ref(props.foo)snapshots the value; it won’t update whenprops.foochanges. The rule should ignore it (no destructuring), but the comment “preserve reactivity” is inaccurate. PrefertoRef(props, 'foo').-// Using ref for individual properties +// Using ref for individual properties (does NOT keep linkage; shown for rule coverage) - const foo = ref(props.foo); + const foo = toRef(props, 'foo');
1-43: Add a namespacedVue.toRefspositive caseTo validate namespaced helpers are treated as safe, consider adding a case with
Vue.toRefs(props).+// Namespaced toRefs +import * as Vue from 'vue'; +export default { + setup(props) { + const { foo } = Vue.toRefs(props); + return () => h('div', foo.value) + } +}crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
206-207: Tiny copy tweak: mentiontoRefalongsidetoRefsThe note suggests
toRefsonly. AddtoReffor single‑prop cases.- "Use 'props.propertyName' or 'toRefs(props)' to maintain reactivity." + "Use 'props.propertyName', 'toRef(props, \"name\")', or 'toRefs(props)' to maintain reactivity."
302-306: Optional: support computed member name ["setup"]Some codebases use computed literal keys:
"setup"via["setup"]. Low frequency, but easy to support.fn is_member_named_setup(name: &biome_js_syntax::AnyJsObjectMemberName) -> bool { - name.as_js_literal_member_name() - .and_then(|literal| literal.name().ok()) - .is_some_and(|token| token.text() == "setup") + if let Some(literal) = name.as_js_literal_member_name() { + return literal.name().ok().is_some_and(|token| token.text() == "setup"); + } + if let Some(comp) = name.as_js_computed_member_name() { + if let Ok(expr) = comp.expression() { + if let Some(str_lit) = expr.as_js_string_literal_expression() { + return str_lit.value_token().ok().is_some_and(|t| t.text_trimmed() == "setup"); + } + } + } + false }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.ts.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (12)
.changeset/grumpy-pigs-chew.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.ts(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_rule_options/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.tscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.jscrates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.tscrates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.jscrates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.tscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changeset files, only use #### or ##### headers; avoid other header levels
Changeset descriptions should use past tense for what you did (e.g., "Added...")
Describe current Biome behavior in present tense within changesets (e.g., "Biome now supports...")
For bug fixes in changesets, start with a link to the issue (e.g., "Fixed #1234: ...")
When referencing rules or assists in changesets, include links to their documentation pages
Include a minimal code block in the changeset when applicable to demonstrate the change
End every sentence in the changeset description with a period
Files:
.changeset/grumpy-pigs-chew.md
🧠 Learnings (2)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Options types must implement serialization/deserialization and schema support using derives: `Serialize`, `Deserialize`, `Deserializable`, and `#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]`
Applied to files:
crates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : In declare_lint_rule! macros, set `version: "next"` for new or updated rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧬 Code graph analysis (7)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.js (4)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (4)
props(7-7)props(18-18)props(31-31)props(44-44)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (4)
props(6-6)props(14-14)props(39-39)foo(30-30)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (2)
foo(21-21)foo(35-35)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (2)
foo(24-24)foo(40-40)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts (5)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (4)
props(7-7)props(18-18)props(31-31)props(44-44)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (4)
props(6-6)props(14-14)props(39-39)foo(30-30)crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
h(131-133)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (2)
foo(21-21)foo(35-35)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (2)
foo(24-24)foo(40-40)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (3)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (4)
props(6-6)props(14-14)props(39-39)foo(30-30)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (2)
foo(21-21)foo(35-35)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (2)
foo(24-24)foo(40-40)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (3)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (4)
props(7-7)props(18-18)props(31-31)props(44-44)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (4)
props(6-6)props(14-14)props(39-39)foo(30-30)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (2)
foo(21-21)foo(35-35)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
crates/biome_analyze/src/rule.rs (1)
inspired(249-254)crates/biome_js_syntax/src/expr_ext.rs (1)
callee(32-37)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js (1)
crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
h(131-133)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (3)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (4)
props(7-7)props(18-18)props(31-31)props(44-44)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (2)
foo(21-21)foo(35-35)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (2)
foo(24-24)foo(40-40)
🔇 Additional comments (6)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (1)
1-37: LGTM: solid coverage of non‑Vue contexts.Scenarios look right for ensuring we don’t over‑match. Nothing to fix here.
crates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs (2)
1-6: Derives and serde config look correct.Matches our options‑type requirements (Serialize/Deserialize/Deserializable + optional JsonSchema). Nice one.
6-6: Add short doc comment to NoVueSetupPropsReactivityLossOptionsTiny docstring helps discoverability in generated docs.
Apply this diff:
-pub struct NoVueSetupPropsReactivityLossOptions {} +/// Options for `vue/noSetupPropsReactivityLoss`. This rule currently has no options. +pub struct NoVueSetupPropsReactivityLossOptions {}Verify the public module export exists in crates/biome_rule_options/src/lib.rs:
rg -nP '^\s*pub\s+mod\s+no_vue_setup_props_reactivity_loss' crates/biome_rule_options/src/lib.rsNo matches were found here — if the rule slug was renamed to
noSetupPropsReactivityLoss, align the type/module/file names and add thepub modexport..changeset/grumpy-pigs-chew.md (1)
5-5: Keep existing slug; fix spacing in the changeset.The repo defines the rule as
noVueSetupPropsReactivityLossin source and tests — do not rename the slug; just add the missing space in the changeset.Locations:
- crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (name: "noVueSetupPropsReactivityLoss")
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js.snap
Apply this diff:
-Added the nursery rule`vue/noVueSetupPropsReactivityLoss`. +Added the nursery rule `vue/noVueSetupPropsReactivityLoss`.Likely an incorrect or invalid review comment.
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (1)
13-19: Looks good:toRefspattern correctly treated as safeThis aligns with the rule’s intent and mirrors Vue guidance. No changes needed.
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (1)
118-126: Rule metadata LGTMVersion set to “next” and source attribution via
inspired()look correct.
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_unicode_table/src/tables.rsis excluded by!**/biome_unicode_table/src/tables.rsand included by**
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧠 Learnings (2)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : In declare_lint_rule! macros, set `version: "next"` for new or updated rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/{lint,assist}/**/*.rs : When banning globals (e.g., `noConsoleLog`), check the semantic model to avoid false positives from locally shadowed bindings
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
crates/biome_analyze/src/rule.rs (4)
domains(584-587)recommended(554-557)sources(569-572)inspired(249-254)crates/biome_js_syntax/src/expr_ext.rs (1)
callee(32-37)
🔇 Additional comments (6)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (6)
1-13: Clean imports and good framework integration.Properly using the existing Vue framework infrastructure. The semantic model integration looks correct.
15-126: Comprehensive rule documentation with excellent examples.The rustdoc is thorough, covering all major scenarios with clear invalid/valid examples. Good adherence to the learned pattern of setting
version: "next".
214-260: Solid component detection logic.Good coverage across different Vue component patterns (Options API, defineComponent, createApp). The filtering by "setup" name is correct.
590-607: Proper parameter destructuring detection.Correctly identifies both object and array binding patterns in setup parameters, with appropriate fallback to identifier binding analysis.
352-364: Fix broken root-scope read detection.The cast is applied to the parent node rather than the reference itself, causing the filter to always fail. Only
toRefs(props)makes object destructuring safe, so missing these detections is critical.Apply this fix:
- for reference in model.as_binding(props_binding).all_reads() { - if let Some(identifier) = - biome_js_syntax::JsReferenceIdentifier::cast(reference.syntax().clone()) + for reference in model.as_binding(props_binding).all_reads() { + if let Some(identifier) = + biome_js_syntax::JsReferenceIdentifier::cast(reference.syntax().clone())
531-588: Over-permissive reactive API validation needs tightening.The current implementation treats
reactive,ref, andtoRefas safe, but onlytoRefs(props)makes object destructuring safe. Also missing support for namespaced calls likeVue.toRefs().Restrict to only accept
toRefsfrom Vue packages and handle static member expressions:-fn is_reactive_api_call(expr: &AnyJsExpression, model: &SemanticModel) -> bool { +fn is_reactive_api_call(expr: &AnyJsExpression, model: &SemanticModel) -> bool { let Some(call_expr) = expr.as_js_call_expression() else { return false; }; let Ok(callee) = call_expr.callee() else { return false; }; - let Some(ident_expr) = callee.as_js_identifier_expression() else { - return false; - }; - - let Ok(name) = ident_expr.name() else { - return false; - }; - - let Ok(token) = name.value_token() else { - return false; - }; - - let function_name = token.text_trimmed(); - - let is_vue_reactive_function = matches!(function_name, "toRefs" | "toRef" | "reactive" | "ref"); - - if !is_vue_reactive_function { - return false; - } + // Support both bare calls (toRefs) and namespaced calls (Vue.toRefs) + let (function_name, identifier_to_check) = match &callee { + AnyJsExpression::JsIdentifierExpression(ident_expr) => { + let name = ident_expr.name().ok()?.value_token().ok()?; + (name.text_trimmed(), ident_expr) + } + AnyJsExpression::JsStaticMemberExpression(member_expr) => { + let prop = member_expr.member().ok()?.value_token().ok()?; + let obj = member_expr.object().ok()?.as_js_identifier_expression()?; + (prop.text_trimmed(), obj) + } + _ => return false, + }; + + // Only toRefs preserves reactivity for object destructuring + if function_name != "toRefs" { + return false; + } - let Some(name_node) = ident_expr.name().ok() else { - return true; - }; + let Some(name_node) = identifier_to_check.name().ok() else { + return false; + }; let Some(binding) = model.binding(&name_node) else { - return true; + return false; }; if !binding.is_imported() { - return true; + return false; }; let binding_node = binding.syntax(); for ancestor in binding_node.ancestors() { let Some(import_decl) = biome_js_syntax::JsImport::cast(ancestor) else { continue; }; let Ok(source) = import_decl.source_text() else { continue; }; let source_value = source.text(); return source_value == "vue" || source_value == "@vue/reactivity" || source_value == "@vue/composition-api"; } - true + false }
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
AsherDe
force-pushed
the
feat/vue-reactivity-rule
branch
from
87c701b to
d813736
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js (1)
1-9: Add a couple of variants to harden import‑origin checks (optional)Consider adding siblings to catch edge cases:
- Namespaced wrong origin
- Aliased wrong origin
- CommonJS require
Examples:
// invalid-wrong-namespace-import.js import * as NotVue from 'some-other-library'; export default { setup(props) { const { bar } = NotVue.toRefs(props); return () => h('div', bar.value) } };// invalid-wrong-alias-import.js import { toRefs as tr } from 'some-other-library'; export default { setup(props) { const { bar } = tr(props); return () => h('div', bar.value) } };// invalid-wrong-package-require.js const { toRefs } = require('some-other-library'); module.exports = { default: { setup(props) { const { bar } = toRefs(props); return () => h('div', bar.value) } } };crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js (1)
17-29: Optional: add a “nested + alias + default” parameter patternA tiny extra guardrail: e.g.
setup({ user: { name: n = 'x' }, foo: f = 1 }) {}to ensure nested patterns are flagged as well.crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (1)
26-38: Optional: add anelsebranch or ternary exampleE.g. destructure in an
else {}orconst x = cond ? ({ foo } = props) : 0;to harden edge cases.crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
262-272: Parenthesised functions aren’t unwrapped
export default (props) => {}orsetup: ((props) => {})won’t be detected becauseJsParenthesizedExpressionisn’t handled.Apply this diff to unwrap parentheses recursively:
fn extract_function_from_expression(expr: &AnyJsExpression) -> Option<AnyJsFunction> { match expr { AnyJsExpression::JsFunctionExpression(func_expr) => { Some(AnyJsFunction::JsFunctionExpression(func_expr.clone())) } AnyJsExpression::JsArrowFunctionExpression(arrow_func) => { Some(AnyJsFunction::JsArrowFunctionExpression(arrow_func.clone())) } + AnyJsExpression::JsParenthesizedExpression(paren) => { + let inner = paren.expression().ok()?; + extract_function_from_expression(&inner) + } _ => None, } }
204-210: Nudge: mentiontoReffor single prop in noteThe note suggests
toRefs(props)only. Add “or toRef(props, 'name') for a single prop” to align with the docs/examples below.- "Use 'props.propertyName' or 'toRefs(props)' to maintain reactivity." + "Use 'props.propertyName' or 'toRefs(props)' (or 'toRef(props, \"name\")' for a single prop) to maintain reactivity."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (18)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.ts.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (16)
.changeset/grumpy-pigs-chew.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.ts(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js
- .changeset/grumpy-pigs-chew.md
- crates/biome_rule_options/src/lib.rs
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.js
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.ts
- crates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.js
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.jscrates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.jscrates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧠 Learnings (2)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : In declare_lint_rule! macros, set `version: "next"` for new or updated rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/{lint,assist}/**/*.rs : When banning globals (e.g., `noConsoleLog`), check the semantic model to avoid false positives from locally shadowed bindings
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧬 Code graph analysis (3)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js (4)
crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
h(131-133)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (2)
foo(25-25)foo(41-41)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (1)
foo(30-30)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (2)
foo(21-21)foo(35-35)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js (3)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js (1)
toRefs(8-8)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js (1)
toRefs(4-4)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (1)
toRefs(17-17)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (3)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (4)
props(6-6)props(14-14)props(39-39)foo(30-30)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (2)
foo(25-25)foo(41-41)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (2)
foo(21-21)foo(35-35)
🔇 Additional comments (7)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js (3)
2-2: Good negative: wrong‑package import should be flaggedThis neatly asserts that only Vue’s own toRefs (or explicitly allowed sources) are exempt. Solid fixture.
6-6: Covers root‑scope destructuring pathDestructuring happens at setup’s root (not a nested fn), so it exercises the rule’s primary failure mode. All good.
6-6: Import-origin resolution for toRefs/namespace: confirmedis_reactive_api_call and is_vue_namespace_or_import in crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (≈lines 531–625) inspect binding.is_imported(), find the import declaration and compare the import source (e.g. "@vue/reactivity"), so the rule does not blindly whitelist unknown packages.
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js (1)
3-43: Solid coverage of parameter destructuring casesCovers defaults, renames, rest, defineComponent and named export variants. Looks good.
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (1)
3-49: Good spread across control‑flow constructsif/try/switch/for are covered nicely and align with the rule’s “root‑scope” focus.
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
349-367: Root‑scope read/destructure logic looks sound — verifiedRan the suggested rg; tests include invalid conditional destructurings and valid nested contexts, confirming root‑scope matching behaves as expected.
214-260: Minor: unify Setup detection to avoid second passHandle AnyVueComponent::Setup inside find_setup_functions so setup() is discovered in one pass and avoid duplicating diagnostics from check_additional_setup_patterns.
Couldn't verify here — ripgrep returned "No files were searched". Run locally to confirm there are no duplicated diagnostics after the change:
rg -n --hidden --glob '!**/target/**' 'NoVueSetupPropsReactivityLoss'
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (3)
467-472: Range-based containment is a bit coarseUsing ranges to test “props within initializer” can misfire on edge cases with overlapping trivia/comments. Ancestor checks are more precise.
Consider replacing with a structural check (walk the initializer AST to find the reference node) instead of range containment.
711-728: Export‑default function fallback broadens scope unnecessarilyChecking export default (() => …) as a “setup” increases false positives. With the earlier gating change, this can be removed.
540-588: Approve — toRefs check matches eslint-plugin-vue; consider adding toRef and import normalisationUpstream treats toRef/toRefs as the safe helpers; your restriction to Vue-imported toRefs is aligned.
- Recognise toRef (singular) in is_reactive_api_call so single-prop conversions are also treated as safe.
- Optional: normalise import-source matching to include monorepo/alias paths (e.g. 'vue/dist/*') if your workspace uses them; otherwise keep strict matching to avoid false positives.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧠 Learnings (2)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : In declare_lint_rule! macros, set `version: "next"` for new or updated rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/{lint,assist}/**/*.rs : When banning globals (e.g., `noConsoleLog`), check the semantic model to avoid false positives from locally shadowed bindings
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
crates/biome_analyze/src/rule.rs (1)
inspired(249-254)crates/biome_js_syntax/src/expr_ext.rs (1)
callee(32-37)
🔇 Additional comments (6)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (6)
118-125: Rule metadata looks spot‑onversion: "next", Vue domain, and source mapping are correctly declared.
296-310: Nice: handles identifier and quoted “setup”Good coverage for setup(props) {} and "setup"() {}.
354-376: Root-scope read detection looks correctCasts the reference node (not parent), checks nearest function ancestor equals setup. This resolves the earlier bug.
175-211: Diagnostic guidance is clear and actionableMessages and notes guide users towards props.property and toRefs(props).
23-27: Docs: don’t suggest toRef for destructuringOnly toRefs(props) makes object destructuring safe. Mentioning toRef here is misleading.
Apply this diff:
- /// - Destructuring assignment of props in the root scope of setup (unless using `toRefs` or `toRef`) + /// - Destructuring assignment of props in the root scope of setup (unless using `toRefs`)
149-173: Limit analysis to recognised Vue components to avoid false positivesWhen VueComponent::from_potential_component returns None, we still lint via the “plain JS” paths, which can flag non‑Vue files. Gate the rule to recognised components only.
Apply this diff:
- match component { - Some(component) => { - for setup_function in find_setup_functions(&component) { - violations.extend(check_setup_function(&setup_function, ctx.model())); - } - } - None => { - violations.extend(check_plain_js_setup_functions(ctx.query(), ctx.model())); - } - } - - violations.extend(check_additional_setup_patterns(ctx.query(), ctx.model())); + if let Some(component) = component { + for setup_function in find_setup_functions(&component) { + violations.extend(check_setup_function(&setup_function, ctx.model())); + } + }
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (5)
24-27: Docs: tighten wording around safe exceptionObject destructuring is only safe when destructuring the result of
toRefs(props).toRefdoesn’t involve object destructuring.- /// - Destructuring assignment of props in the root scope of setup (unless using `toRefs` or `toRef`) + /// - Destructuring assignment of props in the root scope of setup (unless destructuring from `toRefs(props)`)
208-209: Docs: mention toRef for single-property accessSmall clarity boost for users preferring
toReffor one prop.- "Use 'props.propertyName' or 'toRefs(props)' to maintain reactivity." + "Use 'props.propertyName', 'toRefs(props)', or 'toRef(props, \"key\")' to maintain reactivity."
214-260: Avoid duplicate setup discovery across APIsCollecting from both
*_api.setup_func()anditer_declaration_groups()risks double-reporting the same function.match component.kind() { AnyVueComponent::OptionsApi(options_api) => { if let Some(setup_func_expr) = options_api.setup_func() && let Some(func) = extract_function_from_expression(&setup_func_expr) { setup_functions.push(SetupFunction::Function(func)); } @@ } - - setup_functions + { + use std::collections::HashSet; + let mut seen: HashSet<TextRange> = HashSet::new(); + let mut deduped = Vec::new(); + for f in setup_functions { + let range = match &f { + SetupFunction::Function(func) => func.range(), + SetupFunction::Method(method) => method.range(), + }; + if seen.insert(range) { + deduped.push(f); + } + } + deduped + }Run your snapshots to confirm diagnostics don’t double-fire.
147-148: Align rule Options with the published options type (if present)The PR summary mentions
NoVueSetupPropsReactivityLossOptions. If exported, wire it here instead of().- type Options = (); + type Options = biome_rule_options::NoVueSetupPropsReactivityLossOptions;If the options type isn’t actually used, consider removing it from
biome_rule_optionsto avoid dead API.
531-579: SupportVue["toRefs"](...)and similar computed-member formsCurrently only identifier and static member calls are recognised. Computed string member access is common in some codebases/tools.
match &callee { AnyJsExpression::JsIdentifierExpression(ident_expr) => { … } AnyJsExpression::JsStaticMemberExpression(member_expr) => { … } + AnyJsExpression::JsComputedMemberExpression(comp_member) => { + let Ok(prop_expr) = comp_member.member() else { return false; }; + let is_to_refs = prop_expr + .as_js_string_literal_expression() + .and_then(|s| s.value_token().ok()) + .is_some_and(|tok| tok.text_trimmed() == "toRefs"); + if !is_to_refs { + return false; + } + let Ok(object) = comp_member.object() else { return false; }; + is_vue_namespace_or_import(&object, model) + } _ => false, }Optional: normalise
exprto skip parentheses and TS casts before matching (e.g.,toRefs(props) as const). If there’s a helper likeomit_parentheses()/cast skipping in your expr extensions, use it when extractingcallee.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧠 Learnings (1)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/{lint,assist}/**/*.rs : When banning globals (e.g., `noConsoleLog`), check the semantic model to avoid false positives from locally shadowed bindings
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
crates/biome_analyze/src/rule.rs (4)
domains(584-587)recommended(554-557)sources(569-572)inspired(249-254)crates/biome_js_syntax/src/expr_ext.rs (1)
callee(32-37)
🔇 Additional comments (5)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (5)
671-688: Heuristic guard for plain call expressionsIf
VueComponentQueryalready gates these, ignore this. Otherwise, guard on callee resolving todefineComponent/createAppimported from Vue to avoid non‑Vue false positives.fn extract_setup_from_call_expression( call_expr: &biome_js_syntax::JsCallExpression, model: &SemanticModel, ) -> Vec<Violation> { + // Optional guard: + // if !callee_is_vue_factory(call_expr, model) { return Vec::new(); }Happy to sketch
callee_is_vue_factoryif you want it.
352-364: Good fix on reference casting; root-scope reads are now detectedCasting the reference node itself (not its parent) resolves the earlier false negative.
369-381: Root-scope detection logic looks soundNearest function ancestor equals the setup function node; nested callbacks are excluded as intended.
521-529: Safety check limited totoRefsis correctOnly
toRefs(props)preserves reactivity for object destructuring; others (toRef,reactive,ref) should not whitelist destructuring.Add/keep tests for:
const { x } = reactive(props)const { x } = toRef(props, "x")(should be flagged)const { x } = Vue["toRefs"](props)(should be allowed once implemented)
296-301: Fix: recognise identifier-named setup methods (not only quoted)Methods written as
setup(props) {}are currently missed because only literal names are handled.Apply:
-fn is_member_named_setup(name: &biome_js_syntax::AnyJsObjectMemberName) -> bool { - if let Some(lit) = name.as_js_literal_member_name() { - return lit.value().is_ok_and(|tok| tok.text_trimmed() == "setup"); - } - false -} +fn is_member_named_setup(name: &biome_js_syntax::AnyJsObjectMemberName) -> bool { + if let Some(id) = name.as_js_identifier_member_name() { + return id.value_token().is_ok_and(|tok| tok.text_trimmed() == "setup"); + } + if let Some(lit) = name.as_js_literal_member_name() { + return lit.name().is_ok_and(|tok| tok.text_trimmed() == "setup"); + } + false +}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
223-251: Over-filtering by raw text "setup" drops quoted members; let the helper decideThe pre-filter
filter(|(name, _)| name.text() == "setup")misses"setup"(quoted) and duplicates logic already handled byis_member_named_setup. This causes false negatives.Apply this diff to rely solely on
extract_setup_from_object_member:- setup_functions.extend( - options_api - .iter_declaration_groups() - .filter(|(name, _)| name.text() == "setup") - .filter_map(|(_, member)| extract_setup_from_object_member(&member)), - ); + setup_functions.extend( + options_api + .iter_declaration_groups() + .filter_map(|(_, member)| extract_setup_from_object_member(&member)), + ); @@ - setup_functions.extend( - define_component - .iter_declaration_groups() - .filter(|(name, _)| name.text() == "setup") - .filter_map(|(_, member)| extract_setup_from_object_member(&member)), - ); + setup_functions.extend( + define_component + .iter_declaration_groups() + .filter_map(|(_, member)| extract_setup_from_object_member(&member)), + ); @@ - setup_functions.extend( - create_app - .iter_declaration_groups() - .filter(|(name, _)| name.text() == "setup") - .filter_map(|(_, member)| extract_setup_from_object_member(&member)), - ); + setup_functions.extend( + create_app + .iter_declaration_groups() + .filter_map(|(_, member)| extract_setup_from_object_member(&member)), + );
702-746: Optional: further guard plain-call pathIf you ever see false positives here, additionally gate on known Vue factories (defineComponent/createApp) via semantic resolution. Current query likely suffices, so this is optional.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js
🧠 Learnings (1)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/{lint,assist}/**/*.rs : When banning globals (e.g., `noConsoleLog`), check the semantic model to avoid false positives from locally shadowed bindings
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧬 Code graph analysis (2)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (1)
Vue(51-51)crates/biome_js_syntax/src/expr_ext.rs (1)
callee(32-37)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (5)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (4)
props(6-6)props(14-14)props(39-39)foo(30-30)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js (1)
toRefs(4-4)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js (1)
toRefs(6-6)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js (1)
toRefs(8-8)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (2)
foo(21-21)foo(35-35)
🔇 Additional comments (3)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (1)
1-55: LGTM: solid coverage of safe patterns (incl. namespaced/computed).No diagnostics expected; aligns with rule intent.
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
306-324: setup() detection misses identifier member names
is_member_named_setuponly handles literal names; it skips the commonsetup(props) {}identifier form, causing false negatives (esp. in createApp/defineComponent member scanning).Apply this diff to support both identifier and literal member names:
-fn is_member_named_setup(name: &biome_js_syntax::AnyJsObjectMemberName) -> bool { - match name { - biome_js_syntax::AnyJsObjectMemberName::JsLiteralMemberName(lit) => { - lit.value().is_ok_and(|tok| { - let text = tok.text_trimmed(); - if (text.starts_with('"') && text.ends_with('"')) - || (text.starts_with('\'') && text.ends_with('\'')) - { - let inner = &text[1..text.len() - 1]; - inner == "setup" - } else { - text == "setup" - } - }) - } - biome_js_syntax::AnyJsObjectMemberName::JsComputedMemberName(_) => false, - biome_js_syntax::AnyJsObjectMemberName::JsMetavariable(_) => false, - } -} +fn is_member_named_setup(name: &biome_js_syntax::AnyJsObjectMemberName) -> bool { + if let Some(id) = name.as_js_identifier_member_name() { + return id + .value_token() + .is_ok_and(|tok| tok.text_trimmed() == "setup"); + } + if let Some(lit) = name.as_js_literal_member_name() { + return lit.value().is_ok_and(|tok| { + let text = tok.text_trimmed(); + if (text.starts_with('"') && text.ends_with('"')) + || (text.starts_with('\'') && text.ends_with('\'')) + { + &text[1..text.len() - 1] == "setup" + } else { + text == "setup" + } + }); + } + false +}
554-637: Only treat toRefs(...) as safe for destructuring (remove toRef)For object/array destructuring, only
toRefs(props)preserves reactivity. AllowingtoRefhere creates false negatives (e.g.,const { value } = toRef(props, 'k')).Apply this diff to restrict the safe callee to
toRefs:- if function_name != "toRefs" && function_name != "toRef" { + if function_name != "toRefs" { return false; } @@ - if property_name != "toRefs" && property_name != "toRef" { + if property_name != "toRefs" { return false; } @@ - property_name == "toRefs" || property_name == "toRef" + property_name == "toRefs"
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
746-763: Guard plain-call path to Vue factories to avoid non‑Vue false positivesThis path inspects any call whose first arg is an object, which can catch unrelated code. Guard on
defineComponent/createApp(including namespaced forms) imported fromvue.fn extract_setup_from_call_expression( call_expr: &biome_js_syntax::JsCallExpression, model: &SemanticModel, ) -> Vec<Violation> { + // Ensure this is a Vue factory before inspecting its first argument + if let Ok(callee) = call_expr.callee() { + if !is_vue_factory_callee(&callee, model) { + return Vec::new(); + } + } else { + return Vec::new(); + }Add helper:
fn is_vue_factory_callee(callee: &AnyJsExpression, model: &SemanticModel) -> bool { use AnyJsExpression::*; match callee { JsIdentifierExpression(id) => { let Ok(name) = id.name() else { return false; }; let Ok(tok) = name.value_token() else { return false; }; let n = tok.text_trimmed(); if n != "defineComponent" && n != "createApp" { return false; } model .binding(&name) .is_some_and(|b| is_vue_reactive_function_binding(&b)) } JsStaticMemberExpression(m) => { let Ok(member) = m.member() else { return false; }; let Ok(prop_tok) = member.value_token() else { return false; }; let n = prop_tok.text_trimmed(); if n != "defineComponent" && n != "createApp" { return false; } let Ok(obj) = m.object() else { return false; }; is_vue_namespace_or_import(&obj, model) } JsComputedMemberExpression(m) => { let Ok(mem_expr) = m.member() else { return false; }; let name = if let Some(lit) = mem_expr.as_js_string_literal_expression() { lit.value_token().ok().map(|t| t.text_trimmed()) } else { None }; if name != Some("defineComponent") && name != Some("createApp") { return false; } let Ok(obj) = m.object() else { return false; }; is_vue_namespace_or_import(&obj, model) } _ => false, } }
142-147: Consider wiring the dedicated options typeIf
NoVueSetupPropsReactivityLossOptionswas added in biome_rule_options, wire it here for parity with other rules. Keeps surface consistent even if empty for now.- type Options = (); + type Options = biome_rule_options::NoVueSetupPropsReactivityLossOptions;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧠 Learnings (1)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/{lint,assist}/**/*.rs : When banning globals (e.g., `noConsoleLog`), check the semantic model to avoid false positives from locally shadowed bindings
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (3)
crates/biome_analyze/src/rule.rs (4)
domains(584-587)recommended(554-557)sources(569-572)inspired(249-254)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (1)
Vue(51-51)crates/biome_js_syntax/src/expr_ext.rs (1)
callee(32-37)
🔇 Additional comments (3)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (3)
409-421: Root-scope check looks solidThe nearest-function ancestor equality check cleanly excludes nested callbacks/render functions.
Please add/keep tests that cover:
- destructuring inside watch/returned render (not reported),
- destructuring in nested arrow/function inside setup (not reported).
305-341: Detectsetupwith identifier member names too
is_member_named_setupmisses the very commonsetup(props) {}andsetup: (props) => {}cases because it only checks literal/computed names. Add identifier handling.fn is_member_named_setup(name: &biome_js_syntax::AnyJsObjectMemberName) -> bool { - match name { + match name { + biome_js_syntax::AnyJsObjectMemberName::JsIdentifierMemberName(id) => { + id.value_token().is_ok_and(|tok| tok.text_trimmed() == "setup") + } biome_js_syntax::AnyJsObjectMemberName::JsLiteralMemberName(lit) => { lit.value().is_ok_and(|tok| { let text = tok.text_trimmed(); if (text.starts_with('"') && text.ends_with('"')) || (text.starts_with('\'') && text.ends_with('\'')) { let inner = &text[1..text.len() - 1]; inner == "setup" } else { text == "setup" } }) } biome_js_syntax::AnyJsObjectMemberName::JsComputedMemberName(computed) => { computed.expression().is_ok_and(|expr| { let AnyJsExpression::AnyJsLiteralExpression(literal_expr) = expr else { return false; }; let Some(string_literal) = literal_expr.as_js_string_literal_expression() else { return false; }; string_literal.value_token().is_ok_and(|tok| { let text = tok.text_trimmed(); if text.len() >= 2 { let inner = &text[1..text.len() - 1]; inner == "setup" } else { false } }) }) } biome_js_syntax::AnyJsObjectMemberName::JsMetavariable(_) => false, } }
571-654: Don’t treattoRefas a safe destructuring sourceOnly
toRefs(props)preserves reactivity for object destructuring. AcceptingtoRefcauses missed violations.match &callee { AnyJsExpression::JsIdentifierExpression(ident_expr) => { let Ok(name) = ident_expr.name() else { return false; }; let Ok(token) = name.value_token() else { return false; }; let function_name = token.text_trimmed(); - if function_name != "toRefs" && function_name != "toRef" { + if function_name != "toRefs" { return false; } if let Some(binding) = model.binding(&name) { is_vue_reactive_function_binding(&binding) } else { false } } AnyJsExpression::JsStaticMemberExpression(member_expr) => { let Ok(member) = member_expr.member() else { return false; }; let Ok(token) = member.value_token() else { return false; }; let property_name = token.text_trimmed(); - if property_name != "toRefs" && property_name != "toRef" { + if property_name != "toRefs" { return false; } let Ok(object) = member_expr.object() else { return false; }; is_vue_namespace_or_import(&object, model) } AnyJsExpression::JsComputedMemberExpression(computed_member_expr) => { let Ok(member) = computed_member_expr.member() else { return false; }; let is_reactive_function = match &member { AnyJsExpression::AnyJsLiteralExpression(literal_expr) => { let Some(string_literal) = literal_expr.as_js_string_literal_expression() else { return false; }; let Ok(token) = string_literal.value_token() else { return false; }; let text = token.text_trimmed(); if text.len() >= 2 { let property_name = &text[1..text.len() - 1]; - property_name == "toRefs" || property_name == "toRef" + property_name == "toRefs" } else { false } } _ => false, };
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I couldn't review all of it, but there's a lot of that that can be improved. Most important things to address:
- import types from the top of the file and avoid prefixing crates inconsistently
- documentation of the functions. It's hard to review them without understanding the business logic
- avoid let-else when it's not needed, and use if-let chains when possible
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
| fn is_function_like_node(node: &biome_rowan::SyntaxNode<biome_js_syntax::JsLanguage>) -> bool { | ||
| use biome_js_syntax::JsSyntaxKind; | ||
| matches!( | ||
| node.kind(), | ||
| JsSyntaxKind::JS_FUNCTION_DECLARATION | ||
| | JsSyntaxKind::JS_FUNCTION_EXPRESSION | ||
| | JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION | ||
| | JsSyntaxKind::JS_METHOD_CLASS_MEMBER | ||
| | JsSyntaxKind::JS_METHOD_OBJECT_MEMBER | ||
| | JsSyntaxKind::JS_GETTER_CLASS_MEMBER | ||
| | JsSyntaxKind::JS_GETTER_OBJECT_MEMBER | ||
| | JsSyntaxKind::JS_SETTER_CLASS_MEMBER | ||
| | JsSyntaxKind::JS_SETTER_OBJECT_MEMBER | ||
| ) | ||
| } |
There was a problem hiding this comment.
I believe we already have a node for this, but I might be wrong. Check for AnyFunctionLike or something similar
There was a problem hiding this comment.
I checked AnyFunctionLike in function_ext. This function does not contain getters and setters, but returning an object with getters from setup in Vue's setup function is valid and sometimes useful.
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
|
Thanks a lot for the detailed feedback. Just for context, this is my first time contributing to a large-scale developer tool like Biome, so it's been more complex than I anticipated. I'm still learning my way around the codebase. I will work on all the points you've raised—improving the imports, adding documentation, and using more idiomatic control flow. I'll study the existing codebase more closely to better understand the project's patterns. It might take me a bit longer to get this right. I really appreciate your guidance. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (1)
490-493: Critical:toRefis not safe for object destructuring – onlytoRefspreserves reactivity.
toRef(props, 'key')returns a singleRef<T>, not an object of refs. Object destructuring from it (e.g.,const { x } = toRef(props, 'k')) is nonsensical and shouldn't be treated as safe. Past reviews explicitly flagged this: "OnlytoRefs(props)makes object destructuring safe."Apply this diff:
fn is_reactive_function_name(name: &str) -> bool { - name == "toRefs" || name == "toRef" + name == "toRefs" }
🧹 Nitpick comments (2)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
233-233: ImportAnyJsParameterat the top rather than using the fully qualified path.Per past review feedback, import types at the beginning of the file instead of using
biome_js_syntax::prefixes.Apply this diff:
use biome_js_syntax::{ AnyJsArrowFunctionParameters, AnyJsBindingPattern, AnyJsExpression, AnyJsFunction, AnyJsObjectMember, AnyJsObjectMemberName, JsAssignmentExpression, JsCallExpression, JsExportDefaultExpressionClause, JsIdentifierBinding, JsImport, JsLanguage, JsMethodObjectMember, JsObjectMemberList, JsReferenceIdentifier, JsSyntaxKind, - JsVariableDeclarator, + JsVariableDeclarator, AnyJsParameter, AnyJsAssignmentPattern, };Then update line 233:
fn extract_binding_from_formal_parameter( - param: &biome_js_syntax::AnyJsParameter, + param: &AnyJsParameter, ) -> Option<AnyJsBindingPattern> {And line 461:
match left { - biome_js_syntax::AnyJsAssignmentPattern::JsObjectAssignmentPattern(obj_pattern) => { + AnyJsAssignmentPattern::JsObjectAssignmentPattern(obj_pattern) => { Some(DestructuringInfo { destructuring_range: obj_pattern.range(), initializer, }) } - biome_js_syntax::AnyJsAssignmentPattern::JsArrayAssignmentPattern( + AnyJsAssignmentPattern::JsArrayAssignmentPattern( array_pattern, ) => Some(DestructuringInfo {
543-543: ImportBindingat the top rather than using the fully qualified path.Per coding guidelines and past review feedback.
Apply this diff:
+use biome_js_semantic::{Binding, SemanticModel}; -use biome_js_semantic::SemanticModel;Then update line 543:
-fn is_vue_reactive_function_binding(binding: &biome_js_semantic::Binding) -> bool { +fn is_vue_reactive_function_binding(binding: &Binding) -> bool {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (4)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (2)
RuleDomain(904-912)TextRange(9190-9190)crates/biome_analyze/src/rule.rs (1)
inspired(249-254)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (1)
Vue(51-51)crates/biome_rowan/src/ast/mod.rs (1)
cast_ref(142-151)
🔇 Additional comments (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (1)
224-228: Verifyname()handles both identifier and literal member names correctly.A past review suggested explicitly checking both
as_js_identifier_member_name()andas_js_literal_member_name()to handlesetup(props) {}(identifier) and"setup": ...(literal). The current implementation uses a singlename()call, which may be a convenience method that handles both variants. Ifname()is guaranteed to work for allAnyJsObjectMemberNamevariants, this is fine; otherwise, revert to the explicit checks from the past suggestion.If uncertain, consider the explicit approach:
fn is_member_named_setup(name: &AnyJsObjectMemberName) -> bool { // Identifier: setup(props) {} if let Some(id) = name.as_js_identifier_member_name() { return id.value_token().is_ok_and(|tok| tok.text_trimmed() == "setup"); } // Literal: "setup": ... or 'setup'() {} if let Some(lit) = name.as_js_literal_member_name() { return lit.name().is_ok_and(|tok| tok.text_trimmed() == "setup"); } false }
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (2)
RuleDomain(904-912)TextRange(9190-9190)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (1)
Vue(51-51)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
|
No worries @AsherDe BTW, you're not forced to get the rule 100% at the first PR. You're actually encouraged to descope the rule, and break it down in multiple PRs if you think it's too much work. |
AsherDe
force-pushed
the
feat/vue-reactivity-rule
branch
from
e918990 to
7e15df3
Compare
|
@ematipico I was attempting to merge all commit history and accidentally cleared the remote commit log. I apologize, as you may still be reviewing my pull request. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (19)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**
📒 Files selected for processing (18)
.changeset/yummy-melons-serve.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.ts(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid.js(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid.js
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js
- crates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs
- crates/biome_rule_options/src/lib.rs
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-typescript.ts
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js
🧰 Additional context used
📓 Path-based instructions (6)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.jscrates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.tscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.jscrates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.tscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.tscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
.changeset/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/**/*.md: Create changesets using thejust new-changesetcommand; do not author them manually
In changeset markdown, only use headers #### or #####
Changeset descriptions must end every sentence with a full stop (.)
For bug fixes, start the changeset description with a linked issue reference like “Fixed #1234”
Prefer past tense for what was done and present tense for current behavior in changesets
Files:
.changeset/yummy-melons-serve.md
🧬 Code graph analysis (9)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js (5)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (4)
props(7-7)props(18-18)props(31-31)props(44-44)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (4)
props(6-6)props(14-14)props(39-39)foo(30-30)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js (1)
toRefs(8-8)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js (1)
toRefs(6-6)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (3)
toRefs(17-17)foo(25-25)foo(41-41)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.js (1)
crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
h(131-133)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (3)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (4)
props(6-6)props(14-14)props(39-39)foo(30-30)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (2)
foo(21-21)foo(35-35)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (3)
foo(25-25)foo(41-41)bar(52-52)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (2)
RuleDomain(904-912)TextRange(9283-9283)crates/biome_rowan/src/ast/mod.rs (1)
cast_ref(142-151)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-typescript.ts (7)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (4)
props(7-7)props(18-18)props(31-31)props(44-44)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (4)
props(6-6)props(14-14)props(39-39)foo(30-30)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js (1)
toRefs(8-8)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js (1)
toRefs(4-4)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js (1)
toRefs(6-6)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (4)
toRefs(17-17)foo(25-25)foo(41-41)reactiveProps(33-33)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (2)
foo(21-21)foo(35-35)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (5)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (4)
props(7-7)props(18-18)props(31-31)props(44-44)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (4)
props(6-6)props(14-14)props(39-39)foo(30-30)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js (1)
toRefs(8-8)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js (1)
toRefs(4-4)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js (1)
toRefs(6-6)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-wrong-package-import.js (3)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-local-function.js (1)
toRefs(8-8)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js (1)
toRefs(4-4)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (2)
toRefs(17-17)bar(52-52)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.js (3)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-conditional-destructuring.js (4)
props(7-7)props(18-18)props(31-31)props(44-44)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-variable-destructuring.js (4)
props(6-6)props(14-14)props(39-39)foo(30-30)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-safe-patterns.js (3)
foo(25-25)foo(41-41)bar(52-52)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js (1)
crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
h(131-133)
🔇 Additional comments (5)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-namespaced-call.js (1)
1-7: Great addition covering namespaced helpers. Nice to see the tricky SomeLib.toRefs scenario locked in..changeset/yummy-melons-serve.md (1)
1-18: Concise changeset. Reads cleanly and ticks the guideline boxes.crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-unimported-toRefs.js (1)
1-7: Unimported helper case captured. This should stop regressions right where they like to hide.crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js (1)
1-37: Solid guardrail for non-Vue code. Appreciate the mix of shapes to prove we stay quiet in plain JS land.crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-nested-contexts.js (1)
1-55: Nested-context coverage looks spot on. This should keep the rule honest around callback scopes.
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (1)
235-237: Quoted setup keys still not handled properly.This was flagged in previous reviews. Using
.name().text()doesn't handle quoted object keys like"setup": function() {}or'setup': function() {}. You need to explicitly check both identifier and literal member name variants and use.text_trimmed()to strip quotes.Apply the fix from the previous review:
fn is_member_named_setup(name: &AnyJsObjectMemberName) -> bool { - name.name().is_some_and(|text| text.text() == "setup") + if let Some(identifier) = name.as_js_identifier_member_name() { + return identifier + .value_token() + .is_ok_and(|token| token.text_trimmed() == "setup"); + } + + if let Some(literal) = name.as_js_literal_member_name() { + return literal + .name() + .is_ok_and(|token| token.text_trimmed() == "setup"); + } + + false }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid.js
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.209Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : Place all new JavaScript lint rules in the nursery group under biome_js_analyze/lib/src/lint/nursery/<rule_name>.rs
📚 Learning: 2025-10-02T12:57:33.209Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.209Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/specs/nursery/** : Place snapshot tests for new rules under tests/specs/nursery/<ruleName>/ with files prefixed by invalid* and valid*
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (2)
RuleDomain(904-912)TextRange(9283-9283)crates/biome_analyze/src/rule.rs (4)
domains(591-594)recommended(561-564)sources(576-579)inspired(252-257)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
|
I have now updated this current PR to serve as the first step in a three-part series to implement the noVueSetupPropsReactivityLoss rule. The scope of this PR is now limited to: My plan for the follow-up PRs is as follows: |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (2)
RuleDomain(904-912)TextRange(9283-9283)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
AsherDe
force-pushed
the
feat/vue-reactivity-rule
branch
from
dbf6a8b to
01e337b
Compare
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs
Show resolved
Hide resolved
AsherDe
force-pushed
the
feat/vue-reactivity-rule
branch
from
5c29996 to
745084d
Compare
…cturing only
Foundation version with:
- Vue component discovery (export default, defineComponent)
- setup function detection (method & property forms)
- Parameter destructuring check (setup({ foo }))
Removed:
- Root scope destructuring
- toRefs/toRef validation
- Nested scope checking
- TypeScript tests
~604 lines → ~245 lines
AsherDe
force-pushed
the
feat/vue-reactivity-rule
branch
from
745084d to
9450bfe
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (8)
.changeset/yummy-melons-serve.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid.js(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/biome_rule_options/src/lib.rs
- .changeset/yummy-melons-serve.md
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid.js
- crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid-parameter-destructuring.js
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.jscrates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.jscrates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.jscrates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rscrates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs
🧠 Learnings (1)
📚 Learning: 2025-10-02T12:57:33.228Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.228Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/specs/nursery/** : Place snapshot tests for new rules under tests/specs/nursery/<ruleName>/ with files prefixed by invalid* and valid*
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js
🧬 Code graph analysis (3)
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (2)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
RuleDomain(904-912)crates/biome_analyze/src/rule.rs (4)
domains(591-594)recommended(561-564)sources(576-579)inspired(252-257)
crates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
NoVueSetupPropsReactivityLossOptions(8248-8248)
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js (1)
crates/biome_js_analyze/tests/specs/suspicious/noRedeclare/valid-declaration-merging.ts (1)
h(131-133)
🔇 Additional comments (9)
crates/biome_rule_options/src/no_vue_setup_props_reactivity_loss.rs (1)
1-6: LGTM!Empty options struct is correctly scaffolded with appropriate derives and serde configuration.
crates/biome_js_analyze/src/lint/nursery/no_vue_setup_props_reactivity_loss.rs (8)
6-11: Imports look good.All
usestatements are at the beginning of the file and types are imported without redundant crate prefixes.
13-50: Rule declaration is well-structured.Documentation clearly describes the parameter destructuring pattern this PR implements, with appropriate invalid and valid examples. Metadata (version, domain, source) is correct.
52-58: LGTM!Simple
Violation(TextRange)struct follows maintainer feedback.
60-63: LGTM!Enum cleanly handles both method and function property forms.
65-104: Implementation correctly usesVueComponentQuery.The rule properly leverages existing Vue component detection and generates clear diagnostics.
106-125: Helper functions are clean and focused.
check_call_expression_setupandcheck_object_membersuse if-let chains effectively and follow single-responsibility principle.
127-169: Parameter checking logic is sound.The code correctly identifies destructuring patterns (object/array) and appropriately ignores simple identifier parameters. Arrow function handling covers both parameter forms.
171-217: Helper functions are well-structured.
find_setup_function,get_function_from_expression,is_named_setup, andget_first_binding_from_paramsall follow clear, single-purpose patterns. The name checking logic correctly handles both identifier and literal member names via thename()method.
crates/biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/invalid.js
Show resolved
Hide resolved
...biome_js_analyze/tests/specs/nursery/noVueSetupPropsReactivityLoss/valid-non-vue-contexts.js
Show resolved
Hide resolved
AsherDe
force-pushed
the
feat/vue-reactivity-rule
branch
from
9450bfe to
56869c2
Compare
3 participants
Summary
This PR introduces a new lint rule,
vue/noSetupPropsReactivityLoss, ported fromeslint-plugin-vueto prevent a common bug in Vue's Composition API.The motivation for this change is to help developers avoid accidentally losing props reactivity. In Vue, destructuring the
propsobject directly in thesetupfunction's parameters (e.g.,setup({ count })) or in its root scope (e.g.,const { count } = props) breaks the reactive link. This can lead to components that do not update correctly when their props change, which can be difficult to debug.This new rule identifies these specific patterns and provides a clear diagnostic to the user. It correctly handles common exceptions, ignoring destructuring that occurs within nested functions (like
watchcallbacks or returned render functions) and when using reactivity-preserving helpers liketoRefs.A changeset has been added for this new feature.
Fixes #6305
Test Plan
The rule's implementation is verified through a comprehensive suite of spec (snapshot) tests to ensure its correctness and robustness across various edge cases.
Invalid cases that correctly trigger the rule have been tested, including:
setupfunction parameters (setup({ count })).const { count } = props).const,let, andvarfor assignments.Valid cases that are correctly ignored by the rule have also been thoroughly tested:
props.count).watchcallbacks or returned render functions).toRefs().propsobject.setupfunction signatures (e.g., with acontextparameter, as an arrow function, or with no parameters).setupmethod at all.Docs
As this is a new lint rule, the documentation is included directly within the rule's source code (
declare_lint_rule!) asrustdoccomments. This documentation explains the purpose of the rule and providesInvalidandValidcode examples, which are also used in the spec tests.No separate website PR is required for the rule's core documentation at this time.