Skip to content

Commit b4efcd7

Browse files
committed
fix: retry on Suspense render and preserve hook state
- If a parent rerenders while a child suspends, retry rerendering suspended children. This supports patterns like react-freeze where a never-resolving thenable freezes a subtree and a parent re-render clears it. - Preserve hook state (useState, etc.) across suspend/unsuspend for already-mounted components. Discard hook state for components that suspend during initial mount (matching React's WIP fiber discard). - Run effect cleanups (both useEffect and useLayoutEffect) uniformly during suspension. Effects rerun when unsuspended.
1 parent 2459326 commit b4efcd7

File tree

2 files changed

+281
-58
lines changed

2 files changed

+281
-58
lines changed

compat/src/suspense.js

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,21 @@ options.unmount = function (vnode) {
4242
function detachedClone(vnode, detachedParent, parentDom) {
4343
if (vnode) {
4444
if (vnode._component && vnode._component.__hooks) {
45-
vnode._component.__hooks._list.forEach(effect => {
46-
if (typeof effect._cleanup == 'function') effect._cleanup();
47-
});
48-
49-
vnode._component.__hooks = null;
45+
if (vnode._dom != null) {
46+
// Already-mounted component: preserve hook state but run
47+
// effect cleanups. Effects will rerun when unsuspended.
48+
vnode._component.__hooks._list.forEach(effect => {
49+
if (typeof effect._cleanup == 'function') {
50+
effect._cleanup();
51+
effect._cleanup = undefined;
52+
}
53+
});
54+
} else {
55+
// Component suspended during initial mount (never committed
56+
// to DOM). Discard hook state entirely, matching React's
57+
// behavior of discarding the WIP fiber.
58+
vnode._component.__hooks = null;
59+
}
5060
}
5161

5262
vnode = assign({}, vnode);
@@ -206,7 +216,34 @@ Suspense.prototype.render = function (props, state) {
206216
);
207217
}
208218

219+
// Save reference to this vnode. After diffChildren, its _children
220+
// array becomes the "old children" for future diffs. We need this
221+
// reference to swap in original vnodes when retrying after suspension.
222+
this._suspendedVNode = this._vnode;
209223
this._detachOnNextRender = null;
224+
} else if (state._suspended) {
225+
// Parent re-rendered while suspended (not from _childDidSuspend).
226+
// Try rendering children again. If they re-throw, _childDidSuspend
227+
// will re-catch and re-suspend. This supports patterns like
228+
// react-freeze where a never-resolving thenable is thrown to freeze
229+
// a subtree and later cleared by a parent re-render.
230+
231+
// Restore the original vnode tree into the old children array
232+
// so the diff can reuse existing component instances (preserving
233+
// hook state like useState).
234+
const suspendedVNode = state._suspended;
235+
if (this._suspendedVNode && this._suspendedVNode._children) {
236+
this._suspendedVNode._children[0] = removeOriginal(
237+
suspendedVNode,
238+
suspendedVNode._component._parentDom,
239+
suspendedVNode._component._originalParentDom
240+
);
241+
}
242+
243+
this._suspendedVNode = null;
244+
this._pendingSuspensionCount = 0;
245+
this._suspenders = null;
246+
state._suspended = null;
210247
}
211248

212249
return [

compat/test/browser/suspense.test.jsx

Lines changed: 239 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -159,58 +159,7 @@ describe('suspense', () => {
159159
resolve().then(assert).catch(assert);
160160
});
161161

162-
it('should reset hooks of components', () => {
163-
/** @type {(v) => void} */
164-
let set;
165-
const LazyComp = ({ name }) => <div>Hello from {name}</div>;
166-
167-
/** @type {() => Promise<void>} */
168-
let resolve;
169-
const Lazy = lazy(() => {
170-
const p = new Promise(res => {
171-
resolve = () => {
172-
res({ default: LazyComp });
173-
return p;
174-
};
175-
});
176-
177-
return p;
178-
});
179-
180-
const Parent = ({ children }) => {
181-
const [state, setState] = useState(false);
182-
set = setState;
183-
184-
return (
185-
<div>
186-
<p>hi</p>
187-
{state && children}
188-
</div>
189-
);
190-
};
191-
192-
render(
193-
<Suspense fallback={<div>Suspended...</div>}>
194-
<Parent>
195-
<Lazy name="LazyComp" />
196-
</Parent>
197-
</Suspense>,
198-
scratch
199-
);
200-
expect(scratch.innerHTML).to.eql(`<div><p>hi</p></div>`);
201-
202-
set(true);
203-
rerender();
204-
205-
expect(scratch.innerHTML).to.eql('<div>Suspended...</div>');
206-
207-
return resolve().then(() => {
208-
rerender();
209-
expect(scratch.innerHTML).to.eql(`<div><p>hi</p></div>`);
210-
});
211-
});
212-
213-
it('should call effect cleanups', () => {
162+
it('should call effect cleanups when suspending', () => {
214163
/** @type {(v) => void} */
215164
let set;
216165
const effectSpy = vi.fn();
@@ -266,14 +215,14 @@ describe('suspense', () => {
266215
set(true);
267216
rerender();
268217
expect(scratch.innerHTML).to.eql('<div>Suspended...</div>');
218+
// Both effect types should have their cleanups called during suspension
269219
expect(effectSpy).toHaveBeenCalledOnce();
270220
expect(layoutEffectSpy).toHaveBeenCalledOnce();
271221

272222
return resolve().then(() => {
273223
rerender();
274224
expect(effectSpy).toHaveBeenCalledOnce();
275225
expect(layoutEffectSpy).toHaveBeenCalledOnce();
276-
expect(scratch.innerHTML).to.eql(`<div><p>hi</p></div>`);
277226
});
278227
});
279228

@@ -2662,4 +2611,241 @@ describe('suspense', () => {
26622611
expect(renderCount).to.equal(renderCountAfterSuspend);
26632612
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
26642613
});
2614+
2615+
describe('suspension with never-resolving thenable', () => {
2616+
const neverResolve = { then() {} };
2617+
2618+
function ThrowWhen({ suspend, children }) {
2619+
if (suspend) {
2620+
throw neverResolve;
2621+
}
2622+
return createElement(Fragment, null, children);
2623+
}
2624+
2625+
it('should unsuspend when children stop throwing on parent re-render', () => {
2626+
/** @type {(v: boolean) => void} */
2627+
let setSuspend;
2628+
function App() {
2629+
const [suspend, ss] = useState(false);
2630+
setSuspend = ss;
2631+
return (
2632+
<Suspense fallback={<div>Loading...</div>}>
2633+
<ThrowWhen suspend={suspend}>
2634+
<div>Content</div>
2635+
</ThrowWhen>
2636+
</Suspense>
2637+
);
2638+
}
2639+
2640+
render(<App />, scratch);
2641+
expect(scratch.innerHTML).to.equal('<div>Content</div>');
2642+
2643+
act(() => setSuspend(true));
2644+
rerender();
2645+
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
2646+
2647+
act(() => setSuspend(false));
2648+
rerender();
2649+
expect(scratch.innerHTML).to.equal('<div>Content</div>');
2650+
});
2651+
2652+
it('should preserve child state across suspend/unsuspend', () => {
2653+
/** @type {(v: boolean) => void} */
2654+
let setSuspend;
2655+
/** @type {(v: number) => void} */
2656+
let setCount;
2657+
2658+
function Counter() {
2659+
const [count, sc] = useState(42);
2660+
setCount = sc;
2661+
return <div>Count: {count}</div>;
2662+
}
2663+
2664+
function App() {
2665+
const [suspend, ss] = useState(false);
2666+
setSuspend = ss;
2667+
return (
2668+
<Suspense fallback={<div>Loading...</div>}>
2669+
<ThrowWhen suspend={suspend}>
2670+
<Counter />
2671+
</ThrowWhen>
2672+
</Suspense>
2673+
);
2674+
}
2675+
2676+
render(<App />, scratch);
2677+
expect(scratch.innerHTML).to.equal('<div>Count: 42</div>');
2678+
2679+
act(() => setCount(100));
2680+
rerender();
2681+
expect(scratch.innerHTML).to.equal('<div>Count: 100</div>');
2682+
2683+
act(() => setSuspend(true));
2684+
rerender();
2685+
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
2686+
2687+
act(() => setSuspend(false));
2688+
rerender();
2689+
expect(scratch.innerHTML).to.equal('<div>Count: 100</div>');
2690+
});
2691+
2692+
it('should discard hook state for components suspending during initial mount', () => {
2693+
let resolve;
2694+
const LazyComp = ({ name }) => <div>Hello from {name}</div>;
2695+
const Lazy = lazy(() => {
2696+
const p = new Promise(res => {
2697+
resolve = () => {
2698+
res({ default: LazyComp });
2699+
return p;
2700+
};
2701+
});
2702+
return p;
2703+
});
2704+
2705+
/** @type {(v: boolean) => void} */
2706+
let set;
2707+
const Parent = ({ children }) => {
2708+
const [state, setState] = useState(false);
2709+
set = setState;
2710+
return (
2711+
<div>
2712+
<p>hi</p>
2713+
{state && children}
2714+
</div>
2715+
);
2716+
};
2717+
2718+
render(
2719+
<Suspense fallback={<div>Suspended...</div>}>
2720+
<Parent>
2721+
<Lazy name="LazyComp" />
2722+
</Parent>
2723+
</Suspense>,
2724+
scratch
2725+
);
2726+
expect(scratch.innerHTML).to.eql('<div><p>hi</p></div>');
2727+
2728+
// Enable children - triggers Lazy which suspends during mount
2729+
set(true);
2730+
rerender();
2731+
expect(scratch.innerHTML).to.eql('<div>Suspended...</div>');
2732+
2733+
return resolve().then(() => {
2734+
rerender();
2735+
// Parent's hook state is preserved (it was already mounted),
2736+
// so state is still true and Lazy renders.
2737+
expect(scratch.innerHTML).to.eql(
2738+
'<div><p>hi</p><div>Hello from LazyComp</div></div>'
2739+
);
2740+
});
2741+
});
2742+
2743+
it('should restore effects after unsuspend', () => {
2744+
/** @type {(v: boolean) => void} */
2745+
let setSuspend;
2746+
2747+
const effectSpy = vi.fn();
2748+
const cleanupSpy = vi.fn();
2749+
const layoutEffectSpy = vi.fn();
2750+
const layoutCleanupSpy = vi.fn();
2751+
2752+
function Child() {
2753+
useEffect(() => {
2754+
effectSpy();
2755+
return () => cleanupSpy();
2756+
});
2757+
useLayoutEffect(() => {
2758+
layoutEffectSpy();
2759+
return () => layoutCleanupSpy();
2760+
});
2761+
return <div>Child</div>;
2762+
}
2763+
2764+
function App() {
2765+
const [suspend, ss] = useState(false);
2766+
setSuspend = ss;
2767+
return (
2768+
<Suspense fallback={<div>Loading...</div>}>
2769+
<ThrowWhen suspend={suspend}>
2770+
<Child />
2771+
</ThrowWhen>
2772+
</Suspense>
2773+
);
2774+
}
2775+
2776+
act(() => {
2777+
render(<App />, scratch);
2778+
});
2779+
expect(scratch.innerHTML).to.equal('<div>Child</div>');
2780+
expect(effectSpy).toHaveBeenCalledTimes(1);
2781+
expect(layoutEffectSpy).toHaveBeenCalledTimes(1);
2782+
2783+
effectSpy.mockClear();
2784+
cleanupSpy.mockClear();
2785+
layoutEffectSpy.mockClear();
2786+
layoutCleanupSpy.mockClear();
2787+
2788+
act(() => setSuspend(true));
2789+
rerender();
2790+
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
2791+
2792+
// Both effect type cleanups should run during suspension
2793+
expect(layoutCleanupSpy).toHaveBeenCalledTimes(1);
2794+
expect(cleanupSpy).toHaveBeenCalledTimes(1);
2795+
2796+
effectSpy.mockClear();
2797+
cleanupSpy.mockClear();
2798+
layoutEffectSpy.mockClear();
2799+
layoutCleanupSpy.mockClear();
2800+
2801+
act(() => setSuspend(false));
2802+
rerender();
2803+
expect(scratch.innerHTML).to.equal('<div>Child</div>');
2804+
expect(effectSpy).toHaveBeenCalledTimes(1);
2805+
expect(layoutEffectSpy).toHaveBeenCalledTimes(1);
2806+
});
2807+
2808+
it('should not re-render suspended children when parent updates', () => {
2809+
/** @type {(v: boolean) => void} */
2810+
let setSuspend;
2811+
/** @type {(v: number) => void} */
2812+
let setParentCount;
2813+
const childRenderSpy = vi.fn();
2814+
2815+
function Child() {
2816+
childRenderSpy();
2817+
return <div>Child</div>;
2818+
}
2819+
2820+
function App() {
2821+
const [suspend, ss] = useState(false);
2822+
const [count, sc] = useState(0);
2823+
setSuspend = ss;
2824+
setParentCount = sc;
2825+
return (
2826+
<div>
2827+
<span>Parent: {count}</span>
2828+
<Suspense fallback={<div>Loading...</div>}>
2829+
<ThrowWhen suspend={suspend}>
2830+
<Child />
2831+
</ThrowWhen>
2832+
</Suspense>
2833+
</div>
2834+
);
2835+
}
2836+
2837+
act(() => {
2838+
render(<App />, scratch);
2839+
});
2840+
expect(childRenderSpy).toHaveBeenCalledTimes(1);
2841+
2842+
act(() => setSuspend(true));
2843+
rerender();
2844+
childRenderSpy.mockClear();
2845+
2846+
act(() => setParentCount(1));
2847+
rerender();
2848+
expect(childRenderSpy).not.toHaveBeenCalled();
2849+
});
2850+
});
26652851
});

0 commit comments

Comments
 (0)