Skip to content

Commit dc3ccc2

Browse files
committed
Start prerendering Suspense retries immediately
When a component suspends and is replaced by a fallback, we should start prerendering the fallback immediately, even before any new data is received. During the retry, we can enter prerender mode directly if we're sure that no new data was received since we last attempted to render the boundary. To do this, when completing the fallback, we leave behind a pending retry lane on the Suspense boundary. Previously we only did this once a promise resolved, but by assigning a lane during the complete phase, we will know that there's speculative work to be done. Then, upon committing the fallback, we mark the retry lane as suspended — but only if nothing was pinged or updated in the meantime. That allows us to immediately enter prerender mode (i.e. render without skipping any siblings) when performing the retry.
1 parent 725d1b0 commit dc3ccc2

25 files changed

+1253
-189
lines changed

packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ let Suspense;
1818
let TextResource;
1919
let textResourceShouldFail;
2020
let waitForAll;
21+
let waitForPaint;
2122
let assertLog;
2223
let waitForThrow;
2324
let act;
@@ -37,6 +38,7 @@ describe('ReactCache', () => {
3738
waitForAll = InternalTestUtils.waitForAll;
3839
assertLog = InternalTestUtils.assertLog;
3940
waitForThrow = InternalTestUtils.waitForThrow;
41+
waitForPaint = InternalTestUtils.waitForPaint;
4042
act = InternalTestUtils.act;
4143

4244
TextResource = createResource(
@@ -119,7 +121,12 @@ describe('ReactCache', () => {
119121
const root = ReactNoop.createRoot();
120122
root.render(<App />);
121123

122-
await waitForAll(['Suspend! [Hi]', 'Loading...']);
124+
await waitForAll([
125+
'Suspend! [Hi]',
126+
'Loading...',
127+
128+
...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []),
129+
]);
123130

124131
jest.advanceTimersByTime(100);
125132
assertLog(['Promise resolved [Hi]']);
@@ -138,7 +145,12 @@ describe('ReactCache', () => {
138145
const root = ReactNoop.createRoot();
139146
root.render(<App />);
140147

141-
await waitForAll(['Suspend! [Hi]', 'Loading...']);
148+
await waitForAll([
149+
'Suspend! [Hi]',
150+
'Loading...',
151+
152+
...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []),
153+
]);
142154

143155
textResourceShouldFail = true;
144156
let error;
@@ -179,12 +191,19 @@ describe('ReactCache', () => {
179191

180192
if (__DEV__) {
181193
await expect(async () => {
182-
await waitForAll(['App', 'Loading...']);
194+
await waitForAll([
195+
'App',
196+
'Loading...',
197+
198+
...(gate('enableSiblingPrerendering') ? ['App'] : []),
199+
]);
183200
}).toErrorDev([
184201
'Invalid key type. Expected a string, number, symbol, or ' +
185202
"boolean, but instead received: [ 'Hi', 100 ]\n\n" +
186203
'To use non-primitive values as keys, you must pass a hash ' +
187204
'function as the second argument to createResource().',
205+
206+
...(gate('enableSiblingPrerendering') ? ['Invalid key type'] : []),
188207
]);
189208
} else {
190209
await waitForAll(['App', 'Loading...']);
@@ -204,7 +223,7 @@ describe('ReactCache', () => {
204223
<AsyncText ms={100} text={3} />
205224
</Suspense>,
206225
);
207-
await waitForAll(['Suspend! [1]', 'Loading...']);
226+
await waitForPaint(['Suspend! [1]', 'Loading...']);
208227
jest.advanceTimersByTime(100);
209228
assertLog(['Promise resolved [1]']);
210229
await waitForAll([1, 'Suspend! [2]', 1, 'Suspend! [2]', 'Suspend! [3]']);
@@ -225,25 +244,18 @@ describe('ReactCache', () => {
225244
</Suspense>,
226245
);
227246

228-
await waitForAll([1, 'Suspend! [4]', 'Loading...']);
229-
230-
await act(() => jest.advanceTimersByTime(100));
231-
assertLog([
232-
'Promise resolved [4]',
233-
247+
await waitForAll([
234248
1,
235-
4,
236-
'Suspend! [5]',
249+
'Suspend! [4]',
250+
'Loading...',
237251
1,
238-
4,
252+
'Suspend! [4]',
239253
'Suspend! [5]',
240-
241-
'Promise resolved [5]',
242-
1,
243-
4,
244-
5,
245254
]);
246255

256+
await act(() => jest.advanceTimersByTime(100));
257+
assertLog(['Promise resolved [4]', 'Promise resolved [5]', 1, 4, 5]);
258+
247259
expect(root).toMatchRenderedOutput('145');
248260

249261
// We've now rendered values 1, 2, 3, 4, 5, over our limit of 3. The least
@@ -263,24 +275,14 @@ describe('ReactCache', () => {
263275
// 2 and 3 suspend because they were evicted from the cache
264276
'Suspend! [2]',
265277
'Loading...',
266-
]);
267278

268-
await act(() => jest.advanceTimersByTime(100));
269-
assertLog([
270-
'Promise resolved [2]',
271-
272-
1,
273-
2,
274-
'Suspend! [3]',
275279
1,
276-
2,
280+
'Suspend! [2]',
277281
'Suspend! [3]',
278-
279-
'Promise resolved [3]',
280-
1,
281-
2,
282-
3,
283282
]);
283+
284+
await act(() => jest.advanceTimersByTime(100));
285+
assertLog(['Promise resolved [2]', 'Promise resolved [3]', 1, 2, 3]);
284286
expect(root).toMatchRenderedOutput('123');
285287
});
286288

@@ -355,7 +357,12 @@ describe('ReactCache', () => {
355357
</Suspense>,
356358
);
357359

358-
await waitForAll(['Suspend! [Hi]', 'Loading...']);
360+
await waitForAll([
361+
'Suspend! [Hi]',
362+
'Loading...',
363+
364+
...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []),
365+
]);
359366

360367
resolveThenable('Hi');
361368
// This thenable improperly resolves twice. We should not update the

packages/react-dom/src/__tests__/ReactDOMForm-test.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1459,13 +1459,23 @@ describe('ReactDOMForm', () => {
14591459
</Suspense>,
14601460
),
14611461
);
1462-
assertLog(['Suspend! [Count: 0]', 'Loading...']);
1462+
assertLog([
1463+
'Suspend! [Count: 0]',
1464+
'Loading...',
1465+
1466+
...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 0]'] : []),
1467+
]);
14631468
await act(() => resolveText('Count: 0'));
14641469
assertLog(['Count: 0']);
14651470

14661471
// Dispatch outside of a transition. This will trigger a loading state.
14671472
await act(() => dispatch());
1468-
assertLog(['Suspend! [Count: 1]', 'Loading...']);
1473+
assertLog([
1474+
'Suspend! [Count: 1]',
1475+
'Loading...',
1476+
1477+
...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 1]'] : []),
1478+
]);
14691479
expect(container.textContent).toBe('Loading...');
14701480

14711481
await act(() => resolveText('Count: 1'));

packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,13 @@ describe('ReactDOMSuspensePlaceholder', () => {
160160
});
161161

162162
expect(container.textContent).toEqual('Loading...');
163-
assertLog(['A', 'Suspend! [B]', 'Loading...']);
163+
assertLog([
164+
'A',
165+
'Suspend! [B]',
166+
'Loading...',
167+
168+
...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [B]', 'C'] : []),
169+
]);
164170
await act(() => {
165171
resolveText('B');
166172
});

packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,13 @@ test('regression (#20932): return pointer is correct before entering deleted tre
192192
await act(() => {
193193
root.render(<App />);
194194
});
195-
assertLog(['Suspend! [0]', 'Loading Async...', 'Loading Tail...']);
195+
assertLog([
196+
'Suspend! [0]',
197+
'Loading Async...',
198+
'Loading Tail...',
199+
200+
...(gate('enableSiblingPrerendering') ? ['Suspend! [0]'] : []),
201+
]);
196202
await act(() => {
197203
resolveText(0);
198204
});
@@ -205,5 +211,7 @@ test('regression (#20932): return pointer is correct before entering deleted tre
205211
'Loading Async...',
206212
'Suspend! [1]',
207213
'Loading Async...',
214+
215+
...(gate('enableSiblingPrerendering') ? ['Suspend! [1]'] : []),
208216
]);
209217
});

packages/react-reconciler/src/ReactFiberCompleteWork.js

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ import {
155155
getRenderTargetTime,
156156
getWorkInProgressTransitions,
157157
shouldRemainOnPreviousScreen,
158+
markSpawnedRetryLane,
158159
} from './ReactFiberWorkLoop';
159160
import {
160161
OffscreenLane,
@@ -600,25 +601,28 @@ function scheduleRetryEffect(
600601
// Schedule an effect to attach a retry listener to the promise.
601602
// TODO: Move to passive phase
602603
workInProgress.flags |= Update;
603-
} else {
604-
// This boundary suspended, but no wakeables were added to the retry
605-
// queue. Check if the renderer suspended commit. If so, this means
606-
// that once the fallback is committed, we can immediately retry
607-
// rendering again, because rendering wasn't actually blocked. Only
608-
// the commit phase.
609-
// TODO: Consider a model where we always schedule an immediate retry, even
610-
// for normal Suspense. That way the retry can partially render up to the
611-
// first thing that suspends.
612-
if (workInProgress.flags & ScheduleRetry) {
613-
const retryLane =
614-
// TODO: This check should probably be moved into claimNextRetryLane
615-
// I also suspect that we need some further consolidation of offscreen
616-
// and retry lanes.
617-
workInProgress.tag !== OffscreenComponent
618-
? claimNextRetryLane()
619-
: OffscreenLane;
620-
workInProgress.lanes = mergeLanes(workInProgress.lanes, retryLane);
621-
}
604+
}
605+
606+
// Check if we need to schedule an immediate retry. This should happen
607+
// whenever we unwind a suspended tree without fully rendering its siblings;
608+
// we need to begin the retry so we can start prerendering them.
609+
//
610+
// We also use this mechanism for Suspensey Resources (e.g. stylesheets),
611+
// because those don't actually block the render phase, only the commit phase.
612+
// So we can start rendering even before the resources are ready.
613+
if (workInProgress.flags & ScheduleRetry) {
614+
const retryLane =
615+
// TODO: This check should probably be moved into claimNextRetryLane
616+
// I also suspect that we need some further consolidation of offscreen
617+
// and retry lanes.
618+
workInProgress.tag !== OffscreenComponent
619+
? claimNextRetryLane()
620+
: OffscreenLane;
621+
workInProgress.lanes = mergeLanes(workInProgress.lanes, retryLane);
622+
623+
// Track the lanes that have been scheduled for an immediate retry so that
624+
// we can mark them as suspended upon committing the root.
625+
markSpawnedRetryLane(retryLane);
622626
}
623627
}
624628

packages/react-reconciler/src/ReactFiberLane.js

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@ import {
2626
syncLaneExpirationMs,
2727
transitionLaneExpirationMs,
2828
retryLaneExpirationMs,
29+
disableLegacyMode,
2930
} from 'shared/ReactFeatureFlags';
3031
import {isDevToolsPresent} from './ReactFiberDevToolsHook';
3132
import {clz32} from './clz32';
33+
import {LegacyRoot} from './ReactRootTags';
3234

3335
// Lane values below should be kept in sync with getLabelForLane(), used by react-devtools-timeline.
3436
// If those values are changed that package should be rebuilt and redeployed.
@@ -753,10 +755,14 @@ export function markRootPinged(root: FiberRoot, pingedLanes: Lanes) {
753755

754756
export function markRootFinished(
755757
root: FiberRoot,
758+
finishedLanes: Lanes,
756759
remainingLanes: Lanes,
757760
spawnedLane: Lane,
761+
updatedLanes: Lanes,
762+
suspendedRetryLanes: Lanes,
758763
) {
759-
const noLongerPendingLanes = root.pendingLanes & ~remainingLanes;
764+
const previouslyPendingLanes = root.pendingLanes;
765+
const noLongerPendingLanes = previouslyPendingLanes & ~remainingLanes;
760766

761767
root.pendingLanes = remainingLanes;
762768

@@ -812,6 +818,37 @@ export function markRootFinished(
812818
NoLanes,
813819
);
814820
}
821+
822+
// suspendedRetryLanes represents the retry lanes spawned by new Suspense
823+
// boundaries during this render that were not later pinged.
824+
//
825+
// These lanes were marked as pending on their associated Suspense boundary
826+
// fiber during the render phase so that we could start rendering them
827+
// before new data streams in. As soon as the fallback commits, we can try
828+
// to render them again.
829+
//
830+
// But since we know they're still suspended, we can skip straight to the
831+
// "prerender" mode (i.e. don't skip over siblings after something
832+
// suspended) instead of the regular mode (i.e. unwind and skip the siblings
833+
// as soon as something suspends to unblock the rest of the update).
834+
if (
835+
suspendedRetryLanes !== NoLanes &&
836+
// Note that we only do this if there were no updates since we started
837+
// rendering. This mirrors the logic in markRootUpdated — whenever we
838+
// receive an update, we reset all the suspended and pinged lanes.
839+
updatedLanes === NoLanes &&
840+
!(disableLegacyMode && root.tag === LegacyRoot)
841+
) {
842+
// We also need to avoid marking a retry lane as suspended if it was already
843+
// pending before this render. We can't say these are now suspended if they
844+
// weren't included in our attempt.
845+
const freshlySpawnedRetryLanes =
846+
suspendedRetryLanes &
847+
// Remove any retry lane that was already pending before our just-finished
848+
// attempt, and also wasn't included in that attempt.
849+
~(previouslyPendingLanes & ~finishedLanes);
850+
root.suspendedLanes |= freshlySpawnedRetryLanes;
851+
}
815852
}
816853

817854
function markSpawnedDeferredLane(

0 commit comments

Comments
 (0)