diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 4c96207bd6696..71a1e973c3af8 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -221,7 +221,11 @@ function getHighestPriorityLanes(lanes: Lanes | Lane): Lanes { } } -export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { +export function getNextLanes( + root: FiberRoot, + wipLanes: Lanes, + rootHasPendingCommit: boolean, +): Lanes { // Early bailout if there's no pending work left. const pendingLanes = root.pendingLanes; if (pendingLanes === NoLanes) { @@ -246,16 +250,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { // a brief amount of time (i.e. below the "Just Noticeable Difference" // threshold). // - // TODO: finishedLanes is also set when a Suspensey resource, like CSS or - // images, suspends during the commit phase. (We could detect that here by - // checking for root.cancelPendingCommit.) These are also expected to resolve - // quickly, because of preloading, but theoretically they could block forever - // like in a normal "suspend indefinitely" scenario. In the future, we should - // consider only blocking for up to some time limit before discarding the - // commit in favor of prerendering. If we do discard a pending commit, then - // the commit phase callback should act as a ping to try the original - // render again. - const rootHasPendingCommit = root.finishedLanes !== NoLanes; // Do not work on any idle work until all the non-idle work has finished, // even if the work is suspended. diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 29bc7923d893e..4971bb4c2be34 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -61,7 +61,6 @@ function FiberRootNode( this.pendingChildren = null; this.current = null; this.pingCache = null; - this.finishedWork = null; this.timeoutHandle = noTimeout; this.cancelPendingCommit = null; this.context = null; @@ -76,7 +75,6 @@ function FiberRootNode( this.pingedLanes = NoLanes; this.warmLanes = NoLanes; this.expiredLanes = NoLanes; - this.finishedLanes = NoLanes; this.errorRecoveryDisabledLanes = NoLanes; this.shellSuspendCounter = 0; diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index e3791f4c8dcff..d57458fdbf41c 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -69,6 +69,7 @@ import { scheduleMicrotask, shouldAttemptEagerTransition, trackSchedulerEvent, + noTimeout, } from './ReactFiberConfig'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -207,11 +208,15 @@ function flushSyncWorkAcrossRoots_impl( const workInProgressRoot = getWorkInProgressRoot(); const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes(); + const rootHasPendingCommit = + root.cancelPendingCommit !== null || + root.timeoutHandle !== noTimeout; const nextLanes = getNextLanes( root, root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes, + rootHasPendingCommit, ); if ( includesSyncLane(nextLanes) && @@ -335,6 +340,8 @@ function scheduleTaskForRootDuringMicrotask( const pendingPassiveEffectsLanes = getPendingPassiveEffectsLanes(); const workInProgressRoot = getWorkInProgressRoot(); const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes(); + const rootHasPendingCommit = + root.cancelPendingCommit !== null || root.timeoutHandle !== noTimeout; const nextLanes = enableYieldingBeforePassive && root === rootWithPendingPassiveEffects ? // This will schedule the callback at the priority of the lane but we used to @@ -345,6 +352,7 @@ function scheduleTaskForRootDuringMicrotask( : getNextLanes( root, root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes, + rootHasPendingCommit, ); const existingCallbackNode = root.callbackNode; @@ -488,9 +496,12 @@ function performWorkOnRootViaSchedulerTask( // it's available). const workInProgressRoot = getWorkInProgressRoot(); const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes(); + const rootHasPendingCommit = + root.cancelPendingCommit !== null || root.timeoutHandle !== noTimeout; const lanes = getNextLanes( root, root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes, + rootHasPendingCommit, ); if (lanes === NoLanes) { // No more work on this root. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 26544788f1508..ce253f5436cea 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -14,7 +14,6 @@ import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane'; import type {SuspenseState} from './ReactFiberSuspenseComponent'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks'; -import type {EventPriority} from './ReactEventPriorities'; import type { PendingTransitionCallbacks, PendingBoundaries, @@ -1240,16 +1239,12 @@ function finishConcurrentRender( } } - // Only set these if we have a complete tree that is ready to be committed. - // We use these fields to determine later whether or not the work should be - // discarded for a fresh render attempt. - root.finishedWork = finishedWork; - root.finishedLanes = lanes; - if (shouldForceFlushFallbacksInDEV()) { // We're inside an `act` scope. Commit immediately. commitRoot( root, + finishedWork, + lanes, workInProgressRootRecoverableErrors, workInProgressTransitions, workInProgressRootDidIncludeRecursiveRenderUpdate, @@ -1282,7 +1277,7 @@ function finishConcurrentRender( didAttemptEntireTree, ); - const nextLanes = getNextLanes(root, NoLanes); + const nextLanes = getNextLanes(root, NoLanes, true); if (nextLanes !== NoLanes) { // There's additional work we can do on this root. We might as well // attempt to work on that while we're suspended. @@ -1352,6 +1347,8 @@ function commitRootWhenReady( completedRenderStartTime: number, // Profiling-only completedRenderEndTime: number, // Profiling-only ) { + root.timeoutHandle = noTimeout; + // TODO: Combine retry throttling with Suspensey commits. Right now they run // one after the other. const BothVisibilityAndMaySuspendCommit = Visibility | MaySuspendCommit; @@ -1385,6 +1382,8 @@ function commitRootWhenReady( commitRoot.bind( null, root, + finishedWork, + lanes, recoverableErrors, transitions, didIncludeRenderPhaseUpdate, @@ -1406,6 +1405,8 @@ function commitRootWhenReady( // Otherwise, commit immediately.; commitRoot( root, + finishedWork, + lanes, recoverableErrors, transitions, didIncludeRenderPhaseUpdate, @@ -1843,9 +1844,6 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { } } - root.finishedWork = null; - root.finishedLanes = NoLanes; - const timeoutHandle = root.timeoutHandle; if (timeoutHandle !== noTimeout) { // The root previous suspended and scheduled a timeout to commit a fallback @@ -3129,6 +3127,8 @@ const THROTTLED_COMMIT = 2; function commitRoot( root: FiberRoot, + finishedWork: null | Fiber, + lanes: Lanes, recoverableErrors: null | Array>, transitions: Array | null, didIncludeRenderPhaseUpdate: boolean, @@ -3139,48 +3139,9 @@ function commitRoot( suspendedCommitReason: SuspendedCommitReason, // Profiling-only completedRenderStartTime: number, // Profiling-only completedRenderEndTime: number, // Profiling-only -) { - // TODO: This no longer makes any sense. We already wrap the mutation and - // layout phases. Should be able to remove. - const prevTransition = ReactSharedInternals.T; - const previousUpdateLanePriority = getCurrentUpdatePriority(); - try { - setCurrentUpdatePriority(DiscreteEventPriority); - ReactSharedInternals.T = null; - commitRootImpl( - root, - recoverableErrors, - transitions, - didIncludeRenderPhaseUpdate, - previousUpdateLanePriority, - spawnedLane, - updatedLanes, - suspendedRetryLanes, - exitStatus, - suspendedCommitReason, - completedRenderStartTime, - completedRenderEndTime, - ); - } finally { - ReactSharedInternals.T = prevTransition; - setCurrentUpdatePriority(previousUpdateLanePriority); - } -} +): void { + root.cancelPendingCommit = null; -function commitRootImpl( - root: FiberRoot, - recoverableErrors: null | Array>, - transitions: Array | null, - didIncludeRenderPhaseUpdate: boolean, - renderPriorityLevel: EventPriority, - spawnedLane: Lane, - updatedLanes: Lanes, - suspendedRetryLanes: Lanes, - exitStatus: RootExitStatus, // Profiling-only - suspendedCommitReason: SuspendedCommitReason, // Profiling-only - completedRenderStartTime: number, // Profiling-only - completedRenderEndTime: number, // Profiling-only -) { do { // `flushPassiveEffects` will call `flushSyncUpdateQueue` at the end, which // means `flushPassiveEffects` will sometimes result in additional @@ -3196,9 +3157,6 @@ function commitRootImpl( throw new Error('Should not already be working.'); } - const finishedWork = root.finishedWork; - const lanes = root.finishedLanes; - if (enableProfilerTimer && enableComponentPerformanceTrack) { // Log the previous render phase once we commit. I.e. we weren't interrupted. setCurrentTrackFromLanes(lanes); @@ -3234,19 +3192,17 @@ function commitRootImpl( if (enableSchedulingProfiler) { markCommitStopped(); } - return null; + return; } else { if (__DEV__) { if (lanes === NoLanes) { console.error( - 'root.finishedLanes should not be empty during a commit. This is a ' + + 'finishedLanes should not be empty during a commit. This is a ' + 'bug in React.', ); } } } - root.finishedWork = null; - root.finishedLanes = NoLanes; if (finishedWork === root.current) { throw new Error( @@ -3292,7 +3248,6 @@ function commitRootImpl( // might get scheduled in the commit phase. (See #16714.) // TODO: Delete all other places that schedule the passive effect callback // They're redundant. - let rootDoesHavePassiveEffects: boolean = false; if ( // If this subtree rendered with profiling this commit, we need to visit it to log it. (enableProfilerTimer && @@ -3301,7 +3256,6 @@ function commitRootImpl( (finishedWork.subtreeFlags & PassiveMask) !== NoFlags || (finishedWork.flags & PassiveMask) !== NoFlags ) { - rootDoesHavePassiveEffects = true; pendingPassiveEffectsRemainingLanes = remainingLanes; pendingPassiveEffectsRenderEndTime = completedRenderEndTime; // workInProgressTransitions might be overwritten, so we want @@ -3319,7 +3273,6 @@ 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; scheduleCallback(NormalSchedulerPriority, () => { if (enableProfilerTimer && enableComponentPerformanceTrack) { // Track the currently executing event if there is one so we can ignore this @@ -3338,7 +3291,6 @@ function commitRootImpl( // so we can clear the callback now. root.callbackNode = null; root.callbackPriority = NoLane; - root.cancelPendingCommit = null; } if (enableProfilerTimer) { @@ -3355,79 +3307,136 @@ function commitRootImpl( } } + // The commit phase is broken into several sub-phases. We do a separate pass + // of the effect list for each phase: all mutation effects come before all + // layout effects, and so on. + // Check if there are any effects in the whole tree. // TODO: This is left over from the effect list implementation, where we had // to check for the existence of `firstEffect` to satisfy Flow. I think the // only other reason this optimization exists is because it affects profiling. // Reconsider whether this is necessary. - const subtreeHasEffects = - (finishedWork.subtreeFlags & - (BeforeMutationMask | MutationMask | LayoutMask | PassiveMask)) !== - NoFlags; - const rootHasEffect = - (finishedWork.flags & - (BeforeMutationMask | MutationMask | LayoutMask | PassiveMask)) !== + const subtreeHasBeforeMutationEffects = + (finishedWork.subtreeFlags & (BeforeMutationMask | MutationMask)) !== NoFlags; + const rootHasBeforeMutationEffect = + (finishedWork.flags & (BeforeMutationMask | MutationMask)) !== NoFlags; - if (subtreeHasEffects || rootHasEffect) { + if (subtreeHasBeforeMutationEffects || rootHasBeforeMutationEffect) { const prevTransition = ReactSharedInternals.T; ReactSharedInternals.T = null; const previousPriority = getCurrentUpdatePriority(); setCurrentUpdatePriority(DiscreteEventPriority); - const prevExecutionContext = executionContext; executionContext |= CommitContext; + try { + // The first phase a "before mutation" phase. We use this phase to read the + // state of the host tree right before we mutate it. This is where + // getSnapshotBeforeUpdate is called. + commitBeforeMutationEffects(root, finishedWork); + } finally { + // Reset the priority to the previous non-sync value. + executionContext = prevExecutionContext; + setCurrentUpdatePriority(previousPriority); + ReactSharedInternals.T = prevTransition; + } + } + flushMutationEffects(root, finishedWork, lanes); + flushLayoutEffects( + root, + finishedWork, + lanes, + recoverableErrors, + didIncludeRenderPhaseUpdate, + suspendedCommitReason, + completedRenderEndTime, + ); +} - // The commit phase is broken into several sub-phases. We do a separate pass - // of the effect list for each phase: all mutation effects come before all - // layout effects, and so on. - - // The first phase a "before mutation" phase. We use this phase to read the - // state of the host tree right before we mutate it. This is where - // getSnapshotBeforeUpdate is called. - commitBeforeMutationEffects(root, finishedWork); +function flushMutationEffects( + root: FiberRoot, + finishedWork: Fiber, + lanes: Lanes, +): void { + const subtreeMutationHasEffects = + (finishedWork.subtreeFlags & MutationMask) !== NoFlags; + const rootMutationHasEffect = (finishedWork.flags & MutationMask) !== NoFlags; - // The next phase is the mutation phase, where we mutate the host tree. - commitMutationEffects(root, finishedWork, lanes); + if (subtreeMutationHasEffects || rootMutationHasEffect) { + const prevTransition = ReactSharedInternals.T; + ReactSharedInternals.T = null; + const previousPriority = getCurrentUpdatePriority(); + setCurrentUpdatePriority(DiscreteEventPriority); + const prevExecutionContext = executionContext; + executionContext |= CommitContext; + try { + // The next phase is the mutation phase, where we mutate the host tree. + commitMutationEffects(root, finishedWork, lanes); - if (enableCreateEventHandleAPI) { - if (shouldFireAfterActiveInstanceBlur) { - afterActiveInstanceBlur(); + if (enableCreateEventHandleAPI) { + if (shouldFireAfterActiveInstanceBlur) { + afterActiveInstanceBlur(); + } } + resetAfterCommit(root.containerInfo); + } finally { + // Reset the priority to the previous non-sync value. + executionContext = prevExecutionContext; + setCurrentUpdatePriority(previousPriority); + ReactSharedInternals.T = prevTransition; } - resetAfterCommit(root.containerInfo); - - // The work-in-progress tree is now the current tree. This must come after - // the mutation phase, so that the previous tree is still current during - // componentWillUnmount, but before the layout phase, so that the finished - // work is current during componentDidMount/Update. - root.current = finishedWork; - - // The next phase is the layout phase, where we call effects that read - // the host tree after it's been mutated. The idiomatic use case for this is - // layout, but class component lifecycles also fire here for legacy reasons. - if (enableSchedulingProfiler) { - markLayoutEffectsStarted(lanes); - } - commitLayoutEffects(finishedWork, root, lanes); - if (enableSchedulingProfiler) { - markLayoutEffectsStopped(); - } + } - // Tell Scheduler to yield at the end of the frame, so the browser has an - // opportunity to paint. - requestPaint(); + // The work-in-progress tree is now the current tree. This must come after + // the mutation phase, so that the previous tree is still current during + // componentWillUnmount, but before the layout phase, so that the finished + // work is current during componentDidMount/Update. + root.current = finishedWork; +} - executionContext = prevExecutionContext; +function flushLayoutEffects( + root: FiberRoot, + finishedWork: Fiber, + lanes: Lanes, + recoverableErrors: null | Array>, + didIncludeRenderPhaseUpdate: boolean, + suspendedCommitReason: SuspendedCommitReason, // Profiling-only + completedRenderEndTime: number, // Profiling-only +): void { + const subtreeHasLayoutEffects = + (finishedWork.subtreeFlags & LayoutMask) !== NoFlags; + const rootHasLayoutEffect = (finishedWork.flags & LayoutMask) !== NoFlags; - // Reset the priority to the previous non-sync value. - setCurrentUpdatePriority(previousPriority); - ReactSharedInternals.T = prevTransition; - } else { - // No effects. - root.current = finishedWork; + if (subtreeHasLayoutEffects || rootHasLayoutEffect) { + const prevTransition = ReactSharedInternals.T; + ReactSharedInternals.T = null; + const previousPriority = getCurrentUpdatePriority(); + setCurrentUpdatePriority(DiscreteEventPriority); + const prevExecutionContext = executionContext; + executionContext |= CommitContext; + try { + // The next phase is the layout phase, where we call effects that read + // the host tree after it's been mutated. The idiomatic use case for this is + // layout, but class component lifecycles also fire here for legacy reasons. + if (enableSchedulingProfiler) { + markLayoutEffectsStarted(lanes); + } + commitLayoutEffects(finishedWork, root, lanes); + if (enableSchedulingProfiler) { + markLayoutEffectsStopped(); + } + } finally { + // Reset the priority to the previous non-sync value. + executionContext = prevExecutionContext; + setCurrentUpdatePriority(previousPriority); + ReactSharedInternals.T = prevTransition; + } } + // Tell Scheduler to yield at the end of the frame, so the browser has an + // opportunity to paint. + requestPaint(); + if (enableProfilerTimer && enableComponentPerformanceTrack) { recordCommitEndTime(); logCommitPhase( @@ -3439,18 +3448,22 @@ function commitRootImpl( ); } - const rootDidHavePassiveEffects = rootDoesHavePassiveEffects; + const rootDidHavePassiveEffects = // If this subtree rendered with profiling this commit, we need to visit it to log it. + (enableProfilerTimer && + enableComponentPerformanceTrack && + finishedWork.actualDuration !== 0) || + (finishedWork.subtreeFlags & PassiveMask) !== NoFlags || + (finishedWork.flags & PassiveMask) !== NoFlags; - if (rootDoesHavePassiveEffects) { + if (rootDidHavePassiveEffects) { // This commit has passive effects. Stash a reference to them. But don't // schedule a callback until after flushing layout work. - rootDoesHavePassiveEffects = false; rootWithPendingPassiveEffects = root; pendingPassiveEffectsLanes = lanes; } else { // There were no passive effects, so we can immediately release the cache // pool for this render. - releaseRootPooledCache(root, remainingLanes); + releaseRootPooledCache(root, root.pendingLanes); if (__DEV__) { nestedPassiveUpdateCount = 0; rootWithPassiveNestedUpdates = null; @@ -3458,7 +3471,7 @@ function commitRootImpl( } // Read this again, since an effect might have updated it - remainingLanes = root.pendingLanes; + let remainingLanes = root.pendingLanes; // Check if there's remaining work on this root // TODO: This is part of the `componentDidCatch` implementation. Its purpose @@ -3482,7 +3495,8 @@ function commitRootImpl( } } - onCommitRootDevTools(finishedWork.stateNode, renderPriorityLevel); + const renderPriority = lanesToEventPriority(lanes); + onCommitRootDevTools(finishedWork.stateNode, renderPriority); if (enableUpdaterTracking) { if (isDevToolsPresent) { @@ -3495,22 +3509,31 @@ function commitRootImpl( } if (recoverableErrors !== null) { - // There were errors during this render, but recovered from them without - // needing to surface it to the UI. We log them here. - const onRecoverableError = root.onRecoverableError; - for (let i = 0; i < recoverableErrors.length; i++) { - const recoverableError = recoverableErrors[i]; - const errorInfo = makeErrorInfo(recoverableError.stack); - if (__DEV__) { - runWithFiberInDEV( - recoverableError.source, - onRecoverableError, - recoverableError.value, - errorInfo, - ); - } else { - onRecoverableError(recoverableError.value, errorInfo); + const prevTransition = ReactSharedInternals.T; + const previousUpdateLanePriority = getCurrentUpdatePriority(); + setCurrentUpdatePriority(DiscreteEventPriority); + ReactSharedInternals.T = null; + try { + // There were errors during this render, but recovered from them without + // needing to surface it to the UI. We log them here. + const onRecoverableError = root.onRecoverableError; + for (let i = 0; i < recoverableErrors.length; i++) { + const recoverableError = recoverableErrors[i]; + const errorInfo = makeErrorInfo(recoverableError.stack); + if (__DEV__) { + runWithFiberInDEV( + recoverableError.source, + onRecoverableError, + recoverableError.value, + errorInfo, + ); + } else { + onRecoverableError(recoverableError.value, errorInfo); + } } + } finally { + ReactSharedInternals.T = prevTransition; + setCurrentUpdatePriority(previousUpdateLanePriority); } } @@ -3610,8 +3633,6 @@ function commitRootImpl( }); } } - - return null; } function makeErrorInfo(componentStack: ?string) { @@ -3705,7 +3726,6 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { // We've finished our work for this render pass. root.callbackNode = null; root.callbackPriority = NoLane; - root.cancelPendingCommit = null; } if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index afe853202f141..859da37cadf32 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -220,8 +220,6 @@ type BaseFiberRootProperties = { pingCache: WeakMap> | Map> | null, - // A finished work-in-progress HostRoot that's ready to be committed. - finishedWork: Fiber | null, // Timeout handle returned by setTimeout. Used to cancel a pending timeout, if // it's superseded by a new one. timeoutHandle: TimeoutHandle | NoTimeout, @@ -252,8 +250,6 @@ type BaseFiberRootProperties = { errorRecoveryDisabledLanes: Lanes, shellSuspendCounter: number, - finishedLanes: Lanes, - entangledLanes: Lanes, entanglements: LaneMap, diff --git a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js index bbd01cd551e86..f57fc0ac0239a 100644 --- a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js +++ b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js @@ -744,6 +744,7 @@ describe('ReactDeferredValue', () => { , ); // We should switch to pre-rendering the new preview. + await waitForPaint([]); await waitForPaint(['Preview [B]']); expect(root).toMatchRenderedOutput();