Merge pull request #2755 from preactjs/suspense-hydration-compat

compat/src/internal.d.ts

@@ -14,11 +14,8 @@ export interface Component<P = {}, S = {}> extends PreactComponent<P, S> {
 	isPureReactComponent?: true;
 	_patchedLifecycles?: true;
 
-	_childDidSuspend?(
-		error: Promise<void>,
-		suspendingComponent: Component<any, any>,
-		oldVNode?: VNode
-	): void;
+	_childDidSuspend?(error: Promise<void>, suspendingVNode: VNode): void;
+	_suspended: (vnode: VNode) => (unsuspend: () => void) => void;
 	_suspendedComponentWillUnmount?(): void;
 }
 

compat/src/suspense.js

@@ -15,7 +15,7 @@ options._catchError = function(error, newVNode, oldVNode) {
 					newVNode._children = oldVNode._children;
 				}
 				// Don't call oldCatchError if we found a Suspense
-				return component._childDidSuspend(error, newVNode._component);
+				return component._childDidSuspend(error, newVNode);
 			}
 		}
 	}
@@ -63,9 +63,11 @@ Suspense.prototype = new Component();
 
 /**
  * @param {Promise} promise The thrown promise
- * @param {Component<any, any>} suspendingComponent The suspending component
+ * @param {import('./internal').VNode<any, any>} suspendingVNode The suspending component
  */
-Suspense.prototype._childDidSuspend = function(promise, suspendingComponent) {
+Suspense.prototype._childDidSuspend = function(promise, suspendingVNode) {
+	const suspendingComponent = suspendingVNode._component;
+
 	/** @type {import('./internal').SuspenseComponent} */
 	const c = this;
 
@@ -118,8 +120,7 @@ Suspense.prototype._childDidSuspend = function(promise, suspendingComponent) {
 	 * to remain on screen and hydrate it when the suspense actually gets resolved.
 	 * While in non-hydration cases the usual fallback -> component flow would occour.
 	 */
-	const vnode = c._vnode;
-	const wasHydrating = vnode && vnode._hydrating === true;
+	const wasHydrating = suspendingVNode._hydrating === true;
 	if (!wasHydrating && !c._pendingSuspensionCount++) {
 		c.setState({ _suspended: (c._detachOnNextRender = c._vnode._children[0]) });
 	}
@@ -141,6 +142,7 @@ Suspense.prototype.render = function(props, state) {
 	}
 
 	// Wrap fallback tree in a VNode that prevents itself from being marked as aborting mid-hydration:
+	/** @type {import('./internal').VNode} */
 	const fallback =
 		state._suspended && createElement(Fragment, null, props.fallback);
 	if (fallback) fallback._hydrating = null;
@@ -165,10 +167,11 @@ Suspense.prototype.render = function(props, state) {
  * If the parent does not return a callback then the resolved vnode
  * gets unsuspended immediately when it resolves.
  *
- * @param {import('../src/internal').VNode} vnode
+ * @param {import('./internal').VNode} vnode
  * @returns {((unsuspend: () => void) => void)?}
  */
 export function suspended(vnode) {
+	/** @type {import('./internal').Component} */
 	let component = vnode._parent._component;
 	return component && component._suspended && component._suspended(vnode);
 }

compat/test/browser/suspense-hydration.test.js

@@ -0,0 +1,175 @@
+import { setupRerender } from 'preact/test-utils';
+import React, {
+	createElement,
+	hydrate,
+	Component,
+	Fragment,
+	Suspense
+} from 'preact/compat';
+import { logCall, getLog, clearLog } from '../../../test/_util/logCall';
+import { setupScratch, teardown } from '../../../test/_util/helpers';
+import { createLazy } from './suspense-utils';
+
+/* eslint-env browser, mocha */
+describe('suspense hydration', () => {
+	/** @type {HTMLDivElement} */
+	let scratch,
+		rerender,
+		unhandledEvents = [];
+
+	function onUnhandledRejection(event) {
+		unhandledEvents.push(event);
+	}
+
+	/** @type {() => void} */
+	let increment;
+	class Counter extends Component {
+		constructor(props) {
+			super(props);
+
+			this.state = { count: 0 };
+			increment = () => this.setState({ count: ++this.state.count });
+		}
+
+		render(props, { count }) {
+			return <div>Count: {count}</div>;
+		}
+	}
+
+	let resetAppendChild;
+	let resetInsertBefore;
+	let resetRemoveChild;
+	let resetRemove;
+
+	before(() => {
+		resetAppendChild = logCall(Element.prototype, 'appendChild');
+		resetInsertBefore = logCall(Element.prototype, 'insertBefore');
+		resetRemoveChild = logCall(Element.prototype, 'removeChild');
+		resetRemove = logCall(Element.prototype, 'remove');
+	});
+
+	after(() => {
+		resetAppendChild();
+		resetInsertBefore();
+		resetRemoveChild();
+		resetRemove();
+	});
+
+	beforeEach(() => {
+		scratch = setupScratch();
+		rerender = setupRerender();
+
+		unhandledEvents = [];
+		if ('onunhandledrejection' in window) {
+			window.addEventListener('unhandledrejection', onUnhandledRejection);
+		}
+	});
+
+	afterEach(() => {
+		teardown(scratch);
+
+		if ('onunhandledrejection' in window) {
+			window.removeEventListener('unhandledrejection', onUnhandledRejection);
+
+			if (unhandledEvents.length) {
+				throw unhandledEvents[0].reason;
+			}
+		}
+	});
+
+	it('should leave DOM untouched when suspending while hydrating', () => {
+		scratch.innerHTML = '<div>Hello</div>';
+		clearLog();
+
+		const [Lazy, resolve] = createLazy();
+		hydrate(
+			<Suspense>
+				<Lazy />
+			</Suspense>,
+			scratch
+		);
+		rerender(); // Flush rerender queue to mimic what preact will really do
+		expect(scratch.innerHTML).to.equal('<div>Hello</div>');
+		expect(getLog()).to.deep.equal([]);
+		clearLog();
+
+		return resolve(() => <div>Hello</div>).then(() => {
+			rerender();
+			expect(scratch.innerHTML).to.equal('<div>Hello</div>');
+			expect(getLog()).to.deep.equal([]);
+			clearLog();
+		});
+	});
+
+	it('should allow siblings to update around suspense boundary', () => {
+		scratch.innerHTML = '<div>Count: 0</div><div>Hello</div>';
+		clearLog();
+
+		const [Lazy, resolve] = createLazy();
+		hydrate(
+			<Fragment>
+				<Counter />
+				<Suspense>
+					<Lazy />
+				</Suspense>
+			</Fragment>,
+			scratch
+		);
+		rerender(); // Flush rerender queue to mimic what preact will really do
+		expect(scratch.innerHTML).to.equal('<div>Count: 0</div><div>Hello</div>');
+		// Re: DOM OP below - Known issue with hydrating merged text nodes
+		expect(getLog()).to.deep.equal(['<div>Count: .appendChild(#text)']);
+		clearLog();
+
+		increment();
+		rerender();
+
+		expect(scratch.innerHTML).to.equal('<div>Count: 1</div><div>Hello</div>');
+		expect(getLog()).to.deep.equal([]);
+		clearLog();
+
+		return resolve(() => <div>Hello</div>).then(() => {
+			rerender();
+			expect(scratch.innerHTML).to.equal('<div>Count: 1</div><div>Hello</div>');
+			expect(getLog()).to.deep.equal([]);
+			clearLog();
+		});
+	});
+
+	it('should properly hydrate when there is DOM and Components between Suspense and suspender', () => {
+		scratch.innerHTML = '<div><div>Hello</div></div>';
+		clearLog();
+
+		const [Lazy, resolve] = createLazy();
+		hydrate(
+			<Suspense>
+				<div>
+					<Fragment>
+						<Lazy />
+					</Fragment>
+				</div>
+			</Suspense>,
+			scratch
+		);
+		rerender(); // Flush rerender queue to mimic what preact will really do
+		expect(scratch.innerHTML).to.equal('<div><div>Hello</div></div>');
+		expect(getLog()).to.deep.equal([]);
+		clearLog();
+
+		return resolve(() => <div>Hello</div>).then(() => {
+			rerender();
+			expect(scratch.innerHTML).to.equal('<div><div>Hello</div></div>');
+			expect(getLog()).to.deep.equal([]);
+			clearLog();
+		});
+	});
+
+	// TODO:
+	// 1. What if props change between when hydrate suspended and suspense
+	//    resolves?
+	// 2. If using real Suspense, test re-suspending after hydrate suspense
+	// 3. Put some DOM and components with state and event listeners between
+	//    suspender and Suspense boundary
+	// 4. Put some sibling DOM and components with state and event listeners
+	//    sibling to suspender and under Suspense boundary
+});

compat/test/browser/suspense-utils.js

@@ -0,0 +1,116 @@
+import React, { Component, lazy } from 'preact/compat';
+
+const h = React.createElement;
+
+/**
+ * Create a Lazy component whose promise is controlled by by the test. This
+ * function returns 3 values: The Lazy component to render, a `resolve`
+ * function, and a `reject` function. Call `resolve` with the component the Lazy
+ * component should resolve with. Call `reject` with the error the Lazy
+ * component should reject with
+ *
+ * @example
+ * // 1. Create and render the Lazy component
+ * const [Lazy, resolve] = createLazy();
+ * render(
+ * 	<Suspense fallback={<div>Suspended...</div>}>
+ * 		<Lazy />
+ * 	</Suspense>,
+ * 	scratch
+ * );
+ * rerender(); // Rerender is required so the fallback is displayed
+ * expect(scratch.innerHTML).to.eql(`<div>Suspended...</div>`);
+ *
+ * // 2. Resolve the Lazy with a new component to render
+ * return resolve(() => <div>Hello</div>).then(() => {
+ * 	rerender();
+ * 	expect(scratch.innerHTML).to.equal(`<div>Hello</div>`);
+ * });
+ *
+ * @typedef {import('../../../src').ComponentType<any>} ComponentType
+ * @returns {[typeof Component, (c: ComponentType) => Promise<void>, (e: Error) => Promise<void>]}
+ */
+export function createLazy() {
+	/** @type {(c: ComponentType) => Promise<void>} */
+	let resolver, rejecter;
+	const Lazy = lazy(() => {
+		let promise = new Promise((resolve, reject) => {
+			resolver = c => {
+				resolve({ default: c });
+				return promise;
+			};
+
+			rejecter = e => {
+				reject(e);
+				return promise;
+			};
+		});
+
+		return promise;
+	});
+
+	return [Lazy, c => resolver(c), e => rejecter(e)];
+}
+
+/**
+ * Returns a Component and a function (named `suspend`) that will suspend the component when called.
+ * `suspend` will return two functions, `resolve` and `reject`. Call `resolve` with a Component the
+ * suspended component should resume with or reject with the Error the suspended component should
+ * reject with
+ *
+ * @example
+ * // 1. Create a suspender with initial children (argument to createSuspender) and render it
+ * const [Suspender, suspend] = createSuspender(() => <div>Hello</div>);
+ * render(
+ * 	<Suspense fallback={<div>Suspended...</div>}>
+ * 		<Suspender />
+ * 	</Suspense>,
+ * 	scratch
+ * );
+ * expect(scratch.innerHTML).to.eql(`div>Hello</div>`);
+ *
+ * // 2. Cause the component to suspend and rerender the update (i.e. the fallback)
+ * const [resolve] = suspend();
+ * rerender();
+ * expect(scratch.innerHTML).to.eql(`div>Suspended...</div>`);
+ *
+ * // 3. Resolve the suspended component with a new component and rerender
+ * return resolve(() => <div>Hello2</div>).then(() => {
+ * 	rerender();
+ * 	expect(scratch.innerHTML).to.eql(`div>Hello2</div>`);
+ * });
+ *
+ * @typedef {Component<{}, any>} Suspender
+ * @typedef {[(c: ComponentType) => Promise<void>, (error: Error) => Promise<void>]} Resolvers
+ * @param {ComponentType} DefaultComponent
+ * @returns {[typeof Suspender, () => Resolvers]}
+ */
+export function createSuspender(DefaultComponent) {
+	/** @type {(lazy: typeof Component) => void} */
+	let renderLazy;
+	class Suspender extends Component {
+		constructor(props, context) {
+			super(props, context);
+			this.state = { Lazy: null };
+
+			renderLazy = Lazy => this.setState({ Lazy });
+		}
+
+		render(props, state) {
+			return state.Lazy ? h(state.Lazy, props) : h(DefaultComponent, props);
+		}
+	}
+
+	sinon.spy(Suspender.prototype, 'render');
+
+	/**
+	 * @returns {Resolvers}
+	 */
+	function suspend() {
+		const [Lazy, resolve, reject] = createLazy();
+		renderLazy(Lazy);
+		return [resolve, reject];
+	}
+
+	return [Suspender, suspend];
+}