Gathering @developit's suspense hydration work

compat/src/internal.d.ts

@@ -15,7 +15,8 @@ export interface Component<P = {}, S = {}> extends PreactComponent<P, S> {
 
 	_childDidSuspend?(
 		error: Promise<void>,
-		suspendingComponent: Component<any, any>
+		suspendingComponent: Component<any, any>,
+		oldVNode?: VNode
 	): void;
 	_suspendedComponentWillUnmount?(): void;
 }

compat/src/suspense.js

@@ -11,7 +11,7 @@ options._catchError = function(error, newVNode, oldVNode) {
 		for (; (vnode = vnode._parent); ) {
 			if ((component = vnode._component) && component._childDidSuspend) {
 				// Don't call oldCatchError if we found a Suspense
-				return component._childDidSuspend(error, newVNode._component);
+				return component._childDidSuspend(error, newVNode._component, oldVNode);
 			}
 		}
 	}
@@ -42,8 +42,13 @@ Suspense.prototype = new Component();
 /**
  * @param {Promise} promise The thrown promise
  * @param {Component<any, any>} suspendingComponent The suspending component
+ * @param {import('./internal').VNode | null | undefined} oldVNode old VNode caught in the _catchError options
  */
-Suspense.prototype._childDidSuspend = function(promise, suspendingComponent) {
+Suspense.prototype._childDidSuspend = function(
+	promise,
+	suspendingComponent,
+	oldVNode
+) {
 	/** @type {import('./internal').SuspenseComponent} */
 	const c = this;
 
@@ -79,7 +84,12 @@ Suspense.prototype._childDidSuspend = function(promise, suspendingComponent) {
 		}
 	};
 
-	if (!c._suspensions++) {
+	/**
+	 * We do not set `suspended: true` during hydration because we want the actual markup to remain on screen
+	 * and hydrate it when the suspense actually gets resolved.
+	 * While in non-hydration cases the usual fallbac -> component flow would occour.
+	 */
+	if (!(oldVNode && oldVNode._hydrateDom) && !c._suspensions++) {
 		c.setState({ _suspended: (c._detachOnNextRender = c._vnode._children[0]) });
 	}
 	promise.then(onResolved, onResolved);

mangle.json

@@ -27,6 +27,7 @@
       "$_children": "__k",
       "$_suspensions": "__u",
       "$_dom": "__e",
+      "$_hydrateDom": "__s",
       "$_component": "__c",
       "$__html": "__html",
       "$_parent": "__",

src/create-element.js

@@ -70,6 +70,7 @@ export function createVNode(type, props, key, ref) {
 		_dom: null,
 		_lastDomChild: null,
 		_component: null,
+		_hydrateDom: null,
 		constructor: undefined
 	};
 

src/diff/children.js

@@ -123,6 +123,10 @@ export function diffChildren(
 						firstChildDom = newDom;
 					}
 
+					if (excessDomChildren != null) {
+						excessDomChildren[excessDomChildren.indexOf(newDom)] = null;
+					}
+
 					if (childVNode._lastDomChild != null) {
 						// Only Fragments or components that return Fragment like VNodes will
 						// have a non-null _lastDomChild. Continue the diff from the end of

src/diff/index.js

@@ -40,6 +40,14 @@ export function diff(
 	// constructor as undefined. This to prevent JSON-injection.
 	if (newVNode.constructor !== undefined) return null;
 
+	// If this was the innermost VNode at a point where the tree suspended,
+	// pick up diffing where we left off using the saved DOM element and hydration state.
+	if (oldVNode._hydrateDom && excessDomChildren == null) {
+		newVNode._dom = oldDom = oldVNode._hydrateDom;
+		excessDomChildren = [oldDom];
+		oldVNode._hydrateDom = null;
+	}
+
 	if ((tmp = options._diff)) tmp(newVNode);
 
 	try {
@@ -215,6 +223,11 @@ export function diff(
 
 		if ((tmp = options.diffed)) tmp(newVNode);
 	} catch (e) {
+		if (isHydrating) {
+			// Before bailing out, mark the current VNode with the DOM element and hydration state.
+			// We can use this information if we return here to render later on.
+			oldVNode._hydrateDom = newVNode._dom = oldDom;
+		}
 		options._catchError(e, newVNode, oldVNode);
 	}
 
@@ -291,6 +304,10 @@ function diffElementNodes(
 	}
 
 	if (dom == null) {
+		if (excessDomChildren != null) {
+			excessDomChildren[excessDomChildren.indexOf(dom)] = null;
+		}
+
 		if (newVNode.type === null) {
 			return document.createTextNode(newProps);
 		}

test/browser/hydrate.test.js

@@ -1,4 +1,6 @@
 import { createElement, hydrate, Fragment } from 'preact';
+import { Suspense } from 'preact/compat';
+import { useState } from 'preact/hooks';
 import {
 	setupScratch,
 	teardown,
@@ -7,11 +9,13 @@ import {
 } from '../_util/helpers';
 import { ul, li, div } from '../_util/dom';
 import { logCall, clearLog, getLog } from '../_util/logCall';
+import { setupRerender } from 'preact/test-utils';
 
 /** @jsx createElement */
 
 describe('hydrate()', () => {
 	let scratch;
+	let rerender;
 
 	const List = ({ children }) => <ul>{children}</ul>;
 	const ListItem = ({ children }) => <li>{children}</li>;
@@ -27,6 +31,7 @@ describe('hydrate()', () => {
 
 	beforeEach(() => {
 		scratch = setupScratch();
+		rerender = setupRerender();
 	});
 
 	afterEach(() => {
@@ -276,4 +281,45 @@ describe('hydrate()', () => {
 		hydrate(<Component foo="bar" />, element);
 		expect(element.innerHTML).to.equal('<p>hello bar</p>');
 	});
+
+	it('should reuse suspense component markup when suspense resolves during hydration', () => {
+		scratch.innerHTML = '<div id="test"><p>hello bar</p><p>Hello foo</p></div>';
+		const element = scratch;
+		let resolver;
+		const Component = () => {
+			const [state, setState] = useState(false);
+			if (!state) {
+				throw new Promise(resolve => {
+					resolver = () => {
+						setState(true);
+						resolve();
+					};
+				});
+			} else {
+				return <p>hello bar</p>;
+			}
+		};
+		const App = () => {
+			return (
+				<div id="test">
+					<Suspense fallback={<div>baz</div>}>
+						<Component />
+					</Suspense>
+					<p>Hello foo</p>
+				</div>
+			);
+		};
+		hydrate(<App />, element);
+		rerender();
+		expect(element.innerHTML).to.equal(
+			'<div id="test"><p>hello bar</p><p>Hello foo</p></div>'
+		);
+		const removeChildSpy = sinon.spy(element.firstChild, 'removeChild');
+		resolver();
+		rerender();
+		expect(removeChildSpy).to.be.not.called;
+		expect(element.innerHTML).to.equal(
+			'<div id="test"><p>hello bar</p><p>Hello foo</p></div>'
+		);
+	});
 });

test/browser/render.test.js

@@ -1271,21 +1271,25 @@ describe('render()', () => {
 		scratch.appendChild(container);
 		render(<div>Hello</div>, scratch, scratch.firstElementChild);
 		expect(scratch.innerHTML).to.equal('<div>Hello</div>');
+		const child = scratch.firstElementChild;
 
 		render(<div>Hello</div>, scratch, scratch.firstElementChild);
 		expect(scratch.innerHTML).to.equal('<div>Hello</div>');
+		expect(scratch.firstElementChild).to.equal(child);
 	});
 
 	it('should replaceNode after rendering', () => {
 		function App({ i }) {
 			return <p>{i}</p>;
 		}
-
+		scratch.innerHTML = '';
 		render(<App i={2} />, scratch);
 		expect(scratch.innerHTML).to.equal('<p>2</p>');
+		const child = scratch.firstElementChild;
 
-		render(<App i={3} />, scratch, scratch.firstChild);
+		render(<App i={3} />, scratch, child);
 		expect(scratch.innerHTML).to.equal('<p>3</p>');
+		expect(scratch.firstElementChild).to.equal(child);
 	});
 
 	it('should not cause infinite loop with referentially equal props', () => {
@@ -1325,7 +1329,7 @@ describe('render()', () => {
 		it('should use replaceNode as render root and not inject into it', () => {
 			const childA = scratch.querySelector('#a');
 			render(<div id="a">contents</div>, scratch, childA);
-			expect(scratch.querySelector('#a')).to.equalNode(childA);
+			expect(scratch.querySelector('#a')).to.equal(childA);
 			expect(childA.innerHTML).to.equal('contents');
 		});