fix: address scenario where we would crash when replacing a matched v…

…node with null (#4281)

* add test for the issue, TODO convert to class components later

* wip

* correct assertion

* avoid crasshing when replacing existing node

* move test to core

src/diff/children.js

@@ -62,7 +62,6 @@ export function diffChildren(
 
 	for (i = 0; i < newChildrenLength; i++) {
 		childVNode = newParentVNode._children[i];
-
 		if (
 			childVNode == null ||
 			typeof childVNode == 'boolean' ||
@@ -231,11 +230,15 @@ function constructNewChildrenArray(newParentVNode, renderResult, oldChildren) {
 		// Handle unmounting null placeholders, i.e. VNode => null in unkeyed children
 		if (childVNode == null) {
 			oldVNode = oldChildren[i];
-			if (oldVNode && oldVNode.key == null && oldVNode._dom) {
+			if (
+				oldVNode &&
+				oldVNode.key == null &&
+				oldVNode._dom &&
+				(oldVNode._flags & MATCHED) === 0
+			) {
 				if (oldVNode._dom == newParentVNode._nextDom) {
 					newParentVNode._nextDom = getDomSibling(oldVNode);
 				}
-
 				unmount(oldVNode, oldVNode, false);
 
 				// Explicitly nullify this position in oldChildren instead of just
@@ -250,7 +253,6 @@ function constructNewChildrenArray(newParentVNode, renderResult, oldChildren) {
 				oldChildren[i] = null;
 				remainingOldChildren--;
 			}
-
 			continue;
 		}
 

test/browser/render.test.js

@@ -1308,4 +1308,52 @@ describe('render()', () => {
 		expect(divs[1].hasAttribute('role')).to.equal(false);
 		expect(divs[2].hasAttribute('role')).to.equal(false);
 	});
+
+	it('should not crash or repeatedly add the same child when replacing a matched vnode with null', () => {
+		const B = () => <div>B</div>;
+
+		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 />
+						</div>
+					);
+				}
+				return (
+					<div>
+						<div />
+						{null}
+						<B />
+					</div>
+				);
+			}
+		}
+
+		render(<App />, scratch);
+		expect(scratch.innerHTML).to.equal('<div><div>B</div><div></div></div>');
+
+		update();
+		rerender();
+		expect(scratch.innerHTML).to.equal('<div><div></div><div>B</div></div>');
+
+		update();
+		rerender();
+		expect(scratch.innerHTML).to.equal('<div><div>B</div><div></div></div>');
+
+		update();
+		rerender();
+		expect(scratch.innerHTML).to.equal('<div><div></div><div>B</div></div>');
+	});
 });