-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
fix(compat): allow v-model built in modifiers on component #12654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(compat): allow v-model built in modifiers on component #12654
Conversation
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: |
`compatModelEventPrefix + event in props` can only be true if `isCompatEnabled(DeprecationTypes.COMPONENT_V_MODEL, instance)`; see `convertLegacyVModelProps`.
@markrian |
@edison1105 pinging you because it looks like this could be merged. Thanks! |
Co-authored-by: skirtle <[email protected]>
WalkthroughThe changes update the handling of v-model modifiers for Vue components under the Changes
Sequence Diagram(s)sequenceDiagram
participant ParentComponent
participant CustomInput
participant VueCompatRuntime
ParentComponent->>CustomInput: v-model.trim="text"
CustomInput-->>ParentComponent: Emits compat model event (e.g., 'compat:modelValue')
VueCompatRuntime->>CustomInput: Detects compat model event
VueCompatRuntime->>VueCompatRuntime: Applies .trim modifier if present
VueCompatRuntime-->>ParentComponent: Updates 'text' with trimmed value
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/runtime-core/src/componentEmits.ts (2)
154-156
: Add parentheses to avoid precedence ambiguityThe concatenation (
+
),in
, and&&
operators have different precedences.
Although the current expression happens to evaluate correctly, the lack of
parentheses forces the reader to recall those rules and makes the code easy to
mis-read on a quick scan.-const isCompatModelListener = - __COMPAT__ && compatModelEventPrefix + event in props +const isCompatModelListener = + __COMPAT__ && + (compatModelEventPrefix + event) in props
157-159
: Minor: extract helper for modifiers lookup
isCompatModelListener ? props.modelModifiers : getModelModifiers(...)
is duplicated (or very similar) in several places in the code-base. A tiny
utility such asresolveModelModifiers(props, event, isCompat)
would make this
branching logic self-documenting and cut 6–7 bytes from the non-compat build
after minification.Not blocking, just something to consider.
packages/vue-compat/__tests__/componentVModel.spec.ts (1)
110-114
: Avoid the bannedFunction
type
Function
is deliberately broad and flagged by Biome/TSLint/ESLint because it
silently accepts anything callable. Cast to the actual shape you need:- (deprecationData[DeprecationTypes.COMPONENT_V_MODEL].message as Function)( + ( + deprecationData[DeprecationTypes.COMPONENT_V_MODEL] + .message as (c: ComponentOptions) => string + )(This tightens the type-check and removes the linter warning.
🧰 Tools
🪛 Biome (1.9.4)
[error] 111-111: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/runtime-core/src/componentEmits.ts
(1 hunks)packages/vue-compat/__tests__/componentVModel.spec.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/runtime-core/src/componentEmits.ts (2)
packages/runtime-core/src/compat/componentVModel.ts (1)
compatModelEventPrefix
(12-12)packages/runtime-core/src/helpers/useModel.ts (1)
getModelModifiers
(120-129)
packages/vue-compat/__tests__/componentVModel.spec.ts (1)
packages/runtime-core/src/compat/compatConfig.ts (1)
deprecationData
(74-426)
🪛 Biome (1.9.4)
packages/vue-compat/__tests__/componentVModel.spec.ts
[error] 111-111: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🔇 Additional comments (1)
packages/vue-compat/__tests__/componentVModel.spec.ts (1)
116-122
: Nice coverage of.trim
behaviourThe assertions correctly validate both the model update and the reflected input
value after the modifier is applied. Good job extending the test matrix to
cover custom model options as well.
close #12652
This is a rough attempt at fixing #12652.
Summary by CodeRabbit
Bug Fixes
Tests