fix: reuse DOM nodes inside <template> during hydration #4803
Conversation
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
📊 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
|
rschristian
reviewed
Jun 18, 2025
rschristian
reviewed
Jun 18, 2025
|
Size Change: +120 B (+0.26%) Total Size: 46.8 kB
ℹ️ View Unchanged
|
fix: apply precommit hook Co-authored-by: Ryan Christian <[email protected]> refactor: reorder template tag check to avoid expensive instanceof first Co-authored-by: Ryan Christian <[email protected]> fix: import slice from util file
frontman-git
force-pushed
the
fix/template-hydration-reuse-dom
branch
from
16b8b19 to
1dae489
Compare
|
Thanks for the review — commits have been squashed. |
Comment on lines
+57
to
+61
| if ( | ||
| newParentVNode.type === 'template' && | ||
| excessDomChildren?.length === 0 && | ||
| parentDom instanceof DocumentFragment | ||
| ) { |
There was a problem hiding this comment.
Let's make this fast by adding the isHydrating check, we don't need to check excessDomChildren length as we expect this to be 0 as per your assumptions in this PR. I would assume that we are in a DocumentFragment when we are dealing with a template.
Suggested change
| if ( | |
| newParentVNode.type === 'template' && | |
| excessDomChildren?.length === 0 && | |
| parentDom instanceof DocumentFragment | |
| ) { | |
| if (isHydrating && newParentVNode.type === 'template') { |
I assume you could save even more bytes by moving this to
Lines 596 to 611 in 1dae489
newVNode.type === 'template' as well as hoisting the duplicated check
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
fix 3732
When hydrating a
<template>element, Preact fails to reuse existing DOM nodes insidetemplate.content.This is because
<template>elements encapsulate their children within aDocumentFragment, but thediff/hydratelogic doesn't switch the context totemplate.content.As a result, hydration skips the existing fragment content and instead appends new nodes — leading to duplicated content in the DOM.