Iteratively diff children and convert array children to Fragments#2507
Iteratively diff children and convert array children to Fragments#2507marvinhagemeister merged 14 commits intomasterfrom
Conversation
…array in diffChildren. (+8 B) This building up _children avoids in diffChildren avoids having to copy (i.e. slice) the result of render (since we don't direclty assign to _children)
And remove commented out golf attempts
|
Size Change: +115 B (0%) Total Size: 39.4 kB
ℹ️ View Unchanged
|
| // We use `undefined`, as `null` is reserved for empty placeholders | ||
| // (holes). | ||
| oldVNode = oldChildren[i]; | ||
| if (childVNode == null || typeof childVNode == 'boolean') { |
There was a problem hiding this comment.
This big if statement is the internal VNode coercion that toChildArray did. Terser converts into a massive ternary for us :)
| export function toChildArray(children, callback, flattened) { | ||
| if (flattened == null) flattened = []; | ||
|
|
||
| export function toChildArray(children) { |
There was a problem hiding this comment.
Since this function is no longer used in the diff hot path, I rewrote it to optimize for size instead of perf/memory. Let me know what you think. The rewrite saved us about 40 bytes
| null, | ||
| childVNode | ||
| ); | ||
| } else if (Array.isArray(childVNode)) { |
There was a problem hiding this comment.
Putting Array.isArray into a utility function added a byte. Using a .pop property check saves us some bytes (~10) but I worry that the property access will quickly de-opt and become megamorphic since it'll sometimes be a VNode and sometimes by an array. So I think imma leave this for now. Can revisit later
There was a problem hiding this comment.
Good point! iirc checking for .pop() is much slower than Array.isArray() 👍
There was a problem hiding this comment.
Yep. It used to be the other way around. Not sure about other engines.
JoviDeCroock
left a comment
There was a problem hiding this comment.
Awesome, closing my PR. This is some fine work!
At the suggestion from @developit from forever ago, this PR no longer uses
toChildArrayin diffChildren to convertprops.childrento an array. It also no longer flattens nested arrays when diffing children. It now converts them to Fragments.I think this change will help us improve diff perf in the future and it also fixes #2446.