Skip to content

Commit

Permalink
Prerender during same pass if blocked anyway
Browse files Browse the repository at this point in the history
If something suspends in the shell — i.e. we won't replace the
suspended content with a fallback — we might as well prerender the
siblings during the current render pass, instead of spawning a
separate prerender pass.

This is implemented by setting the "is prerendering" flag to true
whenever we suspend in the shell. But only if we haven't already skipped
over some siblings, because if so, then we need to schedule a separate
prerender pass regardless.
  • Loading branch information
acdlite committed Sep 4, 2024
1 parent e10e868 commit 71babee
Show file tree
Hide file tree
Showing 20 changed files with 217 additions and 556 deletions.
31 changes: 4 additions & 27 deletions packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,22 +233,10 @@ describe('ReactCache', () => {
</Suspense>,
);

await waitForAll([1, 'Suspend! [4]', 'Loading...']);
await waitForAll([1, 'Suspend! [4]', 'Suspend! [5]', 'Loading...']);

await act(() => jest.advanceTimersByTime(100));
assertLog([
'Promise resolved [4]',
1,
4,
'Suspend! [5]',
1,
4,
'Suspend! [5]',
'Promise resolved [5]',
1,
4,
5,
]);
assertLog(['Promise resolved [4]', 'Promise resolved [5]', 1, 4, 5]);

expect(root).toMatchRenderedOutput('145');

Expand All @@ -268,23 +256,12 @@ describe('ReactCache', () => {
1,
// 2 and 3 suspend because they were evicted from the cache
'Suspend! [2]',
'Suspend! [3]',
'Loading...',
]);

await act(() => jest.advanceTimersByTime(100));
assertLog([
'Promise resolved [2]',
1,
2,
'Suspend! [3]',
1,
2,
'Suspend! [3]',
'Promise resolved [3]',
1,
2,
3,
]);
assertLog(['Promise resolved [2]', 'Promise resolved [3]', 1, 2, 3]);
expect(root).toMatchRenderedOutput('123');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ describe('ReactDOMFiberAsync', () => {
// Because it suspended, it remains on the current path
expect(div.textContent).toBe('/path/a');
});
assertLog(gate('enableSiblingPrerendering') ? ['Suspend! [/path/b]'] : []);
assertLog([]);

await act(async () => {
resolvePromise();
Expand Down
61 changes: 7 additions & 54 deletions packages/react-dom/src/__tests__/ReactDOMForm-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -699,15 +699,7 @@ describe('ReactDOMForm', () => {
// This should suspend because form actions are implicitly wrapped
// in startTransition.
await submit(formRef.current);
assertLog([
'Pending...',
'Suspend! [Updated]',
'Loading...',

...(gate('enableSiblingPrerendering')
? ['Suspend! [Updated]', 'Loading...']
: []),
]);
assertLog(['Pending...', 'Suspend! [Updated]', 'Loading...']);
expect(container.textContent).toBe('Pending...Initial');

await act(() => resolveText('Updated'));
Expand Down Expand Up @@ -744,15 +736,7 @@ describe('ReactDOMForm', () => {

// Update
await submit(formRef.current);
assertLog([
'Pending...',
'Suspend! [Count: 1]',
'Loading...',

...(gate('enableSiblingPrerendering')
? ['Suspend! [Count: 1]', 'Loading...']
: []),
]);
assertLog(['Pending...', 'Suspend! [Count: 1]', 'Loading...']);
expect(container.textContent).toBe('Pending...Count: 0');

await act(() => resolveText('Count: 1'));
Expand All @@ -761,15 +745,7 @@ describe('ReactDOMForm', () => {

// Update again
await submit(formRef.current);
assertLog([
'Pending...',
'Suspend! [Count: 2]',
'Loading...',

...(gate('enableSiblingPrerendering')
? ['Suspend! [Count: 2]', 'Loading...']
: []),
]);
assertLog(['Pending...', 'Suspend! [Count: 2]', 'Loading...']);
expect(container.textContent).toBe('Pending...Count: 1');

await act(() => resolveText('Count: 2'));
Expand Down Expand Up @@ -813,14 +789,7 @@ describe('ReactDOMForm', () => {
assertLog(['Async action started', 'Pending...']);

await act(() => resolveText('Wait'));
assertLog([
'Suspend! [Updated]',
'Loading...',

...(gate('enableSiblingPrerendering')
? ['Suspend! [Updated]', 'Loading...']
: []),
]);
assertLog(['Suspend! [Updated]', 'Loading...']);
expect(container.textContent).toBe('Pending...Initial');

await act(() => resolveText('Updated'));
Expand Down Expand Up @@ -1506,15 +1475,7 @@ describe('ReactDOMForm', () => {
// Now dispatch inside of a transition. This one does not trigger a
// loading state.
await act(() => startTransition(() => dispatch()));
assertLog([
'Count: 1',
'Suspend! [Count: 2]',
'Loading...',

...(gate('enableSiblingPrerendering')
? ['Suspend! [Count: 2]', 'Loading...']
: []),
]);
assertLog(['Count: 1', 'Suspend! [Count: 2]', 'Loading...']);
expect(container.textContent).toBe('Count: 1');

await act(() => resolveText('Count: 2'));
Expand All @@ -1534,11 +1495,7 @@ describe('ReactDOMForm', () => {

const root = ReactDOMClient.createRoot(container);
await act(() => root.render(<App />));
assertLog([
'Suspend! [Count: 0]',

...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 0]'] : []),
]);
assertLog(['Suspend! [Count: 0]']);
await act(() => resolveText('Count: 0'));
assertLog(['Count: 0']);

Expand All @@ -1551,11 +1508,7 @@ describe('ReactDOMForm', () => {
{withoutStack: true},
],
]);
assertLog([
'Suspend! [Count: 1]',

...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 1]'] : []),
]);
assertLog(['Suspend! [Count: 1]']);
expect(container.textContent).toBe('Count: 0');
});

Expand Down
12 changes: 12 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -1968,6 +1968,18 @@ export function renderDidSuspend(): void {
export function renderDidSuspendDelayIfPossible(): void {
workInProgressRootExitStatus = RootSuspendedWithDelay;

if (
!workInProgressRootDidSkipSuspendedSiblings &&
!includesBlockingLane(workInProgressRootRenderLanes)
) {
// This render may not have originally been scheduled as a prerender, but
// something suspended inside the visible part of the tree, which means we
// won't be able to commit a fallback anyway. Let's proceed as if this were
// a prerender so that we can warm up the siblings without scheduling a
// separate pass.
workInProgressRootIsPrerendering = true;
}

// Check if there are updates that we skipped tree that might have unblocked
// this render.
if (
Expand Down
20 changes: 2 additions & 18 deletions packages/react-reconciler/src/__tests__/ActivitySuspense-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,7 @@ describe('Activity Suspense', () => {
);
});
});
assertLog([
'Open',
'Suspend! [Async]',
'Loading...',

...(gate('enableSiblingPrerendering')
? ['Open', 'Suspend! [Async]', 'Loading...']
: []),
]);
assertLog(['Open', 'Suspend! [Async]', 'Loading...']);
// It should suspend with delay to prevent the already-visible Suspense
// boundary from switching to a fallback
expect(root).toMatchRenderedOutput(<span>Closed</span>);
Expand Down Expand Up @@ -284,15 +276,7 @@ describe('Activity Suspense', () => {
);
});
});
assertLog([
'Open',
'Suspend! [Async]',
'Loading...',

...(gate('enableSiblingPrerendering')
? ['Open', 'Suspend! [Async]', 'Loading...']
: []),
]);
assertLog(['Open', 'Suspend! [Async]', 'Loading...']);
// It should suspend with delay to prevent the already-visible Suspense
// boundary from switching to a fallback
expect(root).toMatchRenderedOutput(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,14 +349,7 @@ describe('act warnings', () => {
root.render(<App showMore={true} />);
});
});
assertLog([
'Suspend! [Async]',
'Loading...',

...(gate('enableSiblingPrerendering')
? ['Suspend! [Async]', 'Loading...']
: []),
]);
assertLog(['Suspend! [Async]', 'Loading...']);
expect(root).toMatchRenderedOutput('(empty)');

// This is a ping, not a retry, because no fallback is showing.
Expand Down
21 changes: 3 additions & 18 deletions packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ describe('ReactAsyncActions', () => {
'Suspend! [A1]',

...(gate('enableSiblingPrerendering')
? ['Pending: false', 'Suspend! [A1]', 'Suspend! [B1]', 'Suspend! [C1]']
? ['Suspend! [B1]', 'Suspend! [C1]']
: []),
]);
expect(root).toMatchRenderedOutput(
Expand All @@ -322,9 +322,7 @@ describe('ReactAsyncActions', () => {
'A1',
'Suspend! [B1]',

...(gate('enableSiblingPrerendering')
? ['Pending: false', 'A1', 'Suspend! [B1]', 'Suspend! [C1]']
: []),
...(gate('enableSiblingPrerendering') ? ['Suspend! [C1]'] : []),
]);
expect(root).toMatchRenderedOutput(
<>
Expand All @@ -333,16 +331,7 @@ describe('ReactAsyncActions', () => {
</>,
);
await act(() => resolveText('B1'));
assertLog([
'Pending: false',
'A1',
'B1',
'Suspend! [C1]',

...(gate('enableSiblingPrerendering')
? ['Pending: false', 'A1', 'B1', 'Suspend! [C1]']
: []),
]);
assertLog(['Pending: false', 'A1', 'B1', 'Suspend! [C1]']);
expect(root).toMatchRenderedOutput(
<>
<span>Pending: true</span>
Expand Down Expand Up @@ -715,10 +704,6 @@ describe('ReactAsyncActions', () => {
// automatically reverted.
'Pending: false',
'Suspend! [B]',

...(gate('enableSiblingPrerendering')
? ['Pending: false', 'Suspend! [B]']
: []),
]);

// Resolve the transition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,16 +209,7 @@ describe('ReactConcurrentErrorRecovery', () => {
root.render(<App step={2} />);
});
});
assertLog([
'Suspend! [A2]',
'Loading...',
'Suspend! [B2]',
'Loading...',

...(gate('enableSiblingPrerendering')
? ['Suspend! [A2]', 'Loading...', 'Suspend! [B2]', 'Loading...']
: []),
]);
assertLog(['Suspend! [A2]', 'Loading...', 'Suspend! [B2]', 'Loading...']);
// Because this is a refresh, we don't switch to a fallback
expect(root).toMatchRenderedOutput('A1B1');

Expand All @@ -229,16 +220,7 @@ describe('ReactConcurrentErrorRecovery', () => {

// Because we're still suspended on A, we can't show an error boundary. We
// should wait for A to resolve.
assertLog([
'Suspend! [A2]',
'Loading...',
'Error! [B2]',
'Oops!',

...(gate('enableSiblingPrerendering')
? ['Suspend! [A2]', 'Loading...', 'Error! [B2]', 'Oops!']
: []),
]);
assertLog(['Suspend! [A2]', 'Loading...', 'Error! [B2]', 'Oops!']);
// Remain on previous screen.
expect(root).toMatchRenderedOutput('A1B1');

Expand Down Expand Up @@ -299,16 +281,7 @@ describe('ReactConcurrentErrorRecovery', () => {
root.render(<App step={2} />);
});
});
assertLog([
'Suspend! [A2]',
'Loading...',
'Suspend! [B2]',
'Loading...',

...(gate('enableSiblingPrerendering')
? ['Suspend! [A2]', 'Loading...', 'Suspend! [B2]', 'Loading...']
: []),
]);
assertLog(['Suspend! [A2]', 'Loading...', 'Suspend! [B2]', 'Loading...']);
// Because this is a refresh, we don't switch to a fallback
expect(root).toMatchRenderedOutput('A1B1');

Expand Down Expand Up @@ -364,11 +337,7 @@ describe('ReactConcurrentErrorRecovery', () => {
root.render(<AsyncText text="Async" />);
});
});
assertLog([
'Suspend! [Async]',

...(gate('enableSiblingPrerendering') ? ['Suspend! [Async]'] : []),
]);
assertLog(['Suspend! [Async]']);
expect(root).toMatchRenderedOutput(null);

// This also works if the suspended component is wrapped with an error
Expand All @@ -384,11 +353,7 @@ describe('ReactConcurrentErrorRecovery', () => {
);
});
});
assertLog([
'Suspend! [Async]',

...(gate('enableSiblingPrerendering') ? ['Suspend! [Async]'] : []),
]);
assertLog(['Suspend! [Async]']);
expect(root).toMatchRenderedOutput(null);

// Continues rendering once data resolves
Expand Down Expand Up @@ -445,7 +410,7 @@ describe('ReactConcurrentErrorRecovery', () => {
'Suspend! [Async]',

...(gate('enableSiblingPrerendering')
? ['Suspend! [Async]', 'Caught an error: Oops!']
? ['Caught an error: Oops!']
: []),
]);
// The render suspended without committing the error.
Expand All @@ -468,7 +433,7 @@ describe('ReactConcurrentErrorRecovery', () => {
'Suspend! [Async]',

...(gate('enableSiblingPrerendering')
? ['Suspend! [Async]', 'Caught an error: Oops!']
? ['Caught an error: Oops!']
: []),
]);
expect(root).toMatchRenderedOutput(null);
Expand Down
Loading

0 comments on commit 71babee

Please sign in to comment.