Keyed children diff optimization #663
Conversation
* Detect when keyed child matches the next old child * Results in fewer "insertBefore" calls * Keyed <input> stay focuses
Codecov Report
@@ Coverage Diff @@
## master #663 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 1 1
Lines 161 166 +5
Branches 50 52 +2
=====================================
+ Hits 161 166 +5
Continue to review full report at Codecov.
|
|
I've done a little more experimenting to see how flexible this change is. As the code implies, this is an optimization for single item removals. It also handles cases like this, where single items are removed in multiple parts of the list. Here's another demonstration which is automated: https://codepen.io/SkaterDad/pen/WzZPxr I believe these are common operations in real apps (data-driven tables, dynamic forms, etc..), and are worth optimizing for. The benchmark results are a nice bonus. This change does not handle multiple consecutive items being removed, or if a non-keyed item is mixed in. The diffing will go back to its current behavior and do a bunch of "insertBefore" DOM operations. Those cases are where a more complicated diffing algorithm like the one in |
|
@jorgebucaran Do you see this going anywhere before 2.0? I've got code I'm waiting to PR for thunks/lazy VNodes which touches code in the same region. I would imagine 2.0 is touching some of the patch & resolveNode code also? I'm fine waiting, just curious what your plans are. |
|
@SkaterDad I didn't know this was blocking you, I'll get this merged and published tomorrow.
It isn't. That algo rewrite is also in the pipeline, but after 2.0. 😅 |
|
No worries! Thanks for getting to it. |
src/index.js
Outdated
| @@ -320,13 +320,22 @@ export function app(state, actions, view, container) { | |||
|
|
|||
| while (k < children.length) { | |||
| var oldKey = getKey(oldChildren[i]) | |||
| var nextOldKey = getKey(oldChildren[i + 1]) | |||
There was a problem hiding this comment.
@SkaterDad Can we inline this call with the if (newKey != null && newKey === nextOldKey) { later below?
There was a problem hiding this comment.
You bet. Looks like that shaves ~3 bytes off after gzip.
Do we want to keep those tests happy though? 🤔 |
If you don't remove the non-keyed child at that point in the code, it stays in the DOM since cleanup doesn't happen until the end and assumes the only children you need to remove are at the end of the oldChildren array. I think mixed keyed/non-keyed is a nice feature to support. I do find myself only keying specific children sometimes, and it behaves intuitively. |
|
Thank you, @SkaterDad! 🎉🎉🎉 |
|
@SkaterDad @jorgebucaran The thing I'm building is highly dependent on having non-keyed elements together with keyed elements in the same parent. If it is keyed, I don't want the algorithm to touch it, contrary if it's not the algo can figure out the most efficient way to modify the dom in order to map to the vdom. |
This hasn't changed. |
|
@SkaterDad I published 1.2.6 by the way. Sorry for the incredible delay. 🙇 |
|
Haha, no worries!
…On Wed, May 30, 2018, 7:46 PM Jorge Bucaran ***@***.***> wrote:
@SkaterDad <https://github.com/SkaterDad> I published 1.2.6 by the way.
Sorry for the incredible delay. 🙇
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#663 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AMpDr6SMDxqEXqwdgwV_AyE_JB88uDuPks5t3z1bgaJpZM4S6N7Z>
.
|
Summary of the change
preactfor the inspiration here).Benefits
Closes Keyed <input> loses focus when earlier sibling is removed #660: Keyed
<input>stays focused if siblings before it are removed (Proof on Codepen). I tried writing test cases for this, but the<input>never fired a "blur" event like it did in-browser.Better keyed list modification performance. Results in fewer "insertBefore" calls when a keyed item is removed but other items are in the same order. Open the "Details" below to see a screenshot from
JS Framework Benchmark. Row removal is 2x faster. Row swap is 5x faster.Bytes
Minified file size changes from 3.45KB to 3.51KB.
Gzipped file size changes from 1.54KB to 1.56KB (using default options in linux gzip command).