From d6cb4e771341ff82489c00f4907990cb8a75696b Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 11 Sep 2024 11:41:54 -0400 Subject: [PATCH] Start prerendering Suspense retries immediately (#30934) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a component suspends and is replaced by a fallback, we should start prerendering the fallback immediately, even before any new data is received. During the retry, we can enter prerender mode directly if we're sure that no new data was received since we last attempted to render the boundary. To do this, when completing the fallback, we leave behind a pending retry lane on the Suspense boundary. Previously we only did this once a promise resolved, but by assigning a lane during the complete phase, we will know that there's speculative work to be done. Then, upon committing the fallback, we mark the retry lane as suspended — but only if nothing was pinged or updated in the meantime. That allows us to immediately enter prerender mode (i.e. render without skipping any siblings) when performing the retry. --- .../__tests__/ReactCacheOld-test.internal.js | 104 ++-- .../src/__tests__/TimelineProfiler-test.js | 104 +++- .../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 | 143 ++++-- .../__tests__/ReactSuspenseCallback-test.js | 12 +- .../ReactSuspenseEffectsSemantics-test.js | 208 +++++++- .../__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, 1911 insertions(+), 467 deletions(-) create mode 100644 packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js diff --git a/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js b/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js index bec901c722..0e9cb549f6 100644 --- a/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js +++ b/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js @@ -18,6 +18,7 @@ let Suspense; let TextResource; let textResourceShouldFail; let waitForAll; +let waitForPaint; let assertLog; let waitForThrow; let act; @@ -37,6 +38,7 @@ describe('ReactCache', () => { waitForAll = InternalTestUtils.waitForAll; assertLog = InternalTestUtils.assertLog; waitForThrow = InternalTestUtils.waitForThrow; + waitForPaint = InternalTestUtils.waitForPaint; act = InternalTestUtils.act; TextResource = createResource( @@ -119,7 +121,12 @@ describe('ReactCache', () => { const root = ReactNoop.createRoot(); root.render(); - await waitForAll(['Suspend! [Hi]', 'Loading...']); + await waitForAll([ + 'Suspend! [Hi]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []), + ]); jest.advanceTimersByTime(100); assertLog(['Promise resolved [Hi]']); @@ -138,7 +145,12 @@ describe('ReactCache', () => { const root = ReactNoop.createRoot(); root.render(); - await waitForAll(['Suspend! [Hi]', 'Loading...']); + await waitForAll([ + 'Suspend! [Hi]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []), + ]); textResourceShouldFail = true; let error; @@ -148,15 +160,7 @@ describe('ReactCache', () => { error = e; } expect(error.message).toMatch('Failed to load: Hi'); - assertLog([ - 'Promise rejected [Hi]', - 'Error! [Hi]', - 'Error! [Hi]', - - ...(gate('enableSiblingPrerendering') - ? ['Error! [Hi]', 'Error! [Hi]'] - : []), - ]); + assertLog(['Promise rejected [Hi]', 'Error! [Hi]', 'Error! [Hi]']); // Should throw again on a subsequent read root.render(); @@ -187,15 +191,27 @@ describe('ReactCache', () => { if (__DEV__) { await expect(async () => { - await waitForAll(['App', 'Loading...']); + await waitForAll([ + 'App', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['App'] : []), + ]); }).toErrorDev([ 'Invalid key type. Expected a string, number, symbol, or ' + "boolean, but instead received: [ 'Hi', 100 ]\n\n" + 'To use non-primitive values as keys, you must pass a hash ' + 'function as the second argument to createResource().', + + ...(gate('enableSiblingPrerendering') ? ['Invalid key type'] : []), ]); } else { - await waitForAll(['App', 'Loading...']); + await waitForAll([ + 'App', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['App'] : []), + ]); } }); @@ -212,13 +228,17 @@ describe('ReactCache', () => { , ); - await waitForAll(['Suspend! [1]', 'Loading...']); + await waitForPaint(['Suspend! [1]', 'Loading...']); jest.advanceTimersByTime(100); assertLog(['Promise resolved [1]']); - await waitForAll([1, 'Suspend! [2]', 1, 'Suspend! [2]', 'Suspend! [3]']); + await waitForAll([1, 'Suspend! [2]']); jest.advanceTimersByTime(100); - assertLog(['Promise resolved [2]', 'Promise resolved [3]']); + assertLog(['Promise resolved [2]']); + await waitForAll([1, 2, 'Suspend! [3]']); + + jest.advanceTimersByTime(100); + assertLog(['Promise resolved [3]']); await waitForAll([1, 2, 3]); await act(() => jest.advanceTimersByTime(100)); @@ -233,24 +253,17 @@ describe('ReactCache', () => { , ); - await waitForAll([1, 'Suspend! [4]', 'Loading...']); + await waitForAll([ + 1, + 'Suspend! [4]', + 'Loading...', + 1, + 'Suspend! [4]', + 'Suspend! [5]', + ]); await act(() => jest.advanceTimersByTime(100)); - assertLog([ - 'Promise resolved [4]', - - 1, - 4, - 'Suspend! [5]', - 1, - 4, - 'Suspend! [5]', - - 'Promise resolved [5]', - 1, - 4, - 5, - ]); + assertLog(['Promise resolved [4]', 'Promise resolved [5]', 1, 4, 5]); expect(root).toMatchRenderedOutput('145'); @@ -271,24 +284,14 @@ describe('ReactCache', () => { // 2 and 3 suspend because they were evicted from the cache 'Suspend! [2]', 'Loading...', + + 1, + 'Suspend! [2]', + 'Suspend! [3]', ]); await act(() => jest.advanceTimersByTime(100)); - assertLog([ - 'Promise resolved [2]', - - 1, - 2, - 'Suspend! [3]', - 1, - 2, - 'Suspend! [3]', - - 'Promise resolved [3]', - 1, - 2, - 3, - ]); + assertLog(['Promise resolved [2]', 'Promise resolved [3]', 1, 2, 3]); expect(root).toMatchRenderedOutput('123'); }); @@ -363,7 +366,12 @@ describe('ReactCache', () => { , ); - await waitForAll(['Suspend! [Hi]', 'Loading...']); + await waitForAll([ + 'Suspend! [Hi]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []), + ]); resolveThenable('Hi'); // This thenable improperly resolves twice. We should not update the diff --git a/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js b/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js index 6861e1aac3..4f31281cf2 100644 --- a/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js +++ b/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js @@ -15,6 +15,17 @@ import { normalizeCodeLocInfo, } from './utils'; +import {ReactVersion} from '../../../../ReactVersions'; +import semver from 'semver'; + +// TODO: This is how other DevTools tests access the version but we should find +// a better solution for this +const ReactVersionTestingAgainst = process.env.REACT_VERSION || ReactVersion; +const enableSiblingPrerendering = semver.gte( + ReactVersionTestingAgainst, + '19.0.0', +); + describe('Timeline profiler', () => { let React; let Scheduler; @@ -1651,7 +1662,11 @@ describe('Timeline profiler', () => { , ); - await waitForAll(['suspended']); + await waitForAll([ + 'suspended', + + ...(enableSiblingPrerendering ? ['suspended'] : []), + ]); Scheduler.unstable_advanceTime(10); resolveFn(); @@ -1662,9 +1677,38 @@ describe('Timeline profiler', () => { const timelineData = stopProfilingAndGetTimelineData(); // Verify the Suspense event and duration was recorded. - expect(timelineData.suspenseEvents).toHaveLength(1); - const suspenseEvent = timelineData.suspenseEvents[0]; - expect(suspenseEvent).toMatchInlineSnapshot(` + 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(` { "componentName": "Example", "depth": 0, @@ -1678,10 +1722,13 @@ 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(2); + expect(timelineData.componentMeasures).toHaveLength( + enableSiblingPrerendering ? 3 : 2, + ); }); it('should mark concurrent render with suspense that rejects', async () => { @@ -1708,7 +1755,11 @@ describe('Timeline profiler', () => { , ); - await waitForAll(['suspended']); + await waitForAll([ + 'suspended', + + ...(enableSiblingPrerendering ? ['suspended'] : []), + ]); Scheduler.unstable_advanceTime(10); rejectFn(); @@ -1719,9 +1770,39 @@ describe('Timeline profiler', () => { const timelineData = stopProfilingAndGetTimelineData(); // Verify the Suspense event and duration was recorded. - expect(timelineData.suspenseEvents).toHaveLength(1); - const suspenseEvent = timelineData.suspenseEvents[0]; - expect(suspenseEvent).toMatchInlineSnapshot(` + 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(` { "componentName": "Example", "depth": 0, @@ -1735,10 +1816,13 @@ 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(2); + expect(timelineData.componentMeasures).toHaveLength( + enableSiblingPrerendering ? 3 : 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 5a44cb6afe..168d44722d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMForm-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMForm-test.js @@ -1459,13 +1459,23 @@ describe('ReactDOMForm', () => { , ), ); - assertLog(['Suspend! [Count: 0]', 'Loading...']); + assertLog([ + 'Suspend! [Count: 0]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 0]'] : []), + ]); await act(() => resolveText('Count: 0')); assertLog(['Count: 0']); // Dispatch outside of a transition. This will trigger a loading state. await act(() => dispatch()); - assertLog(['Suspend! [Count: 1]', 'Loading...']); + assertLog([ + 'Suspend! [Count: 1]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 1]'] : []), + ]); expect(container.textContent).toBe('Loading...'); await act(() => resolveText('Count: 1')); diff --git a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js index 50ba64a478..fa22142702 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js @@ -160,7 +160,13 @@ describe('ReactDOMSuspensePlaceholder', () => { }); expect(container.textContent).toEqual('Loading...'); - assertLog(['A', 'Suspend! [B]', 'Loading...']); + assertLog([ + 'A', + 'Suspend! [B]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [B]', 'C'] : []), + ]); await act(() => { resolveText('B'); }); diff --git a/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js b/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js index 468d5b54e1..796d429625 100644 --- a/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js +++ b/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js @@ -192,7 +192,13 @@ test('regression (#20932): return pointer is correct before entering deleted tre await act(() => { root.render(); }); - assertLog(['Suspend! [0]', 'Loading Async...', 'Loading Tail...']); + assertLog([ + 'Suspend! [0]', + 'Loading Async...', + 'Loading Tail...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [0]'] : []), + ]); await act(() => { resolveText(0); }); @@ -205,5 +211,7 @@ test('regression (#20932): return pointer is correct before entering deleted tre 'Loading Async...', 'Suspend! [1]', 'Loading Async...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [1]'] : []), ]); }); diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index c5d0c1575b..e3fba900dd 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -155,6 +155,7 @@ import { getRenderTargetTime, getWorkInProgressTransitions, shouldRemainOnPreviousScreen, + markSpawnedRetryLane, } from './ReactFiberWorkLoop'; import { OffscreenLane, @@ -600,25 +601,28 @@ function scheduleRetryEffect( // Schedule an effect to attach a retry listener to the promise. // TODO: Move to passive phase workInProgress.flags |= Update; - } else { - // This boundary suspended, but no wakeables were added to the retry - // queue. Check if the renderer suspended commit. If so, this means - // that once the fallback is committed, we can immediately retry - // rendering again, because rendering wasn't actually blocked. Only - // the commit phase. - // TODO: Consider a model where we always schedule an immediate retry, even - // for normal Suspense. That way the retry can partially render up to the - // first thing that suspends. - if (workInProgress.flags & ScheduleRetry) { - const retryLane = - // TODO: This check should probably be moved into claimNextRetryLane - // I also suspect that we need some further consolidation of offscreen - // and retry lanes. - workInProgress.tag !== OffscreenComponent - ? claimNextRetryLane() - : OffscreenLane; - workInProgress.lanes = mergeLanes(workInProgress.lanes, retryLane); - } + } + + // Check if we need to schedule an immediate retry. This should happen + // whenever we unwind a suspended tree without fully rendering its siblings; + // we need to begin the retry so we can start prerendering them. + // + // We also use this mechanism for Suspensey Resources (e.g. stylesheets), + // because those don't actually block the render phase, only the commit phase. + // So we can start rendering even before the resources are ready. + if (workInProgress.flags & ScheduleRetry) { + const retryLane = + // TODO: This check should probably be moved into claimNextRetryLane + // I also suspect that we need some further consolidation of offscreen + // and retry lanes. + workInProgress.tag !== OffscreenComponent + ? claimNextRetryLane() + : OffscreenLane; + workInProgress.lanes = mergeLanes(workInProgress.lanes, retryLane); + + // Track the lanes that have been scheduled for an immediate retry so that + // we can mark them as suspended upon committing the root. + markSpawnedRetryLane(retryLane); } } diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index fe36ce548b..4338d3af58 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -26,9 +26,11 @@ import { syncLaneExpirationMs, transitionLaneExpirationMs, retryLaneExpirationMs, + disableLegacyMode, } from 'shared/ReactFeatureFlags'; import {isDevToolsPresent} from './ReactFiberDevToolsHook'; import {clz32} from './clz32'; +import {LegacyRoot} from './ReactRootTags'; // Lane values below should be kept in sync with getLabelForLane(), used by react-devtools-timeline. // If those values are changed that package should be rebuilt and redeployed. @@ -231,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; @@ -246,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); + } } } } @@ -268,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); + } } } } @@ -753,10 +782,14 @@ export function markRootPinged(root: FiberRoot, pingedLanes: Lanes) { export function markRootFinished( root: FiberRoot, + finishedLanes: Lanes, remainingLanes: Lanes, spawnedLane: Lane, + updatedLanes: Lanes, + suspendedRetryLanes: Lanes, ) { - const noLongerPendingLanes = root.pendingLanes & ~remainingLanes; + const previouslyPendingLanes = root.pendingLanes; + const noLongerPendingLanes = previouslyPendingLanes & ~remainingLanes; root.pendingLanes = remainingLanes; @@ -812,6 +845,37 @@ export function markRootFinished( NoLanes, ); } + + // suspendedRetryLanes represents the retry lanes spawned by new Suspense + // boundaries during this render that were not later pinged. + // + // These lanes were marked as pending on their associated Suspense boundary + // fiber during the render phase so that we could start rendering them + // before new data streams in. As soon as the fallback commits, we can try + // to render them again. + // + // But since we know they're still suspended, we can skip straight to the + // "prerender" mode (i.e. don't skip over siblings after something + // suspended) instead of the regular mode (i.e. unwind and skip the siblings + // as soon as something suspends to unblock the rest of the update). + if ( + suspendedRetryLanes !== NoLanes && + // Note that we only do this if there were no updates since we started + // rendering. This mirrors the logic in markRootUpdated — whenever we + // receive an update, we reset all the suspended and pinged lanes. + updatedLanes === NoLanes && + !(disableLegacyMode && root.tag === LegacyRoot) + ) { + // We also need to avoid marking a retry lane as suspended if it was already + // pending before this render. We can't say these are now suspended if they + // weren't included in our attempt. + const freshlySpawnedRetryLanes = + suspendedRetryLanes & + // Remove any retry lane that was already pending before our just-finished + // attempt, and also wasn't included in that attempt. + ~(previouslyPendingLanes & ~finishedLanes); + root.suspendedLanes |= freshlySpawnedRetryLanes; + } } function markSpawnedDeferredLane( diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 348ca757d2..8322fad7cd 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -128,6 +128,7 @@ import { DidDefer, ShouldSuspendCommit, MaySuspendCommit, + ScheduleRetry, } from './ReactFiberFlags'; import { NoLanes, @@ -365,8 +366,11 @@ let workInProgressRootInterleavedUpdatedLanes: Lanes = NoLanes; let workInProgressRootRenderPhaseUpdatedLanes: Lanes = NoLanes; // Lanes that were pinged (in an interleaved event) during this render. let workInProgressRootPingedLanes: Lanes = NoLanes; -// If this lane scheduled deferred work, this is the lane of the deferred task. +// If this render scheduled deferred work, this is the lane of the deferred task. let workInProgressDeferredLane: Lane = NoLane; +// Represents the retry lanes that were spawned by this render and have not +// been pinged since, implying that they are still suspended. +let workInProgressSuspendedRetryLanes: Lanes = NoLanes; // Errors that are thrown during the render phase. let workInProgressRootConcurrentErrors: Array> | null = null; @@ -993,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; @@ -1138,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( @@ -1146,6 +1154,8 @@ function finishConcurrentRender( workInProgressTransitions, workInProgressRootDidIncludeRecursiveRenderUpdate, workInProgressDeferredLane, + workInProgressRootInterleavedUpdatedLanes, + workInProgressSuspendedRetryLanes, ); } else { if ( @@ -1188,6 +1198,8 @@ function finishConcurrentRender( workInProgressRootDidIncludeRecursiveRenderUpdate, lanes, workInProgressDeferredLane, + workInProgressRootInterleavedUpdatedLanes, + workInProgressSuspendedRetryLanes, workInProgressRootDidSkipSuspendedSiblings, ), msUntilTimeout, @@ -1203,6 +1215,8 @@ function finishConcurrentRender( workInProgressRootDidIncludeRecursiveRenderUpdate, lanes, workInProgressDeferredLane, + workInProgressRootInterleavedUpdatedLanes, + workInProgressSuspendedRetryLanes, workInProgressRootDidSkipSuspendedSiblings, ); } @@ -1216,6 +1230,8 @@ function commitRootWhenReady( didIncludeRenderPhaseUpdate: boolean, lanes: Lanes, spawnedLane: Lane, + updatedLanes: Lanes, + suspendedRetryLanes: Lanes, didSkipSuspendedSiblings: boolean, ) { // TODO: Combine retry throttling with Suspensey commits. Right now they run @@ -1254,6 +1270,9 @@ function commitRootWhenReady( recoverableErrors, transitions, didIncludeRenderPhaseUpdate, + spawnedLane, + updatedLanes, + suspendedRetryLanes, ), ); markRootSuspended(root, lanes, spawnedLane, didSkipSuspendedSiblings); @@ -1261,13 +1280,15 @@ function commitRootWhenReady( } } - // Otherwise, commit immediately. + // Otherwise, commit immediately.; commitRoot( root, recoverableErrors, transitions, didIncludeRenderPhaseUpdate, spawnedLane, + updatedLanes, + suspendedRetryLanes, ); } @@ -1277,7 +1298,13 @@ function isRenderConsistentWithExternalStores(finishedWork: Fiber): boolean { // loop instead of recursion so we can exit early. let node: Fiber = finishedWork; while (true) { - if (node.flags & StoreConsistency) { + const tag = node.tag; + if ( + (tag === FunctionComponent || + tag === ForwardRef || + tag === SimpleMemoComponent) && + node.flags & StoreConsistency + ) { const updateQueue: FunctionComponentUpdateQueue | null = (node.updateQueue: any); if (updateQueue !== null) { @@ -1464,6 +1491,8 @@ export function performSyncWorkOnRoot(root: FiberRoot, lanes: Lanes): null { workInProgressTransitions, workInProgressRootDidIncludeRecursiveRenderUpdate, workInProgressDeferredLane, + workInProgressRootInterleavedUpdatedLanes, + workInProgressSuspendedRetryLanes, ); // Before exiting, make sure there's a callback scheduled for the next @@ -1691,6 +1720,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { workInProgressRootRenderPhaseUpdatedLanes = NoLanes; workInProgressRootPingedLanes = NoLanes; workInProgressDeferredLane = NoLane; + workInProgressSuspendedRetryLanes = NoLanes; workInProgressRootConcurrentErrors = null; workInProgressRootRecoverableErrors = null; workInProgressRootDidIncludeRecursiveRenderUpdate = false; @@ -2104,9 +2134,10 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) { } default: { // Unwind then continue with the normal work loop. + const reason = workInProgressSuspendedReason; workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); + throwAndUnwindWorkLoop(root, unitOfWork, thrownValue, reason); break; } } @@ -2199,6 +2230,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__) { @@ -2226,7 +2265,12 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); + throwAndUnwindWorkLoop( + root, + unitOfWork, + thrownValue, + SuspendedOnError, + ); break; } case SuspendedOnData: { @@ -2284,7 +2328,12 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Otherwise, unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); + throwAndUnwindWorkLoop( + root, + unitOfWork, + thrownValue, + SuspendedAndReadyToContinue, + ); } break; } @@ -2347,7 +2396,12 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Otherwise, unwind then continue with the normal work loop. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); + throwAndUnwindWorkLoop( + root, + unitOfWork, + thrownValue, + SuspendedOnInstanceAndReadyToContinue, + ); break; } case SuspendedOnDeprecatedThrowPromise: { @@ -2357,7 +2411,12 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // always unwind. workInProgressSuspendedReason = NotSuspended; workInProgressThrownValue = null; - throwAndUnwindWorkLoop(root, unitOfWork, thrownValue); + throwAndUnwindWorkLoop( + root, + unitOfWork, + thrownValue, + SuspendedOnDeprecatedThrowPromise, + ); break; } case SuspendedOnHydration: { @@ -2611,6 +2670,7 @@ function throwAndUnwindWorkLoop( root: FiberRoot, unitOfWork: Fiber, thrownValue: mixed, + suspendedReason: SuspendedReason, ) { // This is a fork of performUnitOfWork specifcally for unwinding a fiber // that threw an exception. @@ -2658,16 +2718,43 @@ function throwAndUnwindWorkLoop( // The current algorithm for both hydration and error handling assumes // that the tree is rendered sequentially. So we always skip the siblings. getIsHydrating() || - workInProgressSuspendedReason === SuspendedOnError + suspendedReason === SuspendedOnError ) { skipSiblings = true; // We intentionally don't set workInProgressRootDidSkipSuspendedSiblings, // because we don't want to trigger another prerender attempt. - } else if (!workInProgressRootIsPrerendering) { + } else if ( + // Check whether this is a prerender + !workInProgressRootIsPrerendering && + // Offscreen rendering is also a form of speculative rendering + !includesSomeLane(workInProgressRootRenderLanes, OffscreenLane) + ) { // This is not a prerender. Skip the siblings during this render. A // separate prerender will be scheduled for later. skipSiblings = true; 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; @@ -2688,6 +2775,16 @@ function throwAndUnwindWorkLoop( } } +export function markSpawnedRetryLane(lane: Lane): void { + // Keep track of the retry lanes that were spawned by a fallback during the + // current render and were not later pinged. This will represent the lanes + // that are known to still be suspended. + workInProgressSuspendedRetryLanes = mergeLanes( + workInProgressSuspendedRetryLanes, + lane, + ); +} + function panicOnRootError(root: FiberRoot, error: mixed) { // There's no ancestor that can handle this exception. This should never // happen because the root is supposed to capture all errors that weren't @@ -2866,6 +2963,8 @@ function commitRoot( transitions: Array | null, didIncludeRenderPhaseUpdate: boolean, spawnedLane: Lane, + updatedLanes: Lanes, + suspendedRetryLanes: Lanes, ) { // TODO: This no longer makes any sense. We already wrap the mutation and // layout phases. Should be able to remove. @@ -2881,6 +2980,8 @@ function commitRoot( didIncludeRenderPhaseUpdate, previousUpdateLanePriority, spawnedLane, + updatedLanes, + suspendedRetryLanes, ); } finally { ReactSharedInternals.T = prevTransition; @@ -2897,6 +2998,8 @@ function commitRootImpl( didIncludeRenderPhaseUpdate: boolean, renderPriorityLevel: EventPriority, spawnedLane: Lane, + updatedLanes: Lanes, + suspendedRetryLanes: Lanes, ) { do { // `flushPassiveEffects` will call `flushSyncUpdateQueue` at the end, which @@ -2973,7 +3076,14 @@ function commitRootImpl( const concurrentlyUpdatedLanes = getConcurrentlyUpdatedLanes(); remainingLanes = mergeLanes(remainingLanes, concurrentlyUpdatedLanes); - markRootFinished(root, remainingLanes, spawnedLane); + markRootFinished( + root, + lanes, + remainingLanes, + spawnedLane, + updatedLanes, + suspendedRetryLanes, + ); // Reset this before firing side effects so we can detect recursive updates. didIncludeCommitPhaseUpdate = false; @@ -3679,6 +3789,17 @@ function pingSuspendedRoot( pingedLanes, ); } + + // If something pings the work-in-progress render, any work that suspended + // up to this point may now be unblocked; in other words, no + // longer suspended. + // + // Unlike the broader check above, we only need do this if the lanes match + // exactly. If the lanes don't exactly match, that implies the promise + // was created by an older render. + if (workInProgressSuspendedRetryLanes === workInProgressRootRenderLanes) { + workInProgressSuspendedRetryLanes = NoLanes; + } } ensureRootIsScheduled(root); diff --git a/packages/react-reconciler/src/__tests__/ActivityStrictMode-test.js b/packages/react-reconciler/src/__tests__/ActivityStrictMode-test.js index a8b8d286dc..85365d3e70 100644 --- a/packages/react-reconciler/src/__tests__/ActivityStrictMode-test.js +++ b/packages/react-reconciler/src/__tests__/ActivityStrictMode-test.js @@ -240,6 +240,11 @@ describe('Activity StrictMode', () => { 'Parent mount', 'Parent unmount', 'Parent mount', + + ...(gate('enableSiblingPrerendering') + ? ['Child rendered', 'Child suspended'] + : []), + '------------------------------', 'Child rendered', 'Child rendered', diff --git a/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js b/packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js index eb6fe1687c..ba0451c4c5 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__/ReactActWarnings-test.js b/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js index 03e04e8b1d..561741c108 100644 --- a/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js +++ b/packages/react-reconciler/src/__tests__/ReactActWarnings-test.js @@ -313,13 +313,23 @@ describe('act warnings', () => { act(() => { root.render(); }); - assertLog(['Suspend! [Async]', 'Loading...']); + assertLog([ + 'Suspend! [Async]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Async]'] : []), + ]); expect(root).toMatchRenderedOutput('Loading...'); // This is a retry, not a ping, because we already showed a fallback. expect(() => resolveText('Async')).toErrorDev( - 'A suspended resource finished loading inside a test, but the event ' + - 'was not wrapped in act(...)', + [ + 'A suspended resource finished loading inside a test, but the event ' + + 'was not wrapped in act(...)', + + ...(gate('enableSiblingPrerendering') ? ['not wrapped in act'] : []), + ], + {withoutStack: true}, ); }); diff --git a/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js b/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js index 7f68dcc1b6..b11e78bdec 100644 --- a/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js @@ -109,7 +109,13 @@ describe('ReactBlockingMode', () => { , ); - await waitForAll(['A', 'Suspend! [B]', 'Loading...']); + await waitForAll([ + 'A', + 'Suspend! [B]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [B]', 'C'] : []), + ]); // In Legacy Mode, A and B would mount in a hidden primary tree. In // Concurrent Mode, nothing in the primary tree should mount. But the // fallback should mount immediately. diff --git a/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js index db9d7c336e..4a00607ffb 100644 --- a/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js +++ b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js @@ -292,16 +292,7 @@ describe('ReactConcurrentErrorRecovery', () => { // Because we're still suspended on B, we can't show an error boundary. We // should wait for B to resolve. - assertLog([ - 'Error! [A2]', - 'Oops!', - 'Suspend! [B2]', - 'Loading...', - - ...(gate('enableSiblingPrerendering') - ? ['Error! [A2]', 'Oops!', 'Suspend! [B2]', 'Loading...'] - : []), - ]); + assertLog(['Error! [A2]', 'Oops!', 'Suspend! [B2]', 'Loading...']); // Remain on previous screen. expect(root).toMatchRenderedOutput('A1B1'); diff --git a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js index f6ebd3c574..340e973b2f 100644 --- a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js +++ b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js @@ -399,7 +399,13 @@ describe('ReactLazyContextPropagation', () => { // the fallback displays despite this being a refresh. setContext('B'); }); - assertLog(['Suspend! [B]', 'Loading...', 'B']); + assertLog([ + 'Suspend! [B]', + 'Loading...', + 'B', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), + ]); expect(root).toMatchRenderedOutput('Loading...B'); await act(async () => { @@ -479,7 +485,13 @@ describe('ReactLazyContextPropagation', () => { // the fallback displays despite this being a refresh. setContext('B'); }); - assertLog(['Suspend! [B]', 'Loading...', 'B']); + assertLog([ + 'Suspend! [B]', + 'Loading...', + 'B', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), + ]); expect(root).toMatchRenderedOutput('Loading...B'); await act(async () => { @@ -812,7 +824,12 @@ describe('ReactLazyContextPropagation', () => { await act(() => { setContext('B'); }); - assertLog(['Suspend! [B]', 'Loading...']); + assertLog([ + 'Suspend! [B]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [B]'] : []), + ]); expect(root).toMatchRenderedOutput('Loading...'); await act(async () => { diff --git a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js index b2c38696cc..fd03ba7310 100644 --- a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js +++ b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js @@ -499,6 +499,8 @@ describe('ReactDeferredValue', () => { // The initial value suspended, so we attempt the final value, which // also suspends. 'Suspend! [Final]', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Final]'] : []), ]); expect(root).toMatchRenderedOutput('Fallback'); @@ -630,6 +632,8 @@ describe('ReactDeferredValue', () => { // go straight to attempting the final value. 'Suspend! [Content]', 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [Content]'] : []), ]); // The content suspended, so we show a Suspense fallback expect(root).toMatchRenderedOutput('Loading...'); diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 9b714458c7..556e055972 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -1864,13 +1864,15 @@ describe('ReactHooks', () => { it('does not fire a false positive warning when suspending memo', async () => { const {Suspense, useState} = React; - let wasSuspended = false; + let isSuspended = true; let resolve; function trySuspend() { - if (!wasSuspended) { - throw new Promise(r => { - wasSuspended = true; - resolve = r; + if (isSuspended) { + throw new Promise(res => { + resolve = () => { + isSuspended = false; + res(); + }; }); } } @@ -1900,13 +1902,15 @@ describe('ReactHooks', () => { it('does not fire a false positive warning when suspending forwardRef', async () => { const {Suspense, useState} = React; - let wasSuspended = false; + let isSuspended = true; let resolve; function trySuspend() { - if (!wasSuspended) { - throw new Promise(r => { - wasSuspended = true; - resolve = r; + if (isSuspended) { + throw new Promise(res => { + resolve = () => { + isSuspended = false; + res(); + }; }); } } @@ -1936,13 +1940,15 @@ describe('ReactHooks', () => { it('does not fire a false positive warning when suspending memo(forwardRef)', async () => { const {Suspense, useState} = React; - let wasSuspended = false; + let isSuspended = true; let resolve; function trySuspend() { - if (!wasSuspended) { - throw new Promise(r => { - wasSuspended = true; - resolve = r; + if (isSuspended) { + throw new Promise(res => { + resolve = () => { + isSuspended = false; + res(); + }; }); } } diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index e62852f2a3..0fc64b39da 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3544,7 +3544,13 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); }); - assertLog(['A', 'Suspend! [A]', 'Loading']); + assertLog([ + 'A', + 'Suspend! [A]', + 'Loading', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [A]'] : []), + ]); expect(ReactNoop).toMatchRenderedOutput( <> @@ -4201,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 891cf91049..8526fb5b19 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)); @@ -239,13 +235,6 @@ describe('ReactLazy', () => { assertConsoleErrorDev([ 'Expected the result of a dynamic import() call', 'Expected the result of a dynamic import() call', - - ...(gate('enableSiblingPrerendering') - ? [ - 'Expected the result of a dynamic import() call', - 'Expected the result of a dynamic import() call', - ] - : []), ]); expect(root).not.toMatchRenderedOutput('Hi'); }); @@ -320,7 +309,12 @@ describe('ReactLazy', () => { unstable_isConcurrent: true, }); - await waitForAll(['Suspend! [LazyChildA]', 'Loading...']); + await waitForAll([ + 'Suspend! [LazyChildA]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [LazyChildB]'] : []), + ]); expect(root).not.toMatchRenderedOutput('AB'); await act(async () => { @@ -329,9 +323,23 @@ describe('ReactLazy', () => { // B suspends even though it happens to share the same import as A. // TODO: React.lazy should implement the `status` and `value` fields, so // we can unwrap the result synchronously if it already loaded. Like `use`. - await waitFor(['A', 'Suspend! [LazyChildB]']); + await waitFor([ + 'A', + + // When enableSiblingPrerendering is on, LazyChildB was already + // initialized. So it also already resolved when we called + // resolveFakeImport above. So it doesn't suspend again. + ...(gate('enableSiblingPrerendering') + ? ['B'] + : ['Suspend! [LazyChildB]']), + ]); }); - assertLog(['A', 'B', 'Did mount: A', 'Did mount: B']); + assertLog([ + ...(gate('enableSiblingPrerendering') ? [] : ['A', 'B']), + + 'Did mount: A', + 'Did mount: B', + ]); expect(root).toMatchRenderedOutput('AB'); // Swap the position of A and B @@ -1395,15 +1403,20 @@ describe('ReactLazy', () => { unstable_isConcurrent: true, }); - await waitForAll(['Init A', 'Loading...']); + await waitForAll([ + 'Init A', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Init B'] : []), + ]); expect(root).not.toMatchRenderedOutput('AB'); await act(() => resolveFakeImport(ChildA)); assertLog([ 'A', - 'Init B', - ...(gate('enableSiblingPrerendering') ? ['A'] : []), + // When enableSiblingPrerendering is on, B was already initialized. + ...(gate('enableSiblingPrerendering') ? ['A'] : ['Init B']), ]); await act(() => resolveFakeImport(ChildB)); diff --git a/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js new file mode 100644 index 0000000000..6c24fdc980 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactSiblingPrerendering-test.js @@ -0,0 +1,472 @@ +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 76b4ead3c4..d7ad652f1d 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -169,13 +169,23 @@ describe('ReactSuspense', () => { 'Loading A...', 'Suspend! [B]', 'Loading B...', + + ...(gate('enableSiblingPrerendering') + ? ['Suspend! [A]', 'Suspend! [B]'] + : []), ]); expect(container.innerHTML).toEqual('Loading A...Loading B...'); // Resolve first Suspense's promise and switch back to the normal view. The // second Suspense should still show the placeholder await act(() => resolveText('A')); - assertLog(['A']); + assertLog([ + 'A', + + ...(gate('enableSiblingPrerendering') + ? ['Suspend! [B]', 'Suspend! [B]'] + : []), + ]); expect(container.textContent).toEqual('ALoading B...'); // Resolve the second Suspense's promise resolves and switche back to the @@ -274,19 +284,19 @@ describe('ReactSuspense', () => { root.render(); }); - assertLog(['Foo', 'Suspend! [A]', 'Loading...']); + assertLog([ + 'Foo', + 'Suspend! [A]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') + ? ['Suspend! [A]', 'Suspend! [B]', 'Loading more...'] + : []), + ]); expect(container.textContent).toEqual('Loading...'); await resolveText('A'); - 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 @@ -327,7 +337,15 @@ describe('ReactSuspense', () => { // Render an empty shell const root = ReactDOMClient.createRoot(container); root.render(); - await waitForAll(['Foo', 'Suspend! [A]', 'Loading...']); + await waitForAll([ + 'Foo', + 'Suspend! [A]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') + ? ['Suspend! [A]', 'Suspend! [B]', 'Loading more...'] + : []), + ]); expect(container.textContent).toEqual('Loading...'); // Now resolve A @@ -338,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. @@ -375,19 +386,19 @@ describe('ReactSuspense', () => { await act(() => { root.render(); }); - assertLog(['Foo', 'Suspend! [A]', 'Loading...']); + assertLog([ + 'Foo', + 'Suspend! [A]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') + ? ['Suspend! [A]', 'Suspend! [B]', 'Loading more...'] + : []), + ]); expect(container.textContent).toEqual('Loading...'); await resolveText('A'); - 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 @@ -468,14 +479,24 @@ describe('ReactSuspense', () => { await act(() => { root.render(); }); - assertLog(['Suspend! [default]', 'Loading...']); + assertLog([ + 'Suspend! [default]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), + ]); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog(['Suspend! [new value]', 'Loading...']); + assertLog([ + 'Suspend! [new value]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), + ]); await act(() => resolveText('new value')); assertLog(['new value']); @@ -515,14 +536,24 @@ describe('ReactSuspense', () => { await act(() => { root.render(); }); - assertLog(['Suspend! [default]', 'Loading...']); + assertLog([ + 'Suspend! [default]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), + ]); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog(['Suspend! [new value]', 'Loading...']); + assertLog([ + 'Suspend! [new value]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), + ]); await act(() => resolveText('new value')); assertLog(['new value']); @@ -559,14 +590,24 @@ describe('ReactSuspense', () => { , ); }); - assertLog(['Suspend! [default]', 'Loading...']); + assertLog([ + 'Suspend! [default]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), + ]); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog(['Suspend! [new value]', 'Loading...']); + assertLog([ + 'Suspend! [new value]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), + ]); await act(() => resolveText('new value')); assertLog(['new value']); @@ -603,14 +644,24 @@ describe('ReactSuspense', () => { , ); }); - assertLog(['Suspend! [default]', 'Loading...']); + assertLog([ + 'Suspend! [default]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [default]'] : []), + ]); await act(() => resolveText('default')); assertLog(['default']); expect(container.textContent).toEqual('default'); await act(() => setValue('new value')); - assertLog(['Suspend! [new value]', 'Loading...']); + assertLog([ + 'Suspend! [new value]', + 'Loading...', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [new value]'] : []), + ]); await act(() => resolveText('new value')); assertLog(['new value']); @@ -657,6 +708,10 @@ describe('ReactSuspense', () => { 'Suspend! [Child 2]', 'Loading...', 'destroy layout', + + ...(gate('enableSiblingPrerendering') + ? ['Child 1', 'Suspend! [Child 2]'] + : []), ]); await act(() => resolveText('Child 2')); @@ -679,16 +734,16 @@ describe('ReactSuspense', () => { root.render(); }); - assertLog(['Suspend! [Child 1]', 'Loading...']); - await resolveText('Child 1'); - await waitForAll([ - 'Child 1', - 'Suspend! [Child 2]', + assertLog([ + 'Suspend! [Child 1]', + 'Loading...', ...(gate('enableSiblingPrerendering') - ? ['Child 1', 'Suspend! [Child 2]'] + ? ['Suspend! [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 7f3724fa86..6da6ef7573 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 688ff38a74..4a094dc7fe 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js @@ -274,6 +274,14 @@ describe('ReactSuspenseEffectsSemantics', () => { 'Text:Fallback create passive', 'Text:Outside create passive', 'App create passive', + + ...(gate('enableSiblingPrerendering') + ? [ + 'Text:Inside:Before render', + 'Suspend:Async', + 'ClassText:Inside:After render', + ] + : []), ]); expect(ReactNoop).toMatchRenderedOutput( <> @@ -646,7 +654,17 @@ describe('ReactSuspenseEffectsSemantics', () => { 'Text:Inside:After destroy layout', 'Text:Fallback create layout', ]); - await waitForAll(['Text:Fallback create passive']); + await waitForAll([ + 'Text:Fallback create passive', + + ...(gate('enableSiblingPrerendering') + ? [ + 'Text:Inside:Before render', + 'Suspend:Async', + 'Text:Inside:After render', + ] + : []), + ]); expect(ReactNoop).toMatchRenderedOutput( <>