-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ordering issue with Component re-render #4125
Conversation
📊 Tachometer Benchmark ResultsSummaryduration
usedJSHeapSize
Results02_replace1k
duration
usedJSHeapSize
run-warmup-0
run-warmup-1
run-warmup-2
run-warmup-3
run-warmup-4
run-final
03_update10th1k_x16
duration
usedJSHeapSize
07_create10k
duration
usedJSHeapSize
filter_list
duration
usedJSHeapSize
hydrate1k
duration
usedJSHeapSize
many_updates
duration
usedJSHeapSize
text_update
duration
usedJSHeapSize
todo
duration
usedJSHeapSize
|
Size Change: +30 B (0%) Total Size: 57 kB
ℹ️ View Unchanged
|
@@ -111,6 +111,7 @@ export function diffChildren( | |||
oldVNode = oldChildren[i]; | |||
if (oldVNode && oldVNode.key == null && oldVNode._dom) { | |||
if (oldVNode._dom == oldDom) { | |||
oldVNode._parent = oldParentVNode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am currently trying to figure out why this part matters but it looks like oldVNode._parent
refers to the new version of the parent-vnode here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch!!
Hoping for a release with this fix when you guys get a chance, thanks! 🙏 |
When we have a Fragment as the top-level return from a render function we assign the underlying dom-children with the rendering function as their
_parent
. This however means that when we rungetDomSibling
like we do in ournull
placeholder unmount case https://github.com/preactjs/preact/blob/main/src/diff/children.js#L114 that we will move onto the wrapping root-fragment here which can refer to a stale dom-node, in this case we should leverage the potentially set_nextDom
property to ensure we transferoldDom
back into a valid state.fixes #4123