From 7086f1f4e713ba11100b151fd22e2b8723af21cf Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 5 Sep 2024 11:07:28 -0400 Subject: [PATCH 1/5] Refine tag before checking StoreConsistency flag The StoreConsistency flag (used by function components) is the same bit as the ScheduleRetry flag (used by Suspense components). So we should refine the tag before checking whether it's present. --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 348ca757d2d6d..c485c3459ca8a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -1277,7 +1277,13 @@ function isRenderConsistentWithExternalStores(finishedWork: Fiber): boolean { // loop instead of recursion so we can exit early. let node: Fiber = finishedWork; while (true) { - if (node.flags & StoreConsistency) { + const tag = node.tag; + if ( + (tag === FunctionComponent || + tag === ForwardRef || + tag === SimpleMemoComponent) && + node.flags & StoreConsistency + ) { const updateQueue: FunctionComponentUpdateQueue | null = (node.updateQueue: any); if (updateQueue !== null) { From 1a21c9db7c9a8b7ce1e5df4e8e4694bf288c5d97 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 6 Sep 2024 15:20:03 -0400 Subject: [PATCH 2/5] Don't prerender siblings after something throws (I meant to include this in the last PR but messed up when I was splitting the commits into steps.) The error handling implementation currently assumes that components are rendered sequentially. Therefore we shouldn't prerender the siblings after something errors; we should unwind to the nearest error boundary. However, anything outside the error boundary should not be affected; we should still be able to prerender those components. --- .../__tests__/ReactCacheOld-test.internal.js | 10 +- .../src/ReactFiberWorkLoop.js | 34 ++++- .../ReactConcurrentErrorRecovery-test.js | 11 +- .../src/__tests__/ReactLazy-test.internal.js | 7 - .../ReactSiblingPrerendering-test.js | 120 ++++++++++++++++++ .../ReactSuspenseWithNoopRenderer-test.js | 4 - 6 files changed, 150 insertions(+), 36 deletions(-) create mode 100644 packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js diff --git a/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js b/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js index bec901c722007..3285f0774ece1 100644 --- a/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js +++ b/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js @@ -148,15 +148,7 @@ describe('ReactCache', () => { error = e; } expect(error.message).toMatch('Failed to load: Hi'); - assertLog([ - 'Promise rejected [Hi]', - 'Error! [Hi]', - 'Error! [Hi]', - - ...(gate('enableSiblingPrerendering') - ? ['Error! [Hi]', 'Error! [Hi]'] - : []), - ]); + assertLog(['Promise rejected [Hi]', 'Error! [Hi]', 'Error! [Hi]']); // Should throw again on a subsequent read root.render(); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index c485c3459ca8a..e2e145fabfd47 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -2110,9 +2110,10 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) { } default: { // Unwind then continue with the normal work loop. + const reason = workInProgressSuspendedReason; workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue, reason); break; } } @@ -2232,7 +2233,12 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); + throwAndUnwindWorkLoop( + root, + unitOfWork, + thrownValue, + SuspendedOnError, + ); break; } case SuspendedOnData: { @@ -2290,7 +2296,12 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Otherwise, unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); + throwAndUnwindWorkLoop( + root, + unitOfWork, + thrownValue, + SuspendedAndReadyToContinue, + ); } break; } @@ -2353,7 +2364,12 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Otherwise, unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); + throwAndUnwindWorkLoop( + root, + unitOfWork, + thrownValue, + SuspendedOnInstanceAndReadyToContinue, + ); break; } case SuspendedOnDeprecatedThrowPromise: { @@ -2363,7 +2379,12 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // always unwind. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); + throwAndUnwindWorkLoop( + root, + unitOfWork, + thrownValue, + SuspendedOnDeprecatedThrowPromise, + ); break; } case SuspendedOnHydration: { @@ -2617,6 +2638,7 @@ function throwAndUnwindWorkLoop( root: FiberRoot, unitOfWork: Fiber, thrownValue: mixed, + suspendedReason: SuspendedReason, ) { // This is a fork of performUnitOfWork specifcally for unwinding a fiber // that threw an exception. @@ -2664,7 +2686,7 @@ function throwAndUnwindWorkLoop( // The current algorithm for both hydration and error handling assumes // that the tree is rendered sequentially. So we always skip the siblings. getIsHydrating() || - workInProgressSuspendedReason === SuspendedOnError + suspendedReason === SuspendedOnError ) { skipSiblings = true; // We intentionally don't set workInProgressRootDidSkipSuspendedSiblings, diff --git a/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js index db9d7c336ec2e..4a00607ffb848 100644 --- a/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js +++ b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js @@ -292,16 +292,7 @@ describe('ReactConcurrentErrorRecovery', () => { // Because we're still suspended on B, we can't show an error boundary. We // should wait for B to resolve. - assertLog([ - 'Error! [A2]', - 'Oops!', - 'Suspend! [B2]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') - ? ['Error! [A2]', 'Oops!', 'Suspend! [B2]', 'Loading...'] - : []), - ]); + assertLog(['Error! [A2]', 'Oops!', 'Suspend! [B2]', 'Loading...']); // Remain on previous screen. expect(root).toMatchRenderedOutput('A1B1'); diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 891cf91049833..6eec3112e464b 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -239,13 +239,6 @@ describe('ReactLazy', () => { assertConsoleErrorDev([ 'Expected the result of a dynamic import() call', 'Expected the result of a dynamic import() call', - - ...(gate('enableSiblingPrerendering') - ? [ - 'Expected the result of a dynamic import() call', - 'Expected the result of a dynamic import() call', - ] - : []), ]); expect(root).not.toMatchRenderedOutput('Hi'); }); diff --git a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js new file mode 100644 index 0000000000000..e747d04f6c0ad --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js @@ -0,0 +1,120 @@ +let React; +let ReactNoop; +let Scheduler; +let act; +let assertLog; +let textCache; +let startTransition; + +describe('ReactSiblingPrerendering', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + act = require('internal-test-utils').act; + assertLog = require('internal-test-utils').assertLog; + startTransition = React.startTransition; + + textCache = new Map(); + }); + + function readText(text) { + const record = textCache.get(text); + if (record !== undefined) { + switch (record.status) { + case 'pending': + Scheduler.log(`Suspend! [${text}]`); + throw record.value; + case 'rejected': + throw record.value; + case 'resolved': + return record.value; + } + } else { + Scheduler.log(`Suspend! [${text}]`); + const thenable = { + pings: [], + then(resolve) { + if (newRecord.status === 'pending') { + thenable.pings.push(resolve); + } else { + Promise.resolve().then(() => resolve(newRecord.value)); + } + }, + }; + + const newRecord = { + status: 'pending', + value: thenable, + }; + textCache.set(text, newRecord); + + throw thenable; + } + } + + function Text({text}) { + Scheduler.log(text); + return text; + } + + function AsyncText({text}) { + readText(text); + Scheduler.log(text); + return text; + } + + it("don't prerender siblings when something errors", async () => { + class ErrorBoundary extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + return {error}; + } + render() { + if (this.state.error) { + return ; + } + return this.props.children; + } + } + + function Oops() { + throw new Error('Oops!'); + } + + function App() { + return ( + <> +
+ + + + +
+
+ + +
+ + ); + } + + const root = ReactNoop.createRoot(); + await act(() => startTransition(() => root.render())); + assertLog([ + 'Oops!', + + // A is skipped because we don't prerender siblings when + // something errors. + + 'Suspend! [B]', + + // After B suspends, we're still able to prerender C without starting + // over because there's no fallback, so the root is blocked from + // committing anyway. + ...(gate('enableSiblingPrerendering') ? ['Suspend! [C]'] : []), + ]); + }); +}); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 80e04733bfe35..4de16b544e852 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -506,10 +506,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { // React retries one more time 'Error! [Result]', - ...(gate('enableSiblingPrerendering') - ? ['Error! [Result]', 'Error! [Result]'] - : []), - // Errored again on retry. Now handle it. 'Caught error: Failed to load: Result', ]); From 725d1b01b60454e15e76ab908245eb2f2c1ddef9 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sun, 8 Sep 2024 11:56:41 -0400 Subject: [PATCH 3/5] Don't skip siblings when rendering hidden content Rendering inside a hidden Activity tree is a form of speculative rendering, so we should not skip the siblings after something suspends. --- .../src/ReactFiberWorkLoop.js | 7 ++- .../ReactSiblingPrerendering-test.js | 46 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index e2e145fabfd47..b729d53bc7fe0 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -2691,7 +2691,12 @@ function throwAndUnwindWorkLoop( skipSiblings = true; // We intentionally don't set workInProgressRootDidSkipSuspendedSiblings, // because we don't want to trigger another prerender attempt. - } else if (!workInProgressRootIsPrerendering) { + } else if ( + // Check whether this is a prerender + !workInProgressRootIsPrerendering && + // Offscreen rendering is also a form of speculative rendering + !includesSomeLane(workInProgressRootRenderLanes, OffscreenLane) + ) { // This is not a prerender. Skip the siblings during this render. A // separate prerender will be scheduled for later. skipSiblings = true; diff --git a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js index e747d04f6c0ad..6cc24f9844b2f 100644 --- a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js @@ -3,8 +3,11 @@ let ReactNoop; let Scheduler; let act; let assertLog; +let waitForPaint; let textCache; let startTransition; +let Suspense; +let Activity; describe('ReactSiblingPrerendering', () => { beforeEach(() => { @@ -15,7 +18,10 @@ describe('ReactSiblingPrerendering', () => { Scheduler = require('scheduler'); act = require('internal-test-utils').act; assertLog = require('internal-test-utils').assertLog; + waitForPaint = require('internal-test-utils').waitForPaint; startTransition = React.startTransition; + Suspense = React.Suspense; + Activity = React.unstable_Activity; textCache = new Map(); }); @@ -117,4 +123,44 @@ describe('ReactSiblingPrerendering', () => { ...(gate('enableSiblingPrerendering') ? ['Suspend! [C]'] : []), ]); }); + + // @gate enableActivity + it("don't skip siblings when rendering inside a hidden tree", async () => { + function App() { + return ( + <> + + + }> + + + + + + ); + } + + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(async () => root.render()); + + // The first render includes only the visible part of the tree. The + // hidden content is deferred until later. + await waitForPaint(['A']); + expect(root).toMatchRenderedOutput('A'); + + // The second render is a prerender of the hidden content. + await waitForPaint([ + 'Suspend! [B]', + + // If B and C were visible, C would not have been attempted + // during this pass, because it would prevented the fallback + // from showing. + ...(gate('enableSiblingPrerendering') ? ['Suspend! [C]'] : []), + + 'Loading...', + ]); + expect(root).toMatchRenderedOutput('A'); + }); + }); }); From dc3ccc2bbb560cc5a9f17d0b8cb592a8c6af34f6 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sun, 8 Sep 2024 12:21:53 -0400 Subject: [PATCH 4/5] Start prerendering Suspense retries immediately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../__tests__/ReactCacheOld-test.internal.js | 73 +++-- .../src/__tests__/ReactDOMForm-test.js | 14 +- .../ReactDOMSuspensePlaceholder-test.js | 8 +- .../__tests__/ReactWrongReturnPointer-test.js | 10 +- .../src/ReactFiberCompleteWork.js | 42 +-- .../react-reconciler/src/ReactFiberLane.js | 39 ++- .../src/ReactFiberWorkLoop.js | 82 ++++- .../src/__tests__/ActivityStrictMode-test.js | 5 + .../src/__tests__/ReactActWarnings-test.js | 16 +- .../__tests__/ReactBatching-test.internal.js | 8 +- .../__tests__/ReactContextPropagation-test.js | 23 +- .../src/__tests__/ReactDeferredValue-test.js | 4 + .../src/__tests__/ReactHooks-test.internal.js | 36 ++- .../ReactHooksWithNoopRenderer-test.js | 8 +- .../src/__tests__/ReactLazy-test.internal.js | 36 ++- .../ReactSiblingPrerendering-test.js | 75 +++++ .../__tests__/ReactSuspense-test.internal.js | 111 ++++++- .../ReactSuspenseEffectsSemantics-test.js | 194 +++++++++++- .../__tests__/ReactSuspenseFallback-test.js | 22 +- .../src/__tests__/ReactSuspenseList-test.js | 100 +++++- .../ReactSuspensePlaceholder-test.internal.js | 107 +++++-- .../ReactSuspenseWithNoopRenderer-test.js | 286 +++++++++++++++--- .../__tests__/ReactTransitionTracing-test.js | 120 +++++++- .../src/__tests__/ReactUse-test.js | 15 +- .../src/__tests__/StrictEffectsMode-test.js | 8 + 25 files changed, 1253 insertions(+), 189 deletions(-) diff --git a/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js b/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js index 3285f0774ece1..bffb7b96818c0 100644 --- a/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js +++ b/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js @@ -18,6 +18,7 @@ let Suspense; let TextResource; let textResourceShouldFail; let waitForAll; +let waitForPaint; let assertLog; let waitForThrow; let act; @@ -37,6 +38,7 @@ describe('ReactCache', () => { waitForAll = InternalTestUtils.waitForAll; assertLog = InternalTestUtils.assertLog; waitForThrow = InternalTestUtils.waitForThrow; + waitForPaint = InternalTestUtils.waitForPaint; act = InternalTestUtils.act; TextResource = createResource( @@ -119,7 +121,12 @@ describe('ReactCache', () => { const root = ReactNoop.createRoot(); root.render(); - await waitForAll(['Suspend! [Hi]', 'Loading...']); + await waitForAll([ + 'Suspend! [Hi]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []), + ]); jest.advanceTimersByTime(100); assertLog(['Promise resolved [Hi]']); @@ -138,7 +145,12 @@ describe('ReactCache', () => { const root = ReactNoop.createRoot(); root.render(); - await waitForAll(['Suspend! [Hi]', 'Loading...']); + await waitForAll([ + 'Suspend! [Hi]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []), + ]); textResourceShouldFail = true; let error; @@ -179,12 +191,19 @@ describe('ReactCache', () => { if (__DEV__) { await expect(async () => { - await waitForAll(['App', 'Loading...']); + await waitForAll([ + 'App', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['App'] : []), + ]); }).toErrorDev([ 'Invalid key type. Expected a string, number, symbol, or ' + "boolean, but instead received: [ 'Hi', 100 ]\n\n" + 'To use non-primitive values as keys, you must pass a hash ' + 'function as the second argument to createResource().', + + ...(gate('enableSiblingPrerendering') ? ['Invalid key type'] : []), ]); } else { await waitForAll(['App', 'Loading...']); @@ -204,7 +223,7 @@ describe('ReactCache', () => { , ); - await waitForAll(['Suspend! [1]', 'Loading...']); + await waitForPaint(['Suspend! [1]', 'Loading...']); jest.advanceTimersByTime(100); assertLog(['Promise resolved [1]']); await waitForAll([1, 'Suspend! [2]', 1, 'Suspend! [2]', 'Suspend! [3]']); @@ -225,25 +244,18 @@ describe('ReactCache', () => { , ); - await waitForAll([1, 'Suspend! [4]', 'Loading...']); - - await act(() => jest.advanceTimersByTime(100)); - assertLog([ - 'Promise resolved [4]', - + await waitForAll([ 1, - 4, - 'Suspend! [5]', + 'Suspend! [4]', + 'Loading...', 1, - 4, + 'Suspend! [4]', 'Suspend! [5]', - - 'Promise resolved [5]', - 1, - 4, - 5, ]); + await act(() => jest.advanceTimersByTime(100)); + assertLog(['Promise resolved [4]', 'Promise resolved [5]', 1, 4, 5]); + expect(root).toMatchRenderedOutput('145'); // We've now rendered values 1, 2, 3, 4, 5, over our limit of 3. The least @@ -263,24 +275,14 @@ describe('ReactCache', () => { // 2 and 3 suspend because they were evicted from the cache 'Suspend! [2]', 'Loading...', - ]); - await act(() => jest.advanceTimersByTime(100)); - assertLog([ - 'Promise resolved [2]', - - 1, - 2, - 'Suspend! [3]', 1, - 2, + 'Suspend! [2]', 'Suspend! [3]', - - 'Promise resolved [3]', - 1, - 2, - 3, ]); + + await act(() => jest.advanceTimersByTime(100)); + assertLog(['Promise resolved [2]', 'Promise resolved [3]', 1, 2, 3]); expect(root).toMatchRenderedOutput('123'); }); @@ -355,7 +357,12 @@ describe('ReactCache', () => { , ); - await waitForAll(['Suspend! [Hi]', 'Loading...']); + await waitForAll([ + 'Suspend! [Hi]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []), + ]); resolveThenable('Hi'); // This thenable improperly resolves twice. We should not update the diff --git a/packages/react-dom/src/__tests__/ReactDOMForm-test.js b/packages/react-dom/src/__tests__/ReactDOMForm-test.js index 5a44cb6afec09..168d44722d473 100644 --- a/packages/react-dom/src/__tests__/ReactDOMForm-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMForm-test.js @@ -1459,13 +1459,23 @@ describe('ReactDOMForm', () => { , ), ); - assertLog(['Suspend! [Count: 0]', 'Loading...']); + assertLog([ + 'Suspend! [Count: 0]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 0]'] : []), + ]); await act(() => resolveText('Count: 0')); assertLog(['Count: 0']); // Dispatch outside of a transition. This will trigger a loading state. await act(() => dispatch()); - assertLog(['Suspend! [Count: 1]', 'Loading...']); + assertLog([ + 'Suspend! [Count: 1]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 1]'] : []), + ]); expect(container.textContent).toBe('Loading...'); await act(() => resolveText('Count: 1')); diff --git a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js index 50ba64a478888..fa22142702527 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js @@ -160,7 +160,13 @@ describe('ReactDOMSuspensePlaceholder', () => { }); expect(container.textContent).toEqual('Loading...'); - assertLog(['A', 'Suspend! [B]', 'Loading...']); + assertLog([ + 'A', + 'Suspend! [B]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [B]', 'C'] : []), + ]); await act(() => { resolveText('B'); }); diff --git a/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js b/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js index 468d5b54e1ae2..796d429625a70 100644 --- a/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js +++ b/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js @@ -192,7 +192,13 @@ test('regression (#20932): return pointer is correct before entering deleted tre await act(() => { root.render(); }); - assertLog(['Suspend! [0]', 'Loading Async...', 'Loading Tail...']); + assertLog([ + 'Suspend! [0]', + 'Loading Async...', + 'Loading Tail...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [0]'] : []), + ]); await act(() => { resolveText(0); }); @@ -205,5 +211,7 @@ test('regression (#20932): return pointer is correct before entering deleted tre 'Loading Async...', 'Suspend! [1]', 'Loading Async...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [1]'] : []), ]); }); diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index c5d0c1575be04..e3fba900dd116 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -155,6 +155,7 @@ import { getRenderTargetTime, getWorkInProgressTransitions, shouldRemainOnPreviousScreen, + markSpawnedRetryLane, } from './ReactFiberWorkLoop'; import { OffscreenLane, @@ -600,25 +601,28 @@ function scheduleRetryEffect( // Schedule an effect to attach a retry listener to the promise. // TODO: Move to passive phase workInProgress.flags |= Update; - } else { - // This boundary suspended, but no wakeables were added to the retry - // queue. Check if the renderer suspended commit. If so, this means - // that once the fallback is committed, we can immediately retry - // rendering again, because rendering wasn't actually blocked. Only - // the commit phase. - // TODO: Consider a model where we always schedule an immediate retry, even - // for normal Suspense. That way the retry can partially render up to the - // first thing that suspends. - if (workInProgress.flags & ScheduleRetry) { - const retryLane = - // TODO: This check should probably be moved into claimNextRetryLane - // I also suspect that we need some further consolidation of offscreen - // and retry lanes. - workInProgress.tag !== OffscreenComponent - ? claimNextRetryLane() - : OffscreenLane; - workInProgress.lanes = mergeLanes(workInProgress.lanes, retryLane); - } + } + + // Check if we need to schedule an immediate retry. This should happen + // whenever we unwind a suspended tree without fully rendering its siblings; + // we need to begin the retry so we can start prerendering them. + // + // We also use this mechanism for Suspensey Resources (e.g. stylesheets), + // because those don't actually block the render phase, only the commit phase. + // So we can start rendering even before the resources are ready. + if (workInProgress.flags & ScheduleRetry) { + const retryLane = + // TODO: This check should probably be moved into claimNextRetryLane + // I also suspect that we need some further consolidation of offscreen + // and retry lanes. + workInProgress.tag !== OffscreenComponent + ? claimNextRetryLane() + : OffscreenLane; + workInProgress.lanes = mergeLanes(workInProgress.lanes, retryLane); + + // Track the lanes that have been scheduled for an immediate retry so that + // we can mark them as suspended upon committing the root. + markSpawnedRetryLane(retryLane); } } diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index fe36ce548b33a..8f6f7c9c3f94f 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -26,9 +26,11 @@ import { syncLaneExpirationMs, transitionLaneExpirationMs, retryLaneExpirationMs, + disableLegacyMode, } from 'shared/ReactFeatureFlags'; import {isDevToolsPresent} from './ReactFiberDevToolsHook'; import {clz32} from './clz32'; +import {LegacyRoot} from './ReactRootTags'; // Lane values below should be kept in sync with getLabelForLane(), used by react-devtools-timeline. // If those values are changed that package should be rebuilt and redeployed. @@ -753,10 +755,14 @@ export function markRootPinged(root: FiberRoot, pingedLanes: Lanes) { export function markRootFinished( root: FiberRoot, + finishedLanes: Lanes, remainingLanes: Lanes, spawnedLane: Lane, + updatedLanes: Lanes, + suspendedRetryLanes: Lanes, ) { - const noLongerPendingLanes = root.pendingLanes & ~remainingLanes; + const previouslyPendingLanes = root.pendingLanes; + const noLongerPendingLanes = previouslyPendingLanes & ~remainingLanes; root.pendingLanes = remainingLanes; @@ -812,6 +818,37 @@ export function markRootFinished( NoLanes, ); } + + // suspendedRetryLanes represents the retry lanes spawned by new Suspense + // boundaries during this render that were not later pinged. + // + // These lanes were marked as pending on their associated Suspense boundary + // fiber during the render phase so that we could start rendering them + // before new data streams in. As soon as the fallback commits, we can try + // to render them again. + // + // But since we know they're still suspended, we can skip straight to the + // "prerender" mode (i.e. don't skip over siblings after something + // suspended) instead of the regular mode (i.e. unwind and skip the siblings + // as soon as something suspends to unblock the rest of the update). + if ( + suspendedRetryLanes !== NoLanes && + // Note that we only do this if there were no updates since we started + // rendering. This mirrors the logic in markRootUpdated — whenever we + // receive an update, we reset all the suspended and pinged lanes. + updatedLanes === NoLanes && + !(disableLegacyMode && root.tag === LegacyRoot) + ) { + // We also need to avoid marking a retry lane as suspended if it was already + // pending before this render. We can't say these are now suspended if they + // weren't included in our attempt. + const freshlySpawnedRetryLanes = + suspendedRetryLanes & + // Remove any retry lane that was already pending before our just-finished + // attempt, and also wasn't included in that attempt. + ~(previouslyPendingLanes & ~finishedLanes); + root.suspendedLanes |= freshlySpawnedRetryLanes; + } } function markSpawnedDeferredLane( diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index b729d53bc7fe0..bdba41f0e8d2e 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -128,6 +128,7 @@ import { DidDefer, ShouldSuspendCommit, MaySuspendCommit, + ScheduleRetry, } from './ReactFiberFlags'; import { NoLanes, @@ -365,8 +366,11 @@ let workInProgressRootInterleavedUpdatedLanes: Lanes = NoLanes; let workInProgressRootRenderPhaseUpdatedLanes: Lanes = NoLanes; // Lanes that were pinged (in an interleaved event) during this render. let workInProgressRootPingedLanes: Lanes = NoLanes; -// If this lane scheduled deferred work, this is the lane of the deferred task. +// If this render scheduled deferred work, this is the lane of the deferred task. let workInProgressDeferredLane: Lane = NoLane; +// Represents the retry lanes that were spawned by this render and have not +// been pinged since, implying that they are still suspended. +let workInProgressSuspendedRetryLanes: Lanes = NoLanes; // Errors that are thrown during the render phase. let workInProgressRootConcurrentErrors: Array> | null = null; @@ -1146,6 +1150,8 @@ function finishConcurrentRender( workInProgressTransitions, workInProgressRootDidIncludeRecursiveRenderUpdate, workInProgressDeferredLane, + workInProgressRootInterleavedUpdatedLanes, + workInProgressSuspendedRetryLanes, ); } else { if ( @@ -1188,6 +1194,8 @@ function finishConcurrentRender( workInProgressRootDidIncludeRecursiveRenderUpdate, lanes, workInProgressDeferredLane, + workInProgressRootInterleavedUpdatedLanes, + workInProgressSuspendedRetryLanes, workInProgressRootDidSkipSuspendedSiblings, ), msUntilTimeout, @@ -1203,6 +1211,8 @@ function finishConcurrentRender( workInProgressRootDidIncludeRecursiveRenderUpdate, lanes, workInProgressDeferredLane, + workInProgressRootInterleavedUpdatedLanes, + workInProgressSuspendedRetryLanes, workInProgressRootDidSkipSuspendedSiblings, ); } @@ -1216,6 +1226,8 @@ function commitRootWhenReady( didIncludeRenderPhaseUpdate: boolean, lanes: Lanes, spawnedLane: Lane, + updatedLanes: Lanes, + suspendedRetryLanes: Lanes, didSkipSuspendedSiblings: boolean, ) { // TODO: Combine retry throttling with Suspensey commits. Right now they run @@ -1254,6 +1266,9 @@ function commitRootWhenReady( recoverableErrors, transitions, didIncludeRenderPhaseUpdate, + spawnedLane, + updatedLanes, + suspendedRetryLanes, ), ); markRootSuspended(root, lanes, spawnedLane, didSkipSuspendedSiblings); @@ -1261,13 +1276,15 @@ function commitRootWhenReady( } } - // Otherwise, commit immediately. + // Otherwise, commit immediately.; commitRoot( root, recoverableErrors, transitions, didIncludeRenderPhaseUpdate, spawnedLane, + updatedLanes, + suspendedRetryLanes, ); } @@ -1470,6 +1487,8 @@ export function performSyncWorkOnRoot(root: FiberRoot, lanes: Lanes): null { workInProgressTransitions, workInProgressRootDidIncludeRecursiveRenderUpdate, workInProgressDeferredLane, + workInProgressRootInterleavedUpdatedLanes, + workInProgressSuspendedRetryLanes, ); // Before exiting, make sure there's a callback scheduled for the next @@ -1697,6 +1716,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { workInProgressRootRenderPhaseUpdatedLanes = NoLanes; workInProgressRootPingedLanes = NoLanes; workInProgressDeferredLane = NoLane; + workInProgressSuspendedRetryLanes = NoLanes; workInProgressRootConcurrentErrors = null; workInProgressRootRecoverableErrors = null; workInProgressRootDidIncludeRecursiveRenderUpdate = false; @@ -2701,6 +2721,28 @@ function throwAndUnwindWorkLoop( // separate prerender will be scheduled for later. skipSiblings = true; workInProgressRootDidSkipSuspendedSiblings = true; + + // Because we're skipping the siblings, schedule an immediate retry of + // this boundary. + // + // The reason we do this is because a prerender is only scheduled when + // the root is blocked from committing, i.e. RootSuspendedWithDelay. + // When the root is not blocked, as in the case when we render a + // fallback, the original lane is considered to be finished, and + // therefore no longer in need of being prerendered. However, there's + // still a pending retry that will happen once the data streams in. + // We should start rendering that even before the data streams in so we + // can prerender the siblings. + if ( + suspendedReason === SuspendedOnData || + suspendedReason === SuspendedOnImmediate || + suspendedReason === SuspendedOnDeprecatedThrowPromise + ) { + const boundary = getSuspenseHandler(); + if (boundary !== null && boundary.tag === SuspenseComponent) { + boundary.flags |= ScheduleRetry; + } + } } else { // This is a prerender. Don't skip the siblings. skipSiblings = false; @@ -2721,6 +2763,16 @@ function throwAndUnwindWorkLoop( } } +export function markSpawnedRetryLane(lane: Lane): void { + // Keep track of the retry lanes that were spawned by a fallback during the + // current render and were not later pinged. This will represent the lanes + // that are known to still be suspended. + workInProgressSuspendedRetryLanes = mergeLanes( + workInProgressSuspendedRetryLanes, + lane, + ); +} + function panicOnRootError(root: FiberRoot, error: mixed) { // There's no ancestor that can handle this exception. This should never // happen because the root is supposed to capture all errors that weren't @@ -2899,6 +2951,8 @@ function commitRoot( transitions: Array | null, didIncludeRenderPhaseUpdate: boolean, spawnedLane: Lane, + updatedLanes: Lanes, + suspendedRetryLanes: Lanes, ) { // TODO: This no longer makes any sense. We already wrap the mutation and // layout phases. Should be able to remove. @@ -2914,6 +2968,8 @@ function commitRoot( didIncludeRenderPhaseUpdate, previousUpdateLanePriority, spawnedLane, + updatedLanes, + suspendedRetryLanes, ); } finally { ReactSharedInternals.T = prevTransition; @@ -2930,6 +2986,8 @@ function commitRootImpl( didIncludeRenderPhaseUpdate: boolean, renderPriorityLevel: EventPriority, spawnedLane: Lane, + updatedLanes: Lanes, + suspendedRetryLanes: Lanes, ) { do { // `flushPassiveEffects` will call `flushSyncUpdateQueue` at the end, which @@ -3006,7 +3064,14 @@ function commitRootImpl( const concurrentlyUpdatedLanes = getConcurrentlyUpdatedLanes(); remainingLanes = mergeLanes(remainingLanes, concurrentlyUpdatedLanes); - markRootFinished(root, remainingLanes, spawnedLane); + markRootFinished( + root, + lanes, + remainingLanes, + spawnedLane, + updatedLanes, + suspendedRetryLanes, + ); // Reset this before firing side effects so we can detect recursive updates. didIncludeCommitPhaseUpdate = false; @@ -3712,6 +3777,17 @@ function pingSuspendedRoot( pingedLanes, ); } + + // If something pings the work-in-progress render, any work that suspended + // up to this point may now be unblocked; in other words, no + // longer suspended. + // + // Unlike the broader check above, we only need do this if the lanes match + // exactly. If the lanes don't exactly match, that implies the promise + // was created by an older render. + if (workInProgressSuspendedRetryLanes === workInProgressRootRenderLanes) { + workInProgressSuspendedRetryLanes = NoLanes; + } } ensureRootIsScheduled(root); diff --git a/packages/react-reconciler/src/__tests__/ActivityStrictMode-test.js b/packages/react-reconciler/src/__tests__/ActivityStrictMode-test.js index a8b8d286dc6aa..85365d3e708fc 100644 --- a/packages/react-reconciler/src/__tests__/ActivityStrictMode-test.js +++ b/packages/react-reconciler/src/__tests__/ActivityStrictMode-test.js @@ -240,6 +240,11 @@ describe('Activity StrictMode', () => { 'Parent mount', 'Parent unmount', 'Parent mount', + + ...(gate('enableSiblingPrerendering') + ? ['Child rendered', 'Child suspended'] + : []), + '------------------------------', 'Child rendered', 'Child rendered', diff --git a/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js b/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js index 03e04e8b1dc40..561741c108622 100644 --- a/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js +++ b/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js @@ -313,13 +313,23 @@ describe('act warnings', () => { act(() => { root.render(); }); - assertLog(['Suspend! [Async]', 'Loading...']); + assertLog([ + 'Suspend! [Async]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Async]'] : []), + ]); expect(root).toMatchRenderedOutput('Loading...'); // This is a retry, not a ping, because we already showed a fallback. expect(() => resolveText('Async')).toErrorDev( - 'A suspended resource finished loading inside a test, but the event ' + - 'was not wrapped in act(...)', + [ + 'A suspended resource finished loading inside a test, but the event ' + + 'was not wrapped in act(...)', + + ...(gate('enableSiblingPrerendering') ? ['not wrapped in act'] : []), + ], + {withoutStack: true}, ); }); diff --git a/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js b/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js index 7f68dcc1b6158..b11e78bdec526 100644 --- a/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js @@ -109,7 +109,13 @@ describe('ReactBlockingMode', () => { , ); - await waitForAll(['A', 'Suspend! [B]', 'Loading...']); + await waitForAll([ + 'A', + 'Suspend! [B]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [B]', 'C'] : []), + ]); // In Legacy Mode, A and B would mount in a hidden primary tree. In // Concurrent Mode, nothing in the primary tree should mount. But the // fallback should mount immediately. diff --git a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js index f6ebd3c57494e..340e973b2f0b3 100644 --- a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js +++ b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js @@ -399,7 +399,13 @@ describe('ReactLazyContextPropagation', () => { // the fallback displays despite this being a refresh. setContext('B'); }); - assertLog(['Suspend! [B]', 'Loading...', 'B']); + assertLog([ + 'Suspend! [B]', + 'Loading...', + 'B', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), + ]); expect(root).toMatchRenderedOutput('Loading...B'); await act(async () => { @@ -479,7 +485,13 @@ describe('ReactLazyContextPropagation', () => { // the fallback displays despite this being a refresh. setContext('B'); }); - assertLog(['Suspend! [B]', 'Loading...', 'B']); + assertLog([ + 'Suspend! [B]', + 'Loading...', + 'B', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), + ]); expect(root).toMatchRenderedOutput('Loading...B'); await act(async () => { @@ -812,7 +824,12 @@ describe('ReactLazyContextPropagation', () => { await act(() => { setContext('B'); }); - assertLog(['Suspend! [B]', 'Loading...']); + assertLog([ + 'Suspend! [B]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), + ]); expect(root).toMatchRenderedOutput('Loading...'); await act(async () => { diff --git a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js index b2c38696cc028..fd03ba7310f83 100644 --- a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js +++ b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js @@ -499,6 +499,8 @@ describe('ReactDeferredValue', () => { // The initial value suspended, so we attempt the final value, which // also suspends. 'Suspend! [Final]', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Final]'] : []), ]); expect(root).toMatchRenderedOutput('Fallback'); @@ -630,6 +632,8 @@ describe('ReactDeferredValue', () => { // go straight to attempting the final value. 'Suspend! [Content]', 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Content]'] : []), ]); // The content suspended, so we show a Suspense fallback expect(root).toMatchRenderedOutput('Loading...'); diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 9b714458c74d5..556e0559728e7 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -1864,13 +1864,15 @@ describe('ReactHooks', () => { it('does not fire a false positive warning when suspending memo', async () => { const {Suspense, useState} = React; - let wasSuspended = false; + let isSuspended = true; let resolve; function trySuspend() { - if (!wasSuspended) { - throw new Promise(r => { - wasSuspended = true; - resolve = r; + if (isSuspended) { + throw new Promise(res => { + resolve = () => { + isSuspended = false; + res(); + }; }); } } @@ -1900,13 +1902,15 @@ describe('ReactHooks', () => { it('does not fire a false positive warning when suspending forwardRef', async () => { const {Suspense, useState} = React; - let wasSuspended = false; + let isSuspended = true; let resolve; function trySuspend() { - if (!wasSuspended) { - throw new Promise(r => { - wasSuspended = true; - resolve = r; + if (isSuspended) { + throw new Promise(res => { + resolve = () => { + isSuspended = false; + res(); + }; }); } } @@ -1936,13 +1940,15 @@ describe('ReactHooks', () => { it('does not fire a false positive warning when suspending memo(forwardRef)', async () => { const {Suspense, useState} = React; - let wasSuspended = false; + let isSuspended = true; let resolve; function trySuspend() { - if (!wasSuspended) { - throw new Promise(r => { - wasSuspended = true; - resolve = r; + if (isSuspended) { + throw new Promise(res => { + resolve = () => { + isSuspended = false; + res(); + }; }); } } diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index e62852f2a3ddc..89a07e8e30312 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3544,7 +3544,13 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); }); - assertLog(['A', 'Suspend! [A]', 'Loading']); + assertLog([ + 'A', + 'Suspend! [A]', + 'Loading', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [A]'] : []), + ]); expect(ReactNoop).toMatchRenderedOutput( <> diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 6eec3112e464b..32667704b8a6f 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -313,7 +313,12 @@ describe('ReactLazy', () => { unstable_isConcurrent: true, }); - await waitForAll(['Suspend! [LazyChildA]', 'Loading...']); + await waitForAll([ + 'Suspend! [LazyChildA]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [LazyChildB]'] : []), + ]); expect(root).not.toMatchRenderedOutput('AB'); await act(async () => { @@ -322,9 +327,23 @@ describe('ReactLazy', () => { // B suspends even though it happens to share the same import as A. // TODO: React.lazy should implement the `status` and `value` fields, so // we can unwrap the result synchronously if it already loaded. Like `use`. - await waitFor(['A', 'Suspend! [LazyChildB]']); + await waitFor([ + 'A', + + // When enableSiblingPrerendering is on, LazyChildB was already + // initialized. So it also already resolved when we called + // resolveFakeImport above. So it doesn't suspend again. + ...(gate('enableSiblingPrerendering') + ? ['B'] + : ['Suspend! [LazyChildB]']), + ]); }); - assertLog(['A', 'B', 'Did mount: A', 'Did mount: B']); + assertLog([ + ...(gate('enableSiblingPrerendering') ? [] : ['A', 'B']), + + 'Did mount: A', + 'Did mount: B', + ]); expect(root).toMatchRenderedOutput('AB'); // Swap the position of A and B @@ -1388,15 +1407,20 @@ describe('ReactLazy', () => { unstable_isConcurrent: true, }); - await waitForAll(['Init A', 'Loading...']); + await waitForAll([ + 'Init A', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Init B'] : []), + ]); expect(root).not.toMatchRenderedOutput('AB'); await act(() => resolveFakeImport(ChildA)); assertLog([ 'A', - 'Init B', - ...(gate('enableSiblingPrerendering') ? ['A'] : []), + // When enableSiblingPrerendering is on, B was already initialized. + ...(gate('enableSiblingPrerendering') ? ['A'] : ['Init B']), ]); await act(() => resolveFakeImport(ChildB)); diff --git a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js index 6cc24f9844b2f..902584b2851a4 100644 --- a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js @@ -26,6 +26,22 @@ describe('ReactSiblingPrerendering', () => { textCache = new Map(); }); + // function resolveText(text) { + // const record = textCache.get(text); + // if (record === undefined) { + // const newRecord = { + // status: 'resolved', + // value: text, + // }; + // textCache.set(text, newRecord); + // } else if (record.status === 'pending') { + // const thenable = record.value; + // record.status = 'resolved'; + // record.value = text; + // thenable.pings.forEach(t => t()); + // } + // } + function readText(text) { const record = textCache.get(text); if (record !== undefined) { @@ -61,6 +77,37 @@ describe('ReactSiblingPrerendering', () => { } } + // function getText(text) { + // const record = textCache.get(text); + // if (record === undefined) { + // const thenable = { + // pings: [], + // then(resolve) { + // if (newRecord.status === 'pending') { + // thenable.pings.push(resolve); + // } else { + // Promise.resolve().then(() => resolve(newRecord.value)); + // } + // }, + // }; + // const newRecord = { + // status: 'pending', + // value: thenable, + // }; + // textCache.set(text, newRecord); + // return thenable; + // } else { + // switch (record.status) { + // case 'pending': + // return record.value; + // case 'rejected': + // return Promise.reject(record.value); + // case 'resolved': + // return Promise.resolve(record.value); + // } + // } + // } + function Text({text}) { Scheduler.log(text); return text; @@ -163,4 +210,32 @@ describe('ReactSiblingPrerendering', () => { expect(root).toMatchRenderedOutput('A'); }); }); + + it('start prerendering retries right after the fallback commits', async () => { + function App() { + return ( + }> + + + + ); + } + + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(() => root.render()); + + // On the first attempt, A suspends. Unwind and show a fallback, without + // attempting B. + await waitForPaint(['Suspend! [A]', 'Loading...']); + expect(root).toMatchRenderedOutput('Loading...'); + + // Immediately after the fallback commits, retry the boundary again. This + // time we include B, since we're not blocking the fallback from showing. + if (gate('enableSiblingPrerendering')) { + await waitForPaint(['Suspend! [A]', 'Suspend! [B]']); + } + }); + expect(root).toMatchRenderedOutput('Loading...'); + }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index 76b4ead3c4186..18dd6820e3f85 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -169,13 +169,23 @@ describe('ReactSuspense', () => { 'Loading A...', 'Suspend! [B]', 'Loading B...', + + ...(gate('enableSiblingPrerendering') + ? ['Suspend! [A]', 'Suspend! [B]'] + : []), ]); expect(container.innerHTML).toEqual('Loading A...Loading B...'); // Resolve first Suspense's promise and switch back to the normal view. The // second Suspense should still show the placeholder await act(() => resolveText('A')); - assertLog(['A']); + assertLog([ + 'A', + + ...(gate('enableSiblingPrerendering') + ? ['Suspend! [B]', 'A', 'Suspend! [B]'] + : []), + ]); expect(container.textContent).toEqual('ALoading B...'); // Resolve the second Suspense's promise resolves and switche back to the @@ -274,7 +284,15 @@ describe('ReactSuspense', () => { root.render(); }); - assertLog(['Foo', 'Suspend! [A]', 'Loading...']); + assertLog([ + 'Foo', + 'Suspend! [A]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') + ? ['Suspend! [A]', 'Suspend! [B]', 'Loading more...'] + : []), + ]); expect(container.textContent).toEqual('Loading...'); await resolveText('A'); @@ -327,7 +345,15 @@ describe('ReactSuspense', () => { // Render an empty shell const root = ReactDOMClient.createRoot(container); root.render(); - await waitForAll(['Foo', 'Suspend! [A]', 'Loading...']); + await waitForAll([ + 'Foo', + 'Suspend! [A]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') + ? ['Suspend! [A]', 'Suspend! [B]', 'Loading more...'] + : []), + ]); expect(container.textContent).toEqual('Loading...'); // Now resolve A @@ -375,7 +401,15 @@ describe('ReactSuspense', () => { await act(() => { root.render(); }); - assertLog(['Foo', 'Suspend! [A]', 'Loading...']); + assertLog([ + 'Foo', + 'Suspend! [A]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') + ? ['Suspend! [A]', 'Suspend! [B]', 'Loading more...'] + : []), + ]); expect(container.textContent).toEqual('Loading...'); await resolveText('A'); @@ -468,14 +502,24 @@ describe('ReactSuspense', () => { await act(() => { root.render(); }); - assertLog(['Suspend! [default]', 'Loading...']); + assertLog([ + 'Suspend! [default]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), + ]); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog(['Suspend! [new value]', 'Loading...']); + assertLog([ + 'Suspend! [new value]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), + ]); await act(() => resolveText('new value')); assertLog(['new value']); @@ -515,14 +559,24 @@ describe('ReactSuspense', () => { await act(() => { root.render(); }); - assertLog(['Suspend! [default]', 'Loading...']); + assertLog([ + 'Suspend! [default]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), + ]); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog(['Suspend! [new value]', 'Loading...']); + assertLog([ + 'Suspend! [new value]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), + ]); await act(() => resolveText('new value')); assertLog(['new value']); @@ -559,14 +613,24 @@ describe('ReactSuspense', () => { , ); }); - assertLog(['Suspend! [default]', 'Loading...']); + assertLog([ + 'Suspend! [default]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), + ]); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog(['Suspend! [new value]', 'Loading...']); + assertLog([ + 'Suspend! [new value]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), + ]); await act(() => resolveText('new value')); assertLog(['new value']); @@ -603,14 +667,24 @@ describe('ReactSuspense', () => { , ); }); - assertLog(['Suspend! [default]', 'Loading...']); + assertLog([ + 'Suspend! [default]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), + ]); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog(['Suspend! [new value]', 'Loading...']); + assertLog([ + 'Suspend! [new value]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), + ]); await act(() => resolveText('new value')); assertLog(['new value']); @@ -657,6 +731,10 @@ describe('ReactSuspense', () => { 'Suspend! [Child 2]', 'Loading...', 'destroy layout', + + ...(gate('enableSiblingPrerendering') + ? ['Child 1', 'Suspend! [Child 2]'] + : []), ]); await act(() => resolveText('Child 2')); @@ -679,7 +757,14 @@ describe('ReactSuspense', () => { root.render(); }); - assertLog(['Suspend! [Child 1]', 'Loading...']); + assertLog([ + 'Suspend! [Child 1]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') + ? ['Suspend! [Child 1]', 'Suspend! [Child 2]'] + : []), + ]); await resolveText('Child 1'); await waitForAll([ 'Child 1', diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js index 688ff38a74c0a..4ade3c6ff79cf 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js @@ -274,6 +274,14 @@ describe('ReactSuspenseEffectsSemantics', () => { 'Text:Fallback create passive', 'Text:Outside create passive', 'App create passive', + + ...(gate('enableSiblingPrerendering') + ? [ + 'Text:Inside:Before render', + 'Suspend:Async', + 'ClassText:Inside:After render', + ] + : []), ]); expect(ReactNoop).toMatchRenderedOutput( <> @@ -646,7 +654,17 @@ describe('ReactSuspenseEffectsSemantics', () => { 'Text:Inside:After destroy layout', 'Text:Fallback create layout', ]); - await waitForAll(['Text:Fallback create passive']); + await waitForAll([ + 'Text:Fallback create passive', + + ...(gate('enableSiblingPrerendering') + ? [ + 'Text:Inside:Before render', + 'Suspend:Async', + 'Text:Inside:After render', + ] + : []), + ]); expect(ReactNoop).toMatchRenderedOutput( <>