diff --git a/src/diff/children.js b/src/diff/children.js
index a0812bd1a8..9a1ac8cac9 100644
--- a/src/diff/children.js
+++ b/src/diff/children.js
@@ -180,6 +180,10 @@ function constructNewChildrenArray(
let skew = 0;
+ /** Whether any matched child was found far from its skewed index, i.e. a
+ * real reorder happened and we need to compute the minimal set of moves. */
+ let moved = false;
+
newParentVNode._children = new Array(newChildrenLength);
for (i = 0; i < newChildrenLength; i++) {
// @ts-expect-error We are reusing the childVNode variable to hold both the
@@ -289,38 +293,74 @@ function constructNewChildrenArray(
if (typeof childVNode.type != 'function') {
childVNode._flags |= INSERT_VNODE;
}
- } else if (matchingIndex != skewedIndex) {
- // When we move elements around i.e. [0, 1, 2] --> [1, 0, 2]
- // --> we diff 1, we find it at position 1 while our skewed index is 0 and our skew is 0
- // we set the skew to 1 as we found an offset.
- // --> we diff 0, we find it at position 0 while our skewed index is at 2 and our skew is 1
- // this makes us increase the skew again.
- // --> we diff 2, we find it at position 2 while our skewed index is at 4 and our skew is 2
- //
- // this becomes an optimization question where currently we see a 1 element offset as an insertion
- // or deletion i.e. we optimize for [0, 1, 2] --> [9, 0, 1, 2]
- // while a more than 1 offset we see as a swap.
- // We could probably build heuristics for having an optimized course of action here as well, but
- // might go at the cost of some bytes.
- //
- // If we wanted to optimize for i.e. only swaps we'd just do the last two code-branches and have
- // only the first item be a re-scouting and all the others fall in their skewed counter-part.
- // We could also further optimize for swaps
+ } else {
+ // Matched children are candidates for the minimal-move pass below;
+ // MATCHED on a _new_ vnode is cleared in diffChildren's placement loop.
+ childVNode._flags |= MATCHED;
+
+ // The skew adjustments keep findMatchingIndex's search centered for
+ // shift patterns (insertions/removals at the front). A match further
+ // than one position away means children were genuinely reordered:
+ // flag it so the minimal set of moves is computed below. Off-by-one
+ // matches provably keep matched old indices in increasing order, so
+ // no moves are needed for them.
if (matchingIndex == skewedIndex - 1) {
skew--;
} else if (matchingIndex == skewedIndex + 1) {
skew++;
- } else {
+ } else if (matchingIndex != skewedIndex) {
if (matchingIndex > skewedIndex) {
skew--;
} else {
skew++;
}
- // Move this VNode's DOM if the original index (matchingIndex) doesn't
- // match the new skew index (i + new skew)
- // In the former two branches we know that it matches after skewing
- childVNode._flags |= INSERT_VNODE;
+ moved = true;
+ }
+ }
+ }
+
+ if (moved) {
+ // Children were reordered: mark the minimal set of matched (MATCHED flag)
+ // children for insertion by finding the longest increasing subsequence of
+ // old indices (patience sorting). Children on the subsequence stay in
+ // place, all others get INSERT_VNODE. `_index` still holds the
+ // matchingIndex here.
+ /** @type {number[]} tails[x] is the smallest old index ending an increasing subsequence of length x+1 */
+ let tails = [];
+ /** @type {number[]} length of the longest increasing subsequence ending at child i */
+ let lisLengths = [];
+ for (i = 0; i < newChildrenLength; i++) {
+ childVNode = newParentVNode._children[i];
+ if (childVNode && childVNode._flags & MATCHED) {
+ // Binary search for the insertion point, keeping the pass at
+ // O(n log n) even for pathological reorders.
+ let lo = 0,
+ hi = tails.length;
+ while (lo < hi) {
+ const mid = (lo + hi) >> 1;
+ if (tails[mid] < childVNode._index) {
+ lo = mid + 1;
+ } else {
+ hi = mid;
+ }
+ }
+ tails[lo] = childVNode._index;
+ lisLengths[i] = lo + 1;
+ }
+ }
+
+ // `skew` is dead after the main loop; reuse it as the remaining
+ // subsequence length while walking backwards. Likewise `i` is left at
+ // newChildrenLength by the loop above.
+ skew = tails.length;
+ while (i--) {
+ if (lisLengths[i]) {
+ if (lisLengths[i] == skew) {
+ skew--;
+ } else {
+ newParentVNode._children[i]._flags |= INSERT_VNODE;
+ }
}
}
}
diff --git a/test/browser/fragments.test.jsx b/test/browser/fragments.test.jsx
index 0f8cd6ed1d..0b11ac6031 100644
--- a/test/browser/fragments.test.jsx
+++ b/test/browser/fragments.test.jsx
@@ -803,9 +803,8 @@ describe('Fragment', () => {
expect(scratch.innerHTML).to.equal(htmlForFalse);
expectDomLogToBe(
[
- '
barHellobeep.insertBefore(
bar,
beep)',
- '
Hellobarbeep.appendChild(
Hello)',
- '
barbeepHello.appendChild(
bar)'
+ '
fooHellobeep.insertBefore(
beep,
foo)',
+ '
beepfooHello.insertBefore(
Hello,
foo)'
],
'rendering true to false'
);
@@ -817,8 +816,9 @@ describe('Fragment', () => {
expect(scratch.innerHTML).to.equal(htmlForTrue);
expectDomLogToBe(
[
- '
beepHellofoo.appendChild(
Hello)',
- '
boopfooHello.appendChild(
boop)'
+ '
beepHellofoo.insertBefore(
foo,
beep)',
+ '
foobeepHello.insertBefore(
foo,
beep)',
+ '
foobeepHello.insertBefore(
Hello,
beep)'
],
'rendering false to true'
);
@@ -1594,9 +1594,8 @@ describe('Fragment', () => {
expectDomLogToBe(
[
'
boop.remove()',
- '
barHellobeep.insertBefore(
bar,
beep)',
- '
Hellobarbeep.appendChild(
Hello)',
- '
barbeepHello.appendChild(
bar)'
+ '
fooHellobeep.insertBefore(
beep,
foo)',
+ '
beepfooHello.insertBefore(
Hello,
foo)'
],
'rendering from true to false'
);
@@ -1611,8 +1610,9 @@ describe('Fragment', () => {
);
expectDomLogToBe(
[
- '
beepHellofoo.appendChild(
Hello)',
- '
boopfooHello.appendChild(
boop)',
+ '
beepHellofoo.insertBefore(
foo,
beep)',
+ '
foobeepHello.insertBefore(
foo,
beep)',
+ '
foobeepHello.insertBefore(
Hello,
beep)',
'
.appendChild(#text)',
'
fooHelloboop.appendChild(
boop)'
],
@@ -1680,8 +1680,8 @@ describe('Fragment', () => {
);
expectDomLogToBe(
[
- '
barHellobeepbeepbeep.insertBefore(
bar,
beep)',
- '
Hellobarbeepbeepbeep.appendChild(
Hello)',
+ '
fooHellobeepbeepbeep.appendChild(
Hello)',
+ '
barbeepbeepbeepHello.appendChild(
Hello)',
'
barbeepbeepbeepHello.appendChild(
bar)'
],
'rendering from true to false'
@@ -1697,8 +1697,8 @@ describe('Fragment', () => {
);
expectDomLogToBe(
[
- '
beepbeepbeepHellofoo.appendChild(
Hello)',
- '
beepbeepbeepfooHello.insertBefore(
foo,
beep)',
+ '
beepbeepbeepHellofoo.insertBefore(
foo,
beep)',
+ '
foobeepbeepbeepHello.insertBefore(
foo,
beep)',
'
foobeepbeepbeepHello.insertBefore(
Hello,
beep)'
],
'rendering from false to true'
diff --git a/test/browser/keys.test.jsx b/test/browser/keys.test.jsx
index 54a672da8a..2202bf0be2 100644
--- a/test/browser/keys.test.jsx
+++ b/test/browser/keys.test.jsx
@@ -351,7 +351,7 @@ describe('keys', () => {
render(
, scratch);
expect(scratch.textContent).to.equal('ba');
- expect(getLog()).to.deep.equal(['
ab.appendChild(- a)']);
+ expect(getLog()).to.deep.equal(['
ab.insertBefore(- b,
- a)']);
});
it('should swap existing keyed children in the middle of a list efficiently', () => {
@@ -367,7 +367,7 @@ describe('keys', () => {
render(
, scratch);
expect(scratch.textContent).to.equal('acbd', 'initial swap');
expect(getLog()).to.deep.equal(
- ['abcd.insertBefore(- b,
- d)'],
+ ['
abcd.insertBefore(- c,
- b)'],
'initial swap'
);
@@ -378,11 +378,176 @@ describe('keys', () => {
render(
, scratch);
expect(scratch.textContent).to.equal('abcd', 'swap back');
expect(getLog()).to.deep.equal(
- ['acbd.insertBefore(- c,
- d)'],
+ ['
acbd.insertBefore(- b,
- c)'],
'swap back'
);
});
+ it('should displace multiple keyed children to the end efficiently', () => {
+ const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal('abcdefghij');
+
+ values.push(...values.splice(0, 3));
+ clearLog();
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal('defghijabc');
+ expect(getLog()).to.deep.equal([
+ 'abcdefghij.appendChild(- a)',
+ '
bcdefghija.appendChild(- b)',
+ '
cdefghijab.appendChild(- c)'
+ ]);
+ });
+
+ it('should not displace when the suffix after the match is shorter than the jump', () => {
+ const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal('abcdefghij');
+
+ // Swap two far apart children; only the two swapped children are out of
+ // order, so only those two may move.
+ [values[1], values[8]] = [values[8], values[1]];
+ clearLog();
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal('aicdefghbj');
+ expect(getLog()).to.deep.equal([
+ 'abcdefghij.insertBefore(- i,
- b)',
+ '
aibcdefghj.insertBefore(- b,
- j)'
+ ]);
+ });
+
+ it('should move the shorter suffix when more than half the list is displaced', () => {
+ const values = ['a', 'b', 'c', 'd', 'e', 'f'];
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal('abcdef');
+
+ // Displacing 4 of 6 children: moving the two-child suffix is the
+ // minimal set of moves.
+ values.push(...values.splice(0, 4));
+ clearLog();
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal('efabcd');
+ expect(getLog()).to.deep.equal([
+ 'abcdef.insertBefore(- e,
- a)',
+ '
eabcdf.insertBefore(- f,
- a)'
+ ]);
+ });
+
+ it('should displace multiple keyed children to the end while the list grows', () => {
+ const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal('abcdefghij');
+
+ values.push(...values.splice(0, 3));
+ values.push('k');
+ clearLog();
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal('defghijabck');
+ expect(getLog()).to.deep.equal([
+ 'abcdefghij.appendChild(- a)',
+ '
bcdefghija.appendChild(- b)',
+ '
cdefghijab.appendChild(- c)',
+ '
- .appendChild(#text)',
+ '
defghijabc.appendChild(- k)'
+ ]);
+ });
+
+ it('should displace multiple keyed children to the end while another is removed', () => {
+ const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal('abcdefghij');
+
+ values.push(...values.splice(0, 3));
+ values.splice(values.indexOf('j'), 1);
+ clearLog();
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal('defghiabc');
+ expect(getLog()).to.deep.equal([
+ ' - j.remove()',
+ '
abcdefghi.appendChild(- a)',
+ '
bcdefghia.appendChild(- b)',
+ '
cdefghiab.appendChild(- c)'
+ ]);
+ });
+
+ it('should displace keyed children to the end repeatedly', () => {
+ const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+ const expectedDisplaceLogs = [
+ [
+ '
abcdefghij.appendChild(- a)',
+ '
bcdefghija.appendChild(- b)',
+ '
cdefghijab.appendChild(- c)'
+ ],
+ [
+ '
defghijabc.appendChild(- d)',
+ '
efghijabcd.appendChild(- e)',
+ '
fghijabcde.appendChild(- f)'
+ ],
+ [
+ '
ghijabcdef.appendChild(- g)',
+ '
hijabcdefg.appendChild(- h)',
+ '
ijabcdefgh.appendChild(- i)'
+ ]
+ ];
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal('abcdefghij');
+
+ for (let n = 0; n < 3; n++) {
+ values.push(...values.splice(0, 3));
+ clearLog();
+
+ render(
, scratch);
+ expect(scratch.textContent).to.equal(values.join(''));
+ expect(getLog()).to.deep.equal(expectedDisplaceLogs[n], `round ${n}`);
+ }
+ });
+
+ it('should keep text siblings correct around displaced keyed children', () => {
+ const content = condition => (
+
+ {condition
+ ? [
+ - a
,
+ - b
,
+ - c
,
+ 'mid',
+ - d
,
+ - e
+ ]
+ : [
+ - c
,
+ 'mid',
+ - a
,
+ - b
,
+ - d
,
+ - e
+ ]}
+
+ );
+
+ render(content(true), scratch);
+ expect(scratch.innerHTML).to.equal(
+ '- a
- b
- c
mid- d
- e
'
+ );
+
+ clearLog();
+ render(content(false), scratch);
+ expect(scratch.innerHTML).to.equal(
+ '- c
mid- a
- b
- d
- e
'
+ );
+ });
+
it('should move keyed children to the end of the list', () => {
const values = ['a', 'b', 'c', 'd'];
@@ -463,7 +628,7 @@ describe('keys', () => {
'jihgfabcde.insertBefore(- e,
- a)',
'
jihgfeabcd.insertBefore(- d,
- a)',
'
jihgfedabc.insertBefore(- c,
- a)',
- '
jihgfedcab.appendChild(- a)'
+ '
jihgfedcab.insertBefore(- b,
- a)'
]);
});
diff --git a/test/browser/lifecycles/shouldComponentUpdate.test.jsx b/test/browser/lifecycles/shouldComponentUpdate.test.jsx
index 58cf7d1762..ac483ef4af 100644
--- a/test/browser/lifecycles/shouldComponentUpdate.test.jsx
+++ b/test/browser/lifecycles/shouldComponentUpdate.test.jsx
@@ -1068,7 +1068,7 @@ describe('Lifecycle methods', () => {
items: [7, 6, 5, 4, 3, 2, 1],
expectedLog: [
'
7634521.insertBefore(
5,
3)',
- '
7653421.insertBefore(
3,
2)'
+ '
7653421.insertBefore(
4,
3)'
]
});
});
diff --git a/test/browser/render.test.jsx b/test/browser/render.test.jsx
index 93b69e3b9b..1fb9dff4c7 100644
--- a/test/browser/render.test.jsx
+++ b/test/browser/render.test.jsx
@@ -1754,10 +1754,10 @@ describe('render()', () => {
expect(getLog()).to.deep.equal([
'
.appendChild(#text)',
'
1352640.insertBefore(
11,
1)',
- '
111352640.insertBefore(
1,
5)',
- '
113152640.insertBefore(
6,
0)',
- '
113152460.insertBefore(
2,
0)',
- '
113154620.insertBefore(
5,
0)',
+ '
111352640.insertBefore(
3,
1)',
+ '
113152640.insertBefore(
4,
5)',
+ '
113145260.insertBefore(
6,
5)',
+ '
113146520.insertBefore(
2,
5)',
'
.appendChild(#text)',
'
113146250.appendChild(
9)',
'
.appendChild(#text)',