fix: never lose nodes the parser relocates behind the walk frontier - #22
Merged
Conversation
added 3 commits
July 24, 2026 16:49
…nted node across chunks Also: launch Chrome in beforeAll for the View Transitions describe (bun >=1.3 fails tests whose hooks throw; 1.2.9 swallowed the undefined browser).
…skip ignored nodes by sibling Three related walker fixes: - shouldIgnoreNode hopped by the walk field, so an ignored FIRST child made the walker descend into the ignored node's subtree instead of skipping to its sibling. - The HTML parser can move a late chunk's nodes BEFORE positions the walker already passed (table foster parenting): diff() now runs one settled reconciliation pass over the fully-parsed document (no waits, callbacks not replayed) so those nodes always land. Converged trees make it a no-op. - setAttributes stole Attr nodes from the streamed tree (setNamedItemNS moves them), which made the settled pass see them as removed; new attributes are cloned instead.
An unconditional second pass fought DOM mutations made between insertion and stream end by the page's own code (custom element connectedCallback, scripts). The stream walker now tracks visited nodes (clone-inserted subtrees count as walked) and the reconciliation only runs when the settled document contains nodes the walk never saw — fully-walked pages skip it entirely.
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.
Failing tests first (previous commit), then the fix. Three related walker bugs:
shouldIgnoreNodedescended into an ignored first child. The skip loop hopped by the walk field (firstChild), so an ignored node's content was walked as if it were the parent's child list — losing every following sibling. It now hops bynextSibling. (Likely related to Web Component not loading when navigating with SPA brisa#739-class reports.)Chunk-boundary loss via parser reordering (the "hard to reproduce in a test" case). The HTML parser can relocate a late chunk's nodes before positions the walker already passed — e.g. table foster parenting:
…<tr><td>y</td></tr>in chunk 1, stray text in chunk 2 gets fostered before the<table>. The streamed walk can't revisit, so the node was silently dropped.diff()now runs one reconciliation pass over the settled document when the stream completes: no waits,onNextNodenot replayed, transitions not re-fired — and on converged trees it's a pure no-op (zero mutations).setAttributesstoleAttrnodes from the streamed tree (setNamedItemNSmoves the attribute), so any later comparison against the new tree saw those attributes as missing and removed them from the live DOM — the settled pass surfaced this immediately. New attributes are now cloned.Verification beyond the two new suite tests:
Also: the "Chrome View Transitions API" describe now launches Chrome in
beforeAll— bun ≥1.3 fails tests whose hooks throw (1.2.9 tolerated the lazily-assigned browser), so the suite couldn't run locally on modern bun at all.Known pre-existing (untouched):
tsc --noEmiterror in the test harness (eval(onNextNode)with possibly-undefined arg).🤖 Generated with Claude Code