Avoid lazy components without result going in throw loop (#4939)

Author: JoviDeCroock

compat/src/suspense.js

@@ -208,18 +208,23 @@ Suspense.prototype.render = function (props, state) {
 
 export function lazy(loader) {
 	let prom;
-	let component;
+	let component = null;
 	let error;
+	let resolved;
 
 	function Lazy(props) {
 		if (!prom) {
 			prom = loader();
 			prom.then(
 				exports => {
-					component = exports.default || exports;
+					if (exports) {
+						component = exports.default || exports;
+					}
+					resolved = true;
 				},
 				e => {
 					error = e;
+					resolved = true;
 				}
 			);
 		}
@@ -228,11 +233,11 @@ export function lazy(loader) {
 			throw error;
 		}
 
-		if (!component) {
+		if (!resolved) {
 			throw prom;
 		}
 
-		return createElement(component, props);
+		return component ? createElement(component, props) : null;
 	}
 
 	Lazy.displayName = 'Lazy';

compat/test/browser/suspense.test.jsx

@@ -34,9 +34,11 @@ class Catcher extends Component {
 	}
 
 	render(props, state) {
-		return state.error
-			? <div>Catcher did catch: {state.error.message}</div>
-			: props.children;
+		return state.error ? (
+			<div>Catcher did catch: {state.error.message}</div>
+		) : (
+			props.children
+		);
 	}
 }
 
@@ -104,6 +106,59 @@ describe('suspense', () => {
 		});
 	});
 
+	it('should handle lazy component that rejects without returning a component', async () => {
+		const errorSpy = vi.fn();
+		let renderCount = 0;
+
+		let resolve;
+		function fakeImport() {
+			const p = new Promise((_, reject) => {
+				resolve = () => {
+					reject(new Error('import failed'));
+					return p;
+				};
+			});
+			return p;
+		}
+
+		const SomeComponent = lazy(() =>
+			fakeImport().catch(e => {
+				console.log('caught', e);
+				errorSpy(e);
+			})
+		);
+
+		const App = () => {
+			renderCount++;
+			if (renderCount > 5) {
+				throw new Error('Infinite loop detected!');
+			}
+
+			console.log('RENDER COUNT', renderCount);
+			return (
+				<div>
+					<Suspense fallback={<div>loading</div>}>
+						<SomeComponent />
+					</Suspense>
+				</div>
+			);
+		};
+
+		render(<App />, scratch);
+		rerender();
+
+		expect(scratch.innerHTML).to.contain('loading');
+
+		const assert = () => {
+			rerender();
+
+			expect(scratch.innerHTML).to.contain('<div></div>');
+			expect(errorSpy).toHaveBeenCalledOnce;
+		};
+
+		resolve().then(assert).catch(assert);
+	});
+
 	it('should reset hooks of components', () => {
 		/** @type {(v) => void} */
 		let set;
@@ -190,11 +245,13 @@ describe('suspense', () => {
 				};
 			}, []);
 
-			return state
-				? <div>{children}</div>
-				: <div>
-						<p>hi</p>
-					</div>;
+			return state ? (
+				<div>{children}</div>
+			) : (
+				<div>
+					<p>hi</p>
+				</div>
+			);
 		};
 
 		render(

test/browser/refs.test.jsx

@@ -3,7 +3,7 @@ import { createElement, render, Component, createRef, Fragment } from 'preact';
 import { setupScratch, teardown } from '../_util/helpers';
 import { vi } from 'vitest';
 
-// gives call count and argument errors names (otherwise sinon just uses "spy"):
+// gives call count and argument errors names (otherwise vitest just uses "spy"):
 let spy = (name, ...args) => {
 	let spy = vi.fn(...args);
 	spy.displayName = `spy('${name}')`;
@@ -241,13 +241,15 @@ describe('refs', () => {
 	it('should correctly set nested child refs', () => {
 		const ref = createRef();
 		const App = ({ open }) =>
-			open
-				? <div class="open" key="open">
-						<div ref={ref} />
-					</div>
-				: <div class="closes" key="closed">
-						<div ref={ref} />
-					</div>;
+			open ? (
+				<div class="open" key="open">
+					<div ref={ref} />
+				</div>
+			) : (
+				<div class="closes" key="closed">
+					<div ref={ref} />
+				</div>
+			);
 
 		render(<App />, scratch);
 		expect(ref.current).to.not.be.null;
@@ -360,8 +362,39 @@ describe('refs', () => {
 			render(props, { phase }) {
 				return (
 					<Fragment>
-						{phase === 1
-							? <div>
+						{phase === 1 ? (
+							<div>
+								<div
+									ref={r =>
+										r
+											? calls.push('adding ref to two')
+											: calls.push('removing ref from two')
+									}
+								>
+									Element two
+								</div>
+								<div
+									ref={r =>
+										r
+											? calls.push('adding ref to three')
+											: calls.push('removing ref from three')
+									}
+								>
+									Element three
+								</div>
+							</div>
+						) : phase === 2 ? (
+							<div class="outer">
+								<div
+									ref={r =>
+										r
+											? calls.push('adding ref to one')
+											: calls.push('removing ref from one')
+									}
+								>
+									Element one
+								</div>
+								<div class="wrapper">
 									<div
 										ref={r =>
 											r
@@ -381,39 +414,8 @@ describe('refs', () => {
 										Element three
 									</div>
 								</div>
-							: phase === 2
-								? <div class="outer">
-										<div
-											ref={r =>
-												r
-													? calls.push('adding ref to one')
-													: calls.push('removing ref from one')
-											}
-										>
-											Element one
-										</div>
-										<div class="wrapper">
-											<div
-												ref={r =>
-													r
-														? calls.push('adding ref to two')
-														: calls.push('removing ref from two')
-												}
-											>
-												Element two
-											</div>
-											<div
-												ref={r =>
-													r
-														? calls.push('adding ref to three')
-														: calls.push('removing ref from three')
-												}
-											>
-												Element three
-											</div>
-										</div>
-									</div>
-								: null}
+							</div>
+						) : null}
 					</Fragment>
 				);
 			}