Repository navigation
Retain and improve deferred hydration range markers - #5315
Merged
Merged
Conversation
A component that suspends while hydrating keeps the server DOM between its `<!--$s-->` and `<!--/$s-->` markers, but its vnode could only point at one node of that range. The first suspension pointed `_dom` at the closing marker and a re-suspension (any parent rerender while pending) at the first content node, so: - unmounting a pending boundary left its opening marker and server content behind (after a re-suspension it also left the rest of the content and the closing marker) - a sibling mounted after a re-suspended boundary, or before any pending one, was inserted inside the range, and since 9ba8f34 the resume removes it as unclaimed DOM while its vnode keeps the detached node - several siblings mounted at once before a pending boundary had the same problem, because insert() stepped over the opening marker into the range `_dom` is now the opening marker, the cursor continues after the closing marker, unmount removes the whole range, and insert() no longer steps into a range. The depth-balanced scan is shared with the resume path. preact 4546 -> 4597 B br (+51), 4971 -> 5038 B gz
- rangeEnd() returns nothing unless it starts at a `$s` marker, so the separate isRangeStart() helper and its guards at the call sites go away - unmount removes the range in a single guarded loop instead of if + do/while preact 4597 -> 4583 B br (-14), 5038 -> 5028 B gz
A resumed range starts its cursor at the first kept node. An empty range (`<!--$s--><!--/$s-->`, e.g. a streamed Suspense without a fallback) has none, so the resolved content was appended at the end of the parent instead of between its markers. Fall back to the end marker. preact 4583 -> 4588 B br (+5)
Once a boundary resolved, nothing referenced its `<!--$s-->` markers, so unmounting it left an empty pair behind. With streaming that isn't inert: a `<preact-island>` chunk arriving after the unmount finds the empty range and inserts the server HTML again, so unmounted content reappears. Keep `_excess` after the resume (the resume path is gated on MODE_SUSPENDED, so it is never re-entered) and remove the markers in unmount. A resolved owner only removes the two markers: its content went with its child vnodes, and after a keyed reorder the markers may surround a sibling's content. Resuming stays mutation-free. preact 4588 -> 4595 B br (+7)
📊 Tachometer Benchmark ResultsSummaryduration
usedJSHeapSize
Resultscreate10kduration
usedJSHeapSize
filter-listduration
usedJSHeapSize
hydrate1kduration
usedJSHeapSize
many-updatesduration
usedJSHeapSize
replace1kduration
usedJSHeapSize
run-warmup-0
run-warmup-1
run-warmup-2
run-warmup-3
run-warmup-4
run-final
text-updateduration
usedJSHeapSize
tododuration
usedJSHeapSize
update10th1kduration
usedJSHeapSize
|
|
Size Change: +49 B (+0.3%) Total Size: 16.4 kB 📦 View Changed
ℹ️ View Unchanged
|
JoviDeCroock
force-pushed
the
JoviDeCroock/streamed-range-ownership
branch
from
October 19, 2026 05:56
a64d1e1 to
564ad29
Compare
|
coverage: 99.856% (-0.001%) from 99.857% — JoviDeCroock/streamed-range-ownership into main |
JoviDeCroock
changed the title
JoviDeCroock
changed the title
marvinhagemeister
approved these changes
Oct 19, 2026
rangeEnd() already walks the range, so let it collect the inner nodes instead of re-walking nextSibling until the end marker. The array is only allocated for components that own a range (`_excess` set). -11 B gz / -10 B br.
Merged
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
3 participants
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.
Note
Tests were written by claude, reviewed by me and fix was implemented by me
Problem
During hydration, a component that suspends owns a range of DOM, not a single node. With renderToStringAsync, the server output looks like this:
The vnode has only one pointer,
vnode._dom, plusvnode._component._excess, which points at the start marker. Each part of the diff that reads _dom expects something different.getDomSiblingexpects the first node of a range,diffChildren's cursor the last node of a range,unmountall nodes of a range andinsertwill walk_children.Fix
The vnode now owns the whole range explicitly.
_domis the$scomment marking the start of a suspending range.diff()returnsend.nextSiblingas the cursor fordiffChildren, so siblings continue after the closing marker.insert()tail stops at an$sstarting comment marker.unmountremoves only what the component owns:_excess==_dom) removes its start marker, contents and end marker;_excessis kept after a resume (it used to be cleared), so unmount can still find the markers.oldDom = excessDomChildren[0] || end.All fixed cases are in our tests. This is a follow-up to #5314 and has taken an excruciating amount of time to find all edge cases as only a few were found there and more kept popping up 😅
One thing that I did not fix that would take up a lot more bytes is a keyed reorder that moves a pending boundary, the range stays where it was. Most solutions were coming close to 100b and had some compromises. Need to think that through more.