Keyed nullish placeholders cause re-mounts

src/diff/children.js

@@ -253,7 +253,12 @@ function constructNewChildrenArray(
 
 		if (isMounting) {
 			if (matchingIndex == -1) {
-				skew--;
+				//skew--;
+				if (newChildrenLength > oldChildrenLength) {
+					skew--;
+				} else if (newChildrenLength < oldChildrenLength) {
+					skew++;
+				}
 			}
 
 			// If we are mounting a DOM VNode, mark it for insertion
@@ -407,7 +412,7 @@ function findMatchingIndex(
 		(oldVNode != NULL && (oldVNode._flags & MATCHED) == 0 ? 1 : 0);
 
 	if (
-		oldVNode === NULL ||
+		(oldVNode === NULL && !childVNode.key) ||
 		(oldVNode &&
 			key == oldVNode.key &&
 			type === oldVNode.type &&

test/browser/keys.test.js

@@ -943,4 +943,185 @@ describe('keys', () => {
 
 		expect(scratch.innerHTML).to.eq(`<div>${expected(sorted)}</div>`);
 	});
+
+	it('should handle keyed replacements', () => {
+		const actions = [];
+		class Comp extends Component {
+			componentDidMount() {
+				actions.push('mounted ' + this.props.i);
+			}
+			render() {
+				return <div>Hello</div>;
+			}
+		}
+
+		const App = props => {
+			return (
+				<div>
+					<Comp key={props.y} i={1} />
+					{false}
+					<Comp i={2} />
+					<Comp i={3} />
+				</div>
+			);
+		};
+
+		render(<App y="1" />, scratch);
+		expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']);
+
+		render(<App y="2" />, 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 <div>Hello</div>;
+			}
+		}
+
+		const App = props => {
+			return props.y === '2' ? (
+				<div>
+					<Comp key={1} i={1} />
+					<Comp key={2} i={2} />
+					<Comp key={3} i={3} />
+				</div>
+			) : (
+				<div>
+					{null}
+					<Comp key={1} i={1} />
+					<Comp key={2} i={2} />
+					<Comp key={3} i={3} />
+				</div>
+			);
+		};
+
+		render(<App y="1" />, scratch);
+		expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']);
+
+		render(<App y="2" />, 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 <div>Hello</div>;
+			}
+		}
+
+		const App = props => {
+			return props.y === '1' ? (
+				<div>
+					<Comp key={1} i={1} />
+					<Comp key={2} i={2} />
+					<Comp key={3} i={3} />
+				</div>
+			) : (
+				<div>
+					<Comp key={1} i={1} />
+					{null}
+					<Comp key={3} i={3} />
+				</div>
+			);
+		};
+
+		render(<App y="1" />, scratch);
+		expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']);
+
+		render(<App y="2" />, scratch);
+		expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']);
+
+		render(<App y="1" />, 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 <div>Hello</div>;
+			}
+		}
+
+		const App = props => {
+			return props.y === '2' ? (
+				<div>
+					<Comp key={1} i={1} />
+					<Comp key={2} i={2} />
+					<Comp key={3} i={3} />
+				</div>
+			) : (
+				<div>
+					<Comp key={1} i={1} />
+					<Comp key={2} i={2} />
+					{null}
+					<Comp key={3} i={3} />
+				</div>
+			);
+		};
+
+		render(<App y="1" />, scratch);
+		expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']);
+
+		render(<App y="2" />, 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 <div>Hello</div>;
+			}
+		}
+
+		const App = props => {
+			return props.y === '2' ? (
+				<div>
+					<Comp key={1} i={1} />
+					<Comp key={2} i={2} />
+					<Comp key={3} i={3} />
+				</div>
+			) : (
+				<div>
+					<Comp key={1} i={1} />
+					<Comp key={2} i={2} />
+					<Comp key={3} i={3} />
+					{null}
+				</div>
+			);
+		};
+
+		render(<App y="1" />, scratch);
+		expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']);
+
+		render(<App y="2" />, scratch);
+		expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']);
+	});
 });

test/browser/render.test.js

@@ -1967,4 +1967,33 @@ describe('render()', () => {
 			'<head><title>Test</title></head><body><p>Test</p></body>\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 <div>Hello</div>;
+			}
+		}
+
+		const App = props => {
+			return (
+				<div>
+					{props.y === '1' ? <Comp i={1} /> : <div />}
+					{false}
+					<Comp i={2} />
+					<Comp i={3} />
+				</div>
+			);
+		};
+
+		render(<App y="1" />, scratch);
+		expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']);
+
+		render(<App y="2" />, scratch);
+		expect(actions).to.deep.equal(['mounted 1', 'mounted 2', 'mounted 3']);
+	});
 });