From c2857fedf4084b43b80a7f9b269898103eb6e864 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 10 Sep 2024 13:24:43 -0400 Subject: [PATCH] Prerendering should not cancel a pending commit If there's a pending commit that's expected to run within an short amount of time, we should not cancel it in favor of prerendering. We should wait for the commit to finish before prerendering. This does not apply to commits that are suspended indefinitely, like when you suspend outside of a Suspense boundary, or in the shell during a transition. Because those cases do not represent a complete tree. There's one special case that we intentionally (for now) don't handle, which is Suspensey CSS. These are also expected to resolve quickly, because of preloading, but theoretically they could block forever like in a normal "suspend indefinitely" scenario. In the future, we should consider only blocking for up to some time limit before discarding the commit in favor of prerendering. --- .../__tests__/ReactCacheOld-test.internal.js | 15 +- .../react-reconciler/src/ReactFiberLane.js | 39 ++- .../src/ReactFiberWorkLoop.js | 23 +- .../__tests__/DebugTracing-test.internal.js | 16 +- .../ReactHooksWithNoopRenderer-test.js | 8 +- .../src/__tests__/ReactLazy-test.internal.js | 6 +- .../ReactSiblingPrerendering-test.js | 261 +++++++++++++++++- .../__tests__/ReactSuspense-test.internal.js | 40 +-- .../__tests__/ReactSuspenseCallback-test.js | 12 +- .../ReactSuspenseEffectsSemantics-test.js | 14 +- .../src/__tests__/ReactSuspenseList-test.js | 171 +++--------- .../ReactSuspenseWithNoopRenderer-test.js | 14 +- .../__tests__/ReactTransitionTracing-test.js | 5 +- .../src/__tests__/ReactUse-test.js | 16 +- 14 files changed, 384 insertions(+), 256 deletions(-) diff --git a/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js b/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js index bffb7b96818c0..0e9cb549f653a 100644 --- a/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js +++ b/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js @@ -206,7 +206,12 @@ describe('ReactCache', () => { ...(gate('enableSiblingPrerendering') ? ['Invalid key type'] : []), ]); } else { - await waitForAll(['App', 'Loading...']); + await waitForAll([ + 'App', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['App'] : []), + ]); } }); @@ -226,10 +231,14 @@ describe('ReactCache', () => { await waitForPaint(['Suspend! [1]', 'Loading...']); jest.advanceTimersByTime(100); assertLog(['Promise resolved [1]']); - await waitForAll([1, 'Suspend! [2]', 1, 'Suspend! [2]', 'Suspend! [3]']); + await waitForAll([1, 'Suspend! [2]']); + + jest.advanceTimersByTime(100); + assertLog(['Promise resolved [2]']); + await waitForAll([1, 2, 'Suspend! [3]']); jest.advanceTimersByTime(100); - assertLog(['Promise resolved [2]', 'Promise resolved [3]']); + assertLog(['Promise resolved [3]']); await waitForAll([1, 2, 3]); await act(() => jest.advanceTimersByTime(100)); diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 8f6f7c9c3f94f..4338d3af58546 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -233,6 +233,29 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { const pingedLanes = root.pingedLanes; const warmLanes = root.warmLanes; + // finishedLanes represents a completed tree that is ready to commit. + // + // It's not worth doing discarding the completed tree in favor of performing + // speculative work. So always check this before deciding to warm up + // the siblings. + // + // Note that this is not set in a "suspend indefinitely" scenario, like when + // suspending outside of a Suspense boundary, or in the shell during a + // transition — only in cases where we are very likely to commit the tree in + // a brief amount of time (i.e. below the "Just Noticeable Difference" + // threshold). + // + // TODO: finishedLanes is also set when a Suspensey resource, like CSS or + // images, suspends during the commit phase. (We could detect that here by + // checking for root.cancelPendingCommit.) These are also expected to resolve + // quickly, because of preloading, but theoretically they could block forever + // like in a normal "suspend indefinitely" scenario. In the future, we should + // consider only blocking for up to some time limit before discarding the + // commit in favor of prerendering. If we do discard a pending commit, then + // the commit phase callback should act as a ping to try the original + // render again. + const rootHasPendingCommit = root.finishedLanes !== NoLanes; + // Do not work on any idle work until all the non-idle work has finished, // even if the work is suspended. const nonIdlePendingLanes = pendingLanes & NonIdleLanes; @@ -248,9 +271,11 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { nextLanes = getHighestPriorityLanes(nonIdlePingedLanes); } else { // Nothing has been pinged. Check for lanes that need to be prewarmed. - const lanesToPrewarm = nonIdlePendingLanes & ~warmLanes; - if (lanesToPrewarm !== NoLanes) { - nextLanes = getHighestPriorityLanes(lanesToPrewarm); + if (!rootHasPendingCommit) { + const lanesToPrewarm = nonIdlePendingLanes & ~warmLanes; + if (lanesToPrewarm !== NoLanes) { + nextLanes = getHighestPriorityLanes(lanesToPrewarm); + } } } } @@ -270,9 +295,11 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { nextLanes = getHighestPriorityLanes(pingedLanes); } else { // Nothing has been pinged. Check for lanes that need to be prewarmed. - const lanesToPrewarm = pendingLanes & ~warmLanes; - if (lanesToPrewarm !== NoLanes) { - nextLanes = getHighestPriorityLanes(lanesToPrewarm); + if (!rootHasPendingCommit) { + const lanesToPrewarm = pendingLanes & ~warmLanes; + if (lanesToPrewarm !== NoLanes) { + nextLanes = getHighestPriorityLanes(lanesToPrewarm); + } } } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index bdba41f0e8d2e..f615fc9fa34d1 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -997,8 +997,6 @@ export function performConcurrentWorkOnRoot( // We now have a consistent tree. The next step is either to commit it, // or, if something suspended, wait to commit it after a timeout. - root.finishedWork = finishedWork; - root.finishedLanes = lanes; finishConcurrentRender(root, exitStatus, finishedWork, lanes); } break; @@ -1142,6 +1140,12 @@ function finishConcurrentRender( } } + // Only set these if we have a complete tree that is ready to be committed. + // We use these fields to determine later whether or not the work should be + // discarded for a fresh render attempt. + root.finishedWork = finishedWork; + root.finishedLanes = lanes; + if (shouldForceFlushFallbacksInDEV()) { // We're inside an `act` scope. Commit immediately. commitRoot( @@ -1174,8 +1178,11 @@ function finishConcurrentRender( const nextLanes = getNextLanes(root, NoLanes); if (nextLanes !== NoLanes) { - // There's additional work we can do on this root. We might as well - // attempt to work on that while we're suspended. + // There are additional updates we can do on this root. We might as + // well attempt to work on that while we're suspended. + // + // Notably, this bailout does not occur if the only thing remaining is + // Suspense retries. return; } @@ -2226,6 +2233,14 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { workInProgressTransitions = getTransitionsForLanes(root, lanes); resetRenderTimer(); prepareFreshStack(root, lanes); + } else { + // This is a continuation of an existing work-in-progress. + // + // If we were previously in prerendering mode, check if we received any new + // data during an interleaved event. + if (workInProgressRootIsPrerendering) { + workInProgressRootIsPrerendering = checkIfRootIsPrerendering(root, lanes); + } } if (__DEV__) { diff --git a/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js b/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js index eb6fe1687ccd6..ba0451c4c5f0b 100644 --- a/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js +++ b/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js @@ -187,12 +187,26 @@ describe('DebugTracing', () => { `group: ⚛ render (${DEFAULT_LANE_STRING})`, 'log: ⚛ Example suspended', `groupEnd: ⚛ render (${DEFAULT_LANE_STRING})`, + + ...(gate('enableSiblingPrerendering') + ? [ + `group: ⚛ render (${RETRY_LANE_STRING})`, + 'log: ⚛ Example suspended', + `groupEnd: ⚛ render (${RETRY_LANE_STRING})`, + ] + : []), ]); logs.splice(0); await act(async () => await resolveFakeSuspensePromise()); - expect(logs).toEqual(['log: ⚛ Example resolved']); + expect(logs).toEqual([ + 'log: ⚛ Example resolved', + + ...(gate('enableSiblingPrerendering') + ? ['log: ⚛ Example resolved'] + : []), + ]); }); // @gate experimental && build === 'development' && enableDebugTracing && enableCPUSuspense diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 89a07e8e30312..0fc64b39da899 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -4207,13 +4207,7 @@ describe('ReactHooksWithNoopRenderer', () => { await act(async () => { await resolveText('A'); }); - assertLog([ - 'Promise resolved [A]', - 'A', - 'Suspend! [B]', - - ...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [B]'] : []), - ]); + assertLog(['Promise resolved [A]', 'A', 'Suspend! [B]']); await act(() => { root.render(null); diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 32667704b8a6f..8526fb5b19cd6 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -198,11 +198,7 @@ describe('ReactLazy', () => { await resolveFakeImport(Foo); - await waitForAll([ - 'Foo', - - ...(gate('enableSiblingPrerendering') ? ['Foo'] : []), - ]); + await waitForAll(['Foo']); expect(root).not.toMatchRenderedOutput('FooBar'); await act(() => resolveFakeImport(Bar)); diff --git a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js index 902584b2851a4..6c24fdc980285 100644 --- a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js @@ -3,7 +3,9 @@ let ReactNoop; let Scheduler; let act; let assertLog; +let waitFor; let waitForPaint; +let waitForAll; let textCache; let startTransition; let Suspense; @@ -18,7 +20,9 @@ describe('ReactSiblingPrerendering', () => { Scheduler = require('scheduler'); act = require('internal-test-utils').act; assertLog = require('internal-test-utils').assertLog; + waitFor = require('internal-test-utils').waitFor; waitForPaint = require('internal-test-utils').waitForPaint; + waitForAll = require('internal-test-utils').waitForAll; startTransition = React.startTransition; Suspense = React.Suspense; Activity = React.unstable_Activity; @@ -26,21 +30,21 @@ 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 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); @@ -238,4 +242,231 @@ describe('ReactSiblingPrerendering', () => { }); expect(root).toMatchRenderedOutput('Loading...'); }); + + it('switch back to normal rendering mode if a ping occurs during prerendering', async () => { + function App() { + return ( +
+ }> +
+ + +
+
+ }> + + + +
+ +
+ ); + } + + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(() => root.render()); + + // On the first attempt, B suspends. Unwind and show a fallback, without + // attempting the siblings. + await waitForPaint(['A', 'Suspend! [B]', 'Loading outer...']); + expect(root).toMatchRenderedOutput(
Loading outer...
); + + // Now that the fallback is visible, we can prerender the siblings. Start + // prerendering, then yield to simulate an interleaved event. + if (gate('enableSiblingPrerendering')) { + await waitFor(['A']); + } else { + await waitForAll([]); + } + + // To avoid the Suspense throttling mechanism, let's pretend there's been + // more than a Just Noticeable Difference since we rendered the + // outer fallback. + Scheduler.unstable_advanceTime(500); + + // During the render phase, but before we get to B again, resolve its + // promise. We should re-enter normal rendering mode, but we also + // shouldn't unwind and lose our work-in-progress. + await resolveText('B'); + await waitForPaint([ + // When sibling prerendering is not enabled, we weren't already rendering + // when the data for B came in, so A doesn't get rendered until now. + ...(gate('enableSiblingPrerendering') ? [] : ['A']), + + 'B', + 'Suspend! [C]', + + // If we were still in prerendering mode, then we would have attempted + // to render D here. But since we received new data, we will skip the + // remaining siblings to unblock the inner fallback. + 'Loading inner...', + ]); + + expect(root).toMatchRenderedOutput( +
+
AB
+
Loading inner...
+
, + ); + }); + + // Now that the inner fallback is showing, we can prerender the rest of + // the tree. + assertLog( + gate('enableSiblingPrerendering') + ? [ + // NOTE: C renders twice instead of once because when B resolved, it + // was treated like a retry update, not just a ping. So first it + // regular renders, then it prerenders. TODO: We should be able to + // optimize this by detecting inside the retry listener that the + // outer boundary is no longer suspended, and therefore doesn't need + // to be updated. + 'Suspend! [C]', + + // Now we're in prerender mode, so D is incuded in this attempt. + 'Suspend! [C]', + 'Suspend! [D]', + ] + : [], + ); + expect(root).toMatchRenderedOutput( +
+
AB
+
Loading inner...
+
, + ); + }); + + it("don't throw out completed work in order to prerender", async () => { + function App() { + return ( +
+ }> +
+ +
+
+ }> + + +
+ +
+ ); + } + + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(() => root.render()); + + await waitForPaint(['Suspend! [A]', 'Loading outer...']); + expect(root).toMatchRenderedOutput(
Loading outer...
); + + // Before the prerendering of the inner boundary starts, the data for A + // resolves, so we try rendering that again. + await resolveText('A'); + // This produces a new tree that we can show. However, the commit phase + // is throttled because it's been less than a Just Noticeable Difference + // since the outer fallback was committed. + // + // In the meantime, we could choose to start prerendering B, but instead + // we wait for a JND to elapse and the commit to finish — it's not + // worth discarding the work we've already done. + await waitForAll(['A', 'Suspend! [B]', 'Loading inner...']); + expect(root).toMatchRenderedOutput(
Loading outer...
); + + // Fire the timer to commit the outer fallback. + jest.runAllTimers(); + expect(root).toMatchRenderedOutput( +
+
A
+
Loading inner...
+
, + ); + }); + // Once the outer fallback is committed, we can start prerendering B. + assertLog(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []); + }); + + it( + "don't skip siblings during the retry if there was a ping since the " + + 'first attempt', + 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 or C. + await waitFor([ + 'Suspend! [A]', + 'Loading outer...', + + // Yield to simulate an interleaved event + ]); + + // Ping the promise for A before the render phase has finished, as might + // happen in an interleaved network event + await resolveText('A'); + + // Now continue rendering the rest of the tree. + await waitForPaint(['D']); + expect(root).toMatchRenderedOutput( + <> +
Loading outer...
+
D
+ , + ); + + // Immediately after the fallback commits, retry the boundary again. + // Because the promise for A resolved, this is a normal render, _not_ + // a prerender. So when we proceed to B, and B suspends, we unwind again + // without attempting C. The practical benefit of this is that we don't + // block the inner Suspense fallback from appearing. + await waitForPaint(['A', 'Suspend! [B]', 'Loading inner...']); + // (Since this is a retry, the commit phase is throttled by a timer.) + jest.runAllTimers(); + // The inner fallback is now visible. + expect(root).toMatchRenderedOutput( + <> +
+
A
+
Loading inner...
+
+
D
+ , + ); + + // Now we can proceed to prerendering C. + if (gate('enableSiblingPrerendering')) { + await waitForPaint(['Suspend! [B]', 'Suspend! [C]']); + } + }); + assertLog([]); + }, + ); }); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index 18dd6820e3f85..d7ad652f1da7e 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -183,7 +183,7 @@ describe('ReactSuspense', () => { 'A', ...(gate('enableSiblingPrerendering') - ? ['Suspend! [B]', 'A', 'Suspend! [B]'] + ? ['Suspend! [B]', 'Suspend! [B]'] : []), ]); expect(container.textContent).toEqual('ALoading B...'); @@ -296,15 +296,7 @@ describe('ReactSuspense', () => { expect(container.textContent).toEqual('Loading...'); await resolveText('A'); - await waitForAll([ - 'A', - 'Suspend! [B]', - 'Loading more...', - - ...(gate('enableSiblingPrerendering') - ? ['A', 'Suspend! [B]', 'Loading more...'] - : []), - ]); + await waitForAll(['A', 'Suspend! [B]', 'Loading more...']); // By this point, we have enough info to show "A" and "Loading more..." // However, we've just shown the outer fallback. So we'll delay @@ -364,14 +356,7 @@ describe('ReactSuspense', () => { // B starts loading. Parent boundary is in throttle. // Still shows parent loading under throttle jest.advanceTimersByTime(10); - await waitForAll([ - 'Suspend! [B]', - 'Loading more...', - - ...(gate('enableSiblingPrerendering') - ? ['A', 'Suspend! [B]', 'Loading more...'] - : []), - ]); + await waitForAll(['Suspend! [B]', 'Loading more...']); expect(container.textContent).toEqual('Loading...'); // !! B could have finished before the throttle, but we show a fallback. @@ -413,15 +398,7 @@ describe('ReactSuspense', () => { expect(container.textContent).toEqual('Loading...'); await resolveText('A'); - await waitForAll([ - 'A', - 'Suspend! [B]', - 'Loading more...', - - ...(gate('enableSiblingPrerendering') - ? ['A', 'Suspend! [B]', 'Loading more...'] - : []), - ]); + await waitForAll(['A', 'Suspend! [B]', 'Loading more...']); // By this point, we have enough info to show "A" and "Loading more..." // However, we've just shown the outer fallback. So we'll delay @@ -766,14 +743,7 @@ describe('ReactSuspense', () => { : []), ]); await resolveText('Child 1'); - await waitForAll([ - 'Child 1', - 'Suspend! [Child 2]', - - ...(gate('enableSiblingPrerendering') - ? ['Child 1', 'Suspend! [Child 2]'] - : []), - ]); + await waitForAll(['Child 1', 'Suspend! [Child 2]']); jest.advanceTimersByTime(6000); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js index 7f3724fa86490..6da6ef7573fd9 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js @@ -136,7 +136,11 @@ describe('ReactSuspense', () => { ReactNoop.render(element); await waitForAll([]); expect(ReactNoop).toMatchRenderedOutput('Waiting Tier 1'); - expect(ops).toEqual([new Set([promise2])]); + expect(ops).toEqual([ + new Set([promise2]), + + ...(gate('enableSiblingPrerendering') ? new Set([promise2]) : []), + ]); ops = []; await act(() => resolve2()); @@ -224,7 +228,11 @@ describe('ReactSuspense', () => { await act(() => resolve1()); expect(ReactNoop).toMatchRenderedOutput('Waiting Tier 2Done'); expect(ops1).toEqual([]); - expect(ops2).toEqual([new Set([promise2])]); + expect(ops2).toEqual([ + new Set([promise2]), + + ...(gate('enableSiblingPrerendering') ? new Set([promise2]) : []), + ]); ops1 = []; ops2 = []; diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js index 4ade3c6ff79cf..4a094dc7fed43 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js @@ -1268,22 +1268,16 @@ describe('ReactSuspenseEffectsSemantics', () => { 'Suspend:InnerAsync_2', 'Text:InnerFallback render', - ...(gate('enableSiblingPrerendering') - ? [ - 'Text:Outer render', - 'AsyncText:OuterAsync_1 render', - 'Text:Inner render', - 'Suspend:InnerAsync_2', - 'Text:InnerFallback render', - ] - : []), - 'Text:OuterFallback destroy layout', 'Text:Outer create layout', 'AsyncText:OuterAsync_1 create layout', 'Text:InnerFallback create layout', 'Text:OuterFallback destroy passive', 'AsyncText:OuterAsync_1 create passive', + + ...(gate('enableSiblingPrerendering') + ? ['Text:Inner render', 'Suspend:InnerAsync_2'] + : []), ]); expect(ReactNoop).toMatchRenderedOutput( <> diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js index 59388050f2ade..b4a7bcc186b6d 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js @@ -255,7 +255,7 @@ describe('ReactSuspenseList', () => { await act(() => C.resolve()); assertLog( gate('enableSiblingPrerendering') - ? ['Suspend! [B]', 'C', 'Suspend! [B]', 'C'] + ? ['Suspend! [B]', 'C', 'Suspend! [B]'] : ['C'], ); @@ -390,13 +390,7 @@ describe('ReactSuspenseList', () => { ); await act(() => B.resolve()); - assertLog([ - 'A', - 'B', - 'Suspend! [C]', - - ...(gate('enableSiblingPrerendering') ? ['A', 'B', 'Suspend! [C]'] : []), - ]); + assertLog(['A', 'B', 'Suspend! [C]']); expect(ReactNoop).toMatchRenderedOutput( <> @@ -472,13 +466,7 @@ describe('ReactSuspenseList', () => { ); await act(() => B.resolve()); - assertLog([ - 'A', - 'B', - 'Suspend! [C]', - - ...(gate('enableSiblingPrerendering') ? ['A', 'B', 'Suspend! [C]'] : []), - ]); + assertLog(['A', 'B', 'Suspend! [C]']); expect(ReactNoop).toMatchRenderedOutput( <> @@ -749,7 +737,11 @@ describe('ReactSuspenseList', () => { ReactNoop.render(); - await waitForAll(['Suspend! [A]', 'Loading']); + await waitForAll([ + 'Suspend! [A]', + 'Loading', + ...(gate('enableSiblingPrerendering') ? ['Suspend! [A]'] : []), + ]); expect(ReactNoop).toMatchRenderedOutput(Loading); @@ -786,12 +778,7 @@ describe('ReactSuspenseList', () => { ); await act(() => B.resolve()); - assertLog([ - 'B', - 'Suspend! [C]', - - ...(gate('enableSiblingPrerendering') ? ['B', 'Suspend! [C]'] : []), - ]); + assertLog(['B', 'Suspend! [C]']); // Even though we could now show B, we're still waiting on C. expect(ReactNoop).toMatchRenderedOutput( @@ -878,12 +865,7 @@ describe('ReactSuspenseList', () => { expect(ReactNoop).toMatchRenderedOutput(A); await act(() => B.resolve()); - assertLog([ - 'B', - 'Suspend! [C]', - - ...(gate('enableSiblingPrerendering') ? ['B', 'Suspend! [C]'] : []), - ]); + assertLog(['B', 'Suspend! [C]']); // Even though we could now show B, we're still waiting on C. expect(ReactNoop).toMatchRenderedOutput(A); @@ -948,7 +930,7 @@ describe('ReactSuspenseList', () => { 'A', 'Suspend! [B]', - ...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [B]'] : []), + ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), ]); expect(ReactNoop).toMatchRenderedOutput( @@ -1019,7 +1001,7 @@ describe('ReactSuspenseList', () => { 'C', 'Suspend! [B]', - ...(gate('enableSiblingPrerendering') ? ['C', 'Suspend! [B]'] : []), + ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), ]); expect(ReactNoop).toMatchRenderedOutput( @@ -1125,12 +1107,7 @@ describe('ReactSuspenseList', () => { ); await act(() => A.resolve()); - assertLog([ - 'A', - 'Suspend! [C]', - - ...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [C]'] : []), - ]); + assertLog(['A', 'Suspend! [C]']); // Even though we could show A, it is still in a fallback state because // C is not yet resolved. We need to resolve everything in the head first. @@ -1151,7 +1128,7 @@ describe('ReactSuspenseList', () => { 'C', 'Suspend! [E]', - ...(gate('enableSiblingPrerendering') ? ['A', 'C', 'Suspend! [E]'] : []), + ...(gate('enableSiblingPrerendering') ? ['Suspend! [E]'] : []), ]); // We can now resolve the full head. @@ -1171,7 +1148,7 @@ describe('ReactSuspenseList', () => { 'E', 'Suspend! [F]', - ...(gate('enableSiblingPrerendering') ? ['E', 'Suspend! [F]'] : []), + ...(gate('enableSiblingPrerendering') ? ['Suspend! [F]'] : []), ]); // In the tail we can resolve one-by-one. @@ -1340,12 +1317,7 @@ describe('ReactSuspenseList', () => { await F.resolve(); - await waitForAll([ - 'Suspend! [D]', - 'F', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [D]', 'F'] : []), - ]); + await waitForAll(['Suspend! [D]', 'F']); // Even though we could show F, it is still in a fallback state because // E is not yet resolved. We need to resolve everything in the head first. @@ -1370,7 +1342,7 @@ describe('ReactSuspenseList', () => { 'F', 'Suspend! [B]', - ...(gate('enableSiblingPrerendering') ? ['D', 'F', 'Suspend! [B]'] : []), + ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), ]); // We can now resolve the full head. @@ -1392,7 +1364,7 @@ describe('ReactSuspenseList', () => { 'B', 'Suspend! [A]', - ...(gate('enableSiblingPrerendering') ? ['B', 'Suspend! [A]'] : []), + ...(gate('enableSiblingPrerendering') ? ['Suspend! [A]'] : []), ]); // In the tail we can resolve one-by-one. @@ -1523,15 +1495,7 @@ describe('ReactSuspenseList', () => { await A.resolve(); - await waitForAll([ - 'A', - 'Suspend! [B]', - 'Loading B', - - ...(gate('enableSiblingPrerendering') - ? ['A', 'Suspend! [B]', 'Loading B'] - : []), - ]); + await waitForAll(['A', 'Suspend! [B]', 'Loading B']); // Incremental loading is suspended. jest.advanceTimersByTime(500); @@ -1545,15 +1509,7 @@ describe('ReactSuspenseList', () => { await B.resolve(); - await waitForAll([ - 'B', - 'Suspend! [C]', - 'Loading C', - - ...(gate('enableSiblingPrerendering') - ? ['B', 'Suspend! [C]', 'Loading C'] - : []), - ]); + await waitForAll(['B', 'Suspend! [C]', 'Loading C']); // Incremental loading is suspended. jest.advanceTimersByTime(500); @@ -1777,12 +1733,7 @@ describe('ReactSuspenseList', () => { await B.resolve(); - await waitForAll([ - 'B', - 'Suspend! [C]', - - ...(gate('enableSiblingPrerendering') ? ['B', 'Suspend! [C]'] : []), - ]); + await waitForAll(['B', 'Suspend! [C]']); // Incremental loading is suspended. jest.advanceTimersByTime(500); @@ -1802,17 +1753,7 @@ describe('ReactSuspenseList', () => { await C.resolve(); await E.resolve(); - await waitForAll([ - 'B', - 'C', - 'E', - 'Suspend! [F]', - 'Loading F', - - ...(gate('enableSiblingPrerendering') - ? ['B', 'C', 'E', 'Suspend! [F]', 'Loading F'] - : []), - ]); + await waitForAll(['B', 'C', 'E', 'Suspend! [F]', 'Loading F']); jest.advanceTimersByTime(500); @@ -1929,12 +1870,7 @@ describe('ReactSuspenseList', () => { await D.resolve(); - await waitForAll([ - 'D', - 'Suspend! [E]', - - ...(gate('enableSiblingPrerendering') ? ['D', 'Suspend! [E]'] : []), - ]); + await waitForAll(['D', 'Suspend! [E]']); // Incremental loading is suspended. jest.advanceTimersByTime(500); @@ -1959,17 +1895,7 @@ describe('ReactSuspenseList', () => { await D.resolve(); await E.resolve(); - await waitForAll([ - 'D', - 'E', - 'B', - 'Suspend! [A]', - 'Loading A', - - ...(gate('enableSiblingPrerendering') - ? ['D', 'E', 'B', 'Suspend! [A]', 'Loading A'] - : []), - ]); + await waitForAll(['D', 'E', 'B', 'Suspend! [A]', 'Loading A']); jest.advanceTimersByTime(500); @@ -2100,12 +2026,7 @@ describe('ReactSuspenseList', () => { await B.resolve(); - await waitForAll([ - 'B', - 'Suspend! [C]', - - ...(gate('enableSiblingPrerendering') ? ['B', 'Suspend! [C]'] : []), - ]); + await waitForAll(['B', 'Suspend! [C]']); // Incremental loading is suspended. jest.advanceTimersByTime(500); @@ -2128,17 +2049,7 @@ describe('ReactSuspenseList', () => { await D.resolve(); await E.resolve(); - await waitForAll([ - 'C', - 'D', - 'E', - 'Suspend! [F]', - 'Loading F', - - ...(gate('enableSiblingPrerendering') - ? ['C', 'D', 'E', 'Suspend! [F]', 'Loading F'] - : []), - ]); + await waitForAll(['C', 'D', 'E', 'Suspend! [F]', 'Loading F']); jest.advanceTimersByTime(500); @@ -2201,15 +2112,7 @@ describe('ReactSuspenseList', () => { await A.resolve(); - await waitForAll([ - 'A', - 'Suspend! [B]', - 'Loading B', - - ...(gate('enableSiblingPrerendering') - ? ['A', 'Suspend! [B]', 'Loading B'] - : []), - ]); + await waitForAll(['A', 'Suspend! [B]', 'Loading B']); // Incremental loading is suspended. jest.advanceTimersByTime(500); @@ -2217,15 +2120,7 @@ describe('ReactSuspenseList', () => { expect(ReactNoop).toMatchRenderedOutput(A); await act(() => B.resolve()); - assertLog([ - 'B', - 'Suspend! [C]', - 'Loading C', - - ...(gate('enableSiblingPrerendering') - ? ['B', 'Suspend! [C]', 'Loading C'] - : []), - ]); + assertLog(['B', 'Suspend! [C]', 'Loading C']); // Incremental loading is suspended. jest.advanceTimersByTime(500); @@ -2987,7 +2882,7 @@ describe('ReactSuspenseList', () => { 'C', 'Suspend! [D]', - ...(gate('enableSiblingPrerendering') ? ['C', 'Suspend! [D]'] : []), + ...(gate('enableSiblingPrerendering') ? ['Suspend! [D]'] : []), ]); expect(ReactNoop).toMatchRenderedOutput( <> @@ -2999,12 +2894,12 @@ describe('ReactSuspenseList', () => { ); if (gate('enableSiblingPrerendering')) { - expect(onRender).toHaveBeenCalledTimes(5); + expect(onRender).toHaveBeenCalledTimes(6); // actualDuration - expect(onRender.mock.calls[4][2]).toBe(5 + 12); + expect(onRender.mock.calls[5][2]).toBe(12); // treeBaseDuration - expect(onRender.mock.calls[4][3]).toBe(1 + 4 + 5 + 3); + expect(onRender.mock.calls[5][3]).toBe(1 + 4 + 5 + 3); } else { expect(onRender).toHaveBeenCalledTimes(4); @@ -3095,7 +2990,7 @@ describe('ReactSuspenseList', () => { 'A', 'Suspend! [B]', - ...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [B]'] : []), + ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), ]); expect(ReactNoop).toMatchRenderedOutput( <> diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index f834dd8912a34..39cfebec3487e 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -353,7 +353,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'A', ...(gate('enableSiblingPrerendering') - ? ['Suspend! [B]', 'A', 'Suspend! [B]'] + ? ['Suspend! [B]', 'Suspend! [B]'] : []), ]); expect(ReactNoop).toMatchRenderedOutput( @@ -788,10 +788,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'Outer content', 'Suspend! [Inner content]', 'Loading inner...', - - ...(gate('enableSiblingPrerendering') - ? ['Outer content', 'Suspend! [Inner content]', 'Loading inner...'] - : []), ]); // Don't commit the inner placeholder yet. expect(ReactNoop).toMatchRenderedOutput( @@ -1819,10 +1815,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { // B suspends 'Suspend! [B]', 'Loading more...', - - ...(gate('enableSiblingPrerendering') - ? ['A', 'Suspend! [B]', 'Loading more...'] - : []), ]); // Because we've already been waiting for so long we can // wait a bit longer. Still nothing... @@ -3698,10 +3690,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { ); }); - // This regression test relies on subtle implementation details that happen to - // rely on sibling prerendering being disabled. Not going to bother to rewrite - // it for now; maybe once we land the experiment. - // @gate !enableSiblingPrerendering // @gate enableLegacyCache it('regression: ping at high priority causes update to be dropped', async () => { const {useState, useTransition} = React; diff --git a/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js index c6d79ed76c20a..7a850325e9f2c 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js +++ b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js @@ -2446,11 +2446,10 @@ describe('ReactInteractionTracing', () => { 'Text', 'Suspend [Text Two]', 'Loading Two...', - ...(gate('enableSiblingPrerendering') - ? ['Suspend [Text Two]', 'Suspend [Text Two]'] - : []), + ...(gate('enableSiblingPrerendering') ? ['Suspend [Text Two]'] : []), 'onTransitionStart(transition, 0)', 'onTransitionProgress(transition, 0, 1000, [two])', + ...(gate('enableSiblingPrerendering') ? ['Suspend [Text Two]'] : []), ]); await act(() => { diff --git a/packages/react-reconciler/src/__tests__/ReactUse-test.js b/packages/react-reconciler/src/__tests__/ReactUse-test.js index 66c2e116a5372..97d1df55d57dc 100644 --- a/packages/react-reconciler/src/__tests__/ReactUse-test.js +++ b/packages/react-reconciler/src/__tests__/ReactUse-test.js @@ -1077,25 +1077,13 @@ describe('ReactUse', () => { await act(() => { resolveTextRequests('A'); }); - assertLog([ - 'A', - '(Loading B...)', - - ...(gate('enableSiblingPrerendering') - ? ['A', '(Loading C...)', '(Loading B...)'] - : []), - ]); + assertLog(['A', '(Loading B...)']); expect(root).toMatchRenderedOutput('A(Loading B...)'); await act(() => { resolveTextRequests('B'); }); - assertLog([ - 'B', - '(Loading C...)', - - ...(gate('enableSiblingPrerendering') ? ['B', '(Loading C...)'] : []), - ]); + assertLog(['B', '(Loading C...)']); expect(root).toMatchRenderedOutput('AB(Loading C...)'); await act(() => {