From db281b3d9cd033cdc3d63e00fc9f3153c03aa70c Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 17 Mar 2023 18:05:11 -0400 Subject: [PATCH] Feature: Suspend commit without blocking render (#26398) This adds a new capability for renderers (React DOM, React Native): prevent a tree from being displayed until it is ready, showing a fallback if necessary, but without blocking the React components from being evaluated in the meantime. A concrete example is CSS loading: React DOM can block a commit from being applied until the stylesheet has loaded. This allows us to load the CSS asynchronously, while also preventing a flash of unstyled content. Images and fonts are some of the other use cases. You can think of this as "Suspense for the commit phase". Traditional Suspense, i.e. with `use`, blocking during the render phase: React cannot proceed with rendering until the data is available. But in the case of things like stylesheets, you don't need the CSS in order to evaluate the component. It just needs to be loaded before the tree is committed. Because React buffers its side effects and mutations, it can do work in parallel while the stylesheets load in the background. Like regular Suspense, a "suspensey" stylesheet or image will trigger the nearest Suspense fallback if it hasn't loaded yet. For now, though, we only do this for non-urgent updates, like with startTransition. If you render a suspensey resource during an urgent update, it will revert to today's behavior. (We may or may not add a way to suspend the commit during an urgent update in the future.) In this PR, I have implemented this capability in the reconciler via new methods added to the host config. I've used our internal React "no-op" renderer to write tests that demonstrate the feature. I have not yet implemented Suspensey CSS, images, etc in React DOM. @gnoff and I will work on that in subsequent PRs. --- packages/react-art/src/ReactARTHostConfig.js | 11 + .../src/client/ReactDOMHostConfig.js | 13 + .../src/ReactFabricHostConfig.js | 12 + .../src/ReactNativeHostConfig.js | 12 + packages/react-noop-renderer/src/ReactNoop.js | 3 + .../src/ReactNoopPersistent.js | 3 + .../src/createReactNoop.js | 179 ++++++++++++++ .../src/ReactFiberBeginWork.js | 8 +- .../src/ReactFiberCommitWork.js | 54 ++-- .../src/ReactFiberCompleteWork.js | 163 +++++++++--- .../react-reconciler/src/ReactFiberFlags.js | 71 +++--- .../ReactFiberHostConfigWithNoSingletons.js | 2 +- .../src/ReactFiberOffscreenComponent.js | 3 +- .../react-reconciler/src/ReactFiberRoot.js | 1 + .../src/ReactFiberSuspenseComponent.js | 2 + .../src/ReactFiberThenable.js | 14 ++ .../react-reconciler/src/ReactFiberThrow.js | 58 +++-- .../src/ReactFiberWorkLoop.js | 156 +++++++++--- .../src/ReactInternalTypes.js | 4 + .../ReactFiberHostContext-test.internal.js | 8 + .../ReactSuspenseyCommitPhase-test.js | 231 ++++++++++++++++++ .../src/forks/ReactFiberHostConfig.custom.js | 4 + .../src/ReactTestHostConfig.js | 12 + 23 files changed, 894 insertions(+), 130 deletions(-) create mode 100644 packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js diff --git a/packages/react-art/src/ReactARTHostConfig.js b/packages/react-art/src/ReactARTHostConfig.js index 00c8bbcd67..754af14dcc 100644 --- a/packages/react-art/src/ReactARTHostConfig.js +++ b/packages/react-art/src/ReactARTHostConfig.js @@ -459,6 +459,17 @@ export function requestPostPaintCallback(callback: (time: number) => void) { // noop } +export function shouldSuspendCommit(type, props) { + return false; +} + +export function startSuspendingCommit() {} + +export function suspendInstance(type, props) {} + +export function waitForCommitToBeReady() { + return null; +} // eslint-disable-next-line no-undef export function prepareRendererToRender(container: Container): void { // noop diff --git a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js index b63c805478..fdd4484f71 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js @@ -1608,6 +1608,19 @@ export function requestPostPaintCallback(callback: (time: number) => void) { localRequestAnimationFrame(time => callback(time)); }); } + +export function shouldSuspendCommit(type: Type, props: Props): boolean { + return false; +} + +export function startSuspendingCommit(): void {} + +export function suspendInstance(type: Type, props: Props): void {} + +export function waitForCommitToBeReady(): null { + return null; +} + // ------------------- // Resources // ------------------- diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 689dabcfeb..2c2d5c598b 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -414,6 +414,18 @@ export function requestPostPaintCallback(callback: (time: number) => void) { // noop } +export function shouldSuspendCommit(type: Type, props: Props): boolean { + return false; +} + +export function startSuspendingCommit(): void {} + +export function suspendInstance(type: Type, props: Props): void {} + +export function waitForCommitToBeReady(): null { + return null; +} + export function prepareRendererToRender(container: Container): void { // noop } diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index 501d343b19..f23f73b00d 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -522,6 +522,18 @@ export function requestPostPaintCallback(callback: (time: number) => void) { // noop } +export function shouldSuspendCommit(type: Type, props: Props): boolean { + return false; +} + +export function startSuspendingCommit(): void {} + +export function suspendInstance(type: Type, props: Props): void {} + +export function waitForCommitToBeReady(): null { + return null; +} + export function prepareRendererToRender(container: Container): void { // noop } diff --git a/packages/react-noop-renderer/src/ReactNoop.js b/packages/react-noop-renderer/src/ReactNoop.js index 72d7356926..df55810493 100644 --- a/packages/react-noop-renderer/src/ReactNoop.js +++ b/packages/react-noop-renderer/src/ReactNoop.js @@ -28,6 +28,9 @@ export const { createLegacyRoot, getChildrenAsJSX, getPendingChildrenAsJSX, + getSuspenseyThingStatus, + resolveSuspenseyThing, + resetSuspenseyThingCache, createPortal, render, renderLegacySyncRoot, diff --git a/packages/react-noop-renderer/src/ReactNoopPersistent.js b/packages/react-noop-renderer/src/ReactNoopPersistent.js index f5b74a72f6..040114b86b 100644 --- a/packages/react-noop-renderer/src/ReactNoopPersistent.js +++ b/packages/react-noop-renderer/src/ReactNoopPersistent.js @@ -28,6 +28,9 @@ export const { createLegacyRoot, getChildrenAsJSX, getPendingChildrenAsJSX, + getSuspenseyThingStatus, + resolveSuspenseyThing, + resetSuspenseyThingCache, createPortal, render, renderLegacySyncRoot, diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 67634ab1d5..c6856ac539 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -47,6 +47,7 @@ type Props = { left?: null | number, right?: null | number, top?: null | number, + src?: string, ... }; type Instance = { @@ -72,6 +73,11 @@ type CreateRootOptions = { ... }; +type SuspenseyCommitSubscription = { + pendingCount: number, + commit: null | (() => void), +}; + const NO_CONTEXT = {}; const UPPERCASE_CONTEXT = {}; const UPDATE_SIGNAL = {}; @@ -238,6 +244,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { hidden: !!newProps.hidden, context: instance.context, }; + + if (type === 'suspensey-thing' && typeof newProps.src === 'string') { + clone.src = newProps.src; + } + Object.defineProperty(clone, 'id', { value: clone.id, enumerable: false, @@ -271,6 +282,78 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return hostContext === UPPERCASE_CONTEXT ? rawText.toUpperCase() : rawText; } + type SuspenseyThingRecord = { + status: 'pending' | 'fulfilled', + subscriptions: Array | null, + }; + + let suspenseyThingCache: Map< + SuspenseyThingRecord, + 'pending' | 'fulfilled', + > | null = null; + + // Represents a subscription for all the suspensey things that block a + // particular commit. Once they've all loaded, the commit phase can proceed. + let suspenseyCommitSubscription: SuspenseyCommitSubscription | null = null; + + function startSuspendingCommit(): void { + // This is where we might suspend on things that aren't associated with a + // particular node, like document.fonts.ready. + suspenseyCommitSubscription = null; + } + + function suspendInstance(type: string, props: Props): void { + const src = props.src; + if (type === 'suspensey-thing' && typeof src === 'string') { + // Attach a listener to the suspensey thing and create a subscription + // object that uses reference counting to track when all the suspensey + // things have loaded. + const record = suspenseyThingCache.get(src); + if (record === undefined) { + throw new Error('Could not find record for key.'); + } + if (record.status === 'pending') { + if (suspenseyCommitSubscription === null) { + suspenseyCommitSubscription = { + pendingCount: 1, + commit: null, + }; + } else { + suspenseyCommitSubscription.pendingCount++; + } + } + // Stash the subscription on the record. In `resolveSuspenseyThing`, + // we'll use this fire the commit once all the things have loaded. + if (record.subscriptions === null) { + record.subscriptions = []; + } + record.subscriptions.push(suspenseyCommitSubscription); + } else { + throw new Error( + 'Did not expect this host component to be visited when suspending ' + + 'the commit. Did you check the SuspendCommit flag?', + ); + } + return suspenseyCommitSubscription; + } + + function waitForCommitToBeReady(): + | ((commit: () => mixed) => () => void) + | null { + const subscription = suspenseyCommitSubscription; + if (subscription !== null) { + suspenseyCommitSubscription = null; + return (commit: () => void) => { + subscription.commit = commit; + const cancelCommit = () => { + subscription.commit = null; + }; + return cancelCommit; + }; + } + return null; + } + const sharedHostConfig = { supportsSingletons: false, @@ -322,6 +405,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { hidden: !!props.hidden, context: hostContext, }; + + if (type === 'suspensey-thing' && typeof props.src === 'string') { + inst.src = props.src; + } + // Hide from unit tests Object.defineProperty(inst, 'id', {value: inst.id, enumerable: false}); Object.defineProperty(inst, 'parent', { @@ -480,6 +568,45 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { const endTime = Scheduler.unstable_now(); callback(endTime); }, + + shouldSuspendCommit(type: string, props: Props): boolean { + if (type === 'suspensey-thing' && typeof props.src === 'string') { + if (suspenseyThingCache === null) { + suspenseyThingCache = new Map(); + } + const record = suspenseyThingCache.get(props.src); + if (record === undefined) { + const newRecord: SuspenseyThingRecord = { + status: 'pending', + subscriptions: null, + }; + suspenseyThingCache.set(props.src, newRecord); + const onLoadStart = props.onLoadStart; + if (typeof onLoadStart === 'function') { + onLoadStart(); + } + return props.src; + } else { + if (record.status === 'pending') { + // The resource was already requested, but it hasn't finished + // loading yet. + return true; + } else { + // The resource has already loaded. If the renderer is confident that + // the resource will still be cached by the time the render commits, + // then it can return false, like we do here. + return false; + } + } + } + // Don't need to suspend. + return false; + }, + + startSuspendingCommit, + suspendInstance, + waitForCommitToBeReady, + prepareRendererToRender() {}, resetRendererAfterRender() {}, }; @@ -508,6 +635,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { hostUpdateCounter++; instance.prop = newProps.prop; instance.hidden = !!newProps.hidden; + + if (type === 'suspensey-thing' && typeof newProps.src === 'string') { + instance.src = newProps.src; + } + if (shouldSetTextContent(type, newProps)) { if (__DEV__) { checkPropStringCoercion(newProps.children, 'children'); @@ -689,6 +821,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { if (instance.hidden) { props.hidden = true; } + if (instance.src) { + props.src = instance.src; + } if (children !== null) { props.children = children; } @@ -915,6 +1050,50 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return getPendingChildrenAsJSX(container); }, + getSuspenseyThingStatus(src): string | null { + if (suspenseyThingCache === null) { + return null; + } else { + const record = suspenseyThingCache.get(src); + return record === undefined ? null : record.status; + } + }, + + resolveSuspenseyThing(key: string): void { + if (suspenseyThingCache === null) { + suspenseyThingCache = new Map(); + } + const record = suspenseyThingCache.get(key); + if (record === undefined) { + const newRecord: SuspenseyThingRecord = { + status: 'fulfilled', + subscriptions: null, + }; + suspenseyThingCache.set(key, newRecord); + } else { + if (record.status === 'pending') { + record.status = 'fulfilled'; + const subscriptions = record.subscriptions; + if (subscriptions !== null) { + record.subscriptions = null; + for (let i = 0; i < subscriptions.length; i++) { + const subscription = subscriptions[i]; + subscription.pendingCount--; + if (subscription.pendingCount === 0) { + const commit = subscription.commit; + subscription.commit = null; + commit(); + } + } + } + } + } + }, + + resetSuspenseyThingCache() { + suspenseyThingCache = null; + }, + createPortal( children: ReactNodeList, container: Container, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 4126a7a324..db2159570a 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -2298,7 +2298,7 @@ function updateSuspenseComponent( const newOffscreenQueue: OffscreenQueue = { transitions: currentTransitions, markerInstances: parentMarkerInstances, - wakeables: null, + retryQueue: null, }; primaryChildFragment.updateQueue = newOffscreenQueue; } else { @@ -2399,7 +2399,7 @@ function updateSuspenseComponent( const newOffscreenQueue: OffscreenQueue = { transitions: currentTransitions, markerInstances: parentMarkerInstances, - wakeables: null, + retryQueue: null, }; primaryChildFragment.updateQueue = newOffscreenQueue; } else if (offscreenQueue === currentOffscreenQueue) { @@ -2408,9 +2408,9 @@ function updateSuspenseComponent( const newOffscreenQueue: OffscreenQueue = { transitions: currentTransitions, markerInstances: parentMarkerInstances, - wakeables: + retryQueue: currentOffscreenQueue !== null - ? currentOffscreenQueue.wakeables + ? currentOffscreenQueue.retryQueue : null, }; primaryChildFragment.updateQueue = newOffscreenQueue; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index eb0aa892a3..dfcc972012 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -19,7 +19,7 @@ import type { import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane'; import {NoTimestamp, SyncLane} from './ReactFiberLane'; -import type {SuspenseState} from './ReactFiberSuspenseComponent'; +import type {SuspenseState, RetryQueue} from './ReactFiberSuspenseComponent'; import type {UpdateQueue} from './ReactFiberClassUpdateQueue'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks'; import type {Wakeable} from 'shared/ReactTypes'; @@ -94,6 +94,7 @@ import { LayoutMask, PassiveMask, Visibility, + SuspenseyCommit, } from './ReactFiberFlags'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import { @@ -158,6 +159,7 @@ import { mountHoistable, unmountHoistable, prepareToCommitHoistables, + suspendInstance, } from './ReactFiberHostConfig'; import { captureCommitPhaseError, @@ -2310,9 +2312,9 @@ function commitSuspenseCallback(finishedWork: Fiber) { if (enableSuspenseCallback && newState !== null) { const suspenseCallback = finishedWork.memoizedProps.suspenseCallback; if (typeof suspenseCallback === 'function') { - const wakeables: Set | null = (finishedWork.updateQueue: any); - if (wakeables !== null) { - suspenseCallback(new Set(wakeables)); + const retryQueue: RetryQueue | null = (finishedWork.updateQueue: any); + if (retryQueue !== null) { + suspenseCallback(new Set(retryQueue)); } } else if (__DEV__) { if (suspenseCallback !== undefined) { @@ -2431,7 +2433,7 @@ export function attachOffscreenInstance(instance: OffscreenInstance): void { function attachSuspenseRetryListeners( finishedWork: Fiber, - wakeables: Set, + wakeables: RetryQueue, ) { // If this boundary just timed out, then it will have a set of wakeables. // For each wakeable, attach a listener so that when it resolves, React @@ -2917,10 +2919,10 @@ function commitMutationEffectsOnFiber( } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } - const wakeables: Set | null = (finishedWork.updateQueue: any); - if (wakeables !== null) { + const retryQueue: RetryQueue | null = (finishedWork.updateQueue: any); + if (retryQueue !== null) { finishedWork.updateQueue = null; - attachSuspenseRetryListeners(finishedWork, wakeables); + attachSuspenseRetryListeners(finishedWork, retryQueue); } } return; @@ -3006,10 +3008,10 @@ function commitMutationEffectsOnFiber( const offscreenQueue: OffscreenQueue | null = (finishedWork.updateQueue: any); if (offscreenQueue !== null) { - const wakeables = offscreenQueue.wakeables; - if (wakeables !== null) { - offscreenQueue.wakeables = null; - attachSuspenseRetryListeners(finishedWork, wakeables); + const retryQueue = offscreenQueue.retryQueue; + if (retryQueue !== null) { + offscreenQueue.retryQueue = null; + attachSuspenseRetryListeners(finishedWork, retryQueue); } } } @@ -3020,10 +3022,11 @@ function commitMutationEffectsOnFiber( commitReconciliationEffects(finishedWork); if (flags & Update) { - const wakeables: Set | null = (finishedWork.updateQueue: any); - if (wakeables !== null) { + const retryQueue: Set | null = + (finishedWork.updateQueue: any); + if (retryQueue !== null) { finishedWork.updateQueue = null; - attachSuspenseRetryListeners(finishedWork, wakeables); + attachSuspenseRetryListeners(finishedWork, retryQueue); } } return; @@ -4061,6 +4064,27 @@ export function commitPassiveUnmountEffects(finishedWork: Fiber): void { resetCurrentDebugFiberInDEV(); } +export function recursivelyAccumulateSuspenseyCommit(parentFiber: Fiber): void { + if (parentFiber.subtreeFlags & SuspenseyCommit) { + let child = parentFiber.child; + while (child !== null) { + recursivelyAccumulateSuspenseyCommit(child); + switch (child.tag) { + case HostComponent: + case HostHoistable: { + if (child.flags & SuspenseyCommit) { + const type = child.type; + const props = child.memoizedProps; + suspendInstance(type, props); + } + break; + } + } + child = child.sibling; + } + } +} + function detachAlternateSiblings(parentFiber: Fiber) { // A fiber was deleted from this parent fiber, but it's still part of the // previous (alternate) parent fiber's list of children. Because children diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 4215cade8f..b606632b8e 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -10,11 +10,7 @@ import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {RootState} from './ReactFiberRoot'; import type {Lanes, Lane} from './ReactFiberLane'; -import type { - ReactScopeInstance, - ReactContext, - Wakeable, -} from 'shared/ReactTypes'; +import type {ReactScopeInstance, ReactContext} from 'shared/ReactTypes'; import type { Instance, Type, @@ -25,7 +21,9 @@ import type { import type { SuspenseState, SuspenseListRenderState, + RetryQueue, } from './ReactFiberSuspenseComponent'; +import type {OffscreenQueue} from './ReactFiberOffscreenComponent'; import {isOffscreenManual} from './ReactFiberOffscreenComponent'; import type {OffscreenState} from './ReactFiberOffscreenComponent'; import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent'; @@ -90,6 +88,8 @@ import { Incomplete, ShouldCapture, ForceClientRender, + SuspenseyCommit, + ScheduleRetry, } from './ReactFiberFlags'; import { @@ -111,6 +111,7 @@ import { finalizeContainerChildren, preparePortalMount, prepareScopeUpdate, + shouldSuspendCommit, } from './ReactFiberHostConfig'; import { getRootHostContainer, @@ -150,6 +151,7 @@ import { renderHasNotSuspendedYet, getRenderTargetTime, getWorkInProgressTransitions, + shouldRemainOnPreviousScreen, } from './ReactFiberWorkLoop'; import { OffscreenLane, @@ -157,6 +159,8 @@ import { NoLanes, includesSomeLane, mergeLanes, + claimNextRetryLane, + includesOnlyNonUrgentLanes, } from './ReactFiberLane'; import {resetChildFibers} from './ReactChildFiber'; import {createScopeInstance} from './ReactFiberScope'; @@ -168,6 +172,7 @@ import { popMarkerInstance, popRootMarkerInstance, } from './ReactFiberTracingMarkerComponent'; +import {suspendCommit} from './ReactFiberThenable'; function markUpdate(workInProgress: Fiber) { // Tag the fiber with an update effect. This turns a Placement into @@ -411,6 +416,7 @@ function updateHostComponent( workInProgress: Fiber, type: Type, newProps: Props, + renderLanes: Lanes, ) { if (supportsMutation) { // If we have an alternate, that means this is an update and we need to @@ -427,6 +433,9 @@ function updateHostComponent( // TODO: Split the update API as separate for the props vs. children. // Even better would be if children weren't special cased at all tho. const instance: Instance = workInProgress.stateNode; + + suspendHostCommitIfNeeded(workInProgress, type, newProps, renderLanes); + const currentHostContext = getHostContext(); // TODO: Experiencing an error where oldProps is null. Suggests a host // component is hitting the resume path. Figure out why. Possibly @@ -485,6 +494,9 @@ function updateHostComponent( childrenUnchanged, recyclableInstance, ); + + suspendHostCommitIfNeeded(workInProgress, type, newProps, renderLanes); + if ( finalizeInitialChildren(newInstance, type, newProps, currentHostContext) ) { @@ -502,6 +514,89 @@ function updateHostComponent( } } } + +// TODO: This should ideally move to begin phase, but currently the instance is +// not created until the complete phase. For our existing use cases, host nodes +// that suspend don't have children, so it doesn't matter. But that might not +// always be true in the future. +function suspendHostCommitIfNeeded( + workInProgress: Fiber, + type: Type, + props: Props, + renderLanes: Lanes, +) { + // Ask the renderer if this instance should suspend the commit. + if (!shouldSuspendCommit(type, props)) { + // If this flag was set previously, we can remove it. The flag represents + // whether this particular set of props might ever need to suspend. The + // safest thing to do is for shouldSuspendCommit to always return true, but + // if the renderer is reasonably confident that the underlying resource + // won't be evicted, it can return false as a performance optimization. + workInProgress.flags &= ~SuspenseyCommit; + return; + } + + // Mark this fiber with a flag. We use this right before the commit phase to + // find all the fibers that might need to suspend the commit. In the future + // we'll also use it when revealing a hidden tree. It gets set even if we + // don't end up suspending this particular commit, because if this tree ever + // becomes hidden, we might want to suspend before revealing it again. + workInProgress.flags |= SuspenseyCommit; + + // Check if we're rendering at a "non-urgent" priority. This is the same + // check that `useDeferredValue` does to determine whether it needs to + // defer. This is partly for gradual adoption purposes (i.e. shouldn't start + // suspending until you opt in with startTransition or Suspense) but it + // also happens to be the desired behavior for the concrete use cases we've + // thought of so far, like CSS loading, fonts, images, etc. + // TODO: We may decide to expose a way to force a fallback even during a + // sync update. + if (!includesOnlyNonUrgentLanes(renderLanes)) { + // This is an urgent render. Never suspend or trigger a fallback. + } else { + // Need to decide whether to activate the nearest fallback or to continue + // rendering and suspend right before the commit phase. + if (shouldRemainOnPreviousScreen()) { + // It's OK to block the commit. Don't show a fallback. + } else { + // We shouldn't block the commit. Activate a fallback at the nearest + // Suspense boundary. + suspendCommit(); + } + } +} + +function scheduleRetryEffect( + workInProgress: Fiber, + retryQueue: RetryQueue | null, +) { + const wakeables = retryQueue; + if (wakeables !== null) { + // 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); + } + } +} + function updateHostText( current: Fiber, workInProgress: Fiber, @@ -955,6 +1050,7 @@ function completeWork( workInProgress, workInProgress.type, workInProgress.pendingProps, + renderLanes, ); } bubbleProperties(workInProgress); @@ -968,7 +1064,13 @@ function completeWork( const rootContainerInstance = getRootHostContainer(); const type = workInProgress.type; if (current !== null && workInProgress.stateNode != null) { - updateHostComponent(current, workInProgress, type, newProps); + updateHostComponent( + current, + workInProgress, + type, + newProps, + renderLanes, + ); if (current.ref !== workInProgress.ref) { markRef(workInProgress); @@ -989,19 +1091,22 @@ function completeWork( const currentHostContext = getHostContext(); const wasHydrated = popHydrationState(workInProgress); + let instance: Instance; if (wasHydrated) { // We ignore the boolean indicating there is an updateQueue because // it is used only to set text children and HostSingletons do not // use them. prepareToHydrateHostInstance(workInProgress, currentHostContext); + instance = workInProgress.stateNode; } else { - workInProgress.stateNode = resolveSingletonInstance( + instance = resolveSingletonInstance( type, newProps, rootContainerInstance, currentHostContext, true, ); + workInProgress.stateNode = instance; markUpdate(workInProgress); } @@ -1019,7 +1124,13 @@ function completeWork( popHostContext(workInProgress); const type = workInProgress.type; if (current !== null && workInProgress.stateNode != null) { - updateHostComponent(current, workInProgress, type, newProps); + updateHostComponent( + current, + workInProgress, + type, + newProps, + renderLanes, + ); if (current.ref !== workInProgress.ref) { markRef(workInProgress); @@ -1081,6 +1192,8 @@ function completeWork( } } + suspendHostCommitIfNeeded(workInProgress, type, newProps, renderLanes); + if (workInProgress.ref !== null) { // If there is a ref on a host node we need to schedule a callback markRef(workInProgress); @@ -1227,12 +1340,8 @@ function completeWork( } } - const wakeables: Set | null = (workInProgress.updateQueue: any); - if (wakeables !== null) { - // Schedule an effect to attach a retry listener to the promise. - // TODO: Move to passive phase - workInProgress.flags |= Update; - } + const retryQueue: RetryQueue | null = (workInProgress.updateQueue: any); + scheduleRetryEffect(workInProgress, retryQueue); if ( enableSuspenseCallback && @@ -1337,11 +1446,10 @@ function completeWork( // We might bail out of the loop before finding any but that // doesn't matter since that means that the other boundaries that // we did find already has their listeners attached. - const newThenables = suspended.updateQueue; - if (newThenables !== null) { - workInProgress.updateQueue = newThenables; - workInProgress.flags |= Update; - } + const retryQueue: RetryQueue | null = + (suspended.updateQueue: any); + workInProgress.updateQueue = retryQueue; + scheduleRetryEffect(workInProgress, retryQueue); // Rerender the whole list, but this time, we'll force fallbacks // to stay in place. @@ -1399,11 +1507,9 @@ function completeWork( // Ensure we transfer the update queue to the parent so that it doesn't // get lost if this row ends up dropped during a second pass. - const newThenables = suspended.updateQueue; - if (newThenables !== null) { - workInProgress.updateQueue = newThenables; - workInProgress.flags |= Update; - } + const retryQueue: RetryQueue | null = (suspended.updateQueue: any); + workInProgress.updateQueue = retryQueue; + scheduleRetryEffect(workInProgress, retryQueue); cutOffTailIfNeeded(renderState, true); // This might have been modified. @@ -1566,10 +1672,11 @@ function completeWork( } } - if (workInProgress.updateQueue !== null) { - // Schedule an effect to attach Suspense retry listeners - // TODO: Move to passive phase - workInProgress.flags |= Update; + const offscreenQueue: OffscreenQueue | null = + (workInProgress.updateQueue: any); + if (offscreenQueue !== null) { + const retryQueue = offscreenQueue.retryQueue; + scheduleRetryEffect(workInProgress, retryQueue); } if (enableCache) { diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index f8df8f6f59..c37a648973 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -12,57 +12,63 @@ import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags'; export type Flags = number; // Don't change these values. They're used by React Dev Tools. -export const NoFlags = /* */ 0b000000000000000000000000000; -export const PerformedWork = /* */ 0b000000000000000000000000001; -export const Placement = /* */ 0b000000000000000000000000010; -export const DidCapture = /* */ 0b000000000000000000010000000; -export const Hydrating = /* */ 0b000000000000001000000000000; +export const NoFlags = /* */ 0b0000000000000000000000000000; +export const PerformedWork = /* */ 0b0000000000000000000000000001; +export const Placement = /* */ 0b0000000000000000000000000010; +export const DidCapture = /* */ 0b0000000000000000000010000000; +export const Hydrating = /* */ 0b0000000000000001000000000000; // You can change the rest (and add more). -export const Update = /* */ 0b000000000000000000000000100; -/* Skipped value: 0b000000000000000000000001000; */ +export const Update = /* */ 0b0000000000000000000000000100; +/* Skipped value: 0b0000000000000000000000001000; */ -export const ChildDeletion = /* */ 0b000000000000000000000010000; -export const ContentReset = /* */ 0b000000000000000000000100000; -export const Callback = /* */ 0b000000000000000000001000000; -/* Used by DidCapture: 0b000000000000000000010000000; */ +export const ChildDeletion = /* */ 0b0000000000000000000000010000; +export const ContentReset = /* */ 0b0000000000000000000000100000; +export const Callback = /* */ 0b0000000000000000000001000000; +/* Used by DidCapture: 0b0000000000000000000010000000; */ -export const ForceClientRender = /* */ 0b000000000000000000100000000; -export const Ref = /* */ 0b000000000000000001000000000; -export const Snapshot = /* */ 0b000000000000000010000000000; -export const Passive = /* */ 0b000000000000000100000000000; -/* Used by Hydrating: 0b000000000000001000000000000; */ +export const ForceClientRender = /* */ 0b0000000000000000000100000000; +export const Ref = /* */ 0b0000000000000000001000000000; +export const Snapshot = /* */ 0b0000000000000000010000000000; +export const Passive = /* */ 0b0000000000000000100000000000; +/* Used by Hydrating: 0b0000000000000001000000000000; */ -export const Visibility = /* */ 0b000000000000010000000000000; -export const StoreConsistency = /* */ 0b000000000000100000000000000; +export const Visibility = /* */ 0b0000000000000010000000000000; +export const StoreConsistency = /* */ 0b0000000000000100000000000000; + +// It's OK to reuse this bit because these flags are mutually exclusive for +// different fiber types. We should really be doing this for as many flags as +// possible, because we're about to run out of bits. +export const ScheduleRetry = StoreConsistency; export const LifecycleEffectMask = Passive | Update | Callback | Ref | Snapshot | StoreConsistency; // Union of all commit flags (flags with the lifetime of a particular commit) -export const HostEffectMask = /* */ 0b00000000000011111111111111; +export const HostEffectMask = /* */ 0b0000000000000111111111111111; // These are not really side effects, but we still reuse this field. -export const Incomplete = /* */ 0b000000000001000000000000000; -export const ShouldCapture = /* */ 0b000000000010000000000000000; -export const ForceUpdateForLegacySuspense = /* */ 0b000000000100000000000000000; -export const DidPropagateContext = /* */ 0b000000001000000000000000000; -export const NeedsPropagation = /* */ 0b000000010000000000000000000; -export const Forked = /* */ 0b000000100000000000000000000; +export const Incomplete = /* */ 0b0000000000001000000000000000; +export const ShouldCapture = /* */ 0b0000000000010000000000000000; +export const ForceUpdateForLegacySuspense = /* */ 0b0000000000100000000000000000; +export const DidPropagateContext = /* */ 0b0000000001000000000000000000; +export const NeedsPropagation = /* */ 0b0000000010000000000000000000; +export const Forked = /* */ 0b0000000100000000000000000000; // Static tags describe aspects of a fiber that are not specific to a render, // e.g. a fiber uses a passive effect (even if there are no updates on this particular render). // This enables us to defer more work in the unmount case, // since we can defer traversing the tree during layout to look for Passive effects, // and instead rely on the static flag as a signal that there may be cleanup work. -export const RefStatic = /* */ 0b000001000000000000000000000; -export const LayoutStatic = /* */ 0b000010000000000000000000000; -export const PassiveStatic = /* */ 0b000100000000000000000000000; +export const RefStatic = /* */ 0b0000001000000000000000000000; +export const LayoutStatic = /* */ 0b0000010000000000000000000000; +export const PassiveStatic = /* */ 0b0000100000000000000000000000; +export const SuspenseyCommit = /* */ 0b0001000000000000000000000000; // Flag used to identify newly inserted fibers. It isn't reset after commit unlike `Placement`. -export const PlacementDEV = /* */ 0b001000000000000000000000000; -export const MountLayoutDev = /* */ 0b010000000000000000000000000; -export const MountPassiveDev = /* */ 0b100000000000000000000000000; +export const PlacementDEV = /* */ 0b0010000000000000000000000000; +export const MountLayoutDev = /* */ 0b0100000000000000000000000000; +export const MountPassiveDev = /* */ 0b1000000000000000000000000000; // Groups of flags that are used in the commit phase to skip over trees that // don't contain effects, by checking subtreeFlags. @@ -96,4 +102,5 @@ export const PassiveMask = Passive | Visibility | ChildDeletion; // Union of tags that don't get reset on clones. // This allows certain concepts to persist without recalculating them, // e.g. whether a subtree contains passive effects or portals. -export const StaticMask = LayoutStatic | PassiveStatic | RefStatic; +export const StaticMask = + LayoutStatic | PassiveStatic | RefStatic | SuspenseyCommit; diff --git a/packages/react-reconciler/src/ReactFiberHostConfigWithNoSingletons.js b/packages/react-reconciler/src/ReactFiberHostConfigWithNoSingletons.js index f8cf62243f..f592e95185 100644 --- a/packages/react-reconciler/src/ReactFiberHostConfigWithNoSingletons.js +++ b/packages/react-reconciler/src/ReactFiberHostConfigWithNoSingletons.js @@ -10,7 +10,7 @@ // Renderers that don't support mutation // can re-export everything from this module. -function shim(...args: any) { +function shim(...args: any): any { throw new Error( 'The current renderer does not support Singletons. ' + 'This error is likely caused by a bug in React. ' + diff --git a/packages/react-reconciler/src/ReactFiberOffscreenComponent.js b/packages/react-reconciler/src/ReactFiberOffscreenComponent.js index 2f628d41ee..48b5bc0e6d 100644 --- a/packages/react-reconciler/src/ReactFiberOffscreenComponent.js +++ b/packages/react-reconciler/src/ReactFiberOffscreenComponent.js @@ -15,6 +15,7 @@ import type { Transition, TracingMarkerInstance, } from './ReactFiberTracingMarkerComponent'; +import type {RetryQueue} from './ReactFiberSuspenseComponent'; export type OffscreenProps = { // TODO: Pick an API before exposing the Offscreen type. I've chosen an enum @@ -40,7 +41,7 @@ export type OffscreenState = { export type OffscreenQueue = { transitions: Array | null, markerInstances: Array | null, - wakeables: Set | null, + retryQueue: RetryQueue | null, }; type OffscreenVisibility = number; diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 82f5655250..c8df0a9e14 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -60,6 +60,7 @@ function FiberRootNode( this.pingCache = null; this.finishedWork = null; this.timeoutHandle = noTimeout; + this.cancelPendingCommit = null; this.context = null; this.pendingContext = null; this.callbackNode = null; diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js index 0efbf5592b..e248ad0719 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js @@ -67,6 +67,8 @@ export type SuspenseListRenderState = { tailMode: SuspenseListTailMode, }; +export type RetryQueue = Set; + export function findFirstSuspended(row: Fiber): null | Fiber { let node = row; while (node !== null) { diff --git a/packages/react-reconciler/src/ReactFiberThenable.js b/packages/react-reconciler/src/ReactFiberThenable.js index 81c59baf16..5b2501f33f 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.js +++ b/packages/react-reconciler/src/ReactFiberThenable.js @@ -31,6 +31,12 @@ export const SuspenseException: mixed = new Error( "call the promise's `.catch` method and pass the result to `use`", ); +// This is a noop thenable that we use to trigger a fallback in throwException. +// TODO: It would be better to refactor throwException into multiple functions +// so we can trigger a fallback directly without having to check the type. But +// for now this will do. +export const noopSuspenseyCommitThenable = {then() {}}; + export function createThenableState(): ThenableState { // The ThenableState is created the first time a component suspends. If it // suspends again, we'll reuse the same state. @@ -140,6 +146,14 @@ export function trackUsedThenable( } } +export function suspendCommit(): void { + // This extra indirection only exists so it can handle passing + // noopSuspenseyCommitThenable through to throwException. + // TODO: Factor the thenable check out of throwException + suspendedThenable = noopSuspenseyCommitThenable; + throw SuspenseException; +} + // This is used to track the actual thenable that suspended so it can be // passed to the rest of the Suspense implementation — which, for historical // reasons, expects to receive a thenable. diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index 5d82b8dbe7..37b211876c 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -13,6 +13,7 @@ import type {CapturedValue} from './ReactCapturedValue'; import type {Update} from './ReactFiberClassUpdateQueue'; import type {Wakeable} from 'shared/ReactTypes'; import type {OffscreenQueue} from './ReactFiberOffscreenComponent'; +import type {RetryQueue} from './ReactFiberSuspenseComponent'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import { @@ -33,6 +34,7 @@ import { LifecycleEffectMask, ForceUpdateForLegacySuspense, ForceClientRender, + ScheduleRetry, } from './ReactFiberFlags'; import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode'; import { @@ -79,6 +81,7 @@ import { queueHydrationError, } from './ReactFiberHydrationContext'; import {ConcurrentRoot} from './ReactRootTags'; +import {noopSuspenseyCommitThenable} from './ReactFiberThenable'; function createRootErrorUpdate( fiber: Fiber, @@ -412,33 +415,52 @@ function throwException( // // When the wakeable resolves, we'll attempt to render the boundary // again ("retry"). - const wakeables: Set | null = - (suspenseBoundary.updateQueue: any); - if (wakeables === null) { - suspenseBoundary.updateQueue = new Set([wakeable]); + + // Check if this is a Suspensey resource. We do not attach retry + // listeners to these, because we don't actually need them for + // rendering. Only for committing. Instead, if a fallback commits + // and the only thing that suspended was a Suspensey resource, we + // retry immediately. + // TODO: Refactor throwException so that we don't have to do this type + // check. The caller already knows what the cause was. + const isSuspenseyResource = wakeable === noopSuspenseyCommitThenable; + if (isSuspenseyResource) { + suspenseBoundary.flags |= ScheduleRetry; } else { - wakeables.add(wakeable); + const retryQueue: RetryQueue | null = + (suspenseBoundary.updateQueue: any); + if (retryQueue === null) { + suspenseBoundary.updateQueue = new Set([wakeable]); + } else { + retryQueue.add(wakeable); + } } break; } case OffscreenComponent: { if (suspenseBoundary.mode & ConcurrentMode) { suspenseBoundary.flags |= ShouldCapture; - const offscreenQueue: OffscreenQueue | null = - (suspenseBoundary.updateQueue: any); - if (offscreenQueue === null) { - const newOffscreenQueue: OffscreenQueue = { - transitions: null, - markerInstances: null, - wakeables: new Set([wakeable]), - }; - suspenseBoundary.updateQueue = newOffscreenQueue; + const isSuspenseyResource = + wakeable === noopSuspenseyCommitThenable; + if (isSuspenseyResource) { + suspenseBoundary.flags |= ScheduleRetry; } else { - const wakeables = offscreenQueue.wakeables; - if (wakeables === null) { - offscreenQueue.wakeables = new Set([wakeable]); + const offscreenQueue: OffscreenQueue | null = + (suspenseBoundary.updateQueue: any); + if (offscreenQueue === null) { + const newOffscreenQueue: OffscreenQueue = { + transitions: null, + markerInstances: null, + retryQueue: new Set([wakeable]), + }; + suspenseBoundary.updateQueue = newOffscreenQueue; } else { - wakeables.add(wakeable); + const retryQueue = offscreenQueue.retryQueue; + if (retryQueue === null) { + offscreenQueue.retryQueue = new Set([wakeable]); + } else { + retryQueue.add(wakeable); + } } } break; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 6459cd0775..241c068bb3 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -84,6 +84,8 @@ import { scheduleMicrotask, prepareRendererToRender, resetRendererAfterRender, + startSuspendingCommit, + waitForCommitToBeReady, } from './ReactFiberHostConfig'; import { @@ -161,6 +163,7 @@ import { movePendingFibersToMemoized, addTransitionToLanesMap, getTransitionsForLanes, + includesOnlyNonUrgentLanes, } from './ReactFiberLane'; import { DiscreteEventPriority, @@ -201,6 +204,7 @@ import { invokePassiveEffectMountInDEV, invokeLayoutEffectUnmountInDEV, invokePassiveEffectUnmountInDEV, + recursivelyAccumulateSuspenseyCommit, } from './ReactFiberCommitWork'; import {enqueueUpdate} from './ReactFiberClassUpdateQueue'; import {resetContextDependencies} from './ReactFiberNewContext'; @@ -905,6 +909,18 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { return; } + const cancelPendingCommit = root.cancelPendingCommit; + if (cancelPendingCommit !== null) { + // We should only interrupt a pending commit if the new update + // is urgent. + if (includesOnlyNonUrgentLanes(nextLanes)) { + // The new update is not urgent. Don't interrupt the pending commit. + root.callbackPriority = NoLane; + root.callbackNode = null; + return; + } + } + // We use the highest priority lane to represent the priority of the callback. const newCallbackPriority = getHighestPriorityLane(nextLanes); @@ -1158,7 +1174,7 @@ function performConcurrentWorkOnRoot( // or, if something suspended, wait to commit it after a timeout. root.finishedWork = finishedWork; root.finishedLanes = lanes; - finishConcurrentRender(root, exitStatus, lanes); + finishConcurrentRender(root, exitStatus, finishedWork, lanes); } } @@ -1263,6 +1279,7 @@ export function queueRecoverableErrors(errors: Array>) { function finishConcurrentRender( root: FiberRoot, exitStatus: RootExitStatus, + finishedWork: Fiber, lanes: Lanes, ) { switch (exitStatus) { @@ -1276,10 +1293,12 @@ function finishConcurrentRender( case RootErrored: { // We should have already attempted to retry this tree. If we reached // this point, it errored again. Commit it. - commitRoot( + commitRootWhenReady( root, + finishedWork, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ); break; } @@ -1310,11 +1329,13 @@ function finishConcurrentRender( // lower priority work to do. Instead of committing the fallback // immediately, wait for more data to arrive. root.timeoutHandle = scheduleTimeout( - commitRoot.bind( + commitRootWhenReady.bind( null, root, + finishedWork, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ), msUntilTimeout, ); @@ -1322,10 +1343,12 @@ function finishConcurrentRender( } } // The work expired. Commit immediately. - commitRoot( + commitRootWhenReady( root, + finishedWork, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ); break; } @@ -1357,11 +1380,13 @@ function finishConcurrentRender( // Instead of committing the fallback immediately, wait for more data // to arrive. root.timeoutHandle = scheduleTimeout( - commitRoot.bind( + commitRootWhenReady.bind( null, root, + finishedWork, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ), msUntilTimeout, ); @@ -1370,19 +1395,23 @@ function finishConcurrentRender( } // Commit the placeholder. - commitRoot( + commitRootWhenReady( root, + finishedWork, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ); break; } case RootCompleted: { - // The work completed. Ready to commit. - commitRoot( + // The work completed. + commitRootWhenReady( root, + finishedWork, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ); break; } @@ -1392,6 +1421,53 @@ function finishConcurrentRender( } } +function commitRootWhenReady( + root: FiberRoot, + finishedWork: Fiber, + recoverableErrors: Array> | null, + transitions: Array | null, + lanes: Lanes, +) { + if (includesOnlyNonUrgentLanes(lanes)) { + // Before committing, ask the renderer whether the host tree is ready. + // If it's not, we'll wait until it notifies us. + startSuspendingCommit(); + // This will walk the completed fiber tree and attach listeners to all + // the suspensey resources. The renderer is responsible for accumulating + // all the load events. This all happens in a single synchronous + // transaction, so it track state in its own module scope. + recursivelyAccumulateSuspenseyCommit(finishedWork); + // At the end, ask the renderer if it's ready to commit, or if we should + // suspend. If it's not ready, it will return a callback to subscribe to + // a ready event. + const schedulePendingCommit = waitForCommitToBeReady(); + if (schedulePendingCommit !== null) { + // NOTE: waitForCommitToBeReady returns a subscribe function so that we + // only allocate a function if the commit isn't ready yet. The other + // pattern would be to always pass a callback to waitForCommitToBeReady. + + // Not yet ready to commit. Delay the commit until the renderer notifies + // us that it's ready. This will be canceled if we start work on the + // root again. + root.cancelPendingCommit = schedulePendingCommit( + commitRoot.bind( + null, + root, + workInProgressRootRecoverableErrors, + workInProgressTransitions, + ), + ); + return; + } + } + // Otherwise, commit immediately. + commitRoot( + root, + workInProgressRootRecoverableErrors, + workInProgressTransitions, + ); +} + function isRenderConsistentWithExternalStores(finishedWork: Fiber): boolean { // Search the rendered tree for external store reads, and check whether the // stores were mutated in a concurrent event. Intentionally using an iterative @@ -1714,6 +1790,11 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { // $FlowFixMe Complains noTimeout is not a TimeoutID, despite the check above cancelTimeout(timeoutHandle); } + const cancelPendingCommit = root.cancelPendingCommit; + if (cancelPendingCommit !== null) { + root.cancelPendingCommit = null; + cancelPendingCommit(); + } resetWorkInProgressStack(); workInProgressRoot = root; @@ -1775,9 +1856,21 @@ function handleThrow(root: FiberRoot, thrownValue: any): void { // API for suspending. This implementation detail can change later, once we // deprecate the old API in favor of `use`. thrownValue = getSuspendedThenable(); - workInProgressSuspendedReason = shouldAttemptToSuspendUntilDataResolves() - ? SuspendedOnData - : SuspendedOnImmediate; + workInProgressSuspendedReason = + shouldRemainOnPreviousScreen() && + // Check if there are other pending updates that might possibly unblock this + // component from suspending. This mirrors the check in + // renderDidSuspendDelayIfPossible. We should attempt to unify them somehow. + // TODO: Consider unwinding immediately, using the + // SuspendedOnHydration mechanism. + !includesNonIdleWork(workInProgressRootSkippedLanes) && + !includesNonIdleWork(workInProgressRootInterleavedUpdatedLanes) + ? // Suspend work loop until data resolves + SuspendedOnData + : // Don't suspend work loop, except to check if the data has + // immediately resolved (i.e. in a microtask). Otherwise, trigger the + // nearest Suspense fallback. + SuspendedOnImmediate; } else if (thrownValue === SelectiveHydrationException) { // An update flowed into a dehydrated boundary. Before we can apply the // update, we need to finish hydrating. Interrupt the work-in-progress @@ -1856,28 +1949,28 @@ function handleThrow(root: FiberRoot, thrownValue: any): void { } } -function shouldAttemptToSuspendUntilDataResolves() { - // Check if there are other pending updates that might possibly unblock this - // component from suspending. This mirrors the check in - // renderDidSuspendDelayIfPossible. We should attempt to unify them somehow. - // TODO: Consider unwinding immediately, using the - // SuspendedOnHydration mechanism. - if ( - includesNonIdleWork(workInProgressRootSkippedLanes) || - includesNonIdleWork(workInProgressRootInterleavedUpdatedLanes) - ) { - // Suspend normally. renderDidSuspendDelayIfPossible will handle - // interrupting the work loop. - return false; - } +export function shouldRemainOnPreviousScreen(): boolean { + // This is asking whether it's better to suspend the transition and remain + // on the previous screen, versus showing a fallback as soon as possible. It + // takes into account both the priority of render and also whether showing a + // fallback would produce a desirable user experience. - // TODO: We should be able to remove the equivalent check in - // finishConcurrentRender, and rely just on this one. + // TODO: Once `use` has fully replaced the `throw promise` pattern, we should + // be able to remove the equivalent check in finishConcurrentRender, and rely + // just on this one. if (includesOnlyTransitions(workInProgressRootRenderLanes)) { - // If we're rendering inside the "shell" of the app, it's better to suspend - // rendering and wait for the data to resolve. Otherwise, we should switch - // to a fallback and continue rendering. - return getShellBoundary() === null; + if (getShellBoundary() === null) { + // We're rendering inside the "shell" of the app. Activating the nearest + // fallback would cause visible content to disappear. It's better to + // suspend the transition and remain on the previous screen. + return true; + } else { + // We're rendering content that wasn't part of the previous screen. + // Rather than block the transition, it's better to show a fallback as + // soon as possible. The appearance of any nested fallbacks will be + // throttled to avoid jank. + return false; + } } const handler = getSuspenseHandler(); @@ -2686,6 +2779,7 @@ function commitRootImpl( // So we can clear these now to allow a new callback to be scheduled. root.callbackNode = null; root.callbackPriority = NoLane; + root.cancelPendingCommit = null; // Check which lanes no longer have any work scheduled on them, and mark // those as finished. diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index edc120182e..7f01daa524 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -227,6 +227,10 @@ type BaseFiberRootProperties = { // Timeout handle returned by setTimeout. Used to cancel a pending timeout, if // it's superseded by a new one. timeoutHandle: TimeoutHandle | NoTimeout, + // When a root has a pending commit scheduled, calling this function will + // cancel it. + // TODO: Can this be consolidated with timeoutHandle? + cancelPendingCommit: null | (() => void), // Top context object, used by renderSubtreeIntoContainer context: Object | null, pendingContext: Object | null, diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js index 9cfd5cea5a..10e764fd1d 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js @@ -71,6 +71,14 @@ describe('ReactFiberHostContext', () => { return DefaultEventPriority; }, requestPostPaintCallback: function () {}, + shouldSuspendCommit(type, props) { + return false; + }, + startSuspendingCommit() {}, + suspendInstance(type, props) {}, + waitForCommitToBeReady() { + return null; + }, prepareRendererToRender: function () {}, resetRendererAfterRender: function () {}, supportsMutation: true, diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js new file mode 100644 index 0000000000..19260ebff9 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js @@ -0,0 +1,231 @@ +let React; +let startTransition; +let ReactNoop; +let resolveSuspenseyThing; +let getSuspenseyThingStatus; +let Suspense; +let SuspenseList; +let Scheduler; +let act; +let assertLog; + +describe('ReactSuspenseyCommitPhase', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + Suspense = React.Suspense; + SuspenseList = React.SuspenseList; + if (gate(flags => flags.enableSuspenseList)) { + SuspenseList = React.SuspenseList; + } + startTransition = React.startTransition; + resolveSuspenseyThing = ReactNoop.resolveSuspenseyThing; + getSuspenseyThingStatus = ReactNoop.getSuspenseyThingStatus; + + const InternalTestUtils = require('internal-test-utils'); + act = InternalTestUtils.act; + assertLog = InternalTestUtils.assertLog; + }); + + function Text({text}) { + Scheduler.log(text); + return text; + } + + function SuspenseyImage({src}) { + return ( + Scheduler.log(`Image requested [${src}]`)} + /> + ); + } + + test('suspend commit during initial mount', async () => { + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(() => { + root.render( + }> + + , + ); + }); + }); + assertLog(['Image requested [A]', 'Loading...']); + expect(getSuspenseyThingStatus('A')).toBe('pending'); + expect(root).toMatchRenderedOutput('Loading...'); + + // This should synchronously commit + resolveSuspenseyThing('A'); + expect(root).toMatchRenderedOutput(); + }); + + test('suspend commit during update', async () => { + const root = ReactNoop.createRoot(); + await act(() => resolveSuspenseyThing('A')); + await act(async () => { + startTransition(() => { + root.render( + }> + + , + ); + }); + }); + expect(root).toMatchRenderedOutput(); + + // Update to a new image src. The transition should suspend because + // the src hasn't loaded yet, and the image is in an already-visible tree. + await act(async () => { + startTransition(() => { + root.render( + }> + + , + ); + }); + }); + assertLog(['Image requested [B]']); + expect(getSuspenseyThingStatus('B')).toBe('pending'); + // Should remain on previous screen + expect(root).toMatchRenderedOutput(); + + // This should synchronously commit + resolveSuspenseyThing('B'); + expect(root).toMatchRenderedOutput(); + }); + + test('does not suspend commit during urgent update', async () => { + const root = ReactNoop.createRoot(); + await act(async () => { + root.render( + }> + + , + ); + }); + // NOTE: `shouldSuspendCommit` is called even during synchronous renders + // because if this node is ever hidden, then revealed again, we want to know + // whether it's capable of suspending the commit. We track this using a + // fiber flag. + assertLog(['Image requested [A]']); + expect(getSuspenseyThingStatus('A')).toBe('pending'); + expect(root).toMatchRenderedOutput(); + }); + + test('an urgent update interrupts a suspended commit', async () => { + const root = ReactNoop.createRoot(); + + // Mount an image. This transition will suspend because it's not inside a + // Suspense boundary. + await act(() => { + startTransition(() => { + root.render(); + }); + }); + assertLog(['Image requested [A]']); + // Nothing showing yet. + expect(root).toMatchRenderedOutput(null); + + // If there's an urgent update, it should interrupt the suspended commit. + await act(() => { + root.render(); + }); + assertLog(['Something else']); + expect(root).toMatchRenderedOutput('Something else'); + }); + + test('a non-urgent update does not interrupt a suspended commit', async () => { + const root = ReactNoop.createRoot(); + + // Mount an image. This transition will suspend because it's not inside a + // Suspense boundary. + await act(() => { + startTransition(() => { + root.render(); + }); + }); + assertLog(['Image requested [A]']); + // Nothing showing yet. + expect(root).toMatchRenderedOutput(null); + + // If there's another transition update, it should not interrupt the + // suspended commit. + await act(() => { + startTransition(() => { + root.render(); + }); + }); + // Still suspended. + expect(root).toMatchRenderedOutput(null); + + await act(() => { + // Resolving the image should result in an immediate, synchronous commit. + resolveSuspenseyThing('A'); + expect(root).toMatchRenderedOutput(); + }); + // Then the second transition is unblocked. + // TODO: Right now the only way to unsuspend a commit early is to proceed + // with the commit even if everything isn't ready. Maybe there should also + // be a way to abort a commit so that it can be interrupted by + // another transition. + assertLog(['Something else']); + expect(root).toMatchRenderedOutput('Something else'); + }); + + // @gate enableSuspenseList + test('demonstrate current behavior when used with SuspenseList (not ideal)', async () => { + function App() { + return ( + + }> + + + }> + + + }> + + + + ); + } + + const root = ReactNoop.createRoot(); + await act(() => { + startTransition(() => { + root.render(); + }); + }); + assertLog([ + 'Image requested [A]', + 'Loading A', + 'Loading B', + 'Loading C', + 'Image requested [B]', + 'Image requested [C]', + ]); + expect(root).toMatchRenderedOutput('Loading ALoading BLoading C'); + + // TODO: Notice that none of these items appear until they've all loaded. + // That's not ideal; we should commit each row as it becomes ready to + // commit. That means we need to prepare both the fallback and the primary + // tree during the render phase. Related to Offscreen, too. + resolveSuspenseyThing('A'); + expect(root).toMatchRenderedOutput('Loading ALoading BLoading C'); + resolveSuspenseyThing('B'); + expect(root).toMatchRenderedOutput('Loading ALoading BLoading C'); + resolveSuspenseyThing('C'); + expect(root).toMatchRenderedOutput( + <> + + + + , + ); + }); +}); diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js index 3b9b2f688e..d947045297 100644 --- a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js @@ -68,6 +68,10 @@ export const getInstanceFromScope = $$$hostConfig.getInstanceFromScope; export const getCurrentEventPriority = $$$hostConfig.getCurrentEventPriority; export const detachDeletedInstance = $$$hostConfig.detachDeletedInstance; export const requestPostPaintCallback = $$$hostConfig.requestPostPaintCallback; +export const shouldSuspendCommit = $$$hostConfig.shouldSuspendCommit; +export const startSuspendingCommit = $$$hostConfig.startSuspendingCommit; +export const suspendInstance = $$$hostConfig.suspendInstance; +export const waitForCommitToBeReady = $$$hostConfig.waitForCommitToBeReady; export const prepareRendererToRender = $$$hostConfig.prepareRendererToRender; export const resetRendererAfterRender = $$$hostConfig.resetRendererAfterRender; diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index 8d08e033fa..3c6ada072e 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -324,6 +324,18 @@ export function requestPostPaintCallback(callback: (time: number) => void) { // noop } +export function shouldSuspendCommit(type: Type, props: Props): boolean { + return false; +} + +export function startSuspendingCommit(): void {} + +export function suspendInstance(type: Type, props: Props): void {} + +export function waitForCommitToBeReady(): null { + return null; +} + export function prepareRendererToRender(container: Container): void { // noop }