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
Fragment diff issue #2551
Fragment diff issue #2551
Conversation
|
Size Change: +324 B (0%) Total Size: 40 kB
|
| Filename | Size | Change | |
|---|---|---|---|
compat/dist/compat.js |
3.2 kB | 0 B | |
compat/dist/compat.module.js |
3.23 kB | 0 B | |
compat/dist/compat.umd.js |
3.25 kB | 0 B | |
debug/dist/debug.js |
2.99 kB | 0 B | |
debug/dist/debug.module.js |
2.98 kB | 0 B | |
debug/dist/debug.umd.js |
3.08 kB | 0 B | |
devtools/dist/devtools.js |
184 B | 0 B | |
devtools/dist/devtools.module.js |
195 B | 0 B | |
devtools/dist/devtools.umd.js |
260 B | 0 B | |
hooks/dist/hooks.js |
1.09 kB | 0 B | |
hooks/dist/hooks.module.js |
1.11 kB | 0 B | |
hooks/dist/hooks.umd.js |
1.17 kB | 0 B | |
test-utils/dist/testUtils.js |
437 B | 0 B | |
test-utils/dist/testUtils.module.js |
439 B | 0 B | |
test-utils/dist/testUtils.umd.js |
515 B | 0 B |
|
Nice description - thanks for writing that up! I think much the size savings come from converting Should we revert 8fca6c9 if it doesn't actually change the size? Just thinking we shouldn't give contributors the wrong idea about let/const unless it does affect size. |
|
Yes, we should probably rever that one |
This reverts commit 8fca6c9.
Thanks for pushing this through! Was definitely an interesting fix :)
Co-authored-by: Andre Wiggins <459878+andrewiggins@users.noreply.github.com>
This is amazing! Really impressed with both of you @JoviDeCroock and @andrewiggins on narrowing this down
Fixes #2527
The issue at hand: when updating components with children who are strictly equal or implement SCU they could have reordered due to the parent rendering. This works when the children are just DOM-nodes but becomes an issue when these children are Fragments or nested Fragments. We need to update
_nextDomso the parentsdiffChildrencan know where to continue after it has diffed this child, so for instance when we see the following:The
_nextDomproperty should signal todiffChildren(Parent)that after the diff ofchildren.Childit has to place the<div />after the last dom-property wrapped by theFragmentin this case<div>2</div>, but what happens if ourChildbails out of render, and returnsprops.children? These children could be reordered, … by the Parent component. This means that our dom-reference could be stale. When we aren’t bailing out of render theplaceChildfunction handles this by tracking_nextDomand reordering everything however when we bail out there is no such logic. For this we abstracted out theplaceChildlogic and reused it inSCU/strict-equality bailout, this deeply traverses the DOM when it sees that it isn’t equal and does the placement. This is still an optimized path as compared to non-scu/non-bailout.I guess we could introduce better bailouts but up to now I'm not entirely sure how yet.