Skip to content

Commit ccf8ff0

Browse files
mary-extmary-ext
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 ccf8ff0

File tree

2 files changed

+330
-8
lines changed

2 files changed

+330
-8
lines changed

‎compat/src/suspense.js‎

Lines changed: 40 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,32 @@ 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._pendingSuspensionCount = 0;
244+
state._suspended = this._suspendedVNode = this._suspenders = null;
210245
}
211246

212247
return [

‎compat/test/browser/suspense.test.jsx‎

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

162-
it('should reset hooks of components', () => {
162+
it('should preserve hooks of already-mounted components', () => {
163163
/** @type {(v) => void} */
164164
let set;
165165
const LazyComp = ({ name }) => <div>Hello from {name}</div>;
@@ -206,7 +206,11 @@ describe('suspense', () => {
206206

207207
return resolve().then(() => {
208208
rerender();
209-
expect(scratch.innerHTML).to.eql(`<div><p>hi</p></div>`);
209+
// Parent was already mounted so hook state is preserved (state
210+
// stays true) and the resolved Lazy component renders.
211+
expect(scratch.innerHTML).to.eql(
212+
'<div><p>hi</p><div>Hello from LazyComp</div></div>'
213+
);
210214
});
211215
});
212216

@@ -273,7 +277,10 @@ describe('suspense', () => {
273277
rerender();
274278
expect(effectSpy).toHaveBeenCalledOnce();
275279
expect(layoutEffectSpy).toHaveBeenCalledOnce();
276-
expect(scratch.innerHTML).to.eql(`<div><p>hi</p></div>`);
280+
// Parent state preserved: state=true so renders children branch
281+
expect(scratch.innerHTML).to.eql(
282+
'<div><div>Hello from LazyComp</div></div>'
283+
);
277284
});
278285
});
279286

@@ -2662,4 +2669,284 @@ describe('suspense', () => {
26622669
expect(renderCount).to.equal(renderCountAfterSuspend);
26632670
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
26642671
});
2672+
2673+
describe('suspension with never-resolving thenable', () => {
2674+
const neverResolve = { then() {} };
2675+
2676+
function ThrowWhen({ suspend, children }) {
2677+
if (suspend) {
2678+
throw neverResolve;
2679+
}
2680+
return createElement(Fragment, null, children);
2681+
}
2682+
2683+
it('should unsuspend when children stop throwing on parent re-render', () => {
2684+
/** @type {(v: boolean) => void} */
2685+
let setSuspend;
2686+
function App() {
2687+
const [suspend, ss] = useState(false);
2688+
setSuspend = ss;
2689+
return (
2690+
<Suspense fallback={<div>Loading...</div>}>
2691+
<ThrowWhen suspend={suspend}>
2692+
<div>Content</div>
2693+
</ThrowWhen>
2694+
</Suspense>
2695+
);
2696+
}
2697+
2698+
render(<App />, scratch);
2699+
expect(scratch.innerHTML).to.equal('<div>Content</div>');
2700+
2701+
act(() => setSuspend(true));
2702+
rerender();
2703+
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
2704+
2705+
act(() => setSuspend(false));
2706+
rerender();
2707+
expect(scratch.innerHTML).to.equal('<div>Content</div>');
2708+
});
2709+
2710+
it('should preserve child state across suspend/unsuspend', () => {
2711+
/** @type {(v: boolean) => void} */
2712+
let setSuspend;
2713+
/** @type {(v: number) => void} */
2714+
let setCount;
2715+
2716+
function Counter() {
2717+
const [count, sc] = useState(42);
2718+
setCount = sc;
2719+
return <div>Count: {count}</div>;
2720+
}
2721+
2722+
function App() {
2723+
const [suspend, ss] = useState(false);
2724+
setSuspend = ss;
2725+
return (
2726+
<Suspense fallback={<div>Loading...</div>}>
2727+
<ThrowWhen suspend={suspend}>
2728+
<Counter />
2729+
</ThrowWhen>
2730+
</Suspense>
2731+
);
2732+
}
2733+
2734+
render(<App />, scratch);
2735+
expect(scratch.innerHTML).to.equal('<div>Count: 42</div>');
2736+
2737+
act(() => setCount(100));
2738+
rerender();
2739+
expect(scratch.innerHTML).to.equal('<div>Count: 100</div>');
2740+
2741+
act(() => setSuspend(true));
2742+
rerender();
2743+
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
2744+
2745+
act(() => setSuspend(false));
2746+
rerender();
2747+
expect(scratch.innerHTML).to.equal('<div>Count: 100</div>');
2748+
});
2749+
2750+
it('should restore effects after unsuspend', () => {
2751+
/** @type {(v: boolean) => void} */
2752+
let setSuspend;
2753+
2754+
const effectSpy = vi.fn();
2755+
const cleanupSpy = vi.fn();
2756+
const layoutEffectSpy = vi.fn();
2757+
const layoutCleanupSpy = vi.fn();
2758+
2759+
function Child() {
2760+
useEffect(() => {
2761+
effectSpy();
2762+
return () => cleanupSpy();
2763+
});
2764+
useLayoutEffect(() => {
2765+
layoutEffectSpy();
2766+
return () => layoutCleanupSpy();
2767+
});
2768+
return <div>Child</div>;
2769+
}
2770+
2771+
function App() {
2772+
const [suspend, ss] = useState(false);
2773+
setSuspend = ss;
2774+
return (
2775+
<Suspense fallback={<div>Loading...</div>}>
2776+
<ThrowWhen suspend={suspend}>
2777+
<Child />
2778+
</ThrowWhen>
2779+
</Suspense>
2780+
);
2781+
}
2782+
2783+
act(() => {
2784+
render(<App />, scratch);
2785+
});
2786+
expect(scratch.innerHTML).to.equal('<div>Child</div>');
2787+
expect(effectSpy).toHaveBeenCalledTimes(1);
2788+
expect(layoutEffectSpy).toHaveBeenCalledTimes(1);
2789+
2790+
effectSpy.mockClear();
2791+
cleanupSpy.mockClear();
2792+
layoutEffectSpy.mockClear();
2793+
layoutCleanupSpy.mockClear();
2794+
2795+
act(() => setSuspend(true));
2796+
rerender();
2797+
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
2798+
2799+
// Both effect type cleanups should run during suspension
2800+
expect(layoutCleanupSpy).toHaveBeenCalledTimes(1);
2801+
expect(cleanupSpy).toHaveBeenCalledTimes(1);
2802+
2803+
effectSpy.mockClear();
2804+
cleanupSpy.mockClear();
2805+
layoutEffectSpy.mockClear();
2806+
layoutCleanupSpy.mockClear();
2807+
2808+
act(() => setSuspend(false));
2809+
rerender();
2810+
expect(scratch.innerHTML).to.equal('<div>Child</div>');
2811+
expect(effectSpy).toHaveBeenCalledTimes(1);
2812+
expect(layoutEffectSpy).toHaveBeenCalledTimes(1);
2813+
});
2814+
2815+
it('should handle parent re-rendering twice while children stay suspended', () => {
2816+
/** @type {(v: boolean) => void} */
2817+
let setSuspend;
2818+
/** @type {(v: number) => void} */
2819+
let setCount;
2820+
2821+
function App() {
2822+
const [suspend, ss] = useState(false);
2823+
const [count, sc] = useState(0);
2824+
setSuspend = ss;
2825+
setCount = sc;
2826+
return (
2827+
<Suspense fallback={<div>Loading...</div>}>
2828+
<ThrowWhen suspend={suspend}>
2829+
<div>Count: {count}</div>
2830+
</ThrowWhen>
2831+
</Suspense>
2832+
);
2833+
}
2834+
2835+
render(<App />, scratch);
2836+
expect(scratch.innerHTML).to.equal('<div>Count: 0</div>');
2837+
2838+
act(() => setSuspend(true));
2839+
rerender();
2840+
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
2841+
2842+
// Parent re-renders while still suspended - children re-throw
2843+
act(() => setCount(1));
2844+
rerender();
2845+
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
2846+
2847+
// Parent re-renders again while still suspended
2848+
act(() => setCount(2));
2849+
rerender();
2850+
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
2851+
2852+
// Now unsuspend
2853+
act(() => setSuspend(false));
2854+
rerender();
2855+
expect(scratch.innerHTML).to.equal('<div>Count: 2</div>');
2856+
});
2857+
2858+
it('should handle two consecutive suspend/unsuspend cycles', () => {
2859+
/** @type {(v: boolean) => void} */
2860+
let setSuspend;
2861+
/** @type {(v: number) => void} */
2862+
let setCount;
2863+
2864+
function Counter() {
2865+
const [count, sc] = useState(0);
2866+
setCount = sc;
2867+
return <div>Count: {count}</div>;
2868+
}
2869+
2870+
function App() {
2871+
const [suspend, ss] = useState(false);
2872+
setSuspend = ss;
2873+
return (
2874+
<Suspense fallback={<div>Loading...</div>}>
2875+
<ThrowWhen suspend={suspend}>
2876+
<Counter />
2877+
</ThrowWhen>
2878+
</Suspense>
2879+
);
2880+
}
2881+
2882+
render(<App />, scratch);
2883+
expect(scratch.innerHTML).to.equal('<div>Count: 0</div>');
2884+
2885+
// First cycle: suspend -> unsuspend
2886+
act(() => setSuspend(true));
2887+
rerender();
2888+
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
2889+
2890+
act(() => setSuspend(false));
2891+
rerender();
2892+
expect(scratch.innerHTML).to.equal('<div>Count: 0</div>');
2893+
2894+
// Update state between cycles
2895+
act(() => setCount(5));
2896+
rerender();
2897+
expect(scratch.innerHTML).to.equal('<div>Count: 5</div>');
2898+
2899+
// Second cycle: suspend -> unsuspend
2900+
act(() => setSuspend(true));
2901+
rerender();
2902+
expect(scratch.innerHTML).to.equal('<div>Loading...</div>');
2903+
2904+
act(() => setSuspend(false));
2905+
rerender();
2906+
expect(scratch.innerHTML).to.equal('<div>Count: 5</div>');
2907+
});
2908+
2909+
it('should not re-render suspended children when parent updates', () => {
2910+
/** @type {(v: boolean) => void} */
2911+
let setSuspend;
2912+
/** @type {(v: number) => void} */
2913+
let setParentCount;
2914+
const childRenderSpy = vi.fn();
2915+
2916+
function Child() {
2917+
childRenderSpy();
2918+
return <div>Child</div>;
2919+
}
2920+
2921+
function App() {
2922+
const [suspend, ss] = useState(false);
2923+
const [count, sc] = useState(0);
2924+
setSuspend = ss;
2925+
setParentCount = sc;
2926+
return (
2927+
<div>
2928+
<span>Parent: {count}</span>
2929+
<Suspense fallback={<div>Loading...</div>}>
2930+
<ThrowWhen suspend={suspend}>
2931+
<Child />
2932+
</ThrowWhen>
2933+
</Suspense>
2934+
</div>
2935+
);
2936+
}
2937+
2938+
act(() => {
2939+
render(<App />, scratch);
2940+
});
2941+
expect(childRenderSpy).toHaveBeenCalledTimes(1);
2942+
2943+
act(() => setSuspend(true));
2944+
rerender();
2945+
childRenderSpy.mockClear();
2946+
2947+
act(() => setParentCount(1));
2948+
rerender();
2949+
expect(childRenderSpy).not.toHaveBeenCalled();
2950+
});
2951+
});
26652952
});

0 commit comments

Comments
 (0)