Skip to content

Commit

Permalink
Eagerly set Deletions effect on Fiber when adding child to deletions …
Browse files Browse the repository at this point in the history
…array
  • Loading branch information
Brian Vaughn committed Jul 8, 2020
1 parent d74b606 commit 162a805
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 40 deletions.
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactChildFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,8 @@ function ChildReconciler(shouldTrackSideEffects) {
} else {
returnFiber.deletions.push(childToDelete);
}
// TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions)
returnFiber.effectTag |= Deletion;
childToDelete.nextEffect = null;
childToDelete.effectTag = Deletion;
}
Expand Down
4 changes: 4 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2071,6 +2071,8 @@ function updateSuspensePrimaryChildren(
} else {
workInProgress.deletions.push(currentFallbackChildFragment);
}
// TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions)
workInProgress.effectTag |= Deletion;
}

workInProgress.child = primaryChildFragment;
Expand Down Expand Up @@ -3052,6 +3054,8 @@ function remountFiber(
} else {
returnFiber.deletions.push(current);
}
// TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions)
returnFiber.effectTag |= Deletion;
current.nextEffect = null;
current.effectTag = Deletion;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ function deleteHydratableInstance(
} else {
returnFiber.deletions.push(childToDelete);
}
// TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions)
returnFiber.effectTag |= Deletion;

// This might seem like it belongs on progressedFirstDeletion. However,
// these children are not part of the reconciliation list of children.
Expand Down
44 changes: 4 additions & 40 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,6 @@ let globalMostRecentFallbackTime: number = 0;
const FALLBACK_THROTTLE_MS: number = 500;
const DEFAULT_TIMEOUT_MS: number = 5000;

let nextEffect: Fiber | null = null;
let hasUncaughtError = false;
let firstUncaughtError = null;
let legacyErrorBoundariesThatAlreadyFailed: Set<mixed> | null = null;
Expand Down Expand Up @@ -1788,10 +1787,8 @@ function resetChildLanes(completedWork: Fiber) {
);

subtreeTag |= child.subtreeTag;
// TODO (effects) Document why this exception is important
subtreeTag |= child.effectTag & HostEffectMask;
if (child.deletions !== null) {
subtreeTag |= Deletion;
}

if ((child.effectTag & Incomplete) !== NoEffect) {
childrenDidNotComplete = true;
Expand Down Expand Up @@ -1833,10 +1830,8 @@ function resetChildLanes(completedWork: Fiber) {
);

subtreeTag |= child.subtreeTag;
// TODO (effects) Document why this exception is important
subtreeTag |= child.effectTag & HostEffectMask;
if (child.deletions !== null) {
subtreeTag |= Deletion;
}

if ((child.effectTag & Incomplete) !== NoEffect) {
childrenDidNotComplete = true;
Expand Down Expand Up @@ -1996,8 +1991,6 @@ function commitRootImpl(root, renderPriorityLevel) {
// layout, but class component lifecycles also fire here for legacy reasons.
commitLayoutEffects(finishedWork, root, lanes);

nextEffect = null;

// Tell Scheduler to yield at the end of the frame, so the browser has an
// opportunity to paint.
requestPaint();
Expand Down Expand Up @@ -2030,19 +2023,7 @@ function commitRootImpl(root, renderPriorityLevel) {
pendingPassiveEffectsLanes = lanes;
pendingPassiveEffectsRenderPriority = renderPriorityLevel;
} else {
// We are done with the effect chain at this point so let's clear the
// nextEffect pointers to assist with GC. If we have passive effects, we'll
// clear this in flushPassiveEffects.
// TODO (effects) Traverse with subtreeTag Deletion and detatch deletions array only
nextEffect = firstEffect;
while (nextEffect !== null) {
const nextNextEffect = nextEffect.nextEffect;
nextEffect.nextEffect = null;
if (nextEffect.effectTag & Deletion) {
detachFiberAfterEffects(nextEffect);
}
nextEffect = nextNextEffect;
}
// TODO (effects) Detach sibling pointers for deleted Fibers
}

// Read this again, since an effect might have updated it
Expand Down Expand Up @@ -2626,20 +2607,7 @@ function flushPassiveEffectsImpl() {
}
}

// Note: This currently assumes there are no passive effects on the root fiber
// because the root is not part of its own effect list.
// This could change in the future.
// TODO (effects) Traverse with subtreeTag Deletion and detatch deletions array only
let effect = root.current.firstEffect;
while (effect !== null) {
const nextNextEffect = effect.nextEffect;
// Remove nextEffect pointer to assist GC
effect.nextEffect = null;
if (effect.effectTag & Deletion) {
detachFiberAfterEffects(effect);
}
effect = nextNextEffect;
}
// TODO (effects) Detach sibling pointers for deleted Fibers

if (enableProfilerTimer && enableProfilerCommitHooks) {
const profilerEffects = pendingPassiveProfilerEffects;
Expand Down Expand Up @@ -3733,7 +3701,3 @@ export function act(callback: () => Thenable<mixed>): Thenable<void> {
};
}
}

function detachFiberAfterEffects(fiber: Fiber): void {
fiber.sibling = null;
}

0 comments on commit 162a805

Please sign in to comment.