diff --git a/src/diff/children.js b/src/diff/children.js index 90c5845759..46823e3d1d 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -302,16 +302,27 @@ function constructNewChildrenArray( skew--; } else if (matchingIndex == skewedIndex + 1) { skew++; - } else { - if (matchingIndex > skewedIndex) { - skew--; + } else if (matchingIndex > skewedIndex) { + if ( + matchingIndex - skewedIndex < oldChildrenLength - matchingIndex && + renderResult[i + 1] != NULL && + oldChildren[matchingIndex + 1] != NULL && + renderResult[i + 1].key == oldChildren[matchingIndex + 1].key && + renderResult[i + 1].type == oldChildren[matchingIndex + 1].type + ) { + // A short prefix was moved to the end. The match starts a longer + // contiguous suffix, so keep that suffix in place and insert the + // displaced prefix when reconciliation reaches it. + skew += matchingIndex - skewedIndex; } else { - skew++; - } + 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 + // Move this VNode's DOM if the original index (matchingIndex) + // doesn't match the new skew index (i + new skew) + childVNode._flags |= INSERT_VNODE; + } + } else { + skew++; childVNode._flags |= INSERT_VNODE; } } diff --git a/test/browser/keys.test.jsx b/test/browser/keys.test.jsx index bb00857274..4d196f2067 100644 --- a/test/browser/keys.test.jsx +++ b/test/browser/keys.test.jsx @@ -366,6 +366,156 @@ describe('keys', () => { ); }); + 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; the run following the match (j) is + // shorter than the jump, so the displacement path must not trigger. + [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 displaced prefix would be more + // work than moving the two-child suffix, so the guard must not trigger. + 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', () => { + // The displacement probe compares the key & type of the next pair; a raw + // text child passes that probe against any old text node. Ensure such a + // false positive can only cost extra moves, never correctness. + 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']; @@ -395,6 +545,24 @@ describe('keys', () => { ); }); + 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 move keyed children to the beginning on longer list', () => { // Preact v10 worst case const values = ['a', 'b', 'c', 'd', 'e', 'f'];