Merge pull request #2025 from preactjs/fix/suspense-fallback

Support re-suspending

compat/src/internal.d.ts

@@ -32,4 +32,5 @@ export interface SuspenseState {
 
 export interface SuspenseComponent extends PreactComponent<SuspenseProps, SuspenseState> {
 	_suspensions: Array<Promise<any>>;
+	_fallback: VNode<any>;
 }

compat/src/suspense.js

@@ -1,4 +1,4 @@
-import { Component, createElement, _unmount as unmount, options } from 'preact';
+import { Component, createElement, _unmount as unmount, options, cloneElement } from 'preact';
 import { removeNode } from '../../src/util';
 
 const oldCatchError = options._catchError;
@@ -40,9 +40,10 @@ function detachDom(children) {
 }
 
 // having custom inheritance instead of a class here saves a lot of bytes
-export function Suspense() {
+export function Suspense(props) {
 	// we do not call super here to golf some bytes...
 	this._suspensions = [];
+	this._fallback = props.fallback;
 }
 
 // Things we do here to save some bytes but are not proper JS inheritance:
@@ -67,8 +68,9 @@ Suspense.prototype._childDidSuspend = function(promise) {
 		if (c._suspensions.length == 0) {
 			// If fallback is null, don't try to unmount it
 			// `unmount` expects a real VNode, not null values
-			if (c.props.fallback) {
-				unmount(c.props.fallback);
+			if (c._fallback) {
+				// Unmount current children (should be fallback)
+				unmount(c._fallback);
 			}
 			c._vnode._dom = null;
 
@@ -78,6 +80,7 @@ Suspense.prototype._childDidSuspend = function(promise) {
 	};
 
 	if (c.state._parkedChildren == null) {
+		c._fallback = c._fallback && cloneElement(c._fallback);
 		c.setState({ _parkedChildren: c._vnode._children });
 		detachDom(c._vnode._children);
 		c._vnode._children = [];
@@ -87,7 +90,7 @@ Suspense.prototype._childDidSuspend = function(promise) {
 };
 
 Suspense.prototype.render = function(props, state) {
-	return state._parkedChildren ? props.fallback : props.children;
+	return state._parkedChildren ? this._fallback : props.children;
 };
 
 export function lazy(loader) {

compat/test/browser/suspense.test.js

@@ -4,6 +4,25 @@ import { setupRerender } from 'preact/test-utils';
 import { createElement as h, render, Component, Suspense, lazy, Fragment } from '../../src/index';
 import { setupScratch, teardown } from '../../../test/_util/helpers';
 
+function createLazy() {
+
+	/** @type {(c: ComponentType) => Promise<void>} */
+	let resolver, rejecter, promise;
+	const Lazy = lazy(() => promise = new Promise((resolve, reject) => {
+		resolver = c => {
+			resolve({ default: c });
+			return promise;
+		};
+
+		rejecter = () => {
+			reject();
+			return promise;
+		};
+	}));
+
+	return [Lazy, c => resolver(c), e => rejecter(e)];
+}
+
 /**
  * @typedef {import('../../../src').ComponentType} ComponentType
  * @typedef {[(c: ComponentType) => Promise<void>, (error: Error) => Promise<void>]} Resolvers
@@ -33,23 +52,9 @@ function createSuspender(DefaultComponent) {
 	 * @returns {Resolvers}
 	 */
 	function suspend() {
-
-		/** @type {(c: ComponentType) => Promise<void>} */
-		let resolver, rejecter, promise;
-		const Lazy = lazy(() => promise = new Promise((resolve, reject) => {
-			resolver = c => {
-				resolve({ default: c });
-				return promise;
-			};
-
-			rejecter = () => {
-				reject();
-				return promise;
-			};
-		}));
-
+		const [Lazy, resolve, reject] = createLazy();
 		renderLazy(Lazy);
-		return [c => resolver(c), e => rejecter(e)];
+		return [resolve, reject];
 	}
 
 	return [Suspender, suspend];
@@ -76,6 +81,8 @@ class Catcher extends Component {
 }
 
 describe('suspense', () => {
+
+	/** @type {HTMLDivElement} */
 	let scratch, rerender, unhandledEvents = [];
 
 	function onUnhandledRejection(event) {
@@ -521,7 +528,7 @@ describe('suspense', () => {
 		});
 	});
 
-	it('should allow multiple children to suspend', () => {
+	it('should allow multiple sibling children to suspend', () => {
 		const [Suspender1, suspend1] = createSuspender(() => <div>Hello first</div>);
 		const [Suspender2, suspend2] = createSuspender(() => <div>Hello second</div>);
 
@@ -572,7 +579,7 @@ describe('suspense', () => {
 		});
 	});
 
-	it('should call multiple nested suspending components render in one go', () => {
+	it('should call multiple nested sibling suspending components render in one go', () => {
 		const [Suspender1, suspend1] = createSuspender(() => <div>Hello first</div>);
 		const [Suspender2, suspend2] = createSuspender(() => <div>Hello second</div>);
 
@@ -824,4 +831,124 @@ describe('suspense', () => {
 		});
 	});
 
+	it('should support suspending multiple times', () => {
+		const [Suspender, suspend] = createSuspender(() => <div>initial render</div>);
+		const Loading = () => <div>Suspended...</div>;
+
+		render(
+			<Suspense fallback={<Loading />}>
+				<Suspender />
+			</Suspense>,
+			scratch
+		);
+
+		expect(scratch.innerHTML).to.eql(`<div>initial render</div>`);
+
+		let [resolve] = suspend();
+		rerender();
+
+		expect(scratch.innerHTML).to.eql(`<div>Suspended...</div>`);
+
+		return resolve(() => <div>Hello1</div>)
+			.then(() => {
+				// Rerender promise resolution
+				rerender();
+				expect(scratch.innerHTML).to.eql(`<div>Hello1</div>`);
+
+				// suspend again
+				[resolve] = suspend();
+				rerender();
+
+				expect(scratch.innerHTML).to.eql(`<div>Suspended...</div>`);
+
+				return resolve(() => <div>Hello2</div>);
+			})
+			.then(() => {
+				// Rerender promise resolution
+				rerender();
+				expect(scratch.innerHTML).to.eql(`<div>Hello2</div>`);
+			});
+	});
+
+	it('should correctly render when a suspended component\'s child also suspends', () => {
+		const [Suspender1, suspend1] = createSuspender(() => <div>Hello1</div>);
+		const [LazyChild, resolveChild] = createLazy();
+
+		render(
+			<Suspense fallback={<div>Suspended...</div>}>
+				<Suspender1 />
+			</Suspense>,
+			scratch,
+		);
+
+		expect(scratch.innerHTML).to.equal(`<div>Hello1</div>`);
+
+		let [resolve1] = suspend1();
+		rerender();
+		expect(scratch.innerHTML).to.equal('<div>Suspended...</div>');
+
+
+		return resolve1(() => <LazyChild />)
+			.then(() => {
+				rerender();
+				expect(scratch.innerHTML).to.equal('<div>Suspended...</div>');
+
+				return resolveChild(() => <div>All done!</div>);
+			})
+			.then(() => {
+				rerender();
+				expect(scratch.innerHTML).to.equal('<div>All done!</div>');
+			});
+	});
+
+	it('should correctly render nested Suspense components', () => {
+		// Inspired by the nested-suspense demo from #1865
+		// TODO: Explore writing a test that varies the loading orders
+
+		const [Lazy1, resolve1] = createLazy();
+		const [Lazy2, resolve2] = createLazy();
+		const [Lazy3, resolve3] = createLazy();
+
+		const Loading = () => <div>Suspended...</div>;
+		const loadingHtml = `<div>Suspended...</div>`;
+
+		render(
+			<Suspense fallback={<Loading />}>
+				<Lazy1 />
+				<div>
+					<Suspense fallback={<Loading />}>
+						<Lazy2 />
+					</Suspense>
+					<Lazy3 />
+				</div>
+				<b>4</b>
+			</Suspense>,
+			scratch
+		);
+		rerender(); // Rerender with the fallback HTML
+
+		expect(scratch.innerHTML).to.equal(loadingHtml);
+
+		resolve1(() => <b>1</b>)
+			.then(() => {
+				rerender();
+				expect(scratch.innerHTML).to.equal(loadingHtml);
+
+				return resolve3(() => <b>3</b>);
+			})
+			.then(() => {
+				rerender();
+				expect(scratch.innerHTML).to.equal(
+					`<b>1</b><div>${loadingHtml}<b>3</b></div><b>4</b>`
+				);
+
+				return resolve2(() => <b>2</b>);
+			})
+			.then(() => {
+				rerender();
+				expect(scratch.innerHTML).to.equal(
+					`<b>1</b><div><b>2</b><b>3</b></div><b>4</b>`
+				);
+			});
+	});
 });

demo/index.js

@@ -21,6 +21,8 @@ import SuspenseDemo from './suspense';
 import Redux from './redux';
 import TextFields from './textFields';
 import ReduxBug from './reduxUpdate';
+import SuspenseRouterBug from './suspense-router';
+import NestedSuspenseBug from './nested-suspense';
 
 let isBenchmark = /(\/spiral|\/pythagoras|[#&]bench)/g.test(window.location.href);
 if (!isBenchmark) {
@@ -79,6 +81,8 @@ class App extends Component {
 						<Link href="/suspense" activeClassName="active">Suspense / lazy</Link>
 						<Link href="/textfields" activeClassName="active">Textfields</Link>
 						<Link href="/reduxBug/1" activeClassName="active">Redux Bug</Link>
+						<Link href="/suspense-router" activeClassName="active">Suspense Router Bug</Link>
+						<Link href="/nested-suspense" activeClassName="active">Nested Suspense Bug</Link>
 					</nav>
 				</header>
 				<main>
@@ -111,6 +115,8 @@ class App extends Component {
 						<Redux path="/redux" />
 						<TextFields path="/textfields" />
 						<ReduxBug path="/reduxBug/:start" />
+						<SuspenseRouterBug path="/suspense-router" />
+						<NestedSuspenseBug path="/nested-suspense" />
 					</Router>
 				</main>
 			</div>

demo/nested-suspense/addnewcomponent.js

@@ -0,0 +1,5 @@
+import { createElement } from "react";
+
+export default function AddNewComponent({appearance}) {
+	return <div>AddNewComponent (component #{appearance})</div>;
+}

demo/nested-suspense/component-container.js

@@ -0,0 +1,17 @@
+import { createElement, lazy } from "react";
+
+const pause = timeout =>
+	new Promise(d => setTimeout(d, timeout), console.log(timeout));
+
+const SubComponent = lazy(() =>
+	pause(Math.random() * 1000).then(() => import("./subcomponent.js"))
+);
+
+export default function ComponentContainer({ appearance }) {
+	return (
+		<div>
+			GenerateComponents (component #{appearance})
+			<SubComponent />
+		</div>
+	);
+}

demo/nested-suspense/dropzone.js

@@ -0,0 +1,5 @@
+import { createElement } from "react";
+
+export default function DropZone({appearance}) {
+	return <div>DropZone (component #{appearance})</div>;
+}

demo/nested-suspense/editor.js

@@ -0,0 +1,5 @@
+import { createElement } from "react";
+
+export default function Editor({ children }) {
+	return <div className="Editor">{children}</div>;
+}

demo/nested-suspense/index.js

@@ -0,0 +1,71 @@
+import { createElement, Suspense, lazy, Component } from "react";
+
+const Loading = function() {
+	return <div>Loading...</div>;
+};
+const Error = function({ resetState }) {
+	return (
+		<div>
+			Error!&nbsp;
+			<a onClick={resetState} href="#">
+				Reset app
+			</a>
+		</div>
+	);
+};
+
+const pause = timeout =>
+	new Promise(d => setTimeout(d, timeout), console.log(timeout));
+
+const DropZone = lazy(() =>
+	pause(Math.random() * 1000).then(() => import("./dropzone.js"))
+);
+const Editor = lazy(() =>
+	pause(Math.random() * 1000).then(() => import("./editor.js"))
+);
+const AddNewComponent = lazy(() =>
+	pause(Math.random() * 1000).then(() => import("./addnewcomponent.js"))
+);
+const GenerateComponents = lazy(() =>
+	pause(Math.random() * 1000).then(() =>
+		import("./component-container.js")
+	)
+);
+
+export default class App extends Component {
+	state = { hasError: false };
+
+	static getDerivedStateFromError(error) {
+		// Update state so the next render will show the fallback UI.
+		console.warn(error);
+		return { hasError: true };
+	}
+
+	render() {
+		return this.state.hasError ? (
+			<Error resetState={() => this.setState({ hasError: false })} />
+		) : (
+			<Suspense fallback={<Loading />}>
+				<DropZone appearance={0} />
+				<Editor title="APP_TITLE">
+					<main>
+						<Suspense fallback={<Loading />}>
+							<GenerateComponents appearance={1} />
+						</Suspense>
+						<AddNewComponent appearance={2} />
+					</main>
+					<aside>
+						<section>
+							<Suspense fallback={<Loading />}>
+								<GenerateComponents appearance={3} />
+							</Suspense>
+							<AddNewComponent appearance={4} />
+						</section>
+					</aside>
+				</Editor>
+
+				<footer>Footer here</footer>
+			</Suspense>
+		);
+	}
+}

demo/nested-suspense/subcomponent.js

@@ -0,0 +1,5 @@
+import { createElement } from "react";
+
+export default function SubComponent({ onClick }) {
+	return <div>Lazy loaded sub component</div>;
+}