fix(canon): type-canon static attribute values on component props#3197
Merged
Conversation
The per-prop extraction path only asserted dynamic bindings, so a static
attribute like msg="text" against defineProps<{ msg: number }> produced
no diagnostic from vize check while vue-tsc reports TS2322 at the same
usage. The generic functional prop-checker call is a deliberate no-op
for non-generic components, so nothing checked static values at all.
Static attribute values now emit the same typed assertion as dynamic
bindings, reusing the existing escaped string-literal generation and
mapping the synthetic check identifier back to the authored attribute
value. Valueless attributes keep their boolean-shorthand semantics and
stay out of the per-prop path, and unresolved or any-typed components
keep resolving prop types to unknown, so no new false positives appear.
Found by the create-vue Tier S patch oracle divergence in #2971.
ubugeeei
enabled auto-merge (squash)
July 21, 2026 10:41
📝 WalkthroughWalkthroughStatic component attribute values now generate TypeScript prop checks alongside dynamic bindings. Source mapping covers static value spans, while tests verify literal escaping, diagnostic mapping, and boolean shorthand exclusion. ChangesStatic component prop checks
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
github-actions
Bot
changed the title
vize
@vizejs/fresco
@vizejs/musea-mcp-server
oxlint-plugin-vize
@vizejs/rspack-plugin
@vizejs/unplugin
@vizejs/vite-plugin
@vizejs/vite-plugin-musea
@vizejs/musea-nuxt
@vizejs/nuxt
@vizeui/core
commit: |
Contributor
PR BenchmarkBase:
Raw run timesCompile SFC
Lint
Format
Type check (1T)
Type check (max)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/vize_canon/src/virtual_ts/expressions/component_props.rs (1)
77-89: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle empty attribute values separately.
rfind("")returns the end ofraw_prop, somsg=""maps to a zero-length range after the closing quote instead of the value slot between the quotes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/vize_canon/src/virtual_ts/expressions/component_props.rs` around lines 77 - 89, Update prop_value_source_range to handle empty prop.value strings before calling raw_prop.rfind: locate and return the range for the empty attribute’s value slot between its quotes, rather than using rfind(""). Preserve the existing rfind-based behavior for non-empty values and continue returning None when the source or expected attribute structure cannot be found.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/vize_canon/src/virtual_ts/scope/component_props.rs`:
- Around line 93-97: The declaration-loop guard must stay consistent with the
later value-check loop: update the condition in the component props processing
flow to also allow usages where has_inference_props(usage) is true, ensuring the
corresponding __{component}_Check_{idx} declaration exists before
generate_generic_props_call references it.
---
Outside diff comments:
In `@crates/vize_canon/src/virtual_ts/expressions/component_props.rs`:
- Around line 77-89: Update prop_value_source_range to handle empty prop.value
strings before calling raw_prop.rfind: locate and return the range for the empty
attribute’s value slot between its quotes, rather than using rfind(""). Preserve
the existing rfind-based behavior for non-empty values and continue returning
None when the source or expected attribute structure cannot be found.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 71de9868-8746-40dc-8855-cae9564f3824
📒 Files selected for processing (4)
crates/vize_canon/src/virtual_ts/expressions/component_props.rscrates/vize_canon/src/virtual_ts/expressions/component_props_tests.rscrates/vize_canon/src/virtual_ts/scope/component_prop_checker.rscrates/vize_canon/src/virtual_ts/scope/component_props.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (31)
- GitHub Check: Normalize titles and assignees
- GitHub Check: tool-benchmark
- GitHub Check: test-js-packages
- GitHub Check: test-scripts
- GitHub Check: cargo-semver-checks (vize_carton)
- GitHub Check: cargo-semver-checks (vize_musea)
- GitHub Check: cargo-semver-checks (vize_croquis)
- GitHub Check: check-vize-apps
- GitHub Check: build-js-packages
- GitHub Check: cargo-semver-checks (vize_fresco)
- GitHub Check: clippy-and-test
- GitHub Check: cargo-semver-checks (vize_atelier_sfc)
- GitHub Check: cargo-semver-checks (vize_relief)
- GitHub Check: vue-parity
- GitHub Check: cargo-semver-checks (vize_atelier_vapor)
- GitHub Check: security-audit
- GitHub Check: cargo-semver-checks (vize_atelier_core)
- GitHub Check: editor-extensions
- GitHub Check: pr-benchmark
- GitHub Check: cargo-semver-checks (vize_atelier_dom)
- GitHub Check: cargo-semver-checks (vize_atelier_ssr)
- GitHub Check: check-js
- GitHub Check: cargo-semver-checks (vize_armature)
- GitHub Check: coverage
- GitHub Check: app-readiness
- GitHub Check: miri
- GitHub Check: Publish preview packages
- GitHub Check: criterion-ab
- GitHub Check: dialect-guard
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (1)
crates/**
⚙️ CodeRabbit configuration file
crates/**: Focus on parser/compiler correctness, source locations, UTF-8/UTF-16 offset handling, and panic-free LSP behavior. Flag changes that only work for small fixtures but break real Vue/Nuxt projects.
Files:
crates/vize_canon/src/virtual_ts/expressions/component_props_tests.rscrates/vize_canon/src/virtual_ts/expressions/component_props.rscrates/vize_canon/src/virtual_ts/scope/component_props.rscrates/vize_canon/src/virtual_ts/scope/component_prop_checker.rs
🔇 Additional comments (8)
crates/vize_canon/src/virtual_ts/scope/component_prop_checker.rs (2)
11-22: LGTM!
11-22: LGTM!crates/vize_canon/src/virtual_ts/scope/component_props.rs (2)
93-95: LGTM!Also applies to: 111-111
111-111: LGTM!crates/vize_canon/src/virtual_ts/expressions/component_props.rs (2)
139-145: LGTM!
139-145: 🎯 Functional CorrectnessNo issue: valueless-only props are skipped before the per-prop checker alias and call are emitted, so there’s no dangling
__Child_Check_0reference.> Likely an incorrect or invalid review comment.crates/vize_canon/src/virtual_ts/expressions/component_props_tests.rs (2)
71-115: LGTM!Also applies to: 117-138, 140-159
71-115: LGTM!Also applies to: 117-138, 140-159
Comment on lines
+93
to
97
| let has_value_props = has_value_props(usage); | ||
| let has_navigable_props = component_prop_navigation::has_navigable_props(ctx, usage); | ||
| if !has_dynamic_props && !has_navigable_props { | ||
| if !has_value_props && !has_navigable_props { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Potential TS compilation error for components with only valueless attributes.
There is a mismatch in gating between the type declarations loop and the value checks loop.
If a component usage has only valueless static attributes (e.g., <Child disabled />), has_value_props will evaluate to false. Assuming has_navigable_props is also false, the first loop will continue, skipping the declaration of __{component}_Check_{idx}.
However, has_inference_props will evaluate to true (since it doesn't require prop.value.is_some()). This causes the second loop to call generate_generic_props_call, which emits a reference to the undeclared __{component}_Check_{idx}, leading to a TypeScript Cannot find name error in the generated virtual file.
Consider changing this condition to include has_inference_props(usage), or verify that checkable_usages strictly filters out components that lack value/navigable props beforehand.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/vize_canon/src/virtual_ts/scope/component_props.rs` around lines 93 -
97, The declaration-loop guard must stay consistent with the later value-check
loop: update the condition in the component props processing flow to also allow
usages where has_inference_props(usage) is true, ensuring the corresponding
__{component}_Check_{idx} declaration exists before generate_generic_props_call
references it.
Contributor
Detailed Test ReportCommit: Area Summary
Test InventoryTotal tracked cases: 9463 across 1463 files.
Files
Comment truncated at 64000 characters. Open the workflow run for the full job log. |
Contributor
Tool BenchmarkMeasured: 2026-07-21T10:46:58.430Z
Fairness notes:
Commands: gh workflow run tool-benchmark.yml --ref <branch> -f file_count=3000 -f check_file_count=500 -f vite_file_count=1000 -f nuxt_file_count=250 -f large_blocks=300 -f runs=3 -f warmups=1 -f commit_results=true
node bench/generate.mjs 3000
node bench/compare-tools.mjs --input bench/__in__ --vize-bin target/release/vize --runs 3 --warmups 1 --check-file-count 500 --vite-file-count 1000 --nuxt-file-count 250 --large-blocks 300 --runner-label "blacksmith-32vcpu-ubuntu-2404" --out tool-benchmark-summary.md --json tool-benchmark-results.json --doc performance-blacksmith.mdVariant details and raw run timesSFC compile
Large SFC compile
Large SFC type check
Lint
Format
Type check
Vite build (end-to-end)
Nuxt SPA build (end-to-end)
|
This was referenced Jul 21, 2026
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
1 participant
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
Fixes false negative 1 from the Tier S divergence report in #2971: with
defineProps<{ msg: number }>()inHelloWorld.vue, the static usage<HelloWorld msg="You did it!" />producederrorCount: 0fromvize checkwhile vue-tsc reportsTS2322: Type 'string' is not assignable to type 'number'.Root cause
generate_component_prop_checksgated onprop.is_dynamic, so static attribute values never produced a typed assertion.has_dynamic_props) had the same gate, so a usage with only static props emitted no checks at all.Fix
Static attribute values with a value now flow through the exact same per-prop assertion as dynamic bindings (
const __vize_prop_check_N_msg: __Child_N_prop_msg = "...";), reusing the existing escaped string-literal generation and the same source mapping, so a TS2322 points at the authored attribute value.has_dynamic_propsbecamehas_value_props.Deliberately unchanged:
<Child disabled />) keep boolean-shorthand semantics and stay out of the per-prop path;anycomponents still resolve prop types through__VizePropValue's never→unknown fallback, so statics on them cannot produce false positives;key/ref/dynamic-name attributes remain excluded.Tests
vize_canonsuite green (581 lib tests + integration), zero new clippy warnings, fmt clean.Part of #2971
Need help on this PR? Tag
/codesmithwith what you need. Autofix is enabled.Summary by CodeRabbit
disabled, retain their existing behavior.