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(
  1. a)']); + expect(getLog()).to.deep.equal(['
      ab.insertBefore(
    1. b,
    2. 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(
      1. b,
      2. d)'], + ['
          abcd.insertBefore(
        1. c,
        2. 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(
          1. c,
          2. d)'], + ['
              acbd.insertBefore(
            1. b,
            2. 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(
              1. a)', + '
                  bcdefghija.appendChild(
                1. b)', + '
                    cdefghijab.appendChild(
                  1. 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(
                    1. i,
                    2. b)', + '
                        aibcdefghj.insertBefore(
                      1. b,
                      2. 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(
                        1. e,
                        2. a)', + '
                            eabcdf.insertBefore(
                          1. f,
                          2. 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(
                            1. a)', + '
                                bcdefghija.appendChild(
                              1. b)', + '
                                  cdefghijab.appendChild(
                                1. c)', + '
                                2. .appendChild(#text)', + '
                                    defghijabc.appendChild(
                                  1. 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([ + '
                                  2. j.remove()', + '
                                      abcdefghi.appendChild(
                                    1. a)', + '
                                        bcdefghia.appendChild(
                                      1. b)', + '
                                          cdefghiab.appendChild(
                                        1. 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(
                                          1. a)', + '
                                              bcdefghija.appendChild(
                                            1. b)', + '
                                                cdefghijab.appendChild(
                                              1. c)' + ], + [ + '
                                                  defghijabc.appendChild(
                                                1. d)', + '
                                                    efghijabcd.appendChild(
                                                  1. e)', + '
                                                      fghijabcde.appendChild(
                                                    1. f)' + ], + [ + '
                                                        ghijabcdef.appendChild(
                                                      1. g)', + '
                                                          hijabcdefg.appendChild(
                                                        1. h)', + '
                                                            ijabcdefgh.appendChild(
                                                          1. 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 + ? [ +
                                                            1. a
                                                            2. , +
                                                            3. b
                                                            4. , +
                                                            5. c
                                                            6. , + 'mid', +
                                                            7. d
                                                            8. , +
                                                            9. e
                                                            10. + ] + : [ +
                                                            11. c
                                                            12. , + 'mid', +
                                                            13. a
                                                            14. , +
                                                            15. b
                                                            16. , +
                                                            17. d
                                                            18. , +
                                                            19. e
                                                            20. + ]} +
                                                            + ); + + render(content(true), scratch); + expect(scratch.innerHTML).to.equal( + '
                                                            1. a
                                                            2. b
                                                            3. c
                                                            4. mid
                                                            5. d
                                                            6. e
                                                            ' + ); + + clearLog(); + render(content(false), scratch); + expect(scratch.innerHTML).to.equal( + '
                                                            1. c
                                                            2. mid
                                                            3. a
                                                            4. b
                                                            5. d
                                                            6. 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(
                                                            1. e,
                                                            2. a)', '
                                                                jihgfeabcd.insertBefore(
                                                              1. d,
                                                              2. a)', '
                                                                  jihgfedabc.insertBefore(
                                                                1. c,
                                                                2. a)', - '
                                                                    jihgfedcab.appendChild(
                                                                  1. a)' + '
                                                                      jihgfedcab.insertBefore(
                                                                    1. b,
                                                                    2. 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)',