Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete remaining references to effect list #20625

Merged
merged 1 commit into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
// 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