From be33f7af25bb25f3a584591b3951101f48de8910 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Fri, 4 Sep 2026 07:13:30 +0000 Subject: [PATCH 1/2] Fix hook state loss when Suspense re-suspends --- compat/src/suspense.js | 24 ++++-- .../test/browser/suspense-hydration.test.jsx | 71 +++++++++++++++++ compat/test/browser/suspense.test.jsx | 78 ++++++++++++++++--- hooks/src/index.js | 1 + mangle.json | 1 + 5 files changed, 158 insertions(+), 17 deletions(-) diff --git a/compat/src/suspense.js b/compat/src/suspense.js index 15c44f02f3..d7d5a66577 100644 --- a/compat/src/suspense.js +++ b/compat/src/suspense.js @@ -38,14 +38,19 @@ function initSuspenseHooks() { }; } -function detachedClone(vnode, detachedParent, parentDom) { +function detachedClone(vnode, detachedParent, parentDom, preserveHooks) { if (vnode) { if (vnode._component && vnode._component.__hooks) { - vnode._component.__hooks._list.forEach(effect => { - if (typeof effect._cleanup == 'function') effect._cleanup(); + const hooks = vnode._component.__hooks; + hooks._list.forEach(effect => { + if (!preserveHooks || effect._passive != null) { + const cleanup = effect._cleanup; + effect._cleanup = effect._args = UNDEFINED; + if (typeof cleanup == 'function') cleanup(); + } }); - - vnode._component.__hooks = null; + if (preserveHooks) hooks._pendingEffects = []; + else vnode._component.__hooks = null; } vnode = assign({ constructor: UNDEFINED }, vnode); @@ -62,7 +67,7 @@ function detachedClone(vnode, detachedParent, parentDom) { vnode._children = vnode._children && vnode._children.map(child => - detachedClone(child, detachedParent, parentDom) + detachedClone(child, detachedParent, parentDom, preserveHooks) ); } @@ -186,6 +191,10 @@ 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} @@ -203,7 +212,8 @@ function createSuspense() { this._vnode._children[0] = detachedClone( this._detachOnNextRender, detachedParent, - (detachedComponent._originalParentDom = detachedComponent._parentDom) + (detachedComponent._originalParentDom = detachedComponent._parentDom), + this._unmounted == false ); } diff --git a/compat/test/browser/suspense-hydration.test.jsx b/compat/test/browser/suspense-hydration.test.jsx index b1570c58b8..7d6b5d098a 100644 --- a/compat/test/browser/suspense-hydration.test.jsx +++ b/compat/test/browser/suspense-hydration.test.jsx @@ -2,8 +2,10 @@ import { setupRerender } from 'preact/test-utils'; import React, { createElement, hydrate, + render, Fragment, Suspense, + use, memo, useState, useSyncExternalStore @@ -874,6 +876,75 @@ describe('suspense hydration', () => { }); }); + it('should preserve component state when re-suspending after streaming-style hydration', async () => { + scratch.innerHTML = + '

Hello

'; + + let promise = Promise.resolve('Hello'); + let increment; + function App() { + const message = use(promise); + const [count, setCount] = useState(0); + increment = () => setCount(value => value + 1); + return ( +
+

{message}

+ +
+ ); + } + + hydrate( + Fallback}> + + , + scratch + ); + await promise; + rerender(); + rerender(); + + increment(); + rerender(); + expect(scratch.querySelector('button').textContent).to.equal('Count: 1'); + + promise = Promise.resolve('Hello'); + increment(); + rerender(); + await promise; + rerender(); + rerender(); + + expect(scratch.querySelector('button').textContent).to.equal('Count: 2'); + }); + + it('should preserve component state when re-suspending after client render', async () => { + let promise = Promise.resolve('Hello'); + let increment; + function App() { + use(promise); + const [count, setCount] = useState(0); + increment = () => setCount(value => value + 1); + return ; + } + + render(, scratch); + await promise; + rerender(); + rerender(); + increment(); + rerender(); + expect(scratch.textContent).to.equal('Count: 1'); + + promise = Promise.resolve('Hello'); + increment(); + rerender(); + await promise; + rerender(); + rerender(); + expect(scratch.textContent).to.equal('Count: 2'); + }); + it('should correctly hydrate and rerender a memoized lazy data loader', () => { const originalHtml = '

Count: 5

'; scratch.innerHTML = originalHtml; diff --git a/compat/test/browser/suspense.test.jsx b/compat/test/browser/suspense.test.jsx index 595ca857be..6ef58b1822 100644 --- a/compat/test/browser/suspense.test.jsx +++ b/compat/test/browser/suspense.test.jsx @@ -8,6 +8,7 @@ import React, { Fragment, createContext, useState, + useRef, useEffect, useLayoutEffect, memo @@ -202,9 +203,43 @@ describe('suspense', () => { resolve().then(assert).catch(assert); }); - it('should reset hooks of components', () => { + it('should reset hooks when a component subtree suspends during mount', async () => { + let resolve; + let resolved = false; + let initializations = 0; + const promise = new Promise(r => { + resolve = () => { + resolved = true; + r(); + return promise; + }; + }); + + function App() { + const [value] = useState(() => ++initializations); + if (!resolved) throw promise; + return

{value}

; + } + + render( + + + , + scratch + ); + rerender(); + expect(scratch.textContent).to.equal('loading'); + + await resolve(); + rerender(); + expect(scratch.innerHTML).to.equal('

2

'); + expect(initializations).to.equal(2); + }); + + it('should preserve hooks of mounted components', () => { /** @type {(v) => void} */ let set; + let initialRef; const LazyComp = ({ name }) =>
Hello from {name}
; /** @type {() => Promise} */ @@ -222,7 +257,10 @@ describe('suspense', () => { const Parent = ({ children }) => { const [state, setState] = useState(false); + const ref = useRef({}); set = setState; + if (!initialRef) initialRef = ref; + else expect(ref).to.equal(initialRef); return (
@@ -249,15 +287,21 @@ describe('suspense', () => { return resolve().then(() => { rerender(); - expect(scratch.innerHTML).to.eql(`

hi

`); + expect(scratch.innerHTML).to.eql( + `

hi

Hello from LazyComp
` + ); }); }); - it('should call effect cleanups', () => { + it('should call effect cleanups and setups when hiding and revealing', async () => { /** @type {(v) => void} */ let set; + const effectSetupSpy = vi.fn(); const effectSpy = vi.fn(); + const effectWithoutCleanupSpy = vi.fn(); + const layoutEffectSetupSpy = vi.fn(); const layoutEffectSpy = vi.fn(); + const layoutEffectWithoutCleanupSpy = vi.fn(); const LazyComp = ({ name }) =>
Hello from {name}
; /** @type {() => Promise} */ @@ -277,16 +321,20 @@ describe('suspense', () => { const [state, setState] = useState(false); set = setState; useEffect(() => { + effectSetupSpy(); return () => { effectSpy(); }; - }, []); + }, [state]); + useEffect(effectWithoutCleanupSpy, [state]); useLayoutEffect(() => { + layoutEffectSetupSpy(); return () => { layoutEffectSpy(); }; }, []); + useLayoutEffect(layoutEffectWithoutCleanupSpy, []); return state ? (
{children}
@@ -305,19 +353,29 @@ describe('suspense', () => { , scratch ); + expect(layoutEffectSetupSpy).toHaveBeenCalledOnce(); + expect(layoutEffectWithoutCleanupSpy).toHaveBeenCalledOnce(); set(true); rerender(); expect(scratch.innerHTML).to.eql('
Suspended...
'); + + expect(effectSetupSpy).toHaveBeenCalledOnce(); + expect(effectWithoutCleanupSpy).toHaveBeenCalledOnce(); expect(effectSpy).toHaveBeenCalledOnce(); expect(layoutEffectSpy).toHaveBeenCalledOnce(); - return resolve().then(() => { - rerender(); - expect(effectSpy).toHaveBeenCalledOnce(); - expect(layoutEffectSpy).toHaveBeenCalledOnce(); - expect(scratch.innerHTML).to.eql(`

hi

`); - }); + await resolve(); + await act(() => rerender()); + expect(effectSpy).toHaveBeenCalledOnce(); + expect(layoutEffectSpy).toHaveBeenCalledOnce(); + expect(effectSetupSpy).toHaveBeenCalledTimes(2); + expect(effectWithoutCleanupSpy).toHaveBeenCalledTimes(2); + expect(layoutEffectSetupSpy).toHaveBeenCalledTimes(2); + expect(layoutEffectWithoutCleanupSpy).toHaveBeenCalledTimes(2); + expect(scratch.innerHTML).to.eql( + `
Hello from LazyComp
` + ); }); it('should support a call to setState before rendering the fallback', () => { diff --git a/hooks/src/index.js b/hooks/src/index.js index 7e01bc76ec..f71c60c5b6 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -299,6 +299,7 @@ export function useLayoutEffect(callback, args) { /** @type {import('./internal').EffectHookState} */ const state = getHookState(currentIndex++, 4); if (!options._skipEffects && argsChanged(state._args, args)) { + state._passive = false; state._value = callback; state._pendingArgs = args; diff --git a/mangle.json b/mangle.json index 2e8515a059..5193d8cbf6 100644 --- a/mangle.json +++ b/mangle.json @@ -32,6 +32,7 @@ "$_hydrationMismatch": "__m", "$_list": "__", "$_pendingEffects": "__h", + "$_passive": "__P", "$_value": "__", "$_nextValue": "__N", "$_original": "__v", From aa0cf80db8b759d6796d1d094896edb37896ac4a Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Fri, 4 Sep 2026 09:28:26 +0200 Subject: [PATCH 2/2] 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). --- compat/src/suspense.js | 32 +++++++------- compat/test/browser/suspense.test.jsx | 61 +++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 15 deletions(-) diff --git a/compat/src/suspense.js b/compat/src/suspense.js index d7d5a66577..5f922c79d8 100644 --- a/compat/src/suspense.js +++ b/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) ); } diff --git a/compat/test/browser/suspense.test.jsx b/compat/test/browser/suspense.test.jsx index 6ef58b1822..55f9751cc1 100644 --- a/compat/test/browser/suspense.test.jsx +++ b/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

{n}

; + } + + render( + + + , + 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

x

; + } + + render( + + + , + scratch + ); + rerender(); + }); + it('should support a call to setState before rendering the fallback', () => { const LazyComp = ({ name }) =>
Hello from {name}
;