From 071655d17ba257b7524a6e120a2686be8053af74 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Wed, 18 Sep 2024 18:21:18 -0400 Subject: [PATCH] Revert "Start prerendering Suspense retries immediately (#30934)" This reverts commit d6cb4e771341ff82489c00f4907990cb8a75696b. --- .../__tests__/ReactCacheOld-test.internal.js | 98 ++-- .../src/__tests__/TimelineProfiler-test.js | 93 +--- .../src/__tests__/ReactDOMForm-test.js | 14 +- .../ReactDOMSuspensePlaceholder-test.js | 8 +- .../__tests__/ReactWrongReturnPointer-test.js | 10 +- .../src/ReactFiberCompleteWork.js | 42 +- .../react-reconciler/src/ReactFiberLane.js | 78 +-- .../src/ReactFiberWorkLoop.js | 147 +----- .../src/__tests__/ActivityStrictMode-test.js | 5 - .../__tests__/DebugTracing-test.internal.js | 16 +- .../src/__tests__/ReactActWarnings-test.js | 16 +- .../__tests__/ReactBatching-test.internal.js | 8 +- .../ReactConcurrentErrorRecovery-test.js | 11 +- .../__tests__/ReactContextPropagation-test.js | 23 +- .../src/__tests__/ReactDeferredValue-test.js | 4 - .../src/__tests__/ReactHooks-test.internal.js | 36 +- .../ReactHooksWithNoopRenderer-test.js | 16 +- .../src/__tests__/ReactLazy-test.internal.js | 49 +- .../ReactSiblingPrerendering-test.js | 472 ------------------ .../__tests__/ReactSuspense-test.internal.js | 139 ++---- .../__tests__/ReactSuspenseCallback-test.js | 12 +- .../ReactSuspenseEffectsSemantics-test.js | 209 +------- .../__tests__/ReactSuspenseFallback-test.js | 22 +- .../src/__tests__/ReactSuspenseList-test.js | 263 +++++----- .../ReactSuspensePlaceholder-test.internal.js | 107 +--- .../ReactSuspenseWithNoopRenderer-test.js | 302 +++-------- .../__tests__/ReactTransitionTracing-test.js | 119 +---- .../src/__tests__/ReactUse-test.js | 31 +- .../src/__tests__/StrictEffectsMode-test.js | 8 - 29 files changed, 463 insertions(+), 1895 deletions(-) delete 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 0e9cb549f653a..bec901c722007 100644 --- a/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js +++ b/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js @@ -18,7 +18,6 @@ let Suspense; let TextResource; let textResourceShouldFail; let waitForAll; -let waitForPaint; let assertLog; let waitForThrow; let act; @@ -38,7 +37,6 @@ describe('ReactCache', () => { waitForAll = InternalTestUtils.waitForAll; assertLog = InternalTestUtils.assertLog; waitForThrow = InternalTestUtils.waitForThrow; - waitForPaint = InternalTestUtils.waitForPaint; act = InternalTestUtils.act; TextResource = createResource( @@ -121,12 +119,7 @@ describe('ReactCache', () => { const root = ReactNoop.createRoot(); root.render(); - await waitForAll([ - 'Suspend! [Hi]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []), - ]); + await waitForAll(['Suspend! [Hi]', 'Loading...']); jest.advanceTimersByTime(100); assertLog(['Promise resolved [Hi]']); @@ -145,12 +138,7 @@ describe('ReactCache', () => { const root = ReactNoop.createRoot(); root.render(); - await waitForAll([ - 'Suspend! [Hi]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []), - ]); + await waitForAll(['Suspend! [Hi]', 'Loading...']); textResourceShouldFail = true; let error; @@ -160,7 +148,15 @@ describe('ReactCache', () => { error = e; } expect(error.message).toMatch('Failed to load: Hi'); - assertLog(['Promise rejected [Hi]', 'Error! [Hi]', 'Error! [Hi]']); + assertLog([ + 'Promise rejected [Hi]', + 'Error! [Hi]', + 'Error! [Hi]', + + ...(gate('enableSiblingPrerendering') + ? ['Error! [Hi]', 'Error! [Hi]'] + : []), + ]); // Should throw again on a subsequent read root.render(); @@ -191,27 +187,15 @@ describe('ReactCache', () => { if (__DEV__) { await expect(async () => { - await waitForAll([ - 'App', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['App'] : []), - ]); + await waitForAll(['App', 'Loading...']); }).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...', - - ...(gate('enableSiblingPrerendering') ? ['App'] : []), - ]); + await waitForAll(['App', 'Loading...']); } }); @@ -228,17 +212,13 @@ describe('ReactCache', () => { , ); - await waitForPaint(['Suspend! [1]', 'Loading...']); + await waitForAll(['Suspend! [1]', 'Loading...']); jest.advanceTimersByTime(100); assertLog(['Promise resolved [1]']); - await waitForAll([1, 'Suspend! [2]']); - - jest.advanceTimersByTime(100); - assertLog(['Promise resolved [2]']); - await waitForAll([1, 2, 'Suspend! [3]']); + await waitForAll([1, 'Suspend! [2]', 1, 'Suspend! [2]', 'Suspend! [3]']); jest.advanceTimersByTime(100); - assertLog(['Promise resolved [3]']); + assertLog(['Promise resolved [2]', 'Promise resolved [3]']); await waitForAll([1, 2, 3]); await act(() => jest.advanceTimersByTime(100)); @@ -253,17 +233,24 @@ describe('ReactCache', () => { , ); - await waitForAll([ + await waitForAll([1, 'Suspend! [4]', 'Loading...']); + + await act(() => jest.advanceTimersByTime(100)); + assertLog([ + 'Promise resolved [4]', + 1, - 'Suspend! [4]', - 'Loading...', + 4, + 'Suspend! [5]', 1, - 'Suspend! [4]', + 4, 'Suspend! [5]', - ]); - await act(() => jest.advanceTimersByTime(100)); - assertLog(['Promise resolved [4]', 'Promise resolved [5]', 1, 4, 5]); + 'Promise resolved [5]', + 1, + 4, + 5, + ]); expect(root).toMatchRenderedOutput('145'); @@ -284,14 +271,24 @@ 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, - 'Suspend! [2]', + 2, + 'Suspend! [3]', + 1, + 2, 'Suspend! [3]', - ]); - await act(() => jest.advanceTimersByTime(100)); - assertLog(['Promise resolved [2]', 'Promise resolved [3]', 1, 2, 3]); + 'Promise resolved [3]', + 1, + 2, + 3, + ]); expect(root).toMatchRenderedOutput('123'); }); @@ -366,12 +363,7 @@ describe('ReactCache', () => { , ); - await waitForAll([ - 'Suspend! [Hi]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []), - ]); + await waitForAll(['Suspend! [Hi]', 'Loading...']); resolveThenable('Hi'); // This thenable improperly resolves twice. We should not update the diff --git a/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js b/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js index b4cc9bbc0bafc..a4c55f0bccf64 100644 --- a/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js +++ b/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js @@ -1672,11 +1672,7 @@ describe('Timeline profiler', () => { , ); - await waitForAll([ - 'suspended', - - ...(enableSiblingPrerendering ? ['suspended'] : []), - ]); + await waitForAll(['suspended']); Scheduler.unstable_advanceTime(10); resolveFn(); @@ -1687,38 +1683,9 @@ describe('Timeline profiler', () => { const timelineData = stopProfilingAndGetTimelineData(); // Verify the Suspense event and duration was recorded. - if (enableSiblingPrerendering) { - expect(timelineData.suspenseEvents).toMatchInlineSnapshot(` - [ - { - "componentName": "Example", - "depth": 0, - "duration": 10, - "id": "0", - "phase": "mount", - "promiseName": "", - "resolution": "resolved", - "timestamp": 10, - "type": "suspense", - "warning": null, - }, - { - "componentName": "Example", - "depth": 0, - "duration": 10, - "id": "0", - "phase": "mount", - "promiseName": "", - "resolution": "resolved", - "timestamp": 10, - "type": "suspense", - "warning": null, - }, - ] - `); - } else { - const suspenseEvent = timelineData.suspenseEvents[0]; - expect(suspenseEvent).toMatchInlineSnapshot(` + expect(timelineData.suspenseEvents).toHaveLength(1); + const suspenseEvent = timelineData.suspenseEvents[0]; + expect(suspenseEvent).toMatchInlineSnapshot(` { "componentName": "Example", "depth": 0, @@ -1732,13 +1699,10 @@ describe('Timeline profiler', () => { "warning": null, } `); - } // There should be two batches of renders: Suspeneded and resolved. expect(timelineData.batchUIDToMeasuresMap.size).toBe(2); - expect(timelineData.componentMeasures).toHaveLength( - enableSiblingPrerendering ? 3 : 2, - ); + expect(timelineData.componentMeasures).toHaveLength(2); }); it('should mark concurrent render with suspense that rejects', async () => { @@ -1765,11 +1729,7 @@ describe('Timeline profiler', () => { , ); - await waitForAll([ - 'suspended', - - ...(enableSiblingPrerendering ? ['suspended'] : []), - ]); + await waitForAll(['suspended']); Scheduler.unstable_advanceTime(10); rejectFn(); @@ -1780,39 +1740,9 @@ describe('Timeline profiler', () => { const timelineData = stopProfilingAndGetTimelineData(); // Verify the Suspense event and duration was recorded. - if (enableSiblingPrerendering) { - expect(timelineData.suspenseEvents).toMatchInlineSnapshot(` - [ - { - "componentName": "Example", - "depth": 0, - "duration": 10, - "id": "0", - "phase": "mount", - "promiseName": "", - "resolution": "rejected", - "timestamp": 10, - "type": "suspense", - "warning": null, - }, - { - "componentName": "Example", - "depth": 0, - "duration": 10, - "id": "0", - "phase": "mount", - "promiseName": "", - "resolution": "rejected", - "timestamp": 10, - "type": "suspense", - "warning": null, - }, - ] - `); - } else { - expect(timelineData.suspenseEvents).toHaveLength(1); - const suspenseEvent = timelineData.suspenseEvents[0]; - expect(suspenseEvent).toMatchInlineSnapshot(` + expect(timelineData.suspenseEvents).toHaveLength(1); + const suspenseEvent = timelineData.suspenseEvents[0]; + expect(suspenseEvent).toMatchInlineSnapshot(` { "componentName": "Example", "depth": 0, @@ -1826,13 +1756,10 @@ describe('Timeline profiler', () => { "warning": null, } `); - } // There should be two batches of renders: Suspeneded and resolved. expect(timelineData.batchUIDToMeasuresMap.size).toBe(2); - expect(timelineData.componentMeasures).toHaveLength( - enableSiblingPrerendering ? 3 : 2, - ); + expect(timelineData.componentMeasures).toHaveLength(2); }); it('should mark cascading class component state updates', async () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMForm-test.js b/packages/react-dom/src/__tests__/ReactDOMForm-test.js index 168d44722d473..5a44cb6afec09 100644 --- a/packages/react-dom/src/__tests__/ReactDOMForm-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMForm-test.js @@ -1459,23 +1459,13 @@ describe('ReactDOMForm', () => { , ), ); - assertLog([ - 'Suspend! [Count: 0]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 0]'] : []), - ]); + assertLog(['Suspend! [Count: 0]', 'Loading...']); 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...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 1]'] : []), - ]); + assertLog(['Suspend! [Count: 1]', 'Loading...']); 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 fa22142702527..50ba64a478888 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js @@ -160,13 +160,7 @@ describe('ReactDOMSuspensePlaceholder', () => { }); expect(container.textContent).toEqual('Loading...'); - assertLog([ - 'A', - 'Suspend! [B]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [B]', 'C'] : []), - ]); + assertLog(['A', 'Suspend! [B]', 'Loading...']); 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 796d429625a70..468d5b54e1ae2 100644 --- a/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js +++ b/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js @@ -192,13 +192,7 @@ test('regression (#20932): return pointer is correct before entering deleted tre await act(() => { root.render(); }); - assertLog([ - 'Suspend! [0]', - 'Loading Async...', - 'Loading Tail...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [0]'] : []), - ]); + assertLog(['Suspend! [0]', 'Loading Async...', 'Loading Tail...']); await act(() => { resolveText(0); }); @@ -211,7 +205,5 @@ 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 e3fba900dd116..c5d0c1575be04 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -155,7 +155,6 @@ import { getRenderTargetTime, getWorkInProgressTransitions, shouldRemainOnPreviousScreen, - markSpawnedRetryLane, } from './ReactFiberWorkLoop'; import { OffscreenLane, @@ -601,28 +600,25 @@ function scheduleRetryEffect( // Schedule an effect to attach a retry listener to the promise. // TODO: Move to passive phase workInProgress.flags |= Update; - } - - // 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); + } 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); + } } } diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index f72174e208555..2fd45c85bf569 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -26,11 +26,9 @@ 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. @@ -233,29 +231,6 @@ 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; @@ -271,11 +246,9 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { nextLanes = getHighestPriorityLanes(nonIdlePingedLanes); } else { // Nothing has been pinged. Check for lanes that need to be prewarmed. - if (!rootHasPendingCommit) { - const lanesToPrewarm = nonIdlePendingLanes & ~warmLanes; - if (lanesToPrewarm !== NoLanes) { - nextLanes = getHighestPriorityLanes(lanesToPrewarm); - } + const lanesToPrewarm = nonIdlePendingLanes & ~warmLanes; + if (lanesToPrewarm !== NoLanes) { + nextLanes = getHighestPriorityLanes(lanesToPrewarm); } } } @@ -295,11 +268,9 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { nextLanes = getHighestPriorityLanes(pingedLanes); } else { // Nothing has been pinged. Check for lanes that need to be prewarmed. - if (!rootHasPendingCommit) { - const lanesToPrewarm = pendingLanes & ~warmLanes; - if (lanesToPrewarm !== NoLanes) { - nextLanes = getHighestPriorityLanes(lanesToPrewarm); - } + const lanesToPrewarm = pendingLanes & ~warmLanes; + if (lanesToPrewarm !== NoLanes) { + nextLanes = getHighestPriorityLanes(lanesToPrewarm); } } } @@ -782,14 +753,10 @@ export function markRootPinged(root: FiberRoot, pingedLanes: Lanes) { export function markRootFinished( root: FiberRoot, - finishedLanes: Lanes, remainingLanes: Lanes, spawnedLane: Lane, - updatedLanes: Lanes, - suspendedRetryLanes: Lanes, ) { - const previouslyPendingLanes = root.pendingLanes; - const noLongerPendingLanes = previouslyPendingLanes & ~remainingLanes; + const noLongerPendingLanes = root.pendingLanes & ~remainingLanes; root.pendingLanes = remainingLanes; @@ -845,37 +812,6 @@ 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 dba089e81f3e0..1065c24ee0db3 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -129,7 +129,6 @@ import { DidDefer, ShouldSuspendCommit, MaySuspendCommit, - ScheduleRetry, } from './ReactFiberFlags'; import { NoLanes, @@ -369,11 +368,8 @@ 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 render scheduled deferred work, this is the lane of the deferred task. +// If this lane 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; @@ -999,6 +995,8 @@ 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; @@ -1148,12 +1146,6 @@ 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( @@ -1162,8 +1154,6 @@ function finishConcurrentRender( workInProgressTransitions, workInProgressRootDidIncludeRecursiveRenderUpdate, workInProgressDeferredLane, - workInProgressRootInterleavedUpdatedLanes, - workInProgressSuspendedRetryLanes, ); } else { if ( @@ -1206,8 +1196,6 @@ function finishConcurrentRender( workInProgressRootDidIncludeRecursiveRenderUpdate, lanes, workInProgressDeferredLane, - workInProgressRootInterleavedUpdatedLanes, - workInProgressSuspendedRetryLanes, workInProgressRootDidSkipSuspendedSiblings, ), msUntilTimeout, @@ -1223,8 +1211,6 @@ function finishConcurrentRender( workInProgressRootDidIncludeRecursiveRenderUpdate, lanes, workInProgressDeferredLane, - workInProgressRootInterleavedUpdatedLanes, - workInProgressSuspendedRetryLanes, workInProgressRootDidSkipSuspendedSiblings, ); } @@ -1238,8 +1224,6 @@ 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 @@ -1278,9 +1262,6 @@ function commitRootWhenReady( recoverableErrors, transitions, didIncludeRenderPhaseUpdate, - spawnedLane, - updatedLanes, - suspendedRetryLanes, ), ); markRootSuspended(root, lanes, spawnedLane, didSkipSuspendedSiblings); @@ -1288,15 +1269,13 @@ function commitRootWhenReady( } } - // Otherwise, commit immediately.; + // Otherwise, commit immediately. commitRoot( root, recoverableErrors, transitions, didIncludeRenderPhaseUpdate, spawnedLane, - updatedLanes, - suspendedRetryLanes, ); } @@ -1306,13 +1285,7 @@ function isRenderConsistentWithExternalStores(finishedWork: Fiber): boolean { // loop instead of recursion so we can exit early. let node: Fiber = finishedWork; while (true) { - const tag = node.tag; - if ( - (tag === FunctionComponent || - tag === ForwardRef || - tag === SimpleMemoComponent) && - node.flags & StoreConsistency - ) { + if (node.flags & StoreConsistency) { const updateQueue: FunctionComponentUpdateQueue | null = (node.updateQueue: any); if (updateQueue !== null) { @@ -1503,8 +1476,6 @@ 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 @@ -1732,7 +1703,6 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { workInProgressRootRenderPhaseUpdatedLanes = NoLanes; workInProgressRootPingedLanes = NoLanes; workInProgressDeferredLane = NoLane; - workInProgressSuspendedRetryLanes = NoLanes; workInProgressRootConcurrentErrors = null; workInProgressRootRecoverableErrors = null; workInProgressRootDidIncludeRecursiveRenderUpdate = false; @@ -2146,10 +2116,9 @@ 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, reason); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); break; } } @@ -2242,14 +2211,6 @@ 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__) { @@ -2277,12 +2238,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop( - root, - unitOfWork, - thrownValue, - SuspendedOnError, - ); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); break; } case SuspendedOnData: { @@ -2340,12 +2296,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Otherwise, unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop( - root, - unitOfWork, - thrownValue, - SuspendedAndReadyToContinue, - ); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); } break; } @@ -2408,12 +2359,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Otherwise, unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop( - root, - unitOfWork, - thrownValue, - SuspendedOnInstanceAndReadyToContinue, - ); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); break; } case SuspendedOnDeprecatedThrowPromise: { @@ -2423,12 +2369,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // always unwind. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop( - root, - unitOfWork, - thrownValue, - SuspendedOnDeprecatedThrowPromise, - ); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); break; } case SuspendedOnHydration: { @@ -2682,7 +2623,6 @@ 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. @@ -2730,43 +2670,16 @@ 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() || - suspendedReason === SuspendedOnError + workInProgressSuspendedReason === SuspendedOnError ) { skipSiblings = true; // We intentionally don't set workInProgressRootDidSkipSuspendedSiblings, // because we don't want to trigger another prerender attempt. - } else if ( - // Check whether this is a prerender - !workInProgressRootIsPrerendering && - // Offscreen rendering is also a form of speculative rendering - !includesSomeLane(workInProgressRootRenderLanes, OffscreenLane) - ) { + } else if (!workInProgressRootIsPrerendering) { // This is not a prerender. Skip the siblings during this render. A // 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; @@ -2787,16 +2700,6 @@ 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 @@ -2962,8 +2865,6 @@ 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. @@ -2979,8 +2880,6 @@ function commitRoot( didIncludeRenderPhaseUpdate, previousUpdateLanePriority, spawnedLane, - updatedLanes, - suspendedRetryLanes, ); } finally { ReactSharedInternals.T = prevTransition; @@ -2997,8 +2896,6 @@ function commitRootImpl( didIncludeRenderPhaseUpdate: boolean, renderPriorityLevel: EventPriority, spawnedLane: Lane, - updatedLanes: Lanes, - suspendedRetryLanes: Lanes, ) { do { // `flushPassiveEffects` will call `flushSyncUpdateQueue` at the end, which @@ -3075,14 +2972,7 @@ function commitRootImpl( const concurrentlyUpdatedLanes = getConcurrentlyUpdatedLanes(); remainingLanes = mergeLanes(remainingLanes, concurrentlyUpdatedLanes); - markRootFinished( - root, - lanes, - remainingLanes, - spawnedLane, - updatedLanes, - suspendedRetryLanes, - ); + markRootFinished(root, remainingLanes, spawnedLane); // Reset this before firing side effects so we can detect recursive updates. didIncludeCommitPhaseUpdate = false; @@ -3775,17 +3665,6 @@ 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 85365d3e708fc..a8b8d286dc6aa 100644 --- a/packages/react-reconciler/src/__tests__/ActivityStrictMode-test.js +++ b/packages/react-reconciler/src/__tests__/ActivityStrictMode-test.js @@ -240,11 +240,6 @@ 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__/DebugTracing-test.internal.js b/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js index ba0451c4c5f0b..eb6fe1687ccd6 100644 --- a/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js +++ b/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js @@ -187,26 +187,12 @@ 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', - - ...(gate('enableSiblingPrerendering') - ? ['log: ⚛ Example resolved'] - : []), - ]); + expect(logs).toEqual(['log: ⚛ Example resolved']); }); // @gate experimental && build === 'development' && enableDebugTracing && enableCPUSuspense diff --git a/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js b/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js index 561741c108622..03e04e8b1dc40 100644 --- a/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js +++ b/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js @@ -313,23 +313,13 @@ describe('act warnings', () => { act(() => { root.render(); }); - assertLog([ - 'Suspend! [Async]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [Async]'] : []), - ]); + assertLog(['Suspend! [Async]', 'Loading...']); 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(...)', - - ...(gate('enableSiblingPrerendering') ? ['not wrapped in act'] : []), - ], - + 'A suspended resource finished loading inside a test, but the event ' + + 'was 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 b11e78bdec526..7f68dcc1b6158 100644 --- a/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js @@ -109,13 +109,7 @@ describe('ReactBlockingMode', () => { , ); - await waitForAll([ - 'A', - 'Suspend! [B]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [B]', 'C'] : []), - ]); + await waitForAll(['A', 'Suspend! [B]', 'Loading...']); // 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__/ReactConcurrentErrorRecovery-test.js b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js index 4a00607ffb848..db9d7c336ec2e 100644 --- a/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js +++ b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js @@ -292,7 +292,16 @@ 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...']); + assertLog([ + 'Error! [A2]', + 'Oops!', + 'Suspend! [B2]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') + ? ['Error! [A2]', 'Oops!', 'Suspend! [B2]', 'Loading...'] + : []), + ]); // Remain on previous screen. expect(root).toMatchRenderedOutput('A1B1'); diff --git a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js index 340e973b2f0b3..f6ebd3c57494e 100644 --- a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js +++ b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js @@ -399,13 +399,7 @@ describe('ReactLazyContextPropagation', () => { // the fallback displays despite this being a refresh. setContext('B'); }); - assertLog([ - 'Suspend! [B]', - 'Loading...', - 'B', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), - ]); + assertLog(['Suspend! [B]', 'Loading...', 'B']); expect(root).toMatchRenderedOutput('Loading...B'); await act(async () => { @@ -485,13 +479,7 @@ describe('ReactLazyContextPropagation', () => { // the fallback displays despite this being a refresh. setContext('B'); }); - assertLog([ - 'Suspend! [B]', - 'Loading...', - 'B', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), - ]); + assertLog(['Suspend! [B]', 'Loading...', 'B']); expect(root).toMatchRenderedOutput('Loading...B'); await act(async () => { @@ -824,12 +812,7 @@ describe('ReactLazyContextPropagation', () => { await act(() => { setContext('B'); }); - assertLog([ - 'Suspend! [B]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), - ]); + assertLog(['Suspend! [B]', 'Loading...']); 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 fd03ba7310f83..b2c38696cc028 100644 --- a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js +++ b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js @@ -499,8 +499,6 @@ 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'); @@ -632,8 +630,6 @@ 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 556e0559728e7..9b714458c74d5 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -1864,15 +1864,13 @@ describe('ReactHooks', () => { it('does not fire a false positive warning when suspending memo', async () => { const {Suspense, useState} = React; - let isSuspended = true; + let wasSuspended = false; let resolve; function trySuspend() { - if (isSuspended) { - throw new Promise(res => { - resolve = () => { - isSuspended = false; - res(); - }; + if (!wasSuspended) { + throw new Promise(r => { + wasSuspended = true; + resolve = r; }); } } @@ -1902,15 +1900,13 @@ describe('ReactHooks', () => { it('does not fire a false positive warning when suspending forwardRef', async () => { const {Suspense, useState} = React; - let isSuspended = true; + let wasSuspended = false; let resolve; function trySuspend() { - if (isSuspended) { - throw new Promise(res => { - resolve = () => { - isSuspended = false; - res(); - }; + if (!wasSuspended) { + throw new Promise(r => { + wasSuspended = true; + resolve = r; }); } } @@ -1940,15 +1936,13 @@ describe('ReactHooks', () => { it('does not fire a false positive warning when suspending memo(forwardRef)', async () => { const {Suspense, useState} = React; - let isSuspended = true; + let wasSuspended = false; let resolve; function trySuspend() { - if (isSuspended) { - throw new Promise(res => { - resolve = () => { - isSuspended = false; - res(); - }; + if (!wasSuspended) { + throw new Promise(r => { + wasSuspended = true; + resolve = r; }); } } diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index bc87e47083e2a..48bbaafe3ddde 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3597,13 +3597,7 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); }); - assertLog([ - 'A', - 'Suspend! [A]', - 'Loading', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [A]'] : []), - ]); + assertLog(['A', 'Suspend! [A]', 'Loading']); expect(ReactNoop).toMatchRenderedOutput( <> @@ -4260,7 +4254,13 @@ describe('ReactHooksWithNoopRenderer', () => { await act(async () => { await resolveText('A'); }); - assertLog(['Promise resolved [A]', 'A', 'Suspend! [B]']); + assertLog([ + 'Promise resolved [A]', + 'A', + 'Suspend! [B]', + + ...(gate('enableSiblingPrerendering') ? ['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 8526fb5b19cd6..891cf91049833 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -198,7 +198,11 @@ describe('ReactLazy', () => { await resolveFakeImport(Foo); - await waitForAll(['Foo']); + await waitForAll([ + 'Foo', + + ...(gate('enableSiblingPrerendering') ? ['Foo'] : []), + ]); expect(root).not.toMatchRenderedOutput('FooBar'); await act(() => resolveFakeImport(Bar)); @@ -235,6 +239,13 @@ 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'); }); @@ -309,12 +320,7 @@ describe('ReactLazy', () => { unstable_isConcurrent: true, }); - await waitForAll([ - 'Suspend! [LazyChildA]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [LazyChildB]'] : []), - ]); + await waitForAll(['Suspend! [LazyChildA]', 'Loading...']); expect(root).not.toMatchRenderedOutput('AB'); await act(async () => { @@ -323,23 +329,9 @@ 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', - - // 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]']), - ]); + await waitFor(['A', 'Suspend! [LazyChildB]']); }); - assertLog([ - ...(gate('enableSiblingPrerendering') ? [] : ['A', 'B']), - - 'Did mount: A', - 'Did mount: B', - ]); + assertLog(['A', 'B', 'Did mount: A', 'Did mount: B']); expect(root).toMatchRenderedOutput('AB'); // Swap the position of A and B @@ -1403,20 +1395,15 @@ describe('ReactLazy', () => { unstable_isConcurrent: true, }); - await waitForAll([ - 'Init A', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Init B'] : []), - ]); + await waitForAll(['Init A', 'Loading...']); expect(root).not.toMatchRenderedOutput('AB'); await act(() => resolveFakeImport(ChildA)); assertLog([ 'A', + 'Init B', - // When enableSiblingPrerendering is on, B was already initialized. - ...(gate('enableSiblingPrerendering') ? ['A'] : ['Init B']), + ...(gate('enableSiblingPrerendering') ? ['A'] : []), ]); await act(() => resolveFakeImport(ChildB)); diff --git a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js deleted file mode 100644 index 6c24fdc980285..0000000000000 --- a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js +++ /dev/null @@ -1,472 +0,0 @@ -let React; -let ReactNoop; -let Scheduler; -let act; -let assertLog; -let waitFor; -let waitForPaint; -let waitForAll; -let textCache; -let startTransition; -let Suspense; -let Activity; - -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; - 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; - - 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) { - 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 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; - } - - 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]'] : []), - ]); - }); - - // @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'); - }); - }); - - 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...'); - }); - - 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 d7ad652f1da7e..76b4ead3c4186 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -169,23 +169,13 @@ 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', - - ...(gate('enableSiblingPrerendering') - ? ['Suspend! [B]', 'Suspend! [B]'] - : []), - ]); + assertLog(['A']); expect(container.textContent).toEqual('ALoading B...'); // Resolve the second Suspense's promise resolves and switche back to the @@ -284,19 +274,19 @@ describe('ReactSuspense', () => { root.render(); }); - assertLog([ - 'Foo', - 'Suspend! [A]', - 'Loading...', + assertLog(['Foo', 'Suspend! [A]', 'Loading...']); + expect(container.textContent).toEqual('Loading...'); + + await resolveText('A'); + await waitForAll([ + 'A', + 'Suspend! [B]', + 'Loading more...', ...(gate('enableSiblingPrerendering') - ? ['Suspend! [A]', 'Suspend! [B]', 'Loading more...'] + ? ['A', 'Suspend! [B]', 'Loading more...'] : []), ]); - expect(container.textContent).toEqual('Loading...'); - - await resolveText('A'); - 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 @@ -337,15 +327,7 @@ describe('ReactSuspense', () => { // Render an empty shell const root = ReactDOMClient.createRoot(container); root.render(); - await waitForAll([ - 'Foo', - 'Suspend! [A]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') - ? ['Suspend! [A]', 'Suspend! [B]', 'Loading more...'] - : []), - ]); + await waitForAll(['Foo', 'Suspend! [A]', 'Loading...']); expect(container.textContent).toEqual('Loading...'); // Now resolve A @@ -356,7 +338,14 @@ 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...']); + await waitForAll([ + 'Suspend! [B]', + 'Loading more...', + + ...(gate('enableSiblingPrerendering') + ? ['A', 'Suspend! [B]', 'Loading more...'] + : []), + ]); expect(container.textContent).toEqual('Loading...'); // !! B could have finished before the throttle, but we show a fallback. @@ -386,19 +375,19 @@ describe('ReactSuspense', () => { await act(() => { root.render(); }); - assertLog([ - 'Foo', - 'Suspend! [A]', - 'Loading...', + assertLog(['Foo', 'Suspend! [A]', 'Loading...']); + expect(container.textContent).toEqual('Loading...'); + + await resolveText('A'); + await waitForAll([ + 'A', + 'Suspend! [B]', + 'Loading more...', ...(gate('enableSiblingPrerendering') - ? ['Suspend! [A]', 'Suspend! [B]', 'Loading more...'] + ? ['A', 'Suspend! [B]', 'Loading more...'] : []), ]); - expect(container.textContent).toEqual('Loading...'); - - await resolveText('A'); - 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 @@ -479,24 +468,14 @@ describe('ReactSuspense', () => { await act(() => { root.render(); }); - assertLog([ - 'Suspend! [default]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), - ]); + assertLog(['Suspend! [default]', 'Loading...']); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog([ - 'Suspend! [new value]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), - ]); + assertLog(['Suspend! [new value]', 'Loading...']); await act(() => resolveText('new value')); assertLog(['new value']); @@ -536,24 +515,14 @@ describe('ReactSuspense', () => { await act(() => { root.render(); }); - assertLog([ - 'Suspend! [default]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), - ]); + assertLog(['Suspend! [default]', 'Loading...']); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog([ - 'Suspend! [new value]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), - ]); + assertLog(['Suspend! [new value]', 'Loading...']); await act(() => resolveText('new value')); assertLog(['new value']); @@ -590,24 +559,14 @@ describe('ReactSuspense', () => { , ); }); - assertLog([ - 'Suspend! [default]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), - ]); + assertLog(['Suspend! [default]', 'Loading...']); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog([ - 'Suspend! [new value]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), - ]); + assertLog(['Suspend! [new value]', 'Loading...']); await act(() => resolveText('new value')); assertLog(['new value']); @@ -644,24 +603,14 @@ describe('ReactSuspense', () => { , ); }); - assertLog([ - 'Suspend! [default]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), - ]); + assertLog(['Suspend! [default]', 'Loading...']); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog([ - 'Suspend! [new value]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), - ]); + assertLog(['Suspend! [new value]', 'Loading...']); await act(() => resolveText('new value')); assertLog(['new value']); @@ -708,10 +657,6 @@ describe('ReactSuspense', () => { 'Suspend! [Child 2]', 'Loading...', 'destroy layout', - - ...(gate('enableSiblingPrerendering') - ? ['Child 1', 'Suspend! [Child 2]'] - : []), ]); await act(() => resolveText('Child 2')); @@ -734,16 +679,16 @@ describe('ReactSuspense', () => { root.render(); }); - assertLog([ - 'Suspend! [Child 1]', - 'Loading...', + assertLog(['Suspend! [Child 1]', 'Loading...']); + await resolveText('Child 1'); + await waitForAll([ + 'Child 1', + 'Suspend! [Child 2]', ...(gate('enableSiblingPrerendering') - ? ['Suspend! [Child 1]', 'Suspend! [Child 2]'] + ? ['Child 1', 'Suspend! [Child 2]'] : []), ]); - await resolveText('Child 1'); - 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 6da6ef7573fd9..7f3724fa86490 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js @@ -136,11 +136,7 @@ describe('ReactSuspense', () => { ReactNoop.render(element); await waitForAll([]); expect(ReactNoop).toMatchRenderedOutput('Waiting Tier 1'); - expect(ops).toEqual([ - new Set([promise2]), - - ...(gate('enableSiblingPrerendering') ? new Set([promise2]) : []), - ]); + expect(ops).toEqual([new Set([promise2])]); ops = []; await act(() => resolve2()); @@ -228,11 +224,7 @@ describe('ReactSuspense', () => { await act(() => resolve1()); expect(ReactNoop).toMatchRenderedOutput('Waiting Tier 2Done'); expect(ops1).toEqual([]); - expect(ops2).toEqual([ - new Set([promise2]), - - ...(gate('enableSiblingPrerendering') ? new Set([promise2]) : []), - ]); + expect(ops2).toEqual([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 d45b29aee59ef..ab1a2f6763732 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js @@ -283,14 +283,6 @@ 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( <> @@ -685,17 +677,7 @@ describe('ReactSuspenseEffectsSemantics', () => { 'Text:Fallback create insertion', 'Text:Fallback create layout', ]); - await waitForAll([ - 'Text:Fallback create passive', - - ...(gate('enableSiblingPrerendering') - ? [ - 'Text:Inside:Before render', - 'Suspend:Async', - 'Text:Inside:After render', - ] - : []), - ]); + await waitForAll(['Text:Fallback create passive']); expect(ReactNoop).toMatchRenderedOutput( <>