Merge pull request #4284 from preactjs/fix-and-add-test-for-4283

fix: increment skew when we aren't removing the first pointer

src/diff/children.js

@@ -227,9 +227,11 @@ function constructNewChildrenArray(newParentVNode, renderResult, oldChildren) {
 			childVNode = newParentVNode._children[i] = childVNode;
 		}
 
+		const skewedIndex = i + skew;
+
 		// Handle unmounting null placeholders, i.e. VNode => null in unkeyed children
 		if (childVNode == null) {
-			oldVNode = oldChildren[i];
+			oldVNode = oldChildren[skewedIndex];
 			if (
 				oldVNode &&
 				oldVNode.key == null &&
@@ -250,7 +252,7 @@ function constructNewChildrenArray(newParentVNode, renderResult, oldChildren) {
 				// to unmount this VNode again seeing `_match==true`.  Further,
 				// getDomSibling doesn't know about _match and so would incorrectly
 				// assume DOM nodes in this subtree are mounted and usable.
-				oldChildren[i] = null;
+				oldChildren[skewedIndex] = null;
 				remainingOldChildren--;
 			}
 			continue;
@@ -259,7 +261,6 @@ function constructNewChildrenArray(newParentVNode, renderResult, oldChildren) {
 		childVNode._parent = newParentVNode;
 		childVNode._depth = newParentVNode._depth + 1;
 
-		const skewedIndex = i + skew;
 		const matchingIndex = findMatchingIndex(
 			childVNode,
 			oldChildren,

test/browser/render.test.js

@@ -1356,4 +1356,58 @@ describe('render()', () => {
 		rerender();
 		expect(scratch.innerHTML).to.equal('<div><div></div><div>B</div></div>');
 	});
+
+	it('should not crash or repeatedly add the same child when replacing a matched vnode with null (mixed dom-types)', () => {
+		const B = () => <div>B</div>;
+
+		/** @type {() => void} */
+		let update;
+		class App extends Component {
+			constructor(props) {
+				super(props);
+				this.state = { show: true };
+				update = () => {
+					this.setState(state => ({ show: !state.show }));
+				};
+			}
+
+			render() {
+				if (this.state.show) {
+					return (
+						<div>
+							<B />
+							<div>C</div>
+						</div>
+					);
+				}
+				return (
+					<div>
+						<span>A</span>
+						{null}
+						<B />
+						<div>C</div>
+					</div>
+				);
+			}
+		}
+
+		render(<App />, scratch);
+		expect(scratch.innerHTML).to.equal('<div><div>B</div><div>C</div></div>');
+
+		update();
+		rerender();
+		expect(scratch.innerHTML).to.equal(
+			'<div><span>A</span><div>B</div><div>C</div></div>'
+		);
+
+		update();
+		rerender();
+		expect(scratch.innerHTML).to.equal('<div><div>B</div><div>C</div></div>');
+
+		update();
+		rerender();
+		expect(scratch.innerHTML).to.equal(
+			'<div><span>A</span><div>B</div><div>C</div></div>'
+		);
+	});
 });