Skip to content

Commit

Permalink
Remove remaining references to effect list
Browse files Browse the repository at this point in the history
We no longer use the effect list anywhere in our implementation. It's
been replaced by a recursive traversal in the commit phase.

This removes all references to the effect list in the new fork.
  • Loading branch information
acdlite committed Aug 21, 2020
1 parent 8821965 commit 22ea70e
Show file tree
Hide file tree
Showing 8 changed files with 7 additions and 146 deletions.
15 changes: 0 additions & 15 deletions packages/react-reconciler/src/ReactChildFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
20 changes: 3 additions & 17 deletions packages/react-reconciler/src/ReactFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -374,18 +365,14 @@ 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.
workInProgress.childLanes = NoLanes;
workInProgress.lanes = renderLanes;

workInProgress.child = null;
workInProgress.subtreeTag = NoSubtreeEffect;
workInProgress.memoizedProps = null;
workInProgress.memoizedState = null;
workInProgress.updateQueue = null;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
34 changes: 2 additions & 32 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -2633,7 +2618,6 @@ function initSuspenseListRenderState(
tail: null | Fiber,
lastContentRow: null | Fiber,
tailMode: SuspenseListTailMode,
lastEffectBeforeRendering: null | Fiber,
): void {
const renderState: null | SuspenseListRenderState =
workInProgress.memoizedState;
Expand All @@ -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.
Expand All @@ -2655,7 +2638,6 @@ function initSuspenseListRenderState(
renderState.last = lastContentRow;
renderState.tail = tail;
renderState.tailMode = tailMode;
renderState.lastEffect = lastEffectBeforeRendering;
}
}

Expand Down Expand Up @@ -2737,7 +2719,6 @@ function updateSuspenseListComponent(
tail,
lastContentRow,
tailMode,
workInProgress.lastEffect,
);
break;
}
Expand Down Expand Up @@ -2769,7 +2750,6 @@ function updateSuspenseListComponent(
tail,
null, // last
tailMode,
workInProgress.lastEffect,
);
break;
}
Expand All @@ -2780,7 +2760,6 @@ function updateSuspenseListComponent(
null, // tail
null, // last
undefined,
workInProgress.lastEffect,
);
break;
}
Expand Down Expand Up @@ -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];
Expand All @@ -3055,7 +3027,6 @@ function remountFiber(
} else {
deletions.push(current);
}
current.nextEffect = null;

newWorkInProgress.effectTag |= Placement;

Expand Down Expand Up @@ -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);

Expand Down
22 changes: 1 addition & 21 deletions packages/react-reconciler/src/ReactFiberCompleteWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;

Expand Down
12 changes: 0 additions & 12 deletions packages/react-reconciler/src/ReactFiberHydrationContext.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 0 additions & 2 deletions packages/react-reconciler/src/ReactFiberThrow.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
45 changes: 1 addition & 44 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ import {
import {LegacyRoot} from './ReactRootTags';
import {
NoEffect,
PerformedWork,
Placement,
Update,
PlacementAndUpdate,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -4012,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;
Expand Down

0 comments on commit 22ea70e

Please sign in to comment.