diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 86c95a3978222..fd0350d388eb3 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -187,6 +187,7 @@ import { renderDidSuspendDelayIfPossible, markUnprocessedUpdateTime, getWorkInProgressRoot, + pushRenderExpirationTime, } from './ReactFiberWorkLoop.new'; import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev'; @@ -569,17 +570,30 @@ function updateOffscreenComponent( const nextProps: OffscreenProps = workInProgress.pendingProps; const nextChildren = nextProps.children; + let subtreeRenderTime = renderExpirationTime; if (current !== null) { if (nextProps.mode === 'hidden') { // TODO: Should currently be unreachable because Offscreen is only used as // an implementation detail of Suspense. Once this is a public API, it // will need to create an OffscreenState. } else { - // Clear the offscreen state. - workInProgress.memoizedState = null; + const prevState: OffscreenState | null = current.memoizedState; + if (prevState !== null) { + const baseTime = prevState.baseTime; + subtreeRenderTime = !isSameOrHigherPriority( + baseTime, + renderExpirationTime, + ) + ? baseTime + : renderExpirationTime; + + // Since we're not hidden anymore, reset the state + workInProgress.memoizedState = null; + } } } + pushRenderExpirationTime(workInProgress, subtreeRenderTime); reconcileChildren( current, workInProgress, @@ -1651,33 +1665,32 @@ function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) { } } -function mountSuspenseState( +const SUSPENDED_MARKER: SuspenseState = { + dehydrated: null, + retryTime: NoWork, +}; + +function mountSuspenseOffscreenState( renderExpirationTime: ExpirationTimeOpaque, -): SuspenseState { +): OffscreenState { return { - dehydrated: null, baseTime: renderExpirationTime, - retryTime: NoWork, }; } -function updateSuspenseState( - prevSuspenseState: SuspenseState, +function updateSuspenseOffscreenState( + prevOffscreenState: OffscreenState, renderExpirationTime: ExpirationTimeOpaque, -): SuspenseState { - const prevSuspendedTime = prevSuspenseState.baseTime; +): OffscreenState { + const prevBaseTime = prevOffscreenState.baseTime; return { - dehydrated: null, + // Choose whichever time is inclusive of the other one. This represents + // the union of all the levels that suspended. baseTime: - // Choose whichever time is inclusive of the other one. This represents - // the union of all the levels that suspended. - !isSameExpirationTime( - prevSuspendedTime, - (NoWork: ExpirationTimeOpaque), - ) && !isSameOrHigherPriority(prevSuspendedTime, renderExpirationTime) - ? prevSuspendedTime + !isSameExpirationTime(prevBaseTime, (NoWork: ExpirationTimeOpaque)) && + !isSameOrHigherPriority(prevBaseTime, renderExpirationTime) + ? prevBaseTime : renderExpirationTime, - retryTime: NoWork, }; } @@ -1692,19 +1705,7 @@ function shouldRemainOnFallback( // For example, SuspenseList coordinates when nested content appears. if (current !== null) { const suspenseState: SuspenseState = current.memoizedState; - if (suspenseState !== null) { - // Currently showing a fallback. If the current render includes - // the level that triggered the fallback, we must continue showing it, - // regardless of what the Suspense context says. - const baseTime = suspenseState.baseTime; - if ( - !isSameExpirationTime(baseTime, (NoWork: ExpirationTimeOpaque)) && - !isSameOrHigherPriority(baseTime, renderExpirationTime) - ) { - return true; - } - // Otherwise, fall through to check the Suspense context. - } else { + if (suspenseState === null) { // Currently showing content. Don't hide it, even if ForceSuspenseFallack // is true. More precise name might be "ForceRemainSuspenseFallback". // Note: This is a factoring smell. Can't remain on a fallback if there's @@ -1712,6 +1713,7 @@ function shouldRemainOnFallback( return false; } } + // Not currently showing content. Consult the Suspense context. return hasSuspenseContext( suspenseContext, @@ -1725,20 +1727,6 @@ function getRemainingWorkInPrimaryTree( renderExpirationTime, ) { const currentChildExpirationTime = current.childExpirationTime_opaque; - const currentSuspenseState: SuspenseState = current.memoizedState; - if (currentSuspenseState !== null) { - // This boundary already timed out. Check if this render includes the level - // that previously suspended. - const baseTime = currentSuspenseState.baseTime; - if ( - !isSameExpirationTime(baseTime, (NoWork: ExpirationTimeOpaque)) && - !isSameOrHigherPriority(baseTime, renderExpirationTime) - ) { - // There's pending work at a lower level that might now be unblocked. - return baseTime; - } - } - if ( !isSameOrHigherPriority(currentChildExpirationTime, renderExpirationTime) ) { @@ -1880,8 +1868,10 @@ function updateSuspenseComponent( renderExpirationTime, ); const primaryChildFragment: Fiber = (workInProgress.child: any); - primaryChildFragment.memoizedState = ({baseTime: NoWork}: OffscreenState); - workInProgress.memoizedState = mountSuspenseState(renderExpirationTime); + primaryChildFragment.memoizedState = mountSuspenseOffscreenState( + renderExpirationTime, + ); + workInProgress.memoizedState = SUSPENDED_MARKER; return fallbackFragment; } else { const nextPrimaryChildren = nextProps.children; @@ -1935,14 +1925,10 @@ function updateSuspenseComponent( renderExpirationTime, ); const primaryChildFragment: Fiber = (workInProgress.child: any); - primaryChildFragment.memoizedState = ({ - baseTime: NoWork, - }: OffscreenState); - workInProgress.memoizedState = updateSuspenseState( - current.memoizedState, + primaryChildFragment.memoizedState = mountSuspenseOffscreenState( renderExpirationTime, ); - + workInProgress.memoizedState = SUSPENDED_MARKER; return fallbackChildFragment; } } @@ -1959,18 +1945,21 @@ function updateSuspenseComponent( renderExpirationTime, ); const primaryChildFragment: Fiber = (workInProgress.child: any); - primaryChildFragment.memoizedState = ({ - baseTime: NoWork, - }: OffscreenState); + const prevOffscreenState: OffscreenState | null = (current.child: any) + .memoizedState; + primaryChildFragment.memoizedState = + prevOffscreenState === null + ? mountSuspenseOffscreenState(renderExpirationTime) + : updateSuspenseOffscreenState( + prevOffscreenState, + renderExpirationTime, + ); primaryChildFragment.childExpirationTime_opaque = getRemainingWorkInPrimaryTree( current, workInProgress, renderExpirationTime, ); - workInProgress.memoizedState = updateSuspenseState( - current.memoizedState, - renderExpirationTime, - ); + workInProgress.memoizedState = SUSPENDED_MARKER; return fallbackChildFragment; } else { const nextPrimaryChildren = nextProps.children; @@ -1997,9 +1986,15 @@ function updateSuspenseComponent( renderExpirationTime, ); const primaryChildFragment: Fiber = (workInProgress.child: any); - primaryChildFragment.memoizedState = ({ - baseTime: NoWork, - }: OffscreenState); + const prevOffscreenState: OffscreenState | null = (current.child: any) + .memoizedState; + primaryChildFragment.memoizedState = + prevOffscreenState === null + ? mountSuspenseOffscreenState(renderExpirationTime) + : updateSuspenseOffscreenState( + prevOffscreenState, + renderExpirationTime, + ); primaryChildFragment.childExpirationTime_opaque = getRemainingWorkInPrimaryTree( current, workInProgress, @@ -2007,7 +2002,7 @@ function updateSuspenseComponent( ); // Skip the primary children, and continue working on the // fallback children. - workInProgress.memoizedState = mountSuspenseState(renderExpirationTime); + workInProgress.memoizedState = SUSPENDED_MARKER; return fallbackChildFragment; } else { // Still haven't timed out. Continue rendering the children, like we @@ -3390,6 +3385,10 @@ function beginWork( return null; } } + case OffscreenComponent: { + pushRenderExpirationTime(workInProgress, renderExpirationTime); + break; + } } return bailoutOnAlreadyFinishedWork( current, diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index 2299e61f6e580..4419c5c05f507 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -125,6 +125,7 @@ import { renderDidSuspend, renderDidSuspendDelayIfPossible, renderHasNotSuspendedYet, + popRenderExpirationTime, } from './ReactFiberWorkLoop.new'; import {createFundamentalStateInstance} from './ReactFiberFundamental.new'; import { @@ -1291,6 +1292,7 @@ function completeWork( } break; case OffscreenComponent: { + popRenderExpirationTime(workInProgress); if (current !== null) { const nextState: OffscreenState | null = workInProgress.memoizedState; const prevState: OffscreenState | null = current.memoizedState; diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index 88d7511cef196..f1ed2e73cccdb 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -55,7 +55,7 @@ import { didNotFindHydratableSuspenseInstance, } from './ReactFiberHostConfig'; import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags'; -import {Never, NoWork} from './ReactFiberExpirationTime.new'; +import {Never} from './ReactFiberExpirationTime.new'; // The deepest Fiber on the stack involved in a hydration context. // This may have been an insertion or a hydration. @@ -231,7 +231,6 @@ function tryHydrate(fiber, nextInstance) { if (suspenseInstance !== null) { const suspenseState: SuspenseState = { dehydrated: suspenseInstance, - baseTime: NoWork, retryTime: Never, }; fiber.memoizedState = suspenseState; diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js index 26912af8f11a1..a45f1710138fb 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js @@ -29,10 +29,6 @@ export type SuspenseState = {| // here to indicate that it is dehydrated (flag) and for quick access // to check things like isSuspenseInstancePending. dehydrated: null | SuspenseInstance, - // Represents the work that was deprioritized when we committed the fallback. - // The work outside the boundary already committed at this level, so we cannot - // unhide the content without including it. - baseTime: ExpirationTimeOpaque, // Represents the earliest expiration time we should attempt to hydrate // a dehydrated boundary at. // Never is the default for dehydrated boundaries. diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js index d7d5585b5cc41..ae243c04115a0 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js @@ -20,6 +20,7 @@ import { ContextProvider, SuspenseComponent, SuspenseListComponent, + OffscreenComponent, } from './ReactWorkTags'; import {DidCapture, NoEffect, ShouldCapture} from './ReactSideEffectTags'; import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags'; @@ -33,6 +34,7 @@ import { popTopLevelContextObject as popTopLevelLegacyContextObject, } from './ReactFiberContext.new'; import {popProvider} from './ReactFiberNewContext.new'; +import {popRenderExpirationTime} from './ReactFiberWorkLoop.new'; import invariant from 'shared/invariant'; @@ -105,6 +107,9 @@ function unwindWork( case ContextProvider: popProvider(workInProgress); return null; + case OffscreenComponent: + popRenderExpirationTime(workInProgress); + return null; default: return null; } @@ -141,6 +146,9 @@ function unwindInterruptedWork(interruptedWork: Fiber) { case ContextProvider: popProvider(interruptedWork); break; + case OffscreenComponent: + popRenderExpirationTime(interruptedWork); + break; default: break; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 5d8b99cbc9a90..fed70d4c1a76f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -15,6 +15,7 @@ import type {Interaction} from 'scheduler/src/Tracing'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; import type {SuspenseState} from './ReactFiberSuspenseComponent.new'; import type {Effect as HookEffect} from './ReactFiberHooks.new'; +import type {StackCursor} from './ReactFiberStack.new'; import { warnAboutDeprecatedLifecycles, @@ -168,6 +169,11 @@ import { getIsUpdatingOpaqueValueInRenderPhaseInDEV, } from './ReactFiberHooks.new'; import {createCapturedValue} from './ReactCapturedValue'; +import { + push as pushToStack, + pop as popFromStack, + createCursor, +} from './ReactFiberStack.new'; import { recordCommitTime, @@ -1262,6 +1268,31 @@ export function flushControlled(fn: () => mixed): void { } } +let prevRenderExpirationTime: ExpirationTimeOpaque = NoWork; +let renderExpirationTimeCursorInDEV: StackCursor | null = null; +if (__DEV__) { + // This is only used in DEV to catch push/pop misalignment bugs. + renderExpirationTimeCursorInDEV = createCursor(null); +} + +export function pushRenderExpirationTime( + fiber: Fiber, + subtreeRenderTime: ExpirationTimeOpaque, +) { + if (__DEV__ && renderExpirationTimeCursorInDEV !== null) { + pushToStack(renderExpirationTimeCursorInDEV, null, fiber); + } + prevRenderExpirationTime = renderExpirationTime; + renderExpirationTime = subtreeRenderTime; +} + +export function popRenderExpirationTime(fiber: Fiber) { + if (__DEV__ && renderExpirationTimeCursorInDEV !== null) { + popFromStack(renderExpirationTimeCursorInDEV, fiber); + } + renderExpirationTime = prevRenderExpirationTime; +} + function prepareFreshStack(root, expirationTime) { root.finishedWork = null; root.finishedExpirationTime_opaque = NoWork; diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index f6e7d69382858..bf1f1a696ea2f 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -3180,18 +3180,49 @@ describe('ReactSuspenseWithNoopRenderer', () => { }); setFallbackText('Still loading...'); - expect(Scheduler).toFlushAndYield([ - // First try to render the high pri update. We won't try to re-render - // the suspended tree during this pass, because it still has unfinished - // updates at a lower priority. - 'Loading...', - - // Now try the suspended update again. It's still suspended. - 'Suspend! [C]', - - // Then complete the update to the fallback. - 'Still loading...', - ]); + expect(Scheduler).toFlushAndYield( + gate(flags => + flags.new + ? [ + // First try to render the high pri update. Still suspended. + 'Suspend! [C]', + 'Loading...', + + // In the expiration times model, once the high pri update + // suspends, we can't be sure if there's additional work at a + // lower priority that might unblock the tree. We do know that + // there's a lower priority update *somehwere* in the entire + // root, though (the update to the fallback). So we try + // rendering one more time, just in case. + // TODO: We shouldn't need to do this with lanes, because we + // always know exactly which lanes have pending work in + // each tree. + 'Suspend! [C]', + + // Then complete the update to the fallback. + 'Still loading...', + ] + : [ + // In the old reconciler, we don't attempt to unhdie the + // Suspense boundary at high priority. Instead, we bailout, + // then try again at the original priority that the component + // suspended. This is mostly an implementation compromise, + // though there are some advantages to this behavior, because + // attempt to unhide could slow down the rest of the update. + // + // Render that only includes the fallback, since we bailed + // out on the primary tree. + 'Loading...', + + // Now try the suspended update again at the original + // priority. It's still suspended. + 'Suspend! [C]', + + // Then complete the update to the fallback. + 'Still loading...', + ], + ), + ); expect(root).toMatchRenderedOutput( <>