diff --git a/src/diff/children.js b/src/diff/children.js index c40b4d5b1e..dc98a7ae01 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -253,7 +253,25 @@ function constructNewChildrenArray( if (isMounting) { if (matchingIndex == -1) { - skew--; + // When the array of children is growing we need to decrease the skew + // as we are adding a new element to the array. + // Example: + // [1, 2, 3] --> [0, 1, 2, 3] + // oldChildren newChildren + // + // The new element is at index 0, so our skew is 0, + // we need to decrease the skew as we are adding a new element. + // The decrease will cause us to compare the element at position 1 + // with value 1 with the element at position 0 with value 0. + // + // A linear concept is applied when the array is shrinking, + // if the length is unchanged we can assume that no skew + // changes are needed. + if (newChildrenLength > oldChildrenLength) { + skew--; + } else if (newChildrenLength < oldChildrenLength) { + skew++; + } } // If we are mounting a DOM VNode, mark it for insertion @@ -407,7 +425,7 @@ function findMatchingIndex( (oldVNode != NULL && (oldVNode._flags & MATCHED) == 0 ? 1 : 0); if ( - oldVNode === NULL || + (oldVNode === NULL && childVNode.key == null) || (oldVNode && key == oldVNode.key && type === oldVNode.type && diff --git a/test/browser/keys.test.js b/test/browser/keys.test.js index 8d5a185b91..73c8911c20 100644 --- a/test/browser/keys.test.js +++ b/test/browser/keys.test.js @@ -943,4 +943,185 @@ describe('keys', () => { expect(scratch.innerHTML).to.eq(`
${expected(sorted)}
`); }); + + it('should handle keyed replacements', () => { + const actions = []; + class Comp extends Component { + componentDidMount() { + actions.push('mounted ' + this.props.i); + } + render() { + return
Hello
; + } + } + + const App = props => { + return ( +
+ + {false} + + +
+ ); + }; + + render(, scratch); + expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']); + + render(, scratch); + expect(actions).to.deep.equal([ + 'mounted 1', + 'mounted 2', + 'mounted 3', + 'mounted 1' + ]); + }); + + it('should handle hole prepend', () => { + const actions = []; + class Comp extends Component { + componentDidMount() { + actions.push('mounted ' + this.props.i); + } + render() { + return
Hello
; + } + } + + const App = props => { + return props.y === '2' ? ( +
+ + + +
+ ) : ( +
+ {null} + + + +
+ ); + }; + + render(, scratch); + expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']); + + render(, scratch); + expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']); + }); + + it('should handle hole replace', () => { + const actions = []; + class Comp extends Component { + componentDidMount() { + actions.push('mounted ' + this.props.i); + } + render() { + return
Hello
; + } + } + + const App = props => { + return props.y === '1' ? ( +
+ + + +
+ ) : ( +
+ + {null} + +
+ ); + }; + + render(, scratch); + expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']); + + render(, scratch); + expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']); + + render(, scratch); + expect(actions).to.deep.equal([ + 'mounted 1', + 'mounted 2', + 'mounted 3', + 'mounted 2' + ]); + }); + + it('should handle hole insert', () => { + const actions = []; + class Comp extends Component { + componentDidMount() { + actions.push('mounted ' + this.props.i); + } + render() { + return
Hello
; + } + } + + const App = props => { + return props.y === '2' ? ( +
+ + + +
+ ) : ( +
+ + + {null} + +
+ ); + }; + + render(, scratch); + expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']); + + render(, scratch); + expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']); + }); + + it('should handle hole apppend', () => { + const actions = []; + class Comp extends Component { + componentDidMount() { + actions.push('mounted ' + this.props.i); + } + render() { + return
Hello
; + } + } + + const App = props => { + return props.y === '2' ? ( +
+ + + +
+ ) : ( +
+ + + + {null} +
+ ); + }; + + render(, scratch); + expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']); + + render(, scratch); + expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']); + }); }); diff --git a/test/browser/render.test.js b/test/browser/render.test.js index bb006c6868..87ee33ecb4 100644 --- a/test/browser/render.test.js +++ b/test/browser/render.test.js @@ -1967,4 +1967,33 @@ describe('render()', () => { 'Test

Test

\n' ); }); + + it('should not remount components when replacing a component with a falsy value in-between', () => { + const actions = []; + class Comp extends Component { + componentDidMount() { + actions.push('mounted ' + this.props.i); + } + render() { + return
Hello
; + } + } + + const App = props => { + return ( +
+ {props.y === '1' ? :
} + {false} + + +
+ ); + }; + + render(, scratch); + expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']); + + render(, scratch); + expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']); + }); });