Skip to content

Commit d6cb4e7

Browse files
acdliteacdlite
authored
Start prerendering Suspense retries immediately (facebook#30934)
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 1bb0563 commit d6cb4e7

29 files changed

+1906
-462
lines changed

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

Lines changed: 53 additions & 45 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;
@@ -148,15 +160,7 @@ describe('ReactCache', () => {
148160
error = e;
149161
}
150162
expect(error.message).toMatch('Failed to load: Hi');
151-
assertLog([
152-
'Promise rejected [Hi]',
153-
'Error! [Hi]',
154-
'Error! [Hi]',
155-
156-
...(gate('enableSiblingPrerendering')
157-
? ['Error! [Hi]', 'Error! [Hi]']
158-
: []),
159-
]);
163+
assertLog(['Promise rejected [Hi]', 'Error! [Hi]', 'Error! [Hi]']);
160164

161165
// Should throw again on a subsequent read
162166
root.render(<App />);
@@ -187,15 +191,27 @@ describe('ReactCache', () => {
187191

188192
if (__DEV__) {
189193
await expect(async () => {
190-
await waitForAll(['App', 'Loading...']);
194+
await waitForAll([
195+
'App',
196+
'Loading...',
197+
198+
...(gate('enableSiblingPrerendering') ? ['App'] : []),
199+
]);
191200
}).toErrorDev([
192201
'Invalid key type. Expected a string, number, symbol, or ' +
193202
"boolean, but instead received: [ 'Hi', 100 ]\n\n" +
194203
'To use non-primitive values as keys, you must pass a hash ' +
195204
'function as the second argument to createResource().',
205+
206+
...(gate('enableSiblingPrerendering') ? ['Invalid key type'] : []),
196207
]);
197208
} else {
198-
await waitForAll(['App', 'Loading...']);
209+
await waitForAll([
210+
'App',
211+
'Loading...',
212+
213+
...(gate('enableSiblingPrerendering') ? ['App'] : []),
214+
]);
199215
}
200216
});
201217

@@ -212,13 +228,17 @@ describe('ReactCache', () => {
212228
<AsyncText ms={100} text={3} />
213229
</Suspense>,
214230
);
215-
await waitForAll(['Suspend! [1]', 'Loading...']);
231+
await waitForPaint(['Suspend! [1]', 'Loading...']);
216232
jest.advanceTimersByTime(100);
217233
assertLog(['Promise resolved [1]']);
218-
await waitForAll([1, 'Suspend! [2]', 1, 'Suspend! [2]', 'Suspend! [3]']);
234+
await waitForAll([1, 'Suspend! [2]']);
235+
236+
jest.advanceTimersByTime(100);
237+
assertLog(['Promise resolved [2]']);
238+
await waitForAll([1, 2, 'Suspend! [3]']);
219239

220240
jest.advanceTimersByTime(100);
221-
assertLog(['Promise resolved [2]', 'Promise resolved [3]']);
241+
assertLog(['Promise resolved [3]']);
222242
await waitForAll([1, 2, 3]);
223243

224244
await act(() => jest.advanceTimersByTime(100));
@@ -233,25 +253,18 @@ describe('ReactCache', () => {
233253
</Suspense>,
234254
);
235255

236-
await waitForAll([1, 'Suspend! [4]', 'Loading...']);
237-
238-
await act(() => jest.advanceTimersByTime(100));
239-
assertLog([
240-
'Promise resolved [4]',
241-
256+
await waitForAll([
242257
1,
243-
4,
244-
'Suspend! [5]',
258+
'Suspend! [4]',
259+
'Loading...',
245260
1,
246-
4,
261+
'Suspend! [4]',
247262
'Suspend! [5]',
248-
249-
'Promise resolved [5]',
250-
1,
251-
4,
252-
5,
253263
]);
254264

265+
await act(() => jest.advanceTimersByTime(100));
266+
assertLog(['Promise resolved [4]', 'Promise resolved [5]', 1, 4, 5]);
267+
255268
expect(root).toMatchRenderedOutput('145');
256269

257270
// We've now rendered values 1, 2, 3, 4, 5, over our limit of 3. The least
@@ -271,24 +284,14 @@ describe('ReactCache', () => {
271284
// 2 and 3 suspend because they were evicted from the cache
272285
'Suspend! [2]',
273286
'Loading...',
274-
]);
275-
276-
await act(() => jest.advanceTimersByTime(100));
277-
assertLog([
278-
'Promise resolved [2]',
279287

280288
1,
281-
2,
282-
'Suspend! [3]',
283-
1,
284-
2,
289+
'Suspend! [2]',
285290
'Suspend! [3]',
286-
287-
'Promise resolved [3]',
288-
1,
289-
2,
290-
3,
291291
]);
292+
293+
await act(() => jest.advanceTimersByTime(100));
294+
assertLog(['Promise resolved [2]', 'Promise resolved [3]', 1, 2, 3]);
292295
expect(root).toMatchRenderedOutput('123');
293296
});
294297

@@ -363,7 +366,12 @@ describe('ReactCache', () => {
363366
</Suspense>,
364367
);
365368

366-
await waitForAll(['Suspend! [Hi]', 'Loading...']);
369+
await waitForAll([
370+
'Suspend! [Hi]',
371+
'Loading...',
372+
373+
...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []),
374+
]);
367375

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

‎packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js‎

Lines changed: 94 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,17 @@ import {
1515
normalizeCodeLocInfo,
1616
} from './utils';
1717

18+
import {ReactVersion} from '../../../../ReactVersions';
19+
import semver from 'semver';
20+
21+
// TODO: This is how other DevTools tests access the version but we should find
22+
// a better solution for this
23+
const ReactVersionTestingAgainst = process.env.REACT_VERSION || ReactVersion;
24+
const enableSiblingPrerendering = semver.gte(
25+
ReactVersionTestingAgainst,
26+
'19.0.0',
27+
);
28+
1829
describe('Timeline profiler', () => {
1930
let React;
2031
let Scheduler;
@@ -1651,7 +1662,11 @@ describe('Timeline profiler', () => {
16511662
</React.Suspense>,
16521663
);
16531664

1654-
await waitForAll(['suspended']);
1665+
await waitForAll([
1666+
'suspended',
1667+
1668+
...(enableSiblingPrerendering ? ['suspended'] : []),
1669+
]);
16551670

16561671
Scheduler.unstable_advanceTime(10);
16571672
resolveFn();
@@ -1662,9 +1677,38 @@ describe('Timeline profiler', () => {
16621677
const timelineData = stopProfilingAndGetTimelineData();
16631678

16641679
// Verify the Suspense event and duration was recorded.
1665-
expect(timelineData.suspenseEvents).toHaveLength(1);
1666-
const suspenseEvent = timelineData.suspenseEvents[0];
1667-
expect(suspenseEvent).toMatchInlineSnapshot(`
1680+
if (enableSiblingPrerendering) {
1681+
expect(timelineData.suspenseEvents).toMatchInlineSnapshot(`
1682+
[
1683+
{
1684+
"componentName": "Example",
1685+
"depth": 0,
1686+
"duration": 10,
1687+
"id": "0",
1688+
"phase": "mount",
1689+
"promiseName": "",
1690+
"resolution": "resolved",
1691+
"timestamp": 10,
1692+
"type": "suspense",
1693+
"warning": null,
1694+
},
1695+
{
1696+
"componentName": "Example",
1697+
"depth": 0,
1698+
"duration": 10,
1699+
"id": "0",
1700+
"phase": "mount",
1701+
"promiseName": "",
1702+
"resolution": "resolved",
1703+
"timestamp": 10,
1704+
"type": "suspense",
1705+
"warning": null,
1706+
},
1707+
]
1708+
`);
1709+
} else {
1710+
const suspenseEvent = timelineData.suspenseEvents[0];
1711+
expect(suspenseEvent).toMatchInlineSnapshot(`
16681712
{
16691713
"componentName": "Example",
16701714
"depth": 0,
@@ -1678,10 +1722,13 @@ describe('Timeline profiler', () => {
16781722
"warning": null,
16791723
}
16801724
`);
1725+
}
16811726

16821727
// There should be two batches of renders: Suspeneded and resolved.
16831728
expect(timelineData.batchUIDToMeasuresMap.size).toBe(2);
1684-
expect(timelineData.componentMeasures).toHaveLength(2);
1729+
expect(timelineData.componentMeasures).toHaveLength(
1730+
enableSiblingPrerendering ? 3 : 2,
1731+
);
16851732
});
16861733

16871734
it('should mark concurrent render with suspense that rejects', async () => {
@@ -1708,7 +1755,11 @@ describe('Timeline profiler', () => {
17081755
</React.Suspense>,
17091756
);
17101757

1711-
await waitForAll(['suspended']);
1758+
await waitForAll([
1759+
'suspended',
1760+
1761+
...(enableSiblingPrerendering ? ['suspended'] : []),
1762+
]);
17121763

17131764
Scheduler.unstable_advanceTime(10);
17141765
rejectFn();
@@ -1719,9 +1770,39 @@ describe('Timeline profiler', () => {
17191770
const timelineData = stopProfilingAndGetTimelineData();
17201771

17211772
// Verify the Suspense event and duration was recorded.
1722-
expect(timelineData.suspenseEvents).toHaveLength(1);
1723-
const suspenseEvent = timelineData.suspenseEvents[0];
1724-
expect(suspenseEvent).toMatchInlineSnapshot(`
1773+
if (enableSiblingPrerendering) {
1774+
expect(timelineData.suspenseEvents).toMatchInlineSnapshot(`
1775+
[
1776+
{
1777+
"componentName": "Example",
1778+
"depth": 0,
1779+
"duration": 10,
1780+
"id": "0",
1781+
"phase": "mount",
1782+
"promiseName": "",
1783+
"resolution": "rejected",
1784+
"timestamp": 10,
1785+
"type": "suspense",
1786+
"warning": null,
1787+
},
1788+
{
1789+
"componentName": "Example",
1790+
"depth": 0,
1791+
"duration": 10,
1792+
"id": "0",
1793+
"phase": "mount",
1794+
"promiseName": "",
1795+
"resolution": "rejected",
1796+
"timestamp": 10,
1797+
"type": "suspense",
1798+
"warning": null,
1799+
},
1800+
]
1801+
`);
1802+
} else {
1803+
expect(timelineData.suspenseEvents).toHaveLength(1);
1804+
const suspenseEvent = timelineData.suspenseEvents[0];
1805+
expect(suspenseEvent).toMatchInlineSnapshot(`
17251806
{
17261807
"componentName": "Example",
17271808
"depth": 0,
@@ -1735,10 +1816,13 @@ describe('Timeline profiler', () => {
17351816
"warning": null,
17361817
}
17371818
`);
1819+
}
17381820

17391821
// There should be two batches of renders: Suspeneded and resolved.
17401822
expect(timelineData.batchUIDToMeasuresMap.size).toBe(2);
1741-
expect(timelineData.componentMeasures).toHaveLength(2);
1823+
expect(timelineData.componentMeasures).toHaveLength(
1824+
enableSiblingPrerendering ? 3 : 2,
1825+
);
17421826
});
17431827

17441828
it('should mark cascading class component state updates', async () => {

‎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
});

0 commit comments

Comments
 (0)