diff --git a/packages/react-reconciler/src/ReactChildFiber.new.js b/packages/react-reconciler/src/ReactChildFiber.new.js index 776cb014b77d4..ebc3b59ce47fa 100644 --- a/packages/react-reconciler/src/ReactChildFiber.new.js +++ b/packages/react-reconciler/src/ReactChildFiber.new.js @@ -277,28 +277,13 @@ function ChildReconciler(shouldTrackSideEffects) { // Noop. return; } - // Deletions are added in reversed order so we add it to the front. - // At this point, the return fiber's effect list is empty except for - // deletions, so we can just append the deletion to the list. The remaining - // effects aren't added until the complete phase. Once we implement - // resuming, this may not be true. - // TODO (effects) Get rid of effects list update here. - const last = returnFiber.lastEffect; - if (last !== null) { - last.nextEffect = childToDelete; - returnFiber.lastEffect = childToDelete; - } else { - returnFiber.firstEffect = returnFiber.lastEffect = childToDelete; - } const deletions = returnFiber.deletions; if (deletions === null) { returnFiber.deletions = [childToDelete]; - // TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions) returnFiber.effectTag |= Deletion; } else { deletions.push(childToDelete); } - childToDelete.nextEffect = null; } function deleteRemainingChildren( diff --git a/packages/react-reconciler/src/ReactFiber.new.js b/packages/react-reconciler/src/ReactFiber.new.js index 6a90a307c222c..dd257b06c3583 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -147,10 +147,6 @@ function FiberNode( this.effectTag = NoEffect; this.subtreeTag = NoSubtreeEffect; this.deletions = null; - this.nextEffect = null; - - this.firstEffect = null; - this.lastEffect = null; this.lanes = NoLanes; this.childLanes = NoLanes; @@ -291,11 +287,6 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { workInProgress.subtreeTag = NoSubtreeEffect; workInProgress.deletions = null; - // The effect list is no longer valid. - workInProgress.nextEffect = null; - workInProgress.firstEffect = null; - workInProgress.lastEffect = null; - if (enableProfilerTimer) { // We intentionally reset, rather than copy, actualDuration & actualStartTime. // This prevents time from endlessly accumulating in new commits. @@ -374,11 +365,6 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { // that child fiber is setting, not the reconciliation. workInProgress.effectTag &= Placement; - // The effect list is no longer valid. - workInProgress.nextEffect = null; - workInProgress.firstEffect = null; - workInProgress.lastEffect = null; - const current = workInProgress.alternate; if (current === null) { // Reset to createFiber's initial values. @@ -386,6 +372,7 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { workInProgress.lanes = renderLanes; workInProgress.child = null; + workInProgress.subtreeTag = NoSubtreeEffect; workInProgress.memoizedProps = null; workInProgress.memoizedState = null; workInProgress.updateQueue = null; @@ -406,6 +393,8 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { workInProgress.lanes = current.lanes; workInProgress.child = current.child; + workInProgress.subtreeTag = current.subtreeTag; + workInProgress.deletions = null; workInProgress.memoizedProps = current.memoizedProps; workInProgress.memoizedState = current.memoizedState; workInProgress.updateQueue = current.updateQueue; @@ -830,9 +819,6 @@ export function assignFiberPropertiesInDEV( target.effectTag = source.effectTag; target.subtreeTag = source.subtreeTag; target.deletions = source.deletions; - target.nextEffect = source.nextEffect; - target.firstEffect = source.firstEffect; - target.lastEffect = source.lastEffect; target.lanes = source.lanes; target.childLanes = source.childLanes; target.alternate = source.alternate; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index dd24521b1c6c0..f00e5286751b8 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -2063,8 +2063,6 @@ function updateSuspensePrimaryChildren( primaryChildFragment.sibling = null; if (currentFallbackChildFragment !== null) { // Delete the fallback child fragment - currentFallbackChildFragment.nextEffect = null; - workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChildFragment; const deletions = workInProgress.deletions; if (deletions === null) { workInProgress.deletions = [currentFallbackChildFragment]; @@ -2129,21 +2127,8 @@ function updateSuspenseFallbackChildren( // The fallback fiber was added as a deletion effect during the first pass. // However, since we're going to remain on the fallback, we no longer want - // to delete it. So we need to remove it from the list. Deletions are stored - // on the same list as effects. We want to keep the effects from the primary - // tree. So we copy the primary child fragment's effect list, which does not - // include the fallback deletion effect. - const progressedLastEffect = primaryChildFragment.lastEffect; - if (progressedLastEffect !== null) { - workInProgress.firstEffect = primaryChildFragment.firstEffect; - workInProgress.lastEffect = progressedLastEffect; - progressedLastEffect.nextEffect = null; - workInProgress.deletions = null; - } else { - // TODO: Reset this somewhere else? Lol legacy mode is so weird. - workInProgress.firstEffect = workInProgress.lastEffect = null; - workInProgress.deletions = null; - } + // to delete it. + workInProgress.deletions = null; } else { primaryChildFragment = createWorkInProgressOffscreenFiber( currentPrimaryChildFragment, @@ -2633,7 +2618,6 @@ function initSuspenseListRenderState( tail: null | Fiber, lastContentRow: null | Fiber, tailMode: SuspenseListTailMode, - lastEffectBeforeRendering: null | Fiber, ): void { const renderState: null | SuspenseListRenderState = workInProgress.memoizedState; @@ -2645,7 +2629,6 @@ function initSuspenseListRenderState( last: lastContentRow, tail: tail, tailMode: tailMode, - lastEffect: lastEffectBeforeRendering, }: SuspenseListRenderState); } else { // We can reuse the existing object from previous renders. @@ -2655,7 +2638,6 @@ function initSuspenseListRenderState( renderState.last = lastContentRow; renderState.tail = tail; renderState.tailMode = tailMode; - renderState.lastEffect = lastEffectBeforeRendering; } } @@ -2737,7 +2719,6 @@ function updateSuspenseListComponent( tail, lastContentRow, tailMode, - workInProgress.lastEffect, ); break; } @@ -2769,7 +2750,6 @@ function updateSuspenseListComponent( tail, null, // last tailMode, - workInProgress.lastEffect, ); break; } @@ -2780,7 +2760,6 @@ function updateSuspenseListComponent( null, // tail null, // last undefined, - workInProgress.lastEffect, ); break; } @@ -3040,13 +3019,6 @@ function remountFiber( // Delete the old fiber and place the new one. // Since the old fiber is disconnected, we have to schedule it manually. - const last = returnFiber.lastEffect; - if (last !== null) { - last.nextEffect = current; - returnFiber.lastEffect = current; - } else { - returnFiber.firstEffect = returnFiber.lastEffect = current; - } const deletions = returnFiber.deletions; if (deletions === null) { returnFiber.deletions = [current]; @@ -3055,7 +3027,6 @@ function remountFiber( } else { deletions.push(current); } - current.nextEffect = null; newWorkInProgress.effectTag |= Placement; @@ -3256,7 +3227,6 @@ function beginWork( // update in the past but didn't complete it. renderState.rendering = null; renderState.tail = null; - renderState.lastEffect = null; } pushSuspenseContext(workInProgress, suspenseStackCursor.current); diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index 2e7f0b72593bb..da1a97cab9312 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -1049,18 +1049,8 @@ function completeWork( // Rerender the whole list, but this time, we'll force fallbacks // to stay in place. - // Reset the effect list before doing the second pass since that's now invalid. - if (renderState.lastEffect === null) { - workInProgress.firstEffect = null; - workInProgress.subtreeTag = NoEffect; - let child = workInProgress.child; - while (child !== null) { - child.deletions = null; - child = child.sibling; - } - } - workInProgress.lastEffect = renderState.lastEffect; // Reset the child fibers to their original state. + workInProgress.subtreeTag = NoEffect; resetChildFibers(workInProgress, renderLanes); // Set up the Suspense Context to force suspense and immediately @@ -1128,15 +1118,6 @@ function completeWork( !renderedTail.alternate && !getIsHydrating() // We don't cut it if we're hydrating. ) { - // We need to delete the row we just rendered. - // Reset the effect list to what it was before we rendered this - // child. The nested children have already appended themselves. - const lastEffect = (workInProgress.lastEffect = - renderState.lastEffect); - // Remove any effects that were appended after this point. - if (lastEffect !== null) { - lastEffect.nextEffect = null; - } // We're done. return null; } @@ -1192,7 +1173,6 @@ function completeWork( const next = renderState.tail; renderState.rendering = next; renderState.tail = next.sibling; - renderState.lastEffect = workInProgress.lastEffect; renderState.renderingStartTime = now(); next.sibling = null; diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index cc9965e2ea81a..5baf0c059037a 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -133,18 +133,6 @@ function deleteHydratableInstance( } else { deletions.push(childToDelete); } - - // This might seem like it belongs on progressedFirstDeletion. However, - // these children are not part of the reconciliation list of children. - // Even if we abort and rereconcile the children, that will try to hydrate - // again and the nodes are still in the host tree so these will be - // recreated. - if (returnFiber.lastEffect !== null) { - returnFiber.lastEffect.nextEffect = childToDelete; - returnFiber.lastEffect = childToDelete; - } else { - returnFiber.firstEffect = returnFiber.lastEffect = childToDelete; - } } function insertNonHydratedInstance(returnFiber: Fiber, fiber: Fiber) { diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js index 49e586c232327..ce49b80419fca 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js @@ -49,9 +49,6 @@ export type SuspenseListRenderState = {| tail: null | Fiber, // Tail insertions setting. tailMode: SuspenseListTailMode, - // Last Effect before we rendered the "rendering" item. - // Used to remove new effects added by the rendered item. - lastEffect: null | Fiber, |}; export function shouldCaptureSuspense( diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index d8cacf6569873..12d3eb663c88b 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -185,8 +185,6 @@ function throwException( ) { // The source fiber did not complete. sourceFiber.effectTag |= Incomplete; - // Its effect list is no longer valid. - sourceFiber.firstEffect = sourceFiber.lastEffect = null; if ( value !== null && diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index a1937b8ed48f6..5a3d2fe59b68f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -122,7 +122,6 @@ import { import {LegacyRoot} from './ReactRootTags'; import { NoEffect, - PerformedWork, Placement, Update, PlacementAndUpdate, @@ -1807,45 +1806,6 @@ function completeUnitOfWork(unitOfWork: Fiber): void { } resetChildLanes(completedWork); - - if ( - returnFiber !== null && - // Do not append effects to parents if a sibling failed to complete - (returnFiber.effectTag & Incomplete) === NoEffect - ) { - // Append all the effects of the subtree and this fiber onto the effect - // list of the parent. The completion order of the children affects the - // side-effect order. - if (returnFiber.firstEffect === null) { - returnFiber.firstEffect = completedWork.firstEffect; - } - if (completedWork.lastEffect !== null) { - if (returnFiber.lastEffect !== null) { - returnFiber.lastEffect.nextEffect = completedWork.firstEffect; - } - returnFiber.lastEffect = completedWork.lastEffect; - } - - // If this fiber had side-effects, we append it AFTER the children's - // side-effects. We can perform certain side-effects earlier if needed, - // by doing multiple passes over the effect list. We don't want to - // schedule our own side-effect on our own list because if end up - // reusing children we'll schedule this effect onto itself since we're - // at the end. - const effectTag = completedWork.effectTag; - - // Skip both NoWork and PerformedWork tags when creating the effect - // list. PerformedWork effect is read by React DevTools but shouldn't be - // committed. - if (effectTag > PerformedWork) { - if (returnFiber.lastEffect !== null) { - returnFiber.lastEffect.nextEffect = completedWork; - } else { - returnFiber.firstEffect = completedWork; - } - returnFiber.lastEffect = completedWork; - } - } } else { // This fiber did not complete because something threw. Pop values off // the stack without entering the complete phase. If this is a boundary, @@ -1882,8 +1842,7 @@ function completeUnitOfWork(unitOfWork: Fiber): void { } if (returnFiber !== null) { - // Mark the parent fiber as incomplete and clear its effect list. - returnFiber.firstEffect = returnFiber.lastEffect = null; + // Mark the parent fiber as incomplete returnFiber.effectTag |= Incomplete; returnFiber.subtreeTag = NoSubtreeTag; returnFiber.deletions = null; @@ -2161,25 +2120,24 @@ function commitRootImpl(root, renderPriorityLevel) { // times out. } - // Get the list of effects. - let firstEffect; - if (finishedWork.effectTag > PerformedWork) { - // A fiber's effect list consists only of its children, not itself. So if - // the root has an effect, we need to add it to the end of the list. The - // resulting list is the set that would belong to the root's parent, if it - // had one; that is, all the effects in the tree including the root. - if (finishedWork.lastEffect !== null) { - finishedWork.lastEffect.nextEffect = finishedWork; - firstEffect = finishedWork.firstEffect; - } else { - firstEffect = finishedWork; - } - } else { - // There is no effect on the root. - firstEffect = finishedWork.firstEffect; - } - - if (firstEffect !== null) { + // 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 satsify Flow. I think the + // only other reason this optimization exists is because it affects profiling. + // Reconsider whether this is necessary. + const subtreeHasEffects = + (finishedWork.subtreeTag & + (BeforeMutationSubtreeTag | + MutationSubtreeTag | + LayoutSubtreeTag | + PassiveSubtreeTag)) !== + NoSubtreeTag; + const rootHasEffect = + (finishedWork.effectTag & + (BeforeMutationMask | MutationMask | LayoutMask | PassiveMask)) !== + NoEffect; + + if (subtreeHasEffects || rootHasEffect) { let previousLanePriority; if (decoupleUpdatePriorityFromScheduler) { previousLanePriority = getCurrentUpdateLanePriority(); @@ -4013,8 +3971,6 @@ function detachFiberAfterEffects(fiber: Fiber): void { fiber.child = null; fiber.deletions = null; fiber.dependencies = null; - fiber.firstEffect = null; - fiber.lastEffect = null; fiber.memoizedProps = null; fiber.memoizedState = null; fiber.pendingProps = null; diff --git a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js index 79580d17202fd..9fc02c7bdc101 100644 --- a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js +++ b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js @@ -451,47 +451,23 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); }); - gate(({old}) => { - if (old) { - expect(marks).toEqual([ - `--react-init-${ReactVersion}`, - '--schedule-render-512', - '--render-start-512', - '--render-stop', - '--commit-start-512', - '--layout-effects-start-512', - '--layout-effects-stop', - '--commit-stop', - '--passive-effects-start-512', - '--schedule-state-update-1024-Example', - '--passive-effects-stop', - '--render-start-1024', - '--render-stop', - '--commit-start-1024', - '--commit-stop', - ]); - } else { - expect(marks).toEqual([ - `--react-init-${ReactVersion}`, - '--schedule-render-512', - '--render-start-512', - '--render-stop', - '--commit-start-512', - '--layout-effects-start-512', - '--layout-effects-stop', - '--commit-stop', - '--passive-effects-start-512', - '--schedule-state-update-1024-Example', - '--passive-effects-stop', - '--render-start-1024', - '--render-stop', - '--commit-start-1024', - '--layout-effects-start-1024', - '--layout-effects-stop', - '--commit-stop', - ]); - } - }); + expect(marks).toEqual([ + `--react-init-${ReactVersion}`, + '--schedule-render-512', + '--render-start-512', + '--render-stop', + '--commit-start-512', + '--layout-effects-start-512', + '--layout-effects-stop', + '--commit-stop', + '--passive-effects-start-512', + '--schedule-state-update-1024-Example', + '--passive-effects-stop', + '--render-start-1024', + '--render-stop', + '--commit-start-1024', + '--commit-stop', + ]); }); // @gate enableSchedulingProfiler