From f1d80f6be84fa70507b1a44cbc81435a3d98111d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 15 Mar 2023 12:40:01 -0400 Subject: [PATCH] Feature: Suspend commit without blocking render 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. --- .../src/createReactNoop.js | 149 ++++++++++++++- .../src/ReactFiberCompleteWork.js | 139 +++++++++++++- .../react-reconciler/src/ReactFiberFlags.js | 7 +- .../react-reconciler/src/ReactFiberRoot.js | 1 + .../src/ReactFiberThenable.js | 42 +++++ .../react-reconciler/src/ReactFiberThrow.js | 58 ++++-- .../src/ReactFiberWorkLoop.js | 83 ++++++++- .../src/ReactInternalTypes.js | 4 + .../ReactSuspendedCommitPhase-test.js | 171 ++++++++++++++++++ 9 files changed, 620 insertions(+), 34 deletions(-) create mode 100644 packages/react-reconciler/src/__tests__/ReactSuspendedCommitPhase-test.js diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index cbe6cb12d5696..5b5f9abfedf9c 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 = { @@ -240,6 +241,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { hidden: !!newProps.hidden, context: instance.context, }; + + if (type === 'suspensey-thing' && typeof newProps.src === 'string') { + instance.src = newProps.src; + } + Object.defineProperty(clone, 'id', { value: clone.id, enumerable: false, @@ -273,6 +279,16 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return hostContext === UPPERCASE_CONTEXT ? rawText.toUpperCase() : rawText; } + type SuspenseyThingRecord = { + status: 'pending' | 'fulfilled', + listeners: Set<() => void> | null, + }; + + let trackedSuspendCommitPaylods: Map< + SuspenseyThingRecord, + 'pending' | 'fulfilled', + > | null = null; + const sharedHostConfig = { supportsSingletons: false, @@ -324,6 +340,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', { @@ -488,7 +509,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { type: string, newProps: Props, ): SuspendCommitPayload | null { - return null; + return shouldSuspendCommit(instance, type, newProps); }, shouldSuspendUpdate( @@ -497,12 +518,53 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { oldProps: Props, newProps: Props, ): SuspendCommitPayload | null { - return null; + return shouldSuspendCommit(instance, type, newProps); }, waitForCommitToBeReady( suspenseyThings: Array | null, - ): null { + ): ((commit: () => mixed) => () => void) | null { + if (suspenseyThings !== null) { + const subscribeToOnReady = commit => { + // Attach a listener to all the suspensey things. Once they've all + // fired, we can commit. Unless the commit is canceled. + + let didCancel = false; + let refCount = 0; + const onLoad = () => { + refCount--; + if (refCount === 0 && !didCancel) { + commit(); + } + }; + for (let i = 0; i < suspenseyThings.length; i++) { + const suspenseyThing = suspenseyThings[i]; + const record = trackedSuspendCommitPaylods.get(suspenseyThing); + if (record === undefined) { + throw new Error('Could not find record for key.'); + } + if (record.status === 'pending') { + refCount++; + if (record.listeners === null) { + record.listeners = []; + } + record.listeners.push(onLoad); + } + } + + if (refCount === 0) { + // Nothing is pending anymore. We can commit immediately. + return null; + } + + // React will call this if there's an interruption. + const cancelPendingCommit = () => { + didCancel = true; + }; + return cancelPendingCommit; + }; + return subscribeToOnReady; + } return null; }, @@ -510,6 +572,40 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { resetRendererAfterRender() {}, }; + function shouldSuspendCommit(instance: Instance, type: string, props: Props) { + if (type === 'suspensey-thing' && typeof props.src === 'string') { + if (trackedSuspendCommitPaylods === null) { + trackedSuspendCommitPaylods = new Map(); + } + const record = trackedSuspendCommitPaylods.get(props.src); + if (record === undefined) { + const newRecord: SuspendCommitPayload = { + status: 'pending', + listeners: null, + }; + trackedSuspendCommitPaylods.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 props.src; + } 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 nothing, like we do here. + return null; + } + } + } + // Don't need to suspend. + return null; + } + const hostConfig = useMutation ? { ...sharedHostConfig, @@ -534,6 +630,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'); @@ -715,6 +816,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; } @@ -941,6 +1045,45 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return getPendingChildrenAsJSX(container); }, + getSuspenseyThingStatus(src): string | null { + if (trackedSuspendCommitPaylods === null) { + return null; + } else { + const record = trackedSuspendCommitPaylods.get(src); + return record === undefined ? null : record.status; + } + }, + + async resolveSuspenseyThing(key: string): void { + if (trackedSuspendCommitPaylods === null) { + trackedSuspendCommitPaylods = new Map(); + } + const record = trackedSuspendCommitPaylods.get(key); + if (record === undefined) { + const newRecord: SuspendCommitPayload = { + status: 'fulfilled', + listeners: null, + }; + trackedSuspendCommitPaylods.set(key, newRecord); + } else { + if (record.status === 'pending') { + record.status = 'fulfilled'; + const listeners = record.listeners; + if (listeners !== null) { + record.listeners = null; + for (let i = 0; i < listeners.length; i++) { + const listener = listeners[i]; + listener(); + } + } + } + } + }, + + resetSuspenseyThingCache() { + trackedSuspendCommitPaylods = null; + }, + createPortal( children: ReactNodeList, container: Container, diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 11f00b9aa9b4c..b04bc036ed3f8 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -88,6 +88,7 @@ import { Incomplete, ShouldCapture, ForceClientRender, + SuspendCommit, } from './ReactFiberFlags'; import { @@ -109,6 +110,8 @@ import { finalizeContainerChildren, preparePortalMount, prepareScopeUpdate, + shouldSuspendMount, + shouldSuspendUpdate, } from './ReactFiberHostConfig'; import { getRootHostContainer, @@ -155,6 +158,8 @@ import { NoLanes, includesSomeLane, mergeLanes, + claimNextRetryLane, + includesOnlyNonUrgentLanes, } from './ReactFiberLane'; import {resetChildFibers} from './ReactChildFiber'; import {createScopeInstance} from './ReactFiberScope'; @@ -166,6 +171,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 @@ -409,6 +415,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 @@ -425,6 +432,15 @@ 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; + + suspendCommitOnHostUpdateIfNeeded( + instance, + type, + oldProps, + 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 @@ -483,6 +499,15 @@ function updateHostComponent( childrenUnchanged, recyclableInstance, ); + + suspendCommitOnHostUpdateIfNeeded( + newInstance, + type, + oldProps, + newProps, + renderLanes, + ); + if ( finalizeInitialChildren(newInstance, type, newProps, currentHostContext) ) { @@ -501,6 +526,70 @@ 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 suspendCommitOnHostMountIfNeeded( + instance: Instance, + type: string, + newProps: Props, + renderLanes: Lanes, +) { + // 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. + return; + } + // Ask the renderer if this instance should suspend the commit. + const suspenseyThing = shouldSuspendMount(instance, type, newProps); + if (suspenseyThing !== null) { + suspendCommit(suspenseyThing); + } +} + +// 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 suspendCommitOnHostUpdateIfNeeded( + instance: Instance, + type: string, + oldProps: Props, + newProps: Props, + renderLanes: Lanes, +) { + // 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. + return; + } + // Ask the renderer if this instance should suspend the commit. + const suspenseyThing = shouldSuspendUpdate( + instance, + type, + oldProps, + newProps, + ); + if (suspenseyThing !== null) { + suspendCommit(suspenseyThing); + } +} + function scheduleRetryEffect( workInProgress: Fiber, retryQueue: RetryQueue | null, @@ -510,6 +599,21 @@ 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 & SuspendCommit) { + workInProgress.lanes = mergeLanes( + workInProgress.lanes, + claimNextRetryLane(), + ); + } } } @@ -966,6 +1070,7 @@ function completeWork( workInProgress, workInProgress.type, workInProgress.pendingProps, + renderLanes, ); } bubbleProperties(workInProgress); @@ -979,7 +1084,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); @@ -1000,22 +1111,32 @@ 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); } + suspendCommitOnHostMountIfNeeded( + instance, + type, + newProps, + renderLanes, + ); + if (workInProgress.ref !== null) { // If there is a ref on a host node we need to schedule a callback markRef(workInProgress); @@ -1030,7 +1151,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); @@ -1055,6 +1182,7 @@ function completeWork( // or completeWork depending on whether we want to add them top->down or // bottom->up. Top->down is faster in IE11. const wasHydrated = popHydrationState(workInProgress); + let instance: Instance; if (wasHydrated) { // TODO: Move this and createInstance step into the beginPhase // to consolidate. @@ -1065,9 +1193,10 @@ function completeWork( // commit-phase we mark this as such. markUpdate(workInProgress); } + instance = workInProgress.stateNode; } else { const rootContainerInstance = getRootHostContainer(); - const instance = createInstance( + instance = createInstance( type, newProps, rootContainerInstance, @@ -1092,6 +1221,8 @@ function completeWork( } } + suspendCommitOnHostMountIfNeeded(instance, type, newProps, renderLanes); + if (workInProgress.ref !== null) { // If there is a ref on a host node we need to schedule a callback markRef(workInProgress); diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index f8df8f6f5968d..bc5973fb438b8 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -36,11 +36,16 @@ export const Passive = /* */ 0b000000000000000100000000000; export const Visibility = /* */ 0b000000000000010000000000000; export const StoreConsistency = /* */ 0b000000000000100000000000000; +// 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 SuspendCommit = 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 = /* */ 0b000000000000111111111111111; // These are not really side effects, but we still reuse this field. export const Incomplete = /* */ 0b000000000001000000000000000; diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 82f5655250084..c8df0a9e14673 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/ReactFiberThenable.js b/packages/react-reconciler/src/ReactFiberThenable.js index 81c59baf168d9..639c5e5c0960e 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.js +++ b/packages/react-reconciler/src/ReactFiberThenable.js @@ -14,6 +14,10 @@ import type { RejectedThenable, } from 'shared/ReactTypes'; +import type {SuspendCommitPayload} from './ReactFiberHostConfig'; + +import {shouldRemainOnPreviousScreen} from './ReactFiberWorkLoop'; + import ReactSharedInternals from 'shared/ReactSharedInternals'; const {ReactCurrentActQueue} = ReactSharedInternals; @@ -31,6 +35,14 @@ 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 noopSuspenseyResourceThenable = {then() {}}; + +let suspendCommitPayloads: Array | null = null; + 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 +152,36 @@ export function trackUsedThenable( } } +export function suspendCommit(suspenseyThing: SuspendCommitPayload): void { + // 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. Stash for later. This array will be passed + // back to the renderer via `suspendCommitIfNeeded` right before the + // commit phase. + if (suspendCommitPayloads === null) { + suspendCommitPayloads = [suspenseyThing]; + } else { + suspendCommitPayloads.push(suspenseyThing); + } + } else { + // We shouldn't block the commit. Activate a fallback at the nearest + // Suspense boundary. + suspendedThenable = noopSuspenseyResourceThenable; + throw SuspenseException; + } +} + +export function getSuspendCommitPayloadsCollectedDuringRender(): Array | null { + const things = suspendCommitPayloads; + suspendCommitPayloads = null; + return things; +} + +export function resetSuspendCommitPayloadsCollectedDuringRender(): void { + suspendCommitPayloads = null; +} + // 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 b1b4c4afa4aa6..8e829a4a5f79a 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -34,6 +34,7 @@ import { LifecycleEffectMask, ForceUpdateForLegacySuspense, ForceClientRender, + SuspendCommit, } from './ReactFiberFlags'; import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode'; import { @@ -80,6 +81,7 @@ import { queueHydrationError, } from './ReactFiberHydrationContext'; import {ConcurrentRoot} from './ReactRootTags'; +import {noopSuspenseyResourceThenable} from './ReactFiberThenable'; function createRootErrorUpdate( fiber: Fiber, @@ -413,33 +415,53 @@ function throwException( // // When the wakeable resolves, we'll attempt to render the boundary // again ("retry"). - const retryQueue: RetryQueue | null = - (suspenseBoundary.updateQueue: any); - if (retryQueue === 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 === noopSuspenseyResourceThenable; + if (isSuspenseyResource) { + suspenseBoundary.flags |= SuspendCommit; } else { - retryQueue.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, - retryQueue: new Set([wakeable]), - }; - suspenseBoundary.updateQueue = newOffscreenQueue; + const isSuspenseyResource = + wakeable === noopSuspenseyResourceThenable; + if (isSuspenseyResource) { + suspenseBoundary.flags |= SuspendCommit; } else { - const retryQueue = offscreenQueue.retryQueue; - if (retryQueue === null) { - offscreenQueue.retryQueue = 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 { - retryQueue.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 82a476c5a02b8..d9be521227e47 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -84,6 +84,7 @@ import { scheduleMicrotask, prepareRendererToRender, resetRendererAfterRender, + waitForCommitToBeReady, } from './ReactFiberHostConfig'; import { @@ -161,6 +162,7 @@ import { movePendingFibersToMemoized, addTransitionToLanesMap, getTransitionsForLanes, + includesOnlyNonUrgentLanes, } from './ReactFiberLane'; import { DiscreteEventPriority, @@ -271,6 +273,8 @@ import { SuspenseException, getSuspendedThenable, isThenableResolved, + getSuspendCommitPayloadsCollectedDuringRender, + resetSuspendCommitPayloadsCollectedDuringRender, } from './ReactFiberThenable'; import {schedulePostPaintCallback} from './ReactPostPaintCallback'; import { @@ -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); @@ -1276,10 +1292,11 @@ 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, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ); break; } @@ -1310,11 +1327,12 @@ 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, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ), msUntilTimeout, ); @@ -1322,10 +1340,11 @@ function finishConcurrentRender( } } // The work expired. Commit immediately. - commitRoot( + commitRootWhenReady( root, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ); break; } @@ -1357,11 +1376,12 @@ function finishConcurrentRender( // Instead of committing the fallback immediately, wait for more data // to arrive. root.timeoutHandle = scheduleTimeout( - commitRoot.bind( + commitRootWhenReady.bind( null, root, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ), msUntilTimeout, ); @@ -1370,19 +1390,21 @@ function finishConcurrentRender( } // Commit the placeholder. - commitRoot( + commitRootWhenReady( root, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ); break; } case RootCompleted: { - // The work completed. Ready to commit. - commitRoot( + // The work completed. + commitRootWhenReady( root, workInProgressRootRecoverableErrors, workInProgressTransitions, + lanes, ); break; } @@ -1392,6 +1414,44 @@ function finishConcurrentRender( } } +function commitRootWhenReady( + root: FiberRoot, + 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. + const suspenseyThings = getSuspendCommitPayloadsCollectedDuringRender(); + const schedulePendingCommit = waitForCommitToBeReady(suspenseyThings); + 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 @@ -1736,6 +1796,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; @@ -1754,6 +1819,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { workInProgressRootConcurrentErrors = null; workInProgressRootRecoverableErrors = null; + resetSuspendCommitPayloadsCollectedDuringRender(); finishQueueingConcurrentUpdates(); if (__DEV__) { @@ -1890,7 +1956,7 @@ function handleThrow(root: FiberRoot, thrownValue: any): void { } } -function shouldRemainOnPreviousScreen() { +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 @@ -2720,6 +2786,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 edc120182e958..7f01daa5240da 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__/ReactSuspendedCommitPhase-test.js b/packages/react-reconciler/src/__tests__/ReactSuspendedCommitPhase-test.js new file mode 100644 index 0000000000000..b755887cc5683 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactSuspendedCommitPhase-test.js @@ -0,0 +1,171 @@ +let React; +let startTransition; +let ReactNoop; +let resolveSuspenseyThing; +let getSuspenseyThingStatus; +let Suspense; +let Scheduler; +let act; +let assertLog; + +describe('ReactSuspendedCommitPhase', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + Suspense = React.Suspense; + 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( + }> + + , + ); + }); + // The image wasn't loaded into the cache, but we committed the tree anyway + // because the update did not happen during a transition. + expect(getSuspenseyThingStatus('A')).toBe(null); + 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'); + }); +});