fix(runtime-core): handle non-isomorphic block element update#15002
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
ChangespatchElement full-diff fallback for mismatched dynamicChildren
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
@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: |
Size ReportBundles
Usages
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/runtime-core/src/renderer.ts (1)
858-906: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winSeparate non-isomorphic blocks from normal leaf nodes, and route HMR through the full-diff branch.
Line 866 can null
dynamicChildrenfor HMR, but Line 881 still branches only onisFullDiffRequired; equal-length HMR blocks then callpatchBlockChildren(..., dynamicChildren!)withnull. Also, the current predicate is true when both vnodes simply have no nesteddynamicChildren, forcing full children/props diffs for normal optimized dynamic leaf nodes.Proposed fix
- const isFullDiffRequired = !( - dynamicChildren && - n1.dynamicChildren && - n1.dynamicChildren.length === dynamicChildren.length - ) + const isNonIsomorphicBlock = !!( + (dynamicChildren || n1.dynamicChildren) && + (!dynamicChildren || + !n1.dynamicChildren || + n1.dynamicChildren.length !== dynamicChildren.length) + ) + const forceFullDiff = + isNonIsomorphicBlock || (__DEV__ && isHmrUpdating) // HMR updated, force full diff - if (isFullDiffRequired || (__DEV__ && isHmrUpdating)) { + if (forceFullDiff) { patchFlag = 0 optimized = false dynamicChildren = null } @@ - if (isFullDiffRequired) { + if (dynamicChildren) { + patchBlockChildren( + n1.dynamicChildren!, + dynamicChildren, + el, + parentComponent, + parentSuspense, + resolveChildrenNamespace(n2, namespace), + slotScopeIds, + ) + if (__DEV__) { + // necessary for HMR + traverseStaticChildren(n1, n2) + } + } else if (!optimized) { patchChildren( n1, n2, @@ slotScopeIds, false, ) - } else { - patchBlockChildren( - n1.dynamicChildren!, - dynamicChildren!, - el, - parentComponent, - parentSuspense, - resolveChildrenNamespace(n2, namespace), - slotScopeIds, - ) - if (__DEV__) { - // necessary for HMR - traverseStaticChildren(n1, n2) - } }🤖 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/runtime-core/src/renderer.ts` around lines 858 - 906, Separate non-isomorphic blocks from normal optimized leaf nodes, and ensure HMR always uses the full-diff path. Update the logic around isFullDiffRequired in renderer.ts so it only forces a full diff for user-wrapped/non-isomorphic blocks, not for ordinary vnodes that simply lack dynamicChildren, and keep HMR from falling through to patchBlockChildren when dynamicChildren has been nulled. Use the existing patchChildren, patchBlockChildren, and isHmrUpdating branches to route HMR updates through the full-diff branch and avoid calling patchBlockChildren with null dynamicChildren.
🤖 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.
Outside diff comments:
In `@packages/runtime-core/src/renderer.ts`:
- Around line 858-906: Separate non-isomorphic blocks from normal optimized leaf
nodes, and ensure HMR always uses the full-diff path. Update the logic around
isFullDiffRequired in renderer.ts so it only forces a full diff for
user-wrapped/non-isomorphic blocks, not for ordinary vnodes that simply lack
dynamicChildren, and keep HMR from falling through to patchBlockChildren when
dynamicChildren has been nulled. Use the existing patchChildren,
patchBlockChildren, and isHmrUpdating branches to route HMR updates through the
full-diff branch and avoid calling patchBlockChildren with null dynamicChildren.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1ae86aa-2282-4b74-aeb1-66b53d70e03b
📒 Files selected for processing (2)
packages/runtime-core/__tests__/rendererOptimizedMode.spec.tspackages/runtime-core/src/renderer.ts
|
The safer fix is to deopt before patching when the old and new
By setting see 7042602 |
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
fix #6385
Summary by CodeRabbit