fix(runtime-vapor): preserve hydration cursor for nested insertions#14786
Merged
edison1105 merged 6 commits intominorfrom May 7, 2026
Merged
fix(runtime-vapor): preserve hydration cursor for nested insertions#14786edison1105 merged 6 commits intominorfrom
edison1105 merged 6 commits intominorfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/compiler-vapor
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/runtime-vapor
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
Size ReportBundles
Usages
|
![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 (2)
packages/runtime-vapor/src/apiCreateFragment.ts (1)
30-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard keyed-fragment cursor teardown with
finally.
renderEffect(() => frag.update(...))can synchronously hit user render code on the first pass. If that throws, Line 43 is skipped and the captured hydration cursor remains active for the rest of the hydration walk.Suggested fix
const hydrationCursor: HydrationCursor | null = isHydrating ? captureHydrationCursor() : null - const frag = __DEV__ - ? new DynamicFragment('keyed', true) - : new DynamicFragment(undefined, true) - - renderEffect(() => frag.update(render, key())) - - if (!isHydrating) { - if (_insertionParent) insert(frag, _insertionParent, _insertionAnchor) - } else { - exitHydrationCursor(hydrationCursor) - } - return frag + try { + const frag = __DEV__ + ? new DynamicFragment('keyed', true) + : new DynamicFragment(undefined, true) + + renderEffect(() => frag.update(render, key())) + + if (!isHydrating) { + if (_insertionParent) insert(frag, _insertionParent, _insertionAnchor) + } + return frag + } finally { + if (isHydrating) { + exitHydrationCursor(hydrationCursor) + } + }🤖 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-vapor/src/apiCreateFragment.ts` around lines 30 - 43, Wrap the code path that captures and later exits the hydration cursor in a try/finally so exitHydrationCursor(hydrationCursor) is always called even if renderEffect(() => frag.update(render, key())) throws; specifically, when isHydrating is true call captureHydrationCursor() into hydrationCursor, then call renderEffect as before inside the try block, and call exitHydrationCursor(hydrationCursor) in the finally block (while keeping the existing insertion logic for the non-hydrating branch); reference functions/variables: captureHydrationCursor, hydrationCursor, renderEffect, frag.update, exitHydrationCursor, and isHydrating.packages/runtime-vapor/src/componentSlots.ts (1)
193-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRelease the slot hydration cursor via
finally.The cursor captured/entered here is only released on the happy path. If slot resolution or
fragment.hydrate()throws before Line 312, nested hydration continues with a stale cursor and subsequent insertions can target the wrong range.Suggested fix
if (!isHydrating) resetInsertionState() let hydrationCursor: HydrationCursor | null = null + try { const instance = getScopeOwner()! const rawSlots = instance.rawSlots @@ - if (!isHydrating) { - if (!noSlotted) { - const scopeId = instance.type.__scopeId - if (scopeId) { - setScopeId(fragment, [`${scopeId}-s`]) - } - } - - if (_insertionParent) insert(fragment, _insertionParent, _insertionAnchor) - } else { - if (fragment.insert) { - ;(fragment as VaporFragment).hydrate!() - } - exitHydrationCursor(hydrationCursor) - } - - return fragment + if (!isHydrating) { + if (!noSlotted) { + const scopeId = instance.type.__scopeId + if (scopeId) { + setScopeId(fragment, [`${scopeId}-s`]) + } + } + + if (_insertionParent) insert(fragment, _insertionParent, _insertionAnchor) + } else if (fragment.insert) { + ;(fragment as VaporFragment).hydrate!() + } + + return fragment + } finally { + if (isHydrating) { + exitHydrationCursor(hydrationCursor) + } + }Also applies to: 308-312
🤖 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-vapor/src/componentSlots.ts` around lines 193 - 213, Slot hydration cursor (hydrationCursor) is only released on the happy path; wrap the slot-resolution and subsequent fragment.hydrate() logic (the block that may call enterHydrationCursor() or captureHydrationCursor() and creates fragment/slotFragment) in a try/finally and in the finally release/reset the cursor if set (i.e., call the appropriate cursor cleanup function used elsewhere in this module after enterHydrationCursor()/captureHydrationCursor()), so that hydrationCursor is always cleared even if slot resolution or fragment.hydrate() throws; reference hydrationCursor, enterHydrationCursor(), captureHydrationCursor(), and fragment.hydrate() to locate the code to wrap.
🧹 Nitpick comments (3)
packages/runtime-vapor/__tests__/hydration.spec.ts (2)
5079-5079: ⚡ Quick winAssert hydration output before flushing a tick
At Line 5079, the pre-assertion
await nextTick()can mask hydration-only regressions by allowing post-mount effects to settle first. Assert the initial DOM immediately aftermountWithHydration, then tick for reactive updates.🤖 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-vapor/__tests__/hydration.spec.ts` at line 5079, The test currently awaits nextTick() before asserting hydration output, which hides hydration-only regressions; change the test to assert the initial DOM state immediately after calling mountWithHydration (use the same selectors/expectations used later) and only then call await nextTick() to allow post-mount effects to run and assert subsequent reactive updates—update the test around mountWithHydration and nextTick to place the initial assertion before the await.
5066-5092: ⚡ Quick winVerify anchor identity, not only serialized HTML
The current checks only validate
innerHTML. A regression that recreates anchor comments could still pass. Capture the original start/end anchor nodes and assert they are preserved across hydration and theoktoggle.Suggested test hardening
const teleportContainer = document.createElement('div') teleportContainer.id = 'teleport-empty-if' teleportContainer.innerHTML = `<!--teleport start anchor-->` + `<!--teleport anchor-->` + const startAnchor = teleportContainer.childNodes[0] + const endAnchor = teleportContainer.childNodes[1] document.body.appendChild(teleportContainer) const { container } = await mountWithHydration( '<!--teleport start--><!--teleport end-->', `<teleport to="#teleport-empty-if"> <span v-if="data.ok">{{data.msg}}</span> </teleport>`, data, ) - await nextTick() expect(container.innerHTML).toBe( `<!--teleport start--><!--teleport end-->`, ) expect(teleportContainer.innerHTML).toBe( `<!--teleport start anchor--><!--if--><!--teleport anchor-->`, ) + expect(teleportContainer.childNodes[0]).toBe(startAnchor) + expect(teleportContainer.childNodes[2]).toBe(endAnchor) data.value.ok = true await nextTick() expect(teleportContainer.innerHTML).toBe( `<!--teleport start anchor--><span>foo</span><!--if--><!--teleport anchor-->`, ) + expect(teleportContainer.childNodes[0]).toBe(startAnchor) + expect(teleportContainer.childNodes[3]).toBe(endAnchor)🤖 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-vapor/__tests__/hydration.spec.ts` around lines 5066 - 5092, Store references to the original anchor comment nodes and assert their identity instead of only checking innerHTML: after creating teleportContainer and before calling mountWithHydration, capture the start/end anchor nodes (e.g., via teleportContainer.firstChild / lastChild or by finding the comment nodes), then after mountWithHydration and after toggling data.value.ok (and awaiting nextTick), assert the captured nodes are the very same nodes (using reference equality or Node.isSameNode) to ensure anchors were not recreated; keep the existing innerHTML assertions but add these identity checks around the same spots where teleportContainer.innerHTML is currently asserted.packages/runtime-vapor/src/apiCreateIf.ts (1)
57-62: 💤 Low valueHydration cursor entry inside renderEffect relies on synchronous first execution.
The cursor is entered inside the
renderEffectcallback but exited outside it (line 86). This works because Vue'srenderEffectexecutes synchronously on its first run during hydration. While this is correct behavior, the implicit dependency on synchronous execution is subtle.🤖 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-vapor/src/apiCreateIf.ts` around lines 57 - 62, The hydration cursor is entered inside the renderEffect callback via enterHydrationCursor (when isHydrating is true) but exited outside it, relying implicitly on renderEffect's first-run being synchronous; fix this by pairing entry and exit within the same synchronous execution context: either move enterHydrationCursor out of the renderEffect so you call it immediately before invoking renderEffect (and keep the matching exit where it currently is), or ensure the matching exit occurs inside the same renderEffect callback (so both enterHydrationCursor and its corresponding exitHydrationCursor call are colocated); update the code around renderEffect, enterHydrationCursor and the existing exit call to guarantee the enter/exit happen in the same sync execution.
🤖 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/runtime-vapor/src/apiCreateFor.ts`:
- Around line 93-95: The hydration cursor entered with
enterHydrationCursor(true) (assigned to hydrationCursor) must always be released
even if an exception occurs: wrap the code after the cursor is entered in a
try/finally and in the finally check if hydrationCursor is non-null and call its
release/unwind method (or call exitHydrationCursor/hydrationCursor.exit as
appropriate) to ensure global cursor state is restored; update both the
initial-entry site (where hydrationCursor is set) and the similar block around
lines 560–565 to use the same try/finally pattern so hydrateList()/renderItem()
exceptions cannot leak the cursor.
---
Outside diff comments:
In `@packages/runtime-vapor/src/apiCreateFragment.ts`:
- Around line 30-43: Wrap the code path that captures and later exits the
hydration cursor in a try/finally so exitHydrationCursor(hydrationCursor) is
always called even if renderEffect(() => frag.update(render, key())) throws;
specifically, when isHydrating is true call captureHydrationCursor() into
hydrationCursor, then call renderEffect as before inside the try block, and call
exitHydrationCursor(hydrationCursor) in the finally block (while keeping the
existing insertion logic for the non-hydrating branch); reference
functions/variables: captureHydrationCursor, hydrationCursor, renderEffect,
frag.update, exitHydrationCursor, and isHydrating.
In `@packages/runtime-vapor/src/componentSlots.ts`:
- Around line 193-213: Slot hydration cursor (hydrationCursor) is only released
on the happy path; wrap the slot-resolution and subsequent fragment.hydrate()
logic (the block that may call enterHydrationCursor() or
captureHydrationCursor() and creates fragment/slotFragment) in a try/finally and
in the finally release/reset the cursor if set (i.e., call the appropriate
cursor cleanup function used elsewhere in this module after
enterHydrationCursor()/captureHydrationCursor()), so that hydrationCursor is
always cleared even if slot resolution or fragment.hydrate() throws; reference
hydrationCursor, enterHydrationCursor(), captureHydrationCursor(), and
fragment.hydrate() to locate the code to wrap.
---
Nitpick comments:
In `@packages/runtime-vapor/__tests__/hydration.spec.ts`:
- Line 5079: The test currently awaits nextTick() before asserting hydration
output, which hides hydration-only regressions; change the test to assert the
initial DOM state immediately after calling mountWithHydration (use the same
selectors/expectations used later) and only then call await nextTick() to allow
post-mount effects to run and assert subsequent reactive updates—update the test
around mountWithHydration and nextTick to place the initial assertion before the
await.
- Around line 5066-5092: Store references to the original anchor comment nodes
and assert their identity instead of only checking innerHTML: after creating
teleportContainer and before calling mountWithHydration, capture the start/end
anchor nodes (e.g., via teleportContainer.firstChild / lastChild or by finding
the comment nodes), then after mountWithHydration and after toggling
data.value.ok (and awaiting nextTick), assert the captured nodes are the very
same nodes (using reference equality or Node.isSameNode) to ensure anchors were
not recreated; keep the existing innerHTML assertions but add these identity
checks around the same spots where teleportContainer.innerHTML is currently
asserted.
In `@packages/runtime-vapor/src/apiCreateIf.ts`:
- Around line 57-62: The hydration cursor is entered inside the renderEffect
callback via enterHydrationCursor (when isHydrating is true) but exited outside
it, relying implicitly on renderEffect's first-run being synchronous; fix this
by pairing entry and exit within the same synchronous execution context: either
move enterHydrationCursor out of the renderEffect so you call it immediately
before invoking renderEffect (and keep the matching exit where it currently is),
or ensure the matching exit occurs inside the same renderEffect callback (so
both enterHydrationCursor and its corresponding exitHydrationCursor call are
colocated); update the code around renderEffect, enterHydrationCursor and the
existing exit call to guarantee the enter/exit happen in the same sync
execution.
🪄 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: b7564653-1416-4d54-81b1-7dd4740017c9
⛔ Files ignored due to path filters (9)
packages/compiler-vapor/__tests__/__snapshots__/compile.spec.ts.snapis excluded by!**/*.snappackages/compiler-vapor/__tests__/transforms/__snapshots__/TransformTransition.spec.ts.snapis excluded by!**/*.snappackages/compiler-vapor/__tests__/transforms/__snapshots__/logicalIndex.spec.ts.snapis excluded by!**/*.snappackages/compiler-vapor/__tests__/transforms/__snapshots__/transformChildren.spec.ts.snapis excluded by!**/*.snappackages/compiler-vapor/__tests__/transforms/__snapshots__/transformKey.spec.ts.snapis excluded by!**/*.snappackages/compiler-vapor/__tests__/transforms/__snapshots__/vFor.spec.ts.snapis excluded by!**/*.snappackages/compiler-vapor/__tests__/transforms/__snapshots__/vIf.spec.ts.snapis excluded by!**/*.snappackages/compiler-vapor/__tests__/transforms/__snapshots__/vOnce.spec.ts.snapis excluded by!**/*.snappackages/compiler-vapor/__tests__/transforms/__snapshots__/vSlot.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (20)
packages/compiler-vapor/__tests__/compile.spec.tspackages/compiler-vapor/__tests__/transforms/logicalIndex.spec.tspackages/compiler-vapor/__tests__/transforms/transformChildren.spec.tspackages/compiler-vapor/src/generators/operation.tspackages/compiler-vapor/src/ir/index.tspackages/compiler-vapor/src/transforms/transformChildren.tspackages/runtime-vapor/__tests__/componentSlots.spec.tspackages/runtime-vapor/__tests__/components/Teleport.spec.tspackages/runtime-vapor/__tests__/customElement.spec.tspackages/runtime-vapor/__tests__/hydration.spec.tspackages/runtime-vapor/__tests__/scopeId.spec.tspackages/runtime-vapor/src/apiCreateDynamicComponent.tspackages/runtime-vapor/src/apiCreateFor.tspackages/runtime-vapor/src/apiCreateFragment.tspackages/runtime-vapor/src/apiCreateIf.tspackages/runtime-vapor/src/component.tspackages/runtime-vapor/src/componentSlots.tspackages/runtime-vapor/src/dom/hydration.tspackages/runtime-vapor/src/fragment.tspackages/runtime-vapor/src/insertionState.ts
💤 Files with no reviewable changes (1)
- packages/compiler-vapor/src/ir/index.ts
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 by CodeRabbit
Release Notes
Refactor
Tests