fix(compiler-sfc): infer boolean prop types from generic constraints#15028
Open
ifer47 wants to merge 1 commit into
Open
fix(compiler-sfc): infer boolean prop types from generic constraints#15028ifer47 wants to merge 1 commit into
ifer47 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughExtends ChangesGeneric boolean prop type inference fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
![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
🧹 Nitpick comments (1)
packages/compiler-sfc/__tests__/compileScript/defineProps.spec.ts (1)
241-272: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a direct
neverregression assertion.This fixture proves the boolean-constrained path, but it never hits the empty runtime type list that
packages/compiler-sfc/src/script/defineProps.tsLine 233 now normalizes from[]to['null']. Adding a prop that resolves fully toneverwould cover the exact branch this PR changed and preventtype: []from regressing silently.🤖 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 `@packages/compiler-sfc/__tests__/compileScript/defineProps.spec.ts` around lines 241 - 272, The current defineProps boolean-constraint test covers the generic boolean path but misses the runtime normalization branch in defineProps.ts where an empty type list is converted from [] to ['null']. Extend the existing compileScript/defineProps.spec.ts fixture by adding a prop in the same Props<TMultiple> setup that resolves to never, and assert its generated runtime prop shape so the test directly exercises the empty-runtime-types case and guards the new normalization behavior.
🤖 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 `@packages/compiler-sfc/src/script/resolveType.ts`:
- Around line 2051-2053: `inferRuntimeType` can now return an empty array for
`TSNeverKeyword`, but callers like `defineModel` and the `keyof` index-signature
handling in `resolveType` are not guarding against that shape. Update the
consumers of `inferRuntimeType` so they explicitly handle `[]` before using the
result: in `defineModel`, avoid passing an empty array into
`toRuntimeTypeString` and skip or default the generated runtime `type` when it
would otherwise become undefined; in the `keyof` path in `resolveType`, check
for an empty array before reading `[0]` so `undefined` is never added to the key
set.
---
Nitpick comments:
In `@packages/compiler-sfc/__tests__/compileScript/defineProps.spec.ts`:
- Around line 241-272: The current defineProps boolean-constraint test covers
the generic boolean path but misses the runtime normalization branch in
defineProps.ts where an empty type list is converted from [] to ['null']. Extend
the existing compileScript/defineProps.spec.ts fixture by adding a prop in the
same Props<TMultiple> setup that resolves to never, and assert its generated
runtime prop shape so the test directly exercises the empty-runtime-types case
and guards the new normalization behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5d23ffe-4ec8-42ce-bb4d-7b8a6414901c
⛔ Files ignored due to path filters (1)
packages/compiler-sfc/__tests__/compileScript/__snapshots__/defineProps.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
packages/compiler-sfc/__tests__/compileScript/defineProps.spec.tspackages/compiler-sfc/src/script/defineProps.tspackages/compiler-sfc/src/script/resolveType.ts
Comment on lines
+2051
to
+2053
| case 'TSNeverKeyword': | ||
| return [] | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all inferRuntimeType call sites and surrounding usage to check for [0]/non-empty assumptions
rg -nP -C3 '\binferRuntimeType\s*\(' packages/compiler-sfc/srcRepository: vuejs/core
Length of output: 13450
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant sections around inferRuntimeType and its direct consumers.
sed -n '1688,1738p' packages/compiler-sfc/src/script/resolveType.ts
printf '\n----\n'
sed -n '2166,2192p' packages/compiler-sfc/src/script/resolveType.ts
printf '\n----\n'
sed -n '2008,2058p' packages/compiler-sfc/src/script/resolveType.ts
printf '\n----\n'
sed -n '2100,2140p' packages/compiler-sfc/src/script/resolveType.ts
printf '\n----\n'
sed -n '210,245p' packages/compiler-sfc/src/script/defineProps.ts
printf '\n----\n'
sed -n '118,150p' packages/compiler-sfc/src/script/defineModel.ts
# Find direct index / length-sensitive usage of inferRuntimeType results.
printf '\n==== direct usage search ====\n'
rg -n -C2 'inferRuntimeType\([^)]*\)\s*\[0\]|inferRuntimeType\([^)]*\)\.length|inferRuntimeType\([^)]*\)\s*$' packages/compiler-sfc/src/scriptRepository: vuejs/core
Length of output: 9665
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect runtime-type string generation and other downstream consumers for empty-array assumptions.
rg -n -C2 'function toRuntimeTypeString|const toRuntimeTypeString|toRuntimeTypeString\(' packages/compiler-sfc/src/script
printf '\n----\n'
rg -n -C2 'inferRuntimeType\(ctx, .*?\)\[0\]|\.includes\(UNKNOWN_TYPE\)|\.filter\(t => t !== UNKNOWN_TYPE\)|type\.length \? type' packages/compiler-sfc/src/script
printf '\n----\n'
sed -n '1,120p' packages/compiler-sfc/src/script/defineModel.tsRepository: vuejs/core
Length of output: 8937
Handle the empty-array case before consumers use it
inferRuntimeType can now return [] for TSNeverKeyword, but not all callers handle that shape:
packages/compiler-sfc/src/script/defineModel.ts:131passes the result totoRuntimeTypeString, which returnsundefinedfor an empty array and can emit invalidtype: undefined.packages/compiler-sfc/src/script/resolveType.ts:1713indexes[0]in thekeyofindex-signature path, so an empty result can leakundefinedinto the set.
🤖 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 `@packages/compiler-sfc/src/script/resolveType.ts` around lines 2051 - 2053,
`inferRuntimeType` can now return an empty array for `TSNeverKeyword`, but
callers like `defineModel` and the `keyof` index-signature handling in
`resolveType` are not guarding against that shape. Update the consumers of
`inferRuntimeType` so they explicitly handle `[]` before using the result: in
`defineModel`, avoid passing an empty array into `toRuntimeTypeString` and skip
or default the generated runtime `type` when it would otherwise become
undefined; in the `keyof` path in `resolveType`, check for an empty array before
reading `[0]` so `undefined` is never added to the key set.
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
T extends true ? boolean : nevercan preserveBooleannullwhen runtime inference returns an empty type list, e.g. fromneverRoot Cause
When a generic props alias such as
Props<T extends boolean>was instantiated with an unresolved SFC generic, runtime prop inference kept the unresolved outer type parameter and lost the alias constraint. That made props likemultiple: Tandchecked: T extends true ? boolean : nevercompile without aBooleanruntime constructor, so Vue could not apply boolean casting.Fixes #13787.
Validation
pnpm vitest --project unit packages/compiler-sfc/__tests__/compileScript/defineProps.spec.ts --runpnpm vitest --project unit packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts --runpnpm vitest --project unit packages/compiler-sfc/__tests__ --runpnpm checkpnpm exec prettier --check packages/compiler-sfc/src/script/resolveType.ts packages/compiler-sfc/src/script/defineProps.ts packages/compiler-sfc/__tests__/compileScript/defineProps.spec.tsSummary by CodeRabbit
Bug Fixes
never-like cases are reflected more accurately in generated output.Tests