Skip to content

Commit

Permalink
Delete remaining references to effect list (#20625)
Browse files Browse the repository at this point in the history
I think that's it!
  • Loading branch information
acdlite committed Jan 20, 2021
1 parent 741dcbd commit fceb75e
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 167 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ module.exports = {
'react-internal/no-cross-fork-types': [
ERROR,
{
old: [],
old: ['firstEffect', 'nextEffect'],
new: [],
},
],
Expand Down
14 changes: 0 additions & 14 deletions packages/react-reconciler/src/ReactChildFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,20 +263,6 @@ 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

This comment was marked as spam.

Copy link
@dwanl

dwanl Jan 21, 2021

s

// effects aren't added until the complete phase. Once we implement
// resuming, this may not be true.
const last = returnFiber.lastEffect;
if (last !== null) {
last.nextEffect = childToDelete;
returnFiber.lastEffect = childToDelete;
} else {
returnFiber.firstEffect = returnFiber.lastEffect = childToDelete;
}
childToDelete.nextEffect = null;

const deletions = returnFiber.deletions;
if (deletions === null) {
returnFiber.deletions = [childToDelete];
Expand Down
20 changes: 5 additions & 15 deletions packages/react-reconciler/src/ReactFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,6 @@ function FiberNode(

// Effects
this.flags = NoFlags;
this.nextEffect = null;

this.firstEffect = null;
this.lastEffect = null;
this.subtreeFlags = NoFlags;
this.deletions = null;

Expand Down Expand Up @@ -285,10 +281,7 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
// Reset the effect tag.
workInProgress.flags = NoFlags;

// The effect list is no longer valid.
workInProgress.nextEffect = null;
workInProgress.firstEffect = null;
workInProgress.lastEffect = null;
// The effects are no longer valid.
workInProgress.subtreeFlags = NoFlags;
workInProgress.deletions = null;

Expand Down Expand Up @@ -370,10 +363,7 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) {
// that child fiber is setting, not the reconciliation.
workInProgress.flags &= StaticMask | Placement;

// The effect list is no longer valid.
workInProgress.nextEffect = null;
workInProgress.firstEffect = null;
workInProgress.lastEffect = null;
// The effects are no longer valid.

const current = workInProgress.alternate;
if (current === null) {
Expand Down Expand Up @@ -403,6 +393,9 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) {
workInProgress.lanes = current.lanes;

workInProgress.child = current.child;
// TODO: `subtreeFlags` should be reset to NoFlags, like we do in
// `createWorkInProgress`. Nothing reads this until the complete phase,
// currently, but it might in the future, and we should be consistent.
workInProgress.subtreeFlags = current.subtreeFlags;
workInProgress.deletions = null;
workInProgress.memoizedProps = current.memoizedProps;
Expand Down Expand Up @@ -847,9 +840,6 @@ export function assignFiberPropertiesInDEV(
target.dependencies = source.dependencies;
target.mode = source.mode;
target.flags = source.flags;
target.nextEffect = source.nextEffect;
target.firstEffect = source.firstEffect;
target.lastEffect = source.lastEffect;
target.subtreeFlags = source.subtreeFlags;
target.deletions = source.deletions;
target.lanes = source.lanes;
Expand Down
36 changes: 3 additions & 33 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2198,8 +2198,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 @@ -2261,22 +2259,9 @@ function updateSuspenseFallbackChildren(
currentPrimaryChildFragment.treeBaseDuration;
}

if (currentFallbackChildFragment !== null) {
// 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, and are always added
// to the front. So we know that the first effect must be the fallback
// deletion effect, and everything after that is from the primary free.
const firstPrimaryTreeEffect = currentFallbackChildFragment.nextEffect;
if (firstPrimaryTreeEffect !== null) {
workInProgress.firstEffect = firstPrimaryTreeEffect;
} else {
// TODO: Reset this somewhere else? Lol legacy mode is so weird.
workInProgress.firstEffect = workInProgress.lastEffect = null;
}
}

// The fallback fiber was added as a deletion during the first pass.
// However, since we're going to remain on the fallback, we no longer want
// to delete it.
workInProgress.deletions = null;
} else {
primaryChildFragment = createWorkInProgressOffscreenFiber(
Expand Down Expand Up @@ -2773,7 +2758,6 @@ function initSuspenseListRenderState(
tail: null | Fiber,
lastContentRow: null | Fiber,
tailMode: SuspenseListTailMode,
lastEffectBeforeRendering: null | Fiber,
): void {
const renderState: null | SuspenseListRenderState =
workInProgress.memoizedState;
Expand All @@ -2785,7 +2769,6 @@ function initSuspenseListRenderState(
last: lastContentRow,
tail: tail,
tailMode: tailMode,
lastEffect: lastEffectBeforeRendering,
}: SuspenseListRenderState);
} else {
// We can reuse the existing object from previous renders.
Expand All @@ -2795,7 +2778,6 @@ function initSuspenseListRenderState(
renderState.last = lastContentRow;
renderState.tail = tail;
renderState.tailMode = tailMode;
renderState.lastEffect = lastEffectBeforeRendering;
}
}

Expand Down Expand Up @@ -2877,7 +2859,6 @@ function updateSuspenseListComponent(
tail,
lastContentRow,
tailMode,
workInProgress.lastEffect,
);
break;
}
Expand Down Expand Up @@ -2909,7 +2890,6 @@ function updateSuspenseListComponent(
tail,
null, // last
tailMode,
workInProgress.lastEffect,
);
break;
}
Expand All @@ -2920,7 +2900,6 @@ function updateSuspenseListComponent(
null, // tail
null, // last
undefined,
workInProgress.lastEffect,
);
break;
}
Expand Down Expand Up @@ -3180,15 +3159,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;
}
current.nextEffect = null;

const deletions = returnFiber.deletions;
if (deletions === null) {
returnFiber.deletions = [current];
Expand Down
3 changes: 0 additions & 3 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -1213,9 +1213,6 @@ export function detachFiberAfterEffects(fiber: Fiber): void {
fiber.sibling = null;
fiber.stateNode = null;
fiber.updateQueue = null;
fiber.nextEffect = null;
fiber.firstEffect = null;
fiber.lastEffect = null;

if (__DEV__) {
fiber._debugOwner = null;
Expand Down
57 changes: 35 additions & 22 deletions packages/react-reconciler/src/ReactFiberCompleteWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ import {
NoFlags,
DidCapture,
Snapshot,
ChildDeletion,
StaticMask,
MutationMask,
} from './ReactFiberFlags';
import invariant from 'shared/invariant';

Expand Down Expand Up @@ -173,6 +175,31 @@ function markRef(workInProgress: Fiber) {
workInProgress.flags |= Ref;
}

function hadNoMutationsEffects(current: null | Fiber, completedWork: Fiber) {
const didBailout = current !== null && current.child === completedWork.child;
if (didBailout) {
return true;
}

if ((completedWork.flags & ChildDeletion) !== NoFlags) {
return false;
}

// TODO: If we move the `hadNoMutationsEffects` call after `bubbleProperties`
// then we only have to check the `completedWork.subtreeFlags`.
let child = completedWork.child;
while (child !== null) {
if (
(child.flags & MutationMask) !== NoFlags ||
(child.subtreeFlags & MutationMask) !== NoFlags
) {
return false;
}
child = child.sibling;
}
return true;
}

let appendAllChildren;
let updateHostContainer;
let updateHostComponent;
Expand Down Expand Up @@ -217,7 +244,7 @@ if (supportsMutation) {
}
};

updateHostContainer = function(workInProgress: Fiber) {
updateHostContainer = function(current: null | Fiber, workInProgress: Fiber) {
// Noop
};
updateHostComponent = function(
Expand Down Expand Up @@ -461,13 +488,13 @@ if (supportsMutation) {
node = node.sibling;
}
};
updateHostContainer = function(workInProgress: Fiber) {
updateHostContainer = function(current: null | Fiber, workInProgress: Fiber) {
const portalOrRoot: {
containerInfo: Container,
pendingChildren: ChildSet,
...
} = workInProgress.stateNode;
const childrenUnchanged = workInProgress.firstEffect === null;
const childrenUnchanged = hadNoMutationsEffects(current, workInProgress);
if (childrenUnchanged) {
// No changes, just reuse the existing instance.
} else {
Expand All @@ -492,7 +519,7 @@ if (supportsMutation) {
const oldProps = current.memoizedProps;
// If there are no effects associated with this node, then none of our children had any updates.
// This guarantees that we can reuse all of them.
const childrenUnchanged = workInProgress.firstEffect === null;
const childrenUnchanged = hadNoMutationsEffects(current, workInProgress);
if (childrenUnchanged && oldProps === newProps) {
// No changes, just reuse the existing instance.
// Note that this might release a previous clone.
Expand Down Expand Up @@ -575,7 +602,7 @@ if (supportsMutation) {
};
} else {
// No host operations
updateHostContainer = function(workInProgress: Fiber) {
updateHostContainer = function(current: null | Fiber, workInProgress: Fiber) {
// Noop
};
updateHostComponent = function(
Expand Down Expand Up @@ -847,7 +874,7 @@ function completeWork(
workInProgress.flags |= Snapshot;
}
}
updateHostContainer(workInProgress);
updateHostContainer(current, workInProgress);
bubbleProperties(workInProgress);
return null;
}
Expand Down Expand Up @@ -1142,7 +1169,7 @@ function completeWork(
}
case HostPortal:
popHostContainer(workInProgress);
updateHostContainer(workInProgress);
updateHostContainer(current, workInProgress);
if (current === null) {
preparePortalMount(workInProgress.stateNode.containerInfo);
}
Expand Down Expand Up @@ -1226,11 +1253,7 @@ 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.lastEffect = renderState.lastEffect;
// Reset the effect flags before doing the second pass since that's now invalid.
// Reset the child fibers to their original state.
workInProgress.subtreeFlags = NoFlags;
resetChildFibers(workInProgress, renderLanes);
Expand Down Expand Up @@ -1301,15 +1324,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.
bubbleProperties(workInProgress);
return null;
Expand Down Expand Up @@ -1369,7 +1383,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 @@ -125,18 +125,6 @@ function deleteHydratableInstance(
childToDelete.stateNode = instance;
childToDelete.return = returnFiber;

// 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;
}

const deletions = returnFiber.deletions;
if (deletions === null) {
returnFiber.deletions = [childToDelete];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,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 @@ -188,8 +188,6 @@ function throwException(
) {
// The source fiber did not complete.
sourceFiber.flags |= Incomplete;
// Its effect list is no longer valid.
sourceFiber.firstEffect = sourceFiber.lastEffect = null;

if (
value !== null &&
Expand Down
Loading

0 comments on commit fceb75e

Please sign in to comment.