fix(noUndeclaredVariables): track vue props as bindings#9285
Merged
Conversation
🦋 Changeset detectedLatest commit: 5ff0057 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 |
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
ematipico
approved these changes
Mar 1, 2026
Member
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
I think we should start considering having semantic models for these languages.
Astro suffers the same thing (props and params)
dyc3
force-pushed
the
dyc3/no-undeclared-vars-vue-props
branch
from
699242b to
ab55805
Compare
Contributor
Author
|
Yeah I agree. All these frameworks have significantly different semantics. It would likely help reduce false negatives. |
Base automatically changed from
dyc3/no-undeclared-vars-functions-and-vars
to
main
March 2, 2026 13:16
dyc3
force-pushed
the
dyc3/no-undeclared-vars-vue-props
branch
from
ab55805 to
011701d
Compare
Contributor
WalkthroughThe PR enhances the embedded bindings service to recognise Vue props declared via Options API ( Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/biome_service/src/workspace/document/services/embedded_bindings.rs (1)
172-208: Consolidate duplicated prop-name extraction logic.The object/array prop parsing is duplicated in
visit_js_exportandvisit_define_props_call. A shared helper will keep behaviour in sync and reduce regression risk.♻️ Refactor sketch
+ fn register_props_from_expression(&mut self, expr: AnyJsExpression) { + match expr { + AnyJsExpression::JsObjectExpression(obj) => { + // extract top-level property names + } + AnyJsExpression::JsArrayExpression(arr) => { + // extract string literal values + } + _ => {} + } + } - match props_value { - AnyJsExpression::JsObjectExpression(props_object) => { ... } - AnyJsExpression::JsArrayExpression(props_array) => { ... } - _ => {} - } + self.register_props_from_expression(props_value); - match first_expr { - AnyJsExpression::JsObjectExpression(obj) => { ... } - AnyJsExpression::JsArrayExpression(arr) => { ... } - _ => {} - } + self.register_props_from_expression(first_expr);Also applies to: 352-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs` around lines 172 - 208, The prop-name extraction logic duplicated in visit_js_export and visit_define_props_call should be extracted into a shared helper (e.g., a new method like collect_prop_names or extract_prop_names) that accepts an AnyJsExpression and a mutable reference to self.js_bindings; move the object-handling (JsObjectExpression -> iterate members -> prop_entry.name() -> as_js_literal_member_name -> token.token_text_trimmed()) and array-handling (JsArrayExpression -> iterate elements -> match JsStringLiteralExpression -> inner_string_text or range) into that helper, replace the duplicated blocks in visit_js_export and visit_define_props_call with calls to the helper, and ensure the helper returns/records the same keys (use token.text_trimmed_range()/string_lit.range() and token.token_text_trimmed()/inner text) to keep behavior identical.crates/biome_cli/tests/cases/vue_cross_language_rules.rs (1)
85-228: Please add CLI snapshots for runtimedefinePropsforms too.This suite now covers type-arg and Options API, but the new service logic also handles runtime object/array calls. Adding end-to-end cases for
defineProps({ ... })anddefineProps(['...'])would lock the full integration path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_cli/tests/cases/vue_cross_language_rules.rs` around lines 85 - 228, Add two end-to-end tests that exercise the runtime forms of defineProps by copying the existing test pattern (MemoryFileSystem/BufferConsole, inserting BIOME_CONFIG_HTML_FULL_SUPPORT, invoking run_cli with Args::from(["lint", "--only=noUndeclaredVariables", file.as_str()]) and asserting result.is_ok() and assert_cli_snapshot). Create one test function named no_undeclared_variables_not_triggered_for_define_props_runtime_object that writes a .vue file using <script setup> with defineProps({ loading: Boolean, disabled: Boolean }) and a template referencing loading and disabled, and another named no_undeclared_variables_not_triggered_for_define_props_runtime_array that uses defineProps(['loading','disabled']) (or the equivalent runtime array form) with the template referencing those props; use SnapshotPayload::new with descriptive snapshot names matching the function names. Ensure the tests follow the same structure as the existing functions (e.g., no_undeclared_variables_not_triggered_for_define_props_type_arg) so they integrate with the snapshot harness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs`:
- Around line 315-318: The current early-return only checks callee_text !=
"defineProps" which treats any defineProps call in non-Vue snippets as a
binding; update the guard in the function handling embedded binding extraction
in embedded_bindings.rs to additionally require a Vue-only signal from the
caller (e.g., an is_vue_snippet / is_vue flag or equivalent context parameter)
before treating defineProps(...) as a binding source—i.e., change the condition
to verify callee_text == "defineProps" AND the snippet is marked as Vue, and
propagate or read the existing caller-provided Vue indicator to this function so
non-Vue HTML-ish snippets no longer trigger defineProps extraction.
---
Nitpick comments:
In `@crates/biome_cli/tests/cases/vue_cross_language_rules.rs`:
- Around line 85-228: Add two end-to-end tests that exercise the runtime forms
of defineProps by copying the existing test pattern
(MemoryFileSystem/BufferConsole, inserting BIOME_CONFIG_HTML_FULL_SUPPORT,
invoking run_cli with Args::from(["lint", "--only=noUndeclaredVariables",
file.as_str()]) and asserting result.is_ok() and assert_cli_snapshot). Create
one test function named
no_undeclared_variables_not_triggered_for_define_props_runtime_object that
writes a .vue file using <script setup> with defineProps({ loading: Boolean,
disabled: Boolean }) and a template referencing loading and disabled, and
another named
no_undeclared_variables_not_triggered_for_define_props_runtime_array that uses
defineProps(['loading','disabled']) (or the equivalent runtime array form) with
the template referencing those props; use SnapshotPayload::new with descriptive
snapshot names matching the function names. Ensure the tests follow the same
structure as the existing functions (e.g.,
no_undeclared_variables_not_triggered_for_define_props_type_arg) so they
integrate with the snapshot harness.
In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs`:
- Around line 172-208: The prop-name extraction logic duplicated in
visit_js_export and visit_define_props_call should be extracted into a shared
helper (e.g., a new method like collect_prop_names or extract_prop_names) that
accepts an AnyJsExpression and a mutable reference to self.js_bindings; move the
object-handling (JsObjectExpression -> iterate members -> prop_entry.name() ->
as_js_literal_member_name -> token.token_text_trimmed()) and array-handling
(JsArrayExpression -> iterate elements -> match JsStringLiteralExpression ->
inner_string_text or range) into that helper, replace the duplicated blocks in
visit_js_export and visit_define_props_call with calls to the helper, and ensure
the helper returns/records the same keys (use
token.text_trimmed_range()/string_lit.range() and
token.token_text_trimmed()/inner text) to keep behavior identical.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
crates/biome_cli/tests/snapshots/main_cases_vue_cross_language_rules/no_undeclared_variables_not_triggered_defined_props_options_api.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_vue_cross_language_rules/no_undeclared_variables_not_triggered_for_define_props_type_arg.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_vue_cross_language_rules/no_undeclared_variables_not_triggered_for_define_props_type_arg_2.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/blue-steaks-give.mdcrates/biome_cli/tests/cases/vue_cross_language_rules.rscrates/biome_service/src/workspace/document/services/embedded_bindings.rs
Comment on lines
+315
to
+318
| // defineProps is a macro used in Vue SFCs to define component props. | ||
| // TODO: only bother with this check in Vue files. Currently, this check applies to all html-ish files. | ||
| if callee_text != "defineProps" { | ||
| return; |
Contributor
There was a problem hiding this comment.
Scope defineProps extraction to Vue snippets only.
Tiny gremlin here: this currently treats any defineProps(...) call in embedded HTML-ish snippets as a binding source, which can mask genuine noUndeclaredVariables diagnostics outside Vue. Please gate this path behind a Vue-only signal from the caller.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs`
around lines 315 - 318, The current early-return only checks callee_text !=
"defineProps" which treats any defineProps call in non-Vue snippets as a
binding; update the guard in the function handling embedded binding extraction
in embedded_bindings.rs to additionally require a Vue-only signal from the
caller (e.g., an is_vue_snippet / is_vue flag or equivalent context parameter)
before treating defineProps(...) as a binding source—i.e., change the condition
to verify callee_text == "defineProps" AND the snippet is marked as Vue, and
propagate or read the existing caller-provided Vue indicator to this function so
non-Vue HTML-ish snippets no longer trigger defineProps extraction.
011701d to
09072ff
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/biome_service/src/workspace/document/services/embedded_bindings.rs (1)
141-149:⚠️ Potential issue | 🟠 MajorGate Vue-only prop extraction behind a Vue context flag.
Tiny gremlin still lurking: this logic still runs for all html-ish snippets (see TODO), which can hide legitimate
noUndeclaredVariablesdiagnostics outside Vue. Please guard bothvisit_define_props_calland Vue Options API export handling with a caller-provided Vue signal.Also applies to: 315-318
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs` around lines 141 - 149, The Vue-specific extraction runs unguarded and should only run when a caller-provided Vue context flag is set; add a boolean field (e.g., enable_vue_context) to the visitor struct and early-return None from both visit_define_props_call and visit_js_export when that flag is false, and update any constructors or call sites that instantiate this visitor to accept and propagate that flag so the Vue Options API export handling and defineProps extraction only execute when enable_vue_context is true.
🧹 Nitpick comments (1)
crates/biome_service/src/workspace/document/services/embedded_bindings.rs (1)
45-48: Avoid double-processingdefinePropscalls.
definePropsis currently visited via preorder call-expression scanning and again via statement/declarator handlers. It works, but it’s extra churn and easier to desynchronise later. Prefer a single extraction path.♻️ Minimal direction
- } else if let Some(expr) = JsCallExpression::cast_ref(&node) { - self.visit_define_props_call(&expr); - } + }+// Keep defineProps extraction in module-item visitors only +// (variable declarators + expression statements), so behaviour remains explicit.Also applies to: 63-65, 249-253, 296-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs` around lines 45 - 48, The code double-processes defineProps by handling it both during generic JsCallExpression preorder scanning and again inside module/statement/declarator handlers; remove the duplicate path so defineProps is extracted in only one place. Specifically, stop invoking visit_define_props_call from inside JsModuleItemList/statement/declarator traversal (references: JsModuleItemList::cast_ref, JsCallExpression::cast_ref, visit_define_props_call) and centralize defineProps extraction in the single intended visitor (the existing call-expression handler); update or remove the extra calls found around the blocks referenced (lines near 45-48, 63-65, 249-253, 296-301) so statements/declarators forward to the common call-expression logic instead of re-calling visit_define_props_call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs`:
- Around line 141-149: The Vue-specific extraction runs unguarded and should
only run when a caller-provided Vue context flag is set; add a boolean field
(e.g., enable_vue_context) to the visitor struct and early-return None from both
visit_define_props_call and visit_js_export when that flag is false, and update
any constructors or call sites that instantiate this visitor to accept and
propagate that flag so the Vue Options API export handling and defineProps
extraction only execute when enable_vue_context is true.
---
Nitpick comments:
In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs`:
- Around line 45-48: The code double-processes defineProps by handling it both
during generic JsCallExpression preorder scanning and again inside
module/statement/declarator handlers; remove the duplicate path so defineProps
is extracted in only one place. Specifically, stop invoking
visit_define_props_call from inside JsModuleItemList/statement/declarator
traversal (references: JsModuleItemList::cast_ref, JsCallExpression::cast_ref,
visit_define_props_call) and centralize defineProps extraction in the single
intended visitor (the existing call-expression handler); update or remove the
extra calls found around the blocks referenced (lines near 45-48, 63-65,
249-253, 296-301) so statements/declarators forward to the common
call-expression logic instead of re-calling visit_define_props_call.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
crates/biome_cli/tests/snapshots/main_cases_vue_cross_language_rules/no_undeclared_variables_not_triggered_defined_props_options_api.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_vue_cross_language_rules/no_undeclared_variables_not_triggered_for_define_props_type_arg.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_vue_cross_language_rules/no_undeclared_variables_not_triggered_for_define_props_type_arg_2.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/blue-steaks-give.mdcrates/biome_cli/tests/cases/vue_cross_language_rules.rscrates/biome_service/src/workspace/document/services/embedded_bindings.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_cli/tests/cases/vue_cross_language_rules.rs
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/biome_service/src/workspace/document/services/embedded_bindings.rs (1)
139-141:⚠️ Potential issue | 🟠 MajorGate Vue prop extraction behind a Vue-only context flag.
Tiny gremlin still here: both
visit_js_exportandvisit_define_props_callrun for any JS snippet, so non‑Vue snippets can gain synthetic bindings and hide realnoUndeclaredVariablesdiagnostics.Proposed fix (thread Vue context through the visitor)
-pub(crate) fn visit_js_source_snippet(&mut self, root: &AnyJsRoot) { +pub(crate) fn visit_js_source_snippet(&mut self, root: &AnyJsRoot, is_vue_snippet: bool) { @@ - if let Some(module_item) = JsModuleItemList::cast_ref(&node) { - self.visit_module_item_list(module_item); + if let Some(module_item) = JsModuleItemList::cast_ref(&node) { + self.visit_module_item_list(module_item, is_vue_snippet); } else if let Some(expr) = JsCallExpression::cast_ref(&node) { - self.visit_define_props_call(&expr); + self.visit_define_props_call(&expr, is_vue_snippet); } @@ -fn visit_module_item_list(&mut self, list: JsModuleItemList) { +fn visit_module_item_list(&mut self, list: JsModuleItemList, is_vue_snippet: bool) { @@ - AnyJsModuleItem::JsExport(export) => { - self.visit_js_export(export); + AnyJsModuleItem::JsExport(export) => { + self.visit_js_export(export, is_vue_snippet); } @@ -fn visit_js_export(&mut self, export: JsExport) -> Option<()> { +fn visit_js_export(&mut self, export: JsExport, is_vue_snippet: bool) -> Option<()> { + if !is_vue_snippet { + return Some(()); + } @@ -fn visit_define_props_call(&mut self, call_expression: &JsCallExpression) { +fn visit_define_props_call(&mut self, call_expression: &JsCallExpression, is_vue_snippet: bool) { + if !is_vue_snippet { + return; + }Also applies to: 296-301, 309-319
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs` around lines 139 - 141, The visitor currently extracts Vue props in visit_js_export and visit_define_props_call for any JS, causing synthetic bindings in non-Vue files; modify the visitor to check a Vue-only context flag (e.g., a new or existing is_vue/file_is_vue field on the visitor or context) and return early unless that flag is true before performing extraction in visit_js_export and visit_define_props_call; thread this Vue context through the visitor so the same guard is applied for the other related methods (the ones around the visit_define_props_call region and the export handling), ensuring no Vue-specific synthetic bindings are generated for non-Vue snippets.
🧹 Nitpick comments (1)
crates/biome_service/src/workspace/document/services/embedded_bindings.rs (1)
321-338: Add a direct unit test for type-argumentdefineProps<{...}>extraction.The extraction branch is implemented, but this file only tests runtime object/array forms. A focused local test would lock this behaviour down.
Suggested test case
+ #[test] + fn tracks_define_props_type_argument_object() { + let source = r#" +defineProps<{ title: String; likes: Number }>() +"#; + let mut service = EmbeddedExportedBindings::default(); + let mut builder = service.builder(); + visit_js_root(&mut builder, &parse_js(source)); + service.finish(builder); + assert!(contains_binding(&service, "title")); + assert!(contains_binding(&service, "likes")); + }Also applies to: 649-677
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs` around lines 321 - 338, Add a focused unit test that exercises the type-argument form of defineProps (the branch that checks call_expression.type_arguments() and matches AnyTsType::TsObjectType) to ensure keys are extracted into self.js_bindings via insert(value.text_trimmed_range(), value.token_text_trimmed()); locate the behavior around the code that matches AnyTsType::TsObjectType and the loop over object_type.members() and write a test that parses a snippet like defineProps<{ title: string, count: number }>() (and also repeat for the other similar block around the 649-677 range) asserting the expected trimmed names/ranges are present in the resulting js_bindings map so the type-argument extraction is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs`:
- Around line 139-141: The visitor currently extracts Vue props in
visit_js_export and visit_define_props_call for any JS, causing synthetic
bindings in non-Vue files; modify the visitor to check a Vue-only context flag
(e.g., a new or existing is_vue/file_is_vue field on the visitor or context) and
return early unless that flag is true before performing extraction in
visit_js_export and visit_define_props_call; thread this Vue context through the
visitor so the same guard is applied for the other related methods (the ones
around the visit_define_props_call region and the export handling), ensuring no
Vue-specific synthetic bindings are generated for non-Vue snippets.
---
Nitpick comments:
In `@crates/biome_service/src/workspace/document/services/embedded_bindings.rs`:
- Around line 321-338: Add a focused unit test that exercises the type-argument
form of defineProps (the branch that checks call_expression.type_arguments() and
matches AnyTsType::TsObjectType) to ensure keys are extracted into
self.js_bindings via insert(value.text_trimmed_range(),
value.token_text_trimmed()); locate the behavior around the code that matches
AnyTsType::TsObjectType and the loop over object_type.members() and write a test
that parses a snippet like defineProps<{ title: string, count: number }>() (and
also repeat for the other similar block around the 649-677 range) asserting the
expected trimmed names/ranges are present in the resulting js_bindings map so
the type-argument extraction is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0d83aa81-ee35-4c80-b8b4-93f7dccd3e3c
📒 Files selected for processing (1)
crates/biome_service/src/workspace/document/services/embedded_bindings.rs
ematipico
reviewed
Mar 6, 2026
ematipico
reviewed
Mar 6, 2026
crates/biome_service/src/workspace/document/services/embedded_bindings.rs
Outdated
Show resolved
Hide resolved
ematipico
approved these changes
Mar 6, 2026
Member
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
There's a lot of code in here. I think it's fine to merge, with the reminder for us to actually use the semantic model so we can track the bindings
dyc3
force-pushed
the
dyc3/no-undeclared-vars-vue-props
branch
from
69bc9b5 to
5ff0057
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Props defined in Vue components become available in the template section even if they don't get bound to any variable in the actual script block. We now track them as embedded bindings.
It's not an entirely ideal solution, as I would prefer to use our existing vue component helpers to extract the props. However, using those currently requires the semantic model to be available. This gets us most of the way there though. I'll follow this up with another PR that does the appropriate refactors so we can use our existing helpers.
Mostly planned and implemented by minimax m2.5 and sonnet 4.6
Test Plan
added unit tests and cli tests
Docs