refactor(runtime-vapor): split fragment.ts into domain modules#14951
Merged
Conversation
Pure code motion, no behavior change. fragment.ts had grown into a 1372-line integrator mixing the core branch pipeline with slot boundary ambient state, the slot fallback state machine, and hydration slot-boundary cursor state. Split by domain: - slotBoundary.ts: SlotBoundaryContext + currentSlotBoundary ambient state, trackSlotBoundaryDirtying, hasSlotFallback (leaf module) - slotFragment.ts: the SlotFallbackState machine (render/commit/ recheck/dispose) + detachBlock + getRedirectedBoundary - dom/hydrateFragment.ts: hydrating slot boundary cursor state (withHydratingSlotBoundary, getCurrentSlotEndAnchor, isHydratingSlotFallbackActive et al), which is hydration-cursor state shared by apiCreateFor/vdomInterop rather than slot logic - EMPTY_BLOCK moves to block.ts next to the Block types The SlotFragment class itself stays in fragment.ts: `extends DynamicFragment` reads the base class binding at module evaluation time, and componentSlots.ts (which constructs SlotFragment) sits inside fragment.ts's evaluation closure, so hoisting the class into another module can hit the base class before initialization depending on entry order. The fallback machinery is plain function declarations and therefore cycle-safe. DynamicFragment.hydrate() and its helpers also stay for now: CloseAnchorOwner is a const enum and isolatedModules forbids cross-module const enum usage; they move together in a follow-up. index.ts export surface is unchanged.
…nction Move the hydration anchor-resolution logic out of the DynamicFragment prototype into hydrateDynamicFragmentAnchor() in dom/hydrateFragment.ts, along with its helpers (CloseAnchorOwner, getDynamicCloseOwner, queueAnchorInsert, isReusableDynamicFragmentAnchor) and the deferred hydration cursor handling (prepareDeferredHydrationAnchor). fragment.ts now only value-imports the two entry points; the reverse dependency from dom/hydrateFragment.ts stays type-only. Behavior is unchanged and the optional interop/teleport `hydrate` field on VaporFragment is untouched.
… plan
Split hydrateDynamicFragmentAnchor() into a pure decision step and a side-effect step: resolveDynamicAnchor(frag, isEmpty) maps each SSR output shape to an AnchorPlan ('reuse' | 'create' | 'create-cleanup' | 'create-before-slot-end'), and executeAnchorPlan(frag, plan) performs all mutations (anchor marking, boundary cleanup, queued inserts, stale block reset). The per-path comments documenting each SSR shape stay on the corresponding resolver branch, and the resolver is covered by direct plan-level unit tests in __tests__/dom/hydrateFragment.spec.ts.
Also narrow isSlotFragment() to `val is SlotFragment` so the hydration module reads `forwarded` through the predicate instead of an `as any as SlotFragment` downcast.
Behavior is unchanged; app-only and SSR bundle sizes are byte-identical.
…ment core Replace the inline isLeaving deferral and leave-mode scheduling in DynamicFragment.update() with two registry slots (deferBranchUpdateDuringLeave / removeBranchWithLeave) implemented in components/Transition.ts, following the existing registerTransitionHooks pattern. The now-unused applyTransitionLeaveHooks slot is removed; the impl is called directly within the Transition module. The pending field stays on DynamicFragment but is now exclusively owned by the Transition module.
…Branch Narrow the KeepAlive surface that DynamicFragment.renderBranch needs to understand down to two intent-revealing context methods: - rename VaporKeepAliveContext.getScope to acquireBranchScope (pre-render kept-alive scope reuse, implementation unchanged) - add runBranchRender(frag, fn): owns the keyed cache-key context and marks shape flags once the render settles, replacing the inline withCurrentCacheKey dispatch and the processShapeFlag call in the render finally block The cache-key wrapping range and the setBlockKey -> applyTransitionHooks -> processShapeFlag order are preserved. withCurrentCacheKey disappears from core imports; processShapeFlag stays on the interface for the component / interop boundary-isolation callers. keepAlive.ts gains a type-only DynamicFragment import (no new runtime cycle).
… core SlotFragment now hooks into DynamicFragment.update() through explicit extension points instead of the core pipeline branching on slot state: - drop the shouldInsert 4th parameter of update() (its only caller was SlotFragment.updateContent). The insertion gate is now an overridable getBranchParent(): the base class returns anchor.parentNode and SlotFragment returns null while its fallback is active, which is bit-equivalent to the old flag (no removal/insertion of the detached content, same notifyUpdated condition). update() is back to (render?, key?, noScope?), shrinking the signature contract the vShow monkey-patch forwards. - replace the two `isHydrating && !this.isSlot` reads with an autoHydrate constructor parameter (default true). SlotFragment opts out since updateSlot owns its hydration timing. isSlot is demoted to a pure marker consumed only by the isSlotFragment predicate. Other update() entry points on SlotFragment (updateSlot via updateContent) and the plain DynamicFragment callers in componentSlots/component were audited; behavior is unchanged.
…d fragment subclass
|
Important Review skippedDraft detected. 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 |
Size ReportBundles
Usages
|
@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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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-vapor/src/fragment.ts (1)
137-166:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
runWithFragmentCtx()no longer restorescurrentInstance.Direct deferred-render callers still use this exported helper as the fragment-context wrapper. In
packages/runtime-vapor/src/vdomInterop.ts:1557-1718, slot rendering goes throughrunWithFragmentCtx(frag, ...); after this refactor that path only restores slot-owner / slot-boundary / keep-alive state, so it can now run under the ambient instance (ornull) instead offragment.renderInstance. Please either move the instance swap into the exported helper or switch those call sites to a public wrapper that restores the full render context.🤖 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/fragment.ts` around lines 137 - 166, runWithFragmentCtx currently restores slot-owner, slot-boundary, and keep-alive state but omits restoring currentInstance (so callers like vdomInterop that rely on fragment.renderInstance run under the wrong ambient instance). Fix by saving and restoring the instance around fn: capture prevInstance (via the existing setCurrentInstance / setCurrentRenderInstance helper that manages currentInstance) before applying fragment.renderInstance, set currentInstance to fragment.renderInstance when entering, and restore prevInstance in the finally block; alternatively update all call sites that call runWithFragmentCtx(frag, ...) (e.g., vdomInterop code paths) to use a new public wrapper that sets and restores the full render context including currentInstance instead of calling runWithFragmentCtx directly. Ensure you reference runWithFragmentCtx, fragment.renderInstance, currentInstance and the setCurrentInstance helper when making the change.
🤖 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-vapor/src/fragment.ts`:
- Around line 137-166: runWithFragmentCtx currently restores slot-owner,
slot-boundary, and keep-alive state but omits restoring currentInstance (so
callers like vdomInterop that rely on fragment.renderInstance run under the
wrong ambient instance). Fix by saving and restoring the instance around fn:
capture prevInstance (via the existing setCurrentInstance /
setCurrentRenderInstance helper that manages currentInstance) before applying
fragment.renderInstance, set currentInstance to fragment.renderInstance when
entering, and restore prevInstance in the finally block; alternatively update
all call sites that call runWithFragmentCtx(frag, ...) (e.g., vdomInterop code
paths) to use a new public wrapper that sets and restores the full render
context including currentInstance instead of calling runWithFragmentCtx
directly. Ensure you reference runWithFragmentCtx, fragment.renderInstance,
currentInstance and the setCurrentInstance helper when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf25dfc9-d56a-4d3b-b7e2-5fac90a74002
📒 Files selected for processing (15)
packages/runtime-vapor/__tests__/componentSlots.spec.tspackages/runtime-vapor/__tests__/dom/hydrateFragment.spec.tspackages/runtime-vapor/src/apiCreateFor.tspackages/runtime-vapor/src/block.tspackages/runtime-vapor/src/componentSlots.tspackages/runtime-vapor/src/components/KeepAlive.tspackages/runtime-vapor/src/components/Teleport.tspackages/runtime-vapor/src/components/Transition.tspackages/runtime-vapor/src/dom/hydrateFragment.tspackages/runtime-vapor/src/fragment.tspackages/runtime-vapor/src/keepAlive.tspackages/runtime-vapor/src/slotBoundary.tspackages/runtime-vapor/src/slotFragment.tspackages/runtime-vapor/src/transition.tspackages/runtime-vapor/src/vdomInterop.ts
…d hydration adoptions
…ntics During hydration, prop setters now adopt server-rendered values outright instead of re-writing them: - dev / __FEATURE_PROD_HYDRATION_MISMATCH_DETAILS__: mismatches are warned but never patched, matching vdom's hydrateElement props loop (warn-only; SSR values are preserved) - pure prod: trust-skip with zero DOM reads/writes, only warming the per-element caches so the first post-hydration update diffs correctly - force carve-outs preserved: input/option value & indeterminate and v-bind .prop modifiers still write through - v-html (setHtml/setBlockHtml): server-rendered innerHTML is trusted as-is in all builds (vdom never compares nor writes innerHTML during hydration)
When handleMismatch rebuilds a node from the client template, the server never rendered it, but its render effects still run with isHydrating === true, so prop setters stay check-only (warn + cache, no DOM write). Dynamic attrs/props/class/style were silently dropped, and the polluted cache could skip later updates with the same value. Mark recreated nodes and their template-born descendants with $rcn in the mismatch path, before adopting server children so adopted content keeps normal check-only semantics. Hydration-mode prop setters now write through on marked nodes, matching vdom's "mismatch recovery = full client mount" behavior. Text inside recreated nodes no longer emits the redundant "Hydration text mismatch" warning — the node mismatch was already reported, aligning with vdom's single-warning behavior.
Mirror VDOM's same-vnode guard so keys like 1 and '1' do not collide through the String($key) leaving-cache slot. Also simplify an equivalent in-out branch condition.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/runtime-vapor/src/dom/prop.ts (1)
504-513:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrite block HTML on recreated hydration nodes.
setHtml()skips the trust-SSR fast path for recreated nodes, butsetBlockHtml()returns for every hydration block. If mismatch recovery recreated the target block, this cachesvaluewithout writing the clientinnerHTML, leaving the newly created DOM stale.Suggested fix
export function setBlockHtml( block: Block & { $html?: string }, value: any, ): void { value = value == null ? '' : unsafeToTrustedHTML(value) // trust SSR content during hydration, see setHtml - if (isHydrating) { + if (isHydrating && !normalizeBlock(block).some(isRecreatedNode)) { block.$html = value return } if (block.$html !== value) { setHtmlToBlock(block, (block.$html = value)) } }🤖 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/dom/prop.ts` around lines 504 - 513, The setBlockHtml() function returns early during hydration without checking if the block was recreated. If mismatch recovery recreated the target block, this leaves the newly created DOM stale because the innerHTML is never written. Modify the hydration check in setBlockHtml() to verify whether the block is a recreated node (similar to how setHtml() handles this scenario), and only return early if it is not recreated; for recreated nodes, allow the function to continue and write the HTML content to ensure the client DOM is properly updated.
🤖 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/__tests__/components/Transition.spec.ts`:
- Around line 556-557: Replace the conditional invocation pattern of the
leaveDone callback with explicit assertion-then-invocation to ensure tests fail
deterministically if the callback is undefined. At
packages/runtime-vapor/__tests__/components/Transition.spec.ts lines 556-557,
replace the optional call leaveDone && leaveDone() with
expect(leaveDone).toBeDefined(); leaveDone!(). Apply the same fix at
packages/runtime-vapor/__tests__/components/Transition.spec.ts lines 587-588, at
packages/runtime-vapor/__tests__/components/Transition.spec.ts lines 821-823,
and at packages/runtime-vapor/__tests__/components/TransitionGroup.spec.ts lines
273-274, replacing each instance of the optional leaveDone invocation pattern
with the assertion followed by non-null assertion invocation.
In `@packages/runtime-vapor/__tests__/hydration.spec.ts`:
- Around line 7200-7214: The console.warn spy created at line 7200 is never
restored, which can cause cross-test leakage in subsequent tests. Additionally,
the assertion at line 7213 uses call.includes() to check array membership, but
this misses substring matches when the warning payload is a single concatenated
string. Restore the warn spy after the test completes (add warn.mockRestore() in
the finally block or after the test), and fix the substring check by verifying
that the actual warning message string (accessed via call[0] or call.join())
contains the substring '(start of fragment)' instead of checking array
membership.
---
Outside diff comments:
In `@packages/runtime-vapor/src/dom/prop.ts`:
- Around line 504-513: The setBlockHtml() function returns early during
hydration without checking if the block was recreated. If mismatch recovery
recreated the target block, this leaves the newly created DOM stale because the
innerHTML is never written. Modify the hydration check in setBlockHtml() to
verify whether the block is a recreated node (similar to how setHtml() handles
this scenario), and only return early if it is not recreated; for recreated
nodes, allow the function to continue and write the HTML content to ensure the
client DOM is properly updated.
🪄 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: f053f136-0551-4410-8697-c32de832c22b
📒 Files selected for processing (11)
packages/runtime-core/src/hydration.tspackages/runtime-core/src/index.tspackages/runtime-vapor/__tests__/components/Transition.spec.tspackages/runtime-vapor/__tests__/components/TransitionGroup.spec.tspackages/runtime-vapor/__tests__/hydration.spec.tspackages/runtime-vapor/src/components/Teleport.tspackages/runtime-vapor/src/components/Transition.tspackages/runtime-vapor/src/directives/vShow.tspackages/runtime-vapor/src/dom/hydration.tspackages/runtime-vapor/src/dom/prop.tspackages/runtime-vapor/src/dom/template.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/runtime-vapor/src/components/Teleport.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
Bug Fixes
New Features
Tests