Ensure memoized components re-render after errors (#4880)

Author: JoviDeCroock

compat/test/browser/memo.test.js

@@ -11,7 +11,7 @@ import React, {
 	memo,
 	useState
 } from 'preact/compat';
-import { vi } from 'vitest';
+import { vi, expect } from 'vitest';
 
 const h = React.createElement;
 
@@ -238,4 +238,40 @@ describe('memo()', () => {
 
 		expect(Memoized.type).to.equal(Foo);
 	});
+
+	it('should recover and render siblings when memo child throws once', () => {
+		let causeError = true;
+
+		const TestWithMemo = /** @type {any} */ (memo(
+			/** @type {(props: { n: number }) => any} */ (props => {
+				const { n } = props;
+				if (n === 2 && causeError) {
+					throw new Error('test error');
+				}
+				return <p>test {n}</p>;
+			})
+		));
+
+		class App extends Component {
+			static getDerivedStateFromError() {
+				causeError = false;
+				return {};
+			}
+			render() {
+				return (
+					<div>
+						<h1>Example</h1>
+						<TestWithMemo n={1} />
+						<TestWithMemo n={2} />
+						<TestWithMemo n={3} />
+					</div>
+				);
+			}
+		}
+
+		render(<App />, scratch);
+		rerender();
+
+		expect(scratch.textContent).to.equal('Exampletest 1test 2test 3');
+	});
 });

src/diff/catch-error.js

@@ -3,7 +3,8 @@ import {
 	NULL,
 	COMPONENT_DIRTY,
 	COMPONENT_PENDING_ERROR,
-	COMPONENT_PROCESSING_EXCEPTION
+	COMPONENT_PROCESSING_EXCEPTION,
+	COMPONENT_FORCE
 } from '../constants';
 
 /**
@@ -28,6 +29,7 @@ export function _catchError(error, vnode, oldVNode, errorInfo) {
 			(component = vnode._component) &&
 			!(component._bits & COMPONENT_PROCESSING_EXCEPTION)
 		) {
+			component._bits |= COMPONENT_FORCE;
 			try {
 				ctor = component.constructor;
 

test/browser/lifecycles/componentDidCatch.test.js

@@ -1,7 +1,7 @@
 import { setupRerender } from 'preact/test-utils';
 import { createElement, render, Component, Fragment } from 'preact';
 import { setupScratch, teardown } from '../../_util/helpers';
-import { vi } from 'vitest';
+import { vi, expect } from 'vitest';
 
 /** @jsx createElement */
 
@@ -714,5 +714,49 @@ describe('Lifecycle methods', () => {
 
 			expect(info).to.deep.equal({});
 		});
+
+		// New test mirroring React example but using shouldComponentUpdate instead of memo
+		it('should recover and render siblings when child throws once and uses shouldComponentUpdate', () => {
+			let causeError = true;
+
+			const Render = ({ n }) => <p>test {n}</p>;
+
+			class TestWithSCU extends Component {
+				shouldComponentUpdate(nextProps) {
+					return nextProps.n !== this.props.n;
+				}
+				render(props) {
+					const { n } = props;
+					if (n === 2 && causeError) {
+						throw new Error('test error');
+					}
+					return <Render n={n} />;
+				}
+			}
+
+			class App extends Component {
+				componentDidCatch() {
+					causeError = false;
+					this.setState({});
+				}
+				render() {
+					return (
+						<div>
+							<h1>Example</h1>
+							<TestWithSCU n={1} />
+							<TestWithSCU n={2} />
+							<TestWithSCU n={3} />
+						</div>
+					);
+				}
+			}
+
+			vi.spyOn(App.prototype, 'componentDidCatch');
+			render(<App />, scratch);
+			rerender();
+
+			expect(App.prototype.componentDidCatch).toHaveBeenCalledOnce();
+			expect(scratch.textContent).to.equal('Exampletest 1test 2test 3');
+		});
 	});
 });