From 423d6f571523d8d59777a6ebb58edada3fad7b0d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 9 Jul 2021 15:56:40 -0400 Subject: [PATCH 1/2] Extract early bailout to separate function This block is getting hard to read so I moved it to a separate function. I'm about to refactor the logic that wraps around this path. Ideally this early bailout path would happen before the begin phase phase. Perhaps during reconcilation of the parent fiber's children. --- .../src/ReactFiberBeginWork.new.js | 426 +++++++++--------- .../src/ReactFiberBeginWork.old.js | 426 +++++++++--------- 2 files changed, 428 insertions(+), 424 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 50a5a81a1f099..d15a6d8852880 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -3214,6 +3214,215 @@ function remountFiber( } } +function attemptEarlyBailoutIfNoScheduledUpdate( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, +) { + // This fiber does not have any pending work. Bailout without entering + // the begin phase. There's still some bookkeeping we that needs to be done + // in this optimized path, mostly pushing stuff onto the stack. + switch (workInProgress.tag) { + case HostRoot: + pushHostRootContext(workInProgress); + if (enableCache) { + const root: FiberRoot = workInProgress.stateNode; + const cache: Cache = current.memoizedState.cache; + pushCacheProvider(workInProgress, cache); + pushRootCachePool(root); + } + resetHydrationState(); + break; + case HostComponent: + pushHostContext(workInProgress); + break; + case ClassComponent: { + const Component = workInProgress.type; + if (isLegacyContextProvider(Component)) { + pushLegacyContextProvider(workInProgress); + } + break; + } + case HostPortal: + pushHostContainer(workInProgress, workInProgress.stateNode.containerInfo); + break; + case ContextProvider: { + const newValue = workInProgress.memoizedProps.value; + const context: ReactContext = workInProgress.type._context; + pushProvider(workInProgress, context, newValue); + break; + } + case Profiler: + if (enableProfilerTimer) { + // Profiler should only call onRender when one of its descendants actually rendered. + const hasChildWork = includesSomeLane( + renderLanes, + workInProgress.childLanes, + ); + if (hasChildWork) { + workInProgress.flags |= Update; + } + + if (enableProfilerCommitHooks) { + // Reset effect durations for the next eventual effect phase. + // These are reset during render to allow the DevTools commit hook a chance to read them, + const stateNode = workInProgress.stateNode; + stateNode.effectDuration = 0; + stateNode.passiveEffectDuration = 0; + } + } + break; + case SuspenseComponent: { + const state: SuspenseState | null = workInProgress.memoizedState; + if (state !== null) { + if (enableSuspenseServerRenderer) { + if (state.dehydrated !== null) { + pushSuspenseContext( + workInProgress, + setDefaultShallowSuspenseContext(suspenseStackCursor.current), + ); + // We know that this component will suspend again because if it has + // been unsuspended it has committed as a resolved Suspense component. + // If it needs to be retried, it should have work scheduled on it. + workInProgress.flags |= DidCapture; + // We should never render the children of a dehydrated boundary until we + // upgrade it. We return null instead of bailoutOnAlreadyFinishedWork. + return null; + } + } + + // If this boundary is currently timed out, we need to decide + // whether to retry the primary children, or to skip over it and + // go straight to the fallback. Check the priority of the primary + // child fragment. + const primaryChildFragment: Fiber = (workInProgress.child: any); + const primaryChildLanes = primaryChildFragment.childLanes; + if (includesSomeLane(renderLanes, primaryChildLanes)) { + // The primary children have pending work. Use the normal path + // to attempt to render the primary children again. + return updateSuspenseComponent(current, workInProgress, renderLanes); + } else { + // The primary child fragment does not have pending work marked + // on it + pushSuspenseContext( + workInProgress, + setDefaultShallowSuspenseContext(suspenseStackCursor.current), + ); + // The primary children do not have pending work with sufficient + // priority. Bailout. + const child = bailoutOnAlreadyFinishedWork( + current, + workInProgress, + renderLanes, + ); + if (child !== null) { + // The fallback children have pending work. Skip over the + // primary children and work on the fallback. + return child.sibling; + } else { + // Note: We can return `null` here because we already checked + // whether there were nested context consumers, via the call to + // `bailoutOnAlreadyFinishedWork` above. + return null; + } + } + } else { + pushSuspenseContext( + workInProgress, + setDefaultShallowSuspenseContext(suspenseStackCursor.current), + ); + } + break; + } + case SuspenseListComponent: { + const didSuspendBefore = (current.flags & DidCapture) !== NoFlags; + + let hasChildWork = includesSomeLane( + renderLanes, + workInProgress.childLanes, + ); + + if (enableLazyContextPropagation && !hasChildWork) { + // Context changes may not have been propagated yet. We need to do + // that now, before we can decide whether to bail out. + // TODO: We use `childLanes` as a heuristic for whether there is + // remaining work in a few places, including + // `bailoutOnAlreadyFinishedWork` and + // `updateDehydratedSuspenseComponent`. We should maybe extract this + // into a dedicated function. + lazilyPropagateParentContextChanges( + current, + workInProgress, + renderLanes, + ); + hasChildWork = includesSomeLane(renderLanes, workInProgress.childLanes); + } + + if (didSuspendBefore) { + if (hasChildWork) { + // If something was in fallback state last time, and we have all the + // same children then we're still in progressive loading state. + // Something might get unblocked by state updates or retries in the + // tree which will affect the tail. So we need to use the normal + // path to compute the correct tail. + return updateSuspenseListComponent( + current, + workInProgress, + renderLanes, + ); + } + // If none of the children had any work, that means that none of + // them got retried so they'll still be blocked in the same way + // as before. We can fast bail out. + workInProgress.flags |= DidCapture; + } + + // If nothing suspended before and we're rendering the same children, + // then the tail doesn't matter. Anything new that suspends will work + // in the "together" mode, so we can continue from the state we had. + const renderState = workInProgress.memoizedState; + if (renderState !== null) { + // Reset to the "together" mode in case we've started a different + // update in the past but didn't complete it. + renderState.rendering = null; + renderState.tail = null; + renderState.lastEffect = null; + } + pushSuspenseContext(workInProgress, suspenseStackCursor.current); + + if (hasChildWork) { + break; + } else { + // If none of the children had any work, that means that none of + // them got retried so they'll still be blocked in the same way + // as before. We can fast bail out. + return null; + } + } + case OffscreenComponent: + case LegacyHiddenComponent: { + // Need to check if the tree still needs to be deferred. This is + // almost identical to the logic used in the normal update path, + // so we'll just enter that. The only difference is we'll bail out + // at the next level instead of this one, because the child props + // have not changed. Which is fine. + // TODO: Probably should refactor `beginWork` to split the bailout + // path from the normal path. I'm tempted to do a labeled break here + // but I won't :) + workInProgress.lanes = NoLanes; + return updateOffscreenComponent(current, workInProgress, renderLanes); + } + case CacheComponent: { + if (enableCache) { + const cache: Cache = current.memoizedState.cache; + pushCacheProvider(workInProgress, cache); + } + break; + } + } + return bailoutOnAlreadyFinishedWork(current, workInProgress, renderLanes); +} + function beginWork( current: Fiber | null, workInProgress: Fiber, @@ -3265,218 +3474,11 @@ function beginWork( didReceiveUpdate = true; } else if (!includesSomeLane(renderLanes, updateLanes)) { didReceiveUpdate = false; - // This fiber does not have any pending work. Bailout without entering - // the begin phase. There's still some bookkeeping we that needs to be done - // in this optimized path, mostly pushing stuff onto the stack. - switch (workInProgress.tag) { - case HostRoot: - pushHostRootContext(workInProgress); - if (enableCache) { - const root: FiberRoot = workInProgress.stateNode; - const cache: Cache = current.memoizedState.cache; - pushCacheProvider(workInProgress, cache); - pushRootCachePool(root); - } - resetHydrationState(); - break; - case HostComponent: - pushHostContext(workInProgress); - break; - case ClassComponent: { - const Component = workInProgress.type; - if (isLegacyContextProvider(Component)) { - pushLegacyContextProvider(workInProgress); - } - break; - } - case HostPortal: - pushHostContainer( - workInProgress, - workInProgress.stateNode.containerInfo, - ); - break; - case ContextProvider: { - const newValue = workInProgress.memoizedProps.value; - const context: ReactContext = workInProgress.type._context; - pushProvider(workInProgress, context, newValue); - break; - } - case Profiler: - if (enableProfilerTimer) { - // Profiler should only call onRender when one of its descendants actually rendered. - const hasChildWork = includesSomeLane( - renderLanes, - workInProgress.childLanes, - ); - if (hasChildWork) { - workInProgress.flags |= Update; - } - - if (enableProfilerCommitHooks) { - // Reset effect durations for the next eventual effect phase. - // These are reset during render to allow the DevTools commit hook a chance to read them, - const stateNode = workInProgress.stateNode; - stateNode.effectDuration = 0; - stateNode.passiveEffectDuration = 0; - } - } - break; - case SuspenseComponent: { - const state: SuspenseState | null = workInProgress.memoizedState; - if (state !== null) { - if (enableSuspenseServerRenderer) { - if (state.dehydrated !== null) { - pushSuspenseContext( - workInProgress, - setDefaultShallowSuspenseContext(suspenseStackCursor.current), - ); - // We know that this component will suspend again because if it has - // been unsuspended it has committed as a resolved Suspense component. - // If it needs to be retried, it should have work scheduled on it. - workInProgress.flags |= DidCapture; - // We should never render the children of a dehydrated boundary until we - // upgrade it. We return null instead of bailoutOnAlreadyFinishedWork. - return null; - } - } - - // If this boundary is currently timed out, we need to decide - // whether to retry the primary children, or to skip over it and - // go straight to the fallback. Check the priority of the primary - // child fragment. - const primaryChildFragment: Fiber = (workInProgress.child: any); - const primaryChildLanes = primaryChildFragment.childLanes; - if (includesSomeLane(renderLanes, primaryChildLanes)) { - // The primary children have pending work. Use the normal path - // to attempt to render the primary children again. - return updateSuspenseComponent( - current, - workInProgress, - renderLanes, - ); - } else { - // The primary child fragment does not have pending work marked - // on it - pushSuspenseContext( - workInProgress, - setDefaultShallowSuspenseContext(suspenseStackCursor.current), - ); - // The primary children do not have pending work with sufficient - // priority. Bailout. - const child = bailoutOnAlreadyFinishedWork( - current, - workInProgress, - renderLanes, - ); - if (child !== null) { - // The fallback children have pending work. Skip over the - // primary children and work on the fallback. - return child.sibling; - } else { - // Note: We can return `null` here because we already checked - // whether there were nested context consumers, via the call to - // `bailoutOnAlreadyFinishedWork` above. - return null; - } - } - } else { - pushSuspenseContext( - workInProgress, - setDefaultShallowSuspenseContext(suspenseStackCursor.current), - ); - } - break; - } - case SuspenseListComponent: { - const didSuspendBefore = (current.flags & DidCapture) !== NoFlags; - - let hasChildWork = includesSomeLane( - renderLanes, - workInProgress.childLanes, - ); - - if (enableLazyContextPropagation && !hasChildWork) { - // Context changes may not have been propagated yet. We need to do - // that now, before we can decide whether to bail out. - // TODO: We use `childLanes` as a heuristic for whether there is - // remaining work in a few places, including - // `bailoutOnAlreadyFinishedWork` and - // `updateDehydratedSuspenseComponent`. We should maybe extract this - // into a dedicated function. - lazilyPropagateParentContextChanges( - current, - workInProgress, - renderLanes, - ); - hasChildWork = includesSomeLane( - renderLanes, - workInProgress.childLanes, - ); - } - - if (didSuspendBefore) { - if (hasChildWork) { - // If something was in fallback state last time, and we have all the - // same children then we're still in progressive loading state. - // Something might get unblocked by state updates or retries in the - // tree which will affect the tail. So we need to use the normal - // path to compute the correct tail. - return updateSuspenseListComponent( - current, - workInProgress, - renderLanes, - ); - } - // If none of the children had any work, that means that none of - // them got retried so they'll still be blocked in the same way - // as before. We can fast bail out. - workInProgress.flags |= DidCapture; - } - - // If nothing suspended before and we're rendering the same children, - // then the tail doesn't matter. Anything new that suspends will work - // in the "together" mode, so we can continue from the state we had. - const renderState = workInProgress.memoizedState; - if (renderState !== null) { - // Reset to the "together" mode in case we've started a different - // update in the past but didn't complete it. - renderState.rendering = null; - renderState.tail = null; - renderState.lastEffect = null; - } - pushSuspenseContext(workInProgress, suspenseStackCursor.current); - - if (hasChildWork) { - break; - } else { - // If none of the children had any work, that means that none of - // them got retried so they'll still be blocked in the same way - // as before. We can fast bail out. - return null; - } - } - case OffscreenComponent: - case LegacyHiddenComponent: { - // Need to check if the tree still needs to be deferred. This is - // almost identical to the logic used in the normal update path, - // so we'll just enter that. The only difference is we'll bail out - // at the next level instead of this one, because the child props - // have not changed. Which is fine. - // TODO: Probably should refactor `beginWork` to split the bailout - // path from the normal path. I'm tempted to do a labeled break here - // but I won't :) - workInProgress.lanes = NoLanes; - return updateOffscreenComponent(current, workInProgress, renderLanes); - } - case CacheComponent: { - if (enableCache) { - const cache: Cache = current.memoizedState.cache; - pushCacheProvider(workInProgress, cache); - } - break; - } - } - return bailoutOnAlreadyFinishedWork(current, workInProgress, renderLanes); + return attemptEarlyBailoutIfNoScheduledUpdate( + current, + workInProgress, + renderLanes, + ); } else { if ((current.flags & ForceUpdateForLegacySuspense) !== NoFlags) { // This is a special case that only exists for legacy mode. diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index fa0a9840e758f..322efafaf429e 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -3214,6 +3214,215 @@ function remountFiber( } } +function attemptEarlyBailoutIfNoScheduledUpdate( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, +) { + // This fiber does not have any pending work. Bailout without entering + // the begin phase. There's still some bookkeeping we that needs to be done + // in this optimized path, mostly pushing stuff onto the stack. + switch (workInProgress.tag) { + case HostRoot: + pushHostRootContext(workInProgress); + if (enableCache) { + const root: FiberRoot = workInProgress.stateNode; + const cache: Cache = current.memoizedState.cache; + pushCacheProvider(workInProgress, cache); + pushRootCachePool(root); + } + resetHydrationState(); + break; + case HostComponent: + pushHostContext(workInProgress); + break; + case ClassComponent: { + const Component = workInProgress.type; + if (isLegacyContextProvider(Component)) { + pushLegacyContextProvider(workInProgress); + } + break; + } + case HostPortal: + pushHostContainer(workInProgress, workInProgress.stateNode.containerInfo); + break; + case ContextProvider: { + const newValue = workInProgress.memoizedProps.value; + const context: ReactContext = workInProgress.type._context; + pushProvider(workInProgress, context, newValue); + break; + } + case Profiler: + if (enableProfilerTimer) { + // Profiler should only call onRender when one of its descendants actually rendered. + const hasChildWork = includesSomeLane( + renderLanes, + workInProgress.childLanes, + ); + if (hasChildWork) { + workInProgress.flags |= Update; + } + + if (enableProfilerCommitHooks) { + // Reset effect durations for the next eventual effect phase. + // These are reset during render to allow the DevTools commit hook a chance to read them, + const stateNode = workInProgress.stateNode; + stateNode.effectDuration = 0; + stateNode.passiveEffectDuration = 0; + } + } + break; + case SuspenseComponent: { + const state: SuspenseState | null = workInProgress.memoizedState; + if (state !== null) { + if (enableSuspenseServerRenderer) { + if (state.dehydrated !== null) { + pushSuspenseContext( + workInProgress, + setDefaultShallowSuspenseContext(suspenseStackCursor.current), + ); + // We know that this component will suspend again because if it has + // been unsuspended it has committed as a resolved Suspense component. + // If it needs to be retried, it should have work scheduled on it. + workInProgress.flags |= DidCapture; + // We should never render the children of a dehydrated boundary until we + // upgrade it. We return null instead of bailoutOnAlreadyFinishedWork. + return null; + } + } + + // If this boundary is currently timed out, we need to decide + // whether to retry the primary children, or to skip over it and + // go straight to the fallback. Check the priority of the primary + // child fragment. + const primaryChildFragment: Fiber = (workInProgress.child: any); + const primaryChildLanes = primaryChildFragment.childLanes; + if (includesSomeLane(renderLanes, primaryChildLanes)) { + // The primary children have pending work. Use the normal path + // to attempt to render the primary children again. + return updateSuspenseComponent(current, workInProgress, renderLanes); + } else { + // The primary child fragment does not have pending work marked + // on it + pushSuspenseContext( + workInProgress, + setDefaultShallowSuspenseContext(suspenseStackCursor.current), + ); + // The primary children do not have pending work with sufficient + // priority. Bailout. + const child = bailoutOnAlreadyFinishedWork( + current, + workInProgress, + renderLanes, + ); + if (child !== null) { + // The fallback children have pending work. Skip over the + // primary children and work on the fallback. + return child.sibling; + } else { + // Note: We can return `null` here because we already checked + // whether there were nested context consumers, via the call to + // `bailoutOnAlreadyFinishedWork` above. + return null; + } + } + } else { + pushSuspenseContext( + workInProgress, + setDefaultShallowSuspenseContext(suspenseStackCursor.current), + ); + } + break; + } + case SuspenseListComponent: { + const didSuspendBefore = (current.flags & DidCapture) !== NoFlags; + + let hasChildWork = includesSomeLane( + renderLanes, + workInProgress.childLanes, + ); + + if (enableLazyContextPropagation && !hasChildWork) { + // Context changes may not have been propagated yet. We need to do + // that now, before we can decide whether to bail out. + // TODO: We use `childLanes` as a heuristic for whether there is + // remaining work in a few places, including + // `bailoutOnAlreadyFinishedWork` and + // `updateDehydratedSuspenseComponent`. We should maybe extract this + // into a dedicated function. + lazilyPropagateParentContextChanges( + current, + workInProgress, + renderLanes, + ); + hasChildWork = includesSomeLane(renderLanes, workInProgress.childLanes); + } + + if (didSuspendBefore) { + if (hasChildWork) { + // If something was in fallback state last time, and we have all the + // same children then we're still in progressive loading state. + // Something might get unblocked by state updates or retries in the + // tree which will affect the tail. So we need to use the normal + // path to compute the correct tail. + return updateSuspenseListComponent( + current, + workInProgress, + renderLanes, + ); + } + // If none of the children had any work, that means that none of + // them got retried so they'll still be blocked in the same way + // as before. We can fast bail out. + workInProgress.flags |= DidCapture; + } + + // If nothing suspended before and we're rendering the same children, + // then the tail doesn't matter. Anything new that suspends will work + // in the "together" mode, so we can continue from the state we had. + const renderState = workInProgress.memoizedState; + if (renderState !== null) { + // Reset to the "together" mode in case we've started a different + // update in the past but didn't complete it. + renderState.rendering = null; + renderState.tail = null; + renderState.lastEffect = null; + } + pushSuspenseContext(workInProgress, suspenseStackCursor.current); + + if (hasChildWork) { + break; + } else { + // If none of the children had any work, that means that none of + // them got retried so they'll still be blocked in the same way + // as before. We can fast bail out. + return null; + } + } + case OffscreenComponent: + case LegacyHiddenComponent: { + // Need to check if the tree still needs to be deferred. This is + // almost identical to the logic used in the normal update path, + // so we'll just enter that. The only difference is we'll bail out + // at the next level instead of this one, because the child props + // have not changed. Which is fine. + // TODO: Probably should refactor `beginWork` to split the bailout + // path from the normal path. I'm tempted to do a labeled break here + // but I won't :) + workInProgress.lanes = NoLanes; + return updateOffscreenComponent(current, workInProgress, renderLanes); + } + case CacheComponent: { + if (enableCache) { + const cache: Cache = current.memoizedState.cache; + pushCacheProvider(workInProgress, cache); + } + break; + } + } + return bailoutOnAlreadyFinishedWork(current, workInProgress, renderLanes); +} + function beginWork( current: Fiber | null, workInProgress: Fiber, @@ -3265,218 +3474,11 @@ function beginWork( didReceiveUpdate = true; } else if (!includesSomeLane(renderLanes, updateLanes)) { didReceiveUpdate = false; - // This fiber does not have any pending work. Bailout without entering - // the begin phase. There's still some bookkeeping we that needs to be done - // in this optimized path, mostly pushing stuff onto the stack. - switch (workInProgress.tag) { - case HostRoot: - pushHostRootContext(workInProgress); - if (enableCache) { - const root: FiberRoot = workInProgress.stateNode; - const cache: Cache = current.memoizedState.cache; - pushCacheProvider(workInProgress, cache); - pushRootCachePool(root); - } - resetHydrationState(); - break; - case HostComponent: - pushHostContext(workInProgress); - break; - case ClassComponent: { - const Component = workInProgress.type; - if (isLegacyContextProvider(Component)) { - pushLegacyContextProvider(workInProgress); - } - break; - } - case HostPortal: - pushHostContainer( - workInProgress, - workInProgress.stateNode.containerInfo, - ); - break; - case ContextProvider: { - const newValue = workInProgress.memoizedProps.value; - const context: ReactContext = workInProgress.type._context; - pushProvider(workInProgress, context, newValue); - break; - } - case Profiler: - if (enableProfilerTimer) { - // Profiler should only call onRender when one of its descendants actually rendered. - const hasChildWork = includesSomeLane( - renderLanes, - workInProgress.childLanes, - ); - if (hasChildWork) { - workInProgress.flags |= Update; - } - - if (enableProfilerCommitHooks) { - // Reset effect durations for the next eventual effect phase. - // These are reset during render to allow the DevTools commit hook a chance to read them, - const stateNode = workInProgress.stateNode; - stateNode.effectDuration = 0; - stateNode.passiveEffectDuration = 0; - } - } - break; - case SuspenseComponent: { - const state: SuspenseState | null = workInProgress.memoizedState; - if (state !== null) { - if (enableSuspenseServerRenderer) { - if (state.dehydrated !== null) { - pushSuspenseContext( - workInProgress, - setDefaultShallowSuspenseContext(suspenseStackCursor.current), - ); - // We know that this component will suspend again because if it has - // been unsuspended it has committed as a resolved Suspense component. - // If it needs to be retried, it should have work scheduled on it. - workInProgress.flags |= DidCapture; - // We should never render the children of a dehydrated boundary until we - // upgrade it. We return null instead of bailoutOnAlreadyFinishedWork. - return null; - } - } - - // If this boundary is currently timed out, we need to decide - // whether to retry the primary children, or to skip over it and - // go straight to the fallback. Check the priority of the primary - // child fragment. - const primaryChildFragment: Fiber = (workInProgress.child: any); - const primaryChildLanes = primaryChildFragment.childLanes; - if (includesSomeLane(renderLanes, primaryChildLanes)) { - // The primary children have pending work. Use the normal path - // to attempt to render the primary children again. - return updateSuspenseComponent( - current, - workInProgress, - renderLanes, - ); - } else { - // The primary child fragment does not have pending work marked - // on it - pushSuspenseContext( - workInProgress, - setDefaultShallowSuspenseContext(suspenseStackCursor.current), - ); - // The primary children do not have pending work with sufficient - // priority. Bailout. - const child = bailoutOnAlreadyFinishedWork( - current, - workInProgress, - renderLanes, - ); - if (child !== null) { - // The fallback children have pending work. Skip over the - // primary children and work on the fallback. - return child.sibling; - } else { - // Note: We can return `null` here because we already checked - // whether there were nested context consumers, via the call to - // `bailoutOnAlreadyFinishedWork` above. - return null; - } - } - } else { - pushSuspenseContext( - workInProgress, - setDefaultShallowSuspenseContext(suspenseStackCursor.current), - ); - } - break; - } - case SuspenseListComponent: { - const didSuspendBefore = (current.flags & DidCapture) !== NoFlags; - - let hasChildWork = includesSomeLane( - renderLanes, - workInProgress.childLanes, - ); - - if (enableLazyContextPropagation && !hasChildWork) { - // Context changes may not have been propagated yet. We need to do - // that now, before we can decide whether to bail out. - // TODO: We use `childLanes` as a heuristic for whether there is - // remaining work in a few places, including - // `bailoutOnAlreadyFinishedWork` and - // `updateDehydratedSuspenseComponent`. We should maybe extract this - // into a dedicated function. - lazilyPropagateParentContextChanges( - current, - workInProgress, - renderLanes, - ); - hasChildWork = includesSomeLane( - renderLanes, - workInProgress.childLanes, - ); - } - - if (didSuspendBefore) { - if (hasChildWork) { - // If something was in fallback state last time, and we have all the - // same children then we're still in progressive loading state. - // Something might get unblocked by state updates or retries in the - // tree which will affect the tail. So we need to use the normal - // path to compute the correct tail. - return updateSuspenseListComponent( - current, - workInProgress, - renderLanes, - ); - } - // If none of the children had any work, that means that none of - // them got retried so they'll still be blocked in the same way - // as before. We can fast bail out. - workInProgress.flags |= DidCapture; - } - - // If nothing suspended before and we're rendering the same children, - // then the tail doesn't matter. Anything new that suspends will work - // in the "together" mode, so we can continue from the state we had. - const renderState = workInProgress.memoizedState; - if (renderState !== null) { - // Reset to the "together" mode in case we've started a different - // update in the past but didn't complete it. - renderState.rendering = null; - renderState.tail = null; - renderState.lastEffect = null; - } - pushSuspenseContext(workInProgress, suspenseStackCursor.current); - - if (hasChildWork) { - break; - } else { - // If none of the children had any work, that means that none of - // them got retried so they'll still be blocked in the same way - // as before. We can fast bail out. - return null; - } - } - case OffscreenComponent: - case LegacyHiddenComponent: { - // Need to check if the tree still needs to be deferred. This is - // almost identical to the logic used in the normal update path, - // so we'll just enter that. The only difference is we'll bail out - // at the next level instead of this one, because the child props - // have not changed. Which is fine. - // TODO: Probably should refactor `beginWork` to split the bailout - // path from the normal path. I'm tempted to do a labeled break here - // but I won't :) - workInProgress.lanes = NoLanes; - return updateOffscreenComponent(current, workInProgress, renderLanes); - } - case CacheComponent: { - if (enableCache) { - const cache: Cache = current.memoizedState.cache; - pushCacheProvider(workInProgress, cache); - } - break; - } - } - return bailoutOnAlreadyFinishedWork(current, workInProgress, renderLanes); + return attemptEarlyBailoutIfNoScheduledUpdate( + current, + workInProgress, + renderLanes, + ); } else { if ((current.flags & ForceUpdateForLegacySuspense) !== NoFlags) { // This is a special case that only exists for legacy mode. From d8d34b8ad9e1958d92634e54b4c93443f7c90762 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 10 Jul 2021 12:48:59 -0400 Subject: [PATCH 2/2] Extract state and context check to separate function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The only reason we pass `updateLanes` to some begin functions is to check if we can perform an early bail out. But this is also available as `current.lanes`, so we can read it from there instead. I think the only reason we didn't do it this way originally is because components that have two phases — error and Suspense boundaries — use `workInProgress.lanes` to prevent a bail out, since during the initial render there is no `current`. But we can check the `DidCapture` flag instead, which we use elsewhere to detect the second phase. --- .../src/ReactFiberBeginWork.new.js | 83 +++++++++++-------- .../src/ReactFiberBeginWork.old.js | 83 +++++++++++-------- .../src/ReactFiberThrow.new.js | 2 + .../src/ReactFiberThrow.old.js | 2 + 4 files changed, 98 insertions(+), 72 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index d15a6d8852880..48991e118a0d3 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -409,7 +409,6 @@ function updateMemoComponent( workInProgress: Fiber, Component: any, nextProps: any, - updateLanes: Lanes, renderLanes: Lanes, ): null | Fiber { if (current === null) { @@ -437,7 +436,6 @@ function updateMemoComponent( workInProgress, resolvedType, nextProps, - updateLanes, renderLanes, ); } @@ -482,7 +480,11 @@ function updateMemoComponent( } } const currentChild = ((current.child: any): Fiber); // This is always exactly one child - if (!includesSomeLane(updateLanes, renderLanes)) { + const hasScheduledUpdateOrContext = checkScheduledUpdateOrContext( + current, + renderLanes, + ); + if (!hasScheduledUpdateOrContext) { // This will be the props with resolved defaultProps, // unlike current.memoizedProps which will be the unresolved ones. const prevProps = currentChild.memoizedProps; @@ -507,7 +509,6 @@ function updateSimpleMemoComponent( workInProgress: Fiber, Component: any, nextProps: any, - updateLanes: Lanes, renderLanes: Lanes, ): null | Fiber { // TODO: current can be non-null here even if the component @@ -553,7 +554,7 @@ function updateSimpleMemoComponent( (__DEV__ ? workInProgress.type === current.type : true) ) { didReceiveUpdate = false; - if (!includesSomeLane(renderLanes, updateLanes)) { + if (!checkScheduledUpdateOrContext(current, renderLanes)) { // The pending lanes were cleared at the beginning of beginWork. We're // about to bail out, but there might be other lanes that weren't // included in the current render. Usually, the priority level of the @@ -740,7 +741,6 @@ const updateLegacyHiddenComponent = updateOffscreenComponent; function updateCacheComponent( current: Fiber | null, workInProgress: Fiber, - updateLanes: Lanes, renderLanes: Lanes, ) { if (!enableCache) { @@ -762,7 +762,7 @@ function updateCacheComponent( pushCacheProvider(workInProgress, freshCache); } else { // Check for updates - if (includesSomeLane(renderLanes, updateLanes)) { + if (includesSomeLane(current.lanes, renderLanes)) { cloneUpdateQueue(current, workInProgress); processUpdateQueue(workInProgress, null, null, renderLanes); } @@ -1306,7 +1306,6 @@ function mountLazyComponent( _current, workInProgress, elementType, - updateLanes, renderLanes, ) { if (_current !== null) { @@ -1396,7 +1395,6 @@ function mountLazyComponent( workInProgress, Component, resolveDefaultProps(Component.type, resolvedProps), // The inner type can have defaults too - updateLanes, renderLanes, ); return child; @@ -3214,6 +3212,27 @@ function remountFiber( } } +function checkScheduledUpdateOrContext( + current: Fiber, + renderLanes: Lanes, +): boolean { + // Before performing an early bailout, we must check if there are pending + // updates or context. + const updateLanes = current.lanes; + if (includesSomeLane(updateLanes, renderLanes)) { + return true; + } + // No pending update, but because context is propagated lazily, we need + // to check for a context change before we bail out. + if (enableLazyContextPropagation) { + const dependencies = current.dependencies; + if (dependencies !== null && checkIfContextChanged(dependencies)) { + return true; + } + } + return false; +} + function attemptEarlyBailoutIfNoScheduledUpdate( current: Fiber, workInProgress: Fiber, @@ -3428,8 +3447,6 @@ function beginWork( workInProgress: Fiber, renderLanes: Lanes, ): Fiber | null { - let updateLanes = workInProgress.lanes; - if (__DEV__) { if (workInProgress._debugNeedsRemount && current !== null) { // This will restart the begin phase with a new fiber. @@ -3449,17 +3466,6 @@ function beginWork( } if (current !== null) { - // TODO: The factoring of this block is weird. - if ( - enableLazyContextPropagation && - !includesSomeLane(renderLanes, updateLanes) - ) { - const dependencies = current.dependencies; - if (dependencies !== null && checkIfContextChanged(dependencies)) { - updateLanes = mergeLanes(updateLanes, renderLanes); - } - } - const oldProps = current.memoizedProps; const newProps = workInProgress.pendingProps; @@ -3472,14 +3478,27 @@ function beginWork( // If props or context changed, mark the fiber as having performed work. // This may be unset if the props are determined to be equal later (memo). didReceiveUpdate = true; - } else if (!includesSomeLane(renderLanes, updateLanes)) { - didReceiveUpdate = false; - return attemptEarlyBailoutIfNoScheduledUpdate( + } else { + // Neither props nor legacy context changes. Check if there's a pending + // update or context change. + const hasScheduledUpdateOrContext = checkScheduledUpdateOrContext( current, - workInProgress, renderLanes, ); - } else { + if ( + !hasScheduledUpdateOrContext && + // If this is the second pass of an error or suspense boundary, there + // may not be work scheduled on `current`, so we check for this flag. + (workInProgress.flags & DidCapture) === NoFlags + ) { + // No pending updates or context. Bail out now. + didReceiveUpdate = false; + return attemptEarlyBailoutIfNoScheduledUpdate( + current, + workInProgress, + renderLanes, + ); + } if ((current.flags & ForceUpdateForLegacySuspense) !== NoFlags) { // This is a special case that only exists for legacy mode. // See https://github.com/facebook/react/pull/19216. @@ -3518,7 +3537,6 @@ function beginWork( current, workInProgress, elementType, - updateLanes, renderLanes, ); } @@ -3611,7 +3629,6 @@ function beginWork( workInProgress, type, resolvedProps, - updateLanes, renderLanes, ); } @@ -3621,7 +3638,6 @@ function beginWork( workInProgress, workInProgress.type, workInProgress.pendingProps, - updateLanes, renderLanes, ); } @@ -3657,12 +3673,7 @@ function beginWork( } case CacheComponent: { if (enableCache) { - return updateCacheComponent( - current, - workInProgress, - updateLanes, - renderLanes, - ); + return updateCacheComponent(current, workInProgress, renderLanes); } break; } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 322efafaf429e..0615563f70eeb 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -409,7 +409,6 @@ function updateMemoComponent( workInProgress: Fiber, Component: any, nextProps: any, - updateLanes: Lanes, renderLanes: Lanes, ): null | Fiber { if (current === null) { @@ -437,7 +436,6 @@ function updateMemoComponent( workInProgress, resolvedType, nextProps, - updateLanes, renderLanes, ); } @@ -482,7 +480,11 @@ function updateMemoComponent( } } const currentChild = ((current.child: any): Fiber); // This is always exactly one child - if (!includesSomeLane(updateLanes, renderLanes)) { + const hasScheduledUpdateOrContext = checkScheduledUpdateOrContext( + current, + renderLanes, + ); + if (!hasScheduledUpdateOrContext) { // This will be the props with resolved defaultProps, // unlike current.memoizedProps which will be the unresolved ones. const prevProps = currentChild.memoizedProps; @@ -507,7 +509,6 @@ function updateSimpleMemoComponent( workInProgress: Fiber, Component: any, nextProps: any, - updateLanes: Lanes, renderLanes: Lanes, ): null | Fiber { // TODO: current can be non-null here even if the component @@ -553,7 +554,7 @@ function updateSimpleMemoComponent( (__DEV__ ? workInProgress.type === current.type : true) ) { didReceiveUpdate = false; - if (!includesSomeLane(renderLanes, updateLanes)) { + if (!checkScheduledUpdateOrContext(current, renderLanes)) { // The pending lanes were cleared at the beginning of beginWork. We're // about to bail out, but there might be other lanes that weren't // included in the current render. Usually, the priority level of the @@ -740,7 +741,6 @@ const updateLegacyHiddenComponent = updateOffscreenComponent; function updateCacheComponent( current: Fiber | null, workInProgress: Fiber, - updateLanes: Lanes, renderLanes: Lanes, ) { if (!enableCache) { @@ -762,7 +762,7 @@ function updateCacheComponent( pushCacheProvider(workInProgress, freshCache); } else { // Check for updates - if (includesSomeLane(renderLanes, updateLanes)) { + if (includesSomeLane(current.lanes, renderLanes)) { cloneUpdateQueue(current, workInProgress); processUpdateQueue(workInProgress, null, null, renderLanes); } @@ -1306,7 +1306,6 @@ function mountLazyComponent( _current, workInProgress, elementType, - updateLanes, renderLanes, ) { if (_current !== null) { @@ -1396,7 +1395,6 @@ function mountLazyComponent( workInProgress, Component, resolveDefaultProps(Component.type, resolvedProps), // The inner type can have defaults too - updateLanes, renderLanes, ); return child; @@ -3214,6 +3212,27 @@ function remountFiber( } } +function checkScheduledUpdateOrContext( + current: Fiber, + renderLanes: Lanes, +): boolean { + // Before performing an early bailout, we must check if there are pending + // updates or context. + const updateLanes = current.lanes; + if (includesSomeLane(updateLanes, renderLanes)) { + return true; + } + // No pending update, but because context is propagated lazily, we need + // to check for a context change before we bail out. + if (enableLazyContextPropagation) { + const dependencies = current.dependencies; + if (dependencies !== null && checkIfContextChanged(dependencies)) { + return true; + } + } + return false; +} + function attemptEarlyBailoutIfNoScheduledUpdate( current: Fiber, workInProgress: Fiber, @@ -3428,8 +3447,6 @@ function beginWork( workInProgress: Fiber, renderLanes: Lanes, ): Fiber | null { - let updateLanes = workInProgress.lanes; - if (__DEV__) { if (workInProgress._debugNeedsRemount && current !== null) { // This will restart the begin phase with a new fiber. @@ -3449,17 +3466,6 @@ function beginWork( } if (current !== null) { - // TODO: The factoring of this block is weird. - if ( - enableLazyContextPropagation && - !includesSomeLane(renderLanes, updateLanes) - ) { - const dependencies = current.dependencies; - if (dependencies !== null && checkIfContextChanged(dependencies)) { - updateLanes = mergeLanes(updateLanes, renderLanes); - } - } - const oldProps = current.memoizedProps; const newProps = workInProgress.pendingProps; @@ -3472,14 +3478,27 @@ function beginWork( // If props or context changed, mark the fiber as having performed work. // This may be unset if the props are determined to be equal later (memo). didReceiveUpdate = true; - } else if (!includesSomeLane(renderLanes, updateLanes)) { - didReceiveUpdate = false; - return attemptEarlyBailoutIfNoScheduledUpdate( + } else { + // Neither props nor legacy context changes. Check if there's a pending + // update or context change. + const hasScheduledUpdateOrContext = checkScheduledUpdateOrContext( current, - workInProgress, renderLanes, ); - } else { + if ( + !hasScheduledUpdateOrContext && + // If this is the second pass of an error or suspense boundary, there + // may not be work scheduled on `current`, so we check for this flag. + (workInProgress.flags & DidCapture) === NoFlags + ) { + // No pending updates or context. Bail out now. + didReceiveUpdate = false; + return attemptEarlyBailoutIfNoScheduledUpdate( + current, + workInProgress, + renderLanes, + ); + } if ((current.flags & ForceUpdateForLegacySuspense) !== NoFlags) { // This is a special case that only exists for legacy mode. // See https://github.com/facebook/react/pull/19216. @@ -3518,7 +3537,6 @@ function beginWork( current, workInProgress, elementType, - updateLanes, renderLanes, ); } @@ -3611,7 +3629,6 @@ function beginWork( workInProgress, type, resolvedProps, - updateLanes, renderLanes, ); } @@ -3621,7 +3638,6 @@ function beginWork( workInProgress, workInProgress.type, workInProgress.pendingProps, - updateLanes, renderLanes, ); } @@ -3657,12 +3673,7 @@ function beginWork( } case CacheComponent: { if (enableCache) { - return updateCacheComponent( - current, - workInProgress, - updateLanes, - renderLanes, - ); + return updateCacheComponent(current, workInProgress, renderLanes); } break; } diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 398ae98abef60..874340a1eeaf2 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -383,6 +383,8 @@ function throwException( attachPingListener(root, wakeable, rootRenderLanes); workInProgress.flags |= ShouldCapture; + // TODO: I think we can remove this, since we now use `DidCapture` in + // the begin phase to prevent an early bailout. workInProgress.lanes = rootRenderLanes; return; diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 42e2557938579..db11692c6ce39 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -383,6 +383,8 @@ function throwException( attachPingListener(root, wakeable, rootRenderLanes); workInProgress.flags |= ShouldCapture; + // TODO: I think we can remove this, since we now use `DidCapture` in + // the begin phase to prevent an early bailout. workInProgress.lanes = rootRenderLanes; return;