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/ReactFiberHostConfigWithNoSingletons.js b/packages/react-reconciler/src/ReactFiberHostConfigWithNoSingletons.js index f8cf62243f41f..f592e9518591f 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/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'); + }); +});