fix(compat): should not warn COMPILER_V_BIND_OBJECT_ORDER when using v-bind together with v-for #12993
Conversation
…OBJECT_ORDER warning
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-ssr
@vue/compiler-sfc
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
@vue/compat
vue
commit: |
|
Perhaps it would be more appropriate to skip the v-for when check it |
edison1105
added
ready to merge
WalkthroughThe changes update the compiler's handling of Changes
Sequence Diagram(s)sequenceDiagram
participant TemplateParser
participant buildProps
participant CompatibilityChecker
TemplateParser->>buildProps: Process v-bind in v-for
buildProps->>CompatibilityChecker: Run compatibility check
CompatibilityChecker-->>buildProps: Compatibility logic (may modify mergeArgs)
buildProps->>buildProps: pushRefVForMarker()
buildProps->>buildProps: pushMergeArg()
Assessment against linked issues
Poem
✨ Finishing Touches
🪧 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 (
|
![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 (1)
packages/vue-compat/__tests__/compiler.spec.ts (1)
96-104: Good test for verifying the fixThis test correctly verifies that no deprecation warning is emitted when using v-bind with an object inside a v-for loop, addressing the issue described in the PR.
Consider a minor grammatical improvement to the test name:
-test('should not warn COMPILER_V_BIND_OBJECT_ORDER work with vFor', () => { +test('should not warn COMPILER_V_BIND_OBJECT_ORDER when used with vFor', () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/compiler-core/src/transforms/transformElement.ts(2 hunks)packages/vue-compat/__tests__/compiler.spec.ts(1 hunks)
🔇 Additional comments (2)
packages/compiler-core/src/transforms/transformElement.ts (2)
598-599: LGTM - Merging early for compat build checkAdding the early merge step here ensures proper handling of compatibility checks before performing other operations, which prevents the incorrect COMPILER_V_BIND_OBJECT_ORDER warning when v-bind appears before v-for.
642-644:✅ Verification successful
Correct sequencing of pushRefVForMarker and pushMergeArg
This reordering ensures that the ref marker for v-for and merge arguments are processed before pushing the expression, which correctly handles the case when v-bind is used with v-for. The added comment also helpfully documents the reasoning.
Could you verify whether the earlier call to
pushMergeArg()on line 644 might now be redundant in non-compat builds? It seems like it could potentially be moved inside theif (__COMPAT__) {check, since for non-compat builds we would only need a single call.
🏁 Script executed:
#!/bin/bash # Check for other usages of pushMergeArg to understand its behavior rg -A 5 -B 5 "pushMergeArg" packages/compiler-core/src/transforms/transformElement.tsLength of output: 2193
Keep
pushMergeArg()outside__COMPAT__
The call insideif (__COMPAT__) { … }only runs in compatibility builds; the subsequentpushMergeArg()immediately afterpushRefVForMarker()is required in non-compat builds to flush pending props before pushing the v-bind expression. Removing or moving it into the compat block would break non-compat behavior.
edison1105
changed the title
fix #12992
Summary by CodeRabbit
Bug Fixes
v-bindwith objects insidev-forloops to ensure compatibility warnings are not incorrectly triggered.Tests
v-bindwith an object inside av-forloop.