Preserve mounted hook state on re-suspend with a smaller footprint

Alternative to the approach in #5239: instead of tracking whether the
Suspense boundary has committed via lifecycle hooks and threading a
preserveHooks flag through detachedClone, discard hooks per component in
options._catchError when the suspending vnode never committed
(oldVNode has no component). detachedClone then always keeps hook state,
runs effect cleanups, and clears effect args so effects re-run on reveal.

Also drops effects queued by the aborted render (_pendingEffects and
_renderCallbacks), which otherwise made a changed-deps layout effect run
twice on reveal.

Size: compat +36 B br, hooks +6 B br (vs +70 B / +6 B for #5239).

Author: JoviDeCroock

compat/src/suspense.js

@@ -17,6 +17,10 @@ function initSuspenseHooks() {
 
 			while ((vnode = vnode._parent)) {
 				if ((component = vnode._component) && component._childDidSuspend) {
+					// A component that suspends before ever committing has no state
+					// worth keeping; mounted ones keep their hooks while parked.
+					if (oldVNode && !oldVNode._component)
+						newVNode._component.__hooks = UNDEFINED;
 					// Don't call oldCatchError if we found a Suspense
 					return component._childDidSuspend(error, newVNode);
 				}
@@ -38,19 +42,22 @@ function initSuspenseHooks() {
 	};
 }
 
-function detachedClone(vnode, detachedParent, parentDom, preserveHooks) {
+function detachedClone(vnode, detachedParent, parentDom) {
 	if (vnode) {
-		if (vnode._component && vnode._component.__hooks) {
-			const hooks = vnode._component.__hooks;
+		const hooks = vnode._component && vnode._component.__hooks;
+		if (hooks) {
 			hooks._list.forEach(effect => {
-				if (!preserveHooks || effect._passive != null) {
-					const cleanup = effect._cleanup;
+				// Only effects carry `_passive`; clearing `_args` makes them run
+				// again when the tree is revealed, memo/ref state stays intact.
+				if (effect._passive != null) {
+					if (typeof effect._cleanup == 'function') effect._cleanup();
 					effect._cleanup = effect._args = UNDEFINED;
-					if (typeof cleanup == 'function') cleanup();
 				}
 			});
-			if (preserveHooks) hooks._pendingEffects = [];
-			else vnode._component.__hooks = null;
+			// Drop effects queued by the aborted render; `options._render` swaps in
+			// a fresh `_pendingEffects` array before anything is pushed again, so
+			// sharing one empty array here is safe.
+			hooks._pendingEffects = vnode._component._renderCallbacks = [];
 		}
 
 		vnode = assign({ constructor: UNDEFINED }, vnode);
@@ -67,7 +74,7 @@ function detachedClone(vnode, detachedParent, parentDom, preserveHooks) {
 		vnode._children =
 			vnode._children &&
 			vnode._children.map(child =>
-				detachedClone(child, detachedParent, parentDom, preserveHooks)
+				detachedClone(child, detachedParent, parentDom)
 			);
 	}
 
@@ -191,10 +198,6 @@ function createSuspense() {
 	Suspense.prototype.componentWillUnmount = function () {
 		this._suspenders = [];
 	};
-	Suspense.prototype.componentDidMount =
-		Suspense.prototype.componentDidUpdate = function () {
-			if (!this._pendingSuspensionCount) this._unmounted = false;
-		};
 
 	/**
 	 * @this {import('./internal').SuspenseComponent}
@@ -212,8 +215,7 @@ function createSuspense() {
 				this._vnode._children[0] = detachedClone(
 					this._detachOnNextRender,
 					detachedParent,
-					(detachedComponent._originalParentDom = detachedComponent._parentDom),
-					this._unmounted == false
+					(detachedComponent._originalParentDom = detachedComponent._parentDom)
 				);
 			}
 

compat/test/browser/suspense.test.jsx

@@ -378,6 +378,67 @@ describe('suspense', () => {
 		);
 	});
 
+	it('should run a layout effect with changed deps once when revealed', async () => {
+		let promise = Promise.resolve();
+		let set;
+		let first = true;
+		const setup = vi.fn();
+		const cleanup = vi.fn();
+		function App() {
+			const [n, setN] = useState(0);
+			set = setN;
+			useLayoutEffect(() => {
+				setup(n);
+				return () => cleanup(n);
+			}, [n]);
+			if (n && first) {
+				first = false;
+				throw promise;
+			}
+			return <p>{n}</p>;
+		}
+
+		render(
+			<Suspense fallback="loading">
+				<App />
+			</Suspense>,
+			scratch
+		);
+		expect(setup).toHaveBeenCalledTimes(1);
+
+		set(1);
+		rerender();
+		expect(scratch.textContent).to.equal('loading');
+		expect(cleanup).toHaveBeenCalledTimes(1);
+
+		await promise;
+		await act(() => rerender());
+		expect(scratch.textContent).to.equal('1');
+		expect(setup.mock.calls).to.deep.equal([[0], [1]]);
+		expect(cleanup).toHaveBeenCalledTimes(1);
+	});
+
+	it('should handle a promise thrown from a layout effect', () => {
+		let threw = false;
+		function App() {
+			useLayoutEffect(() => {
+				if (!threw) {
+					threw = true;
+					throw Promise.resolve();
+				}
+			});
+			return <p>x</p>;
+		}
+
+		render(
+			<Suspense fallback="loading">
+				<App />
+			</Suspense>,
+			scratch
+		);
+		rerender();
+	});
+
 	it('should support a call to setState before rendering the fallback', () => {
 		const LazyComp = ({ name }) => <div>Hello from {name}</div>;