Skip to content

Commit 6a6dbe4

Browse files
acdliteacdlite
committed
Extract passive unmount effects to separate functions
 I'm about to add a "disconnect passive effects" function that will share much of the same code as commitPassiveUnmountOnFiber. To minimize the duplicated code, I've extracted the shared parts into separate functions, similar to what I did for commitLayoutEffectOnFiber and reappearLayoutEffects. This may not save much on code size because Closure will likely inline some of it, anyway, but it makes it harder for the two paths to accidentally diverge.
1 parent 820bb1d commit 6a6dbe4

File tree

2 files changed

+130
-128
lines changed

2 files changed

+130
-128
lines changed

‎packages/react-reconciler/src/ReactFiberCommitWork.new.js

Lines changed: 65 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -3553,6 +3553,49 @@ export function commitPassiveUnmountEffects(finishedWork: Fiber): void {
35533553
resetCurrentDebugFiberInDEV();
35543554
}
35553555

3556+
function detachAlternateSiblings(parentFiber: Fiber) {
3557+
if (deletedTreeCleanUpLevel >= 1) {
3558+
const previousFiber = parentFiber.alternate;
3559+
if (previousFiber !== null) {
3560+
let detachedChild = previousFiber.child;
3561+
if (detachedChild !== null) {
3562+
previousFiber.child = null;
3563+
do {
3564+
const detachedSibling = detachedChild.sibling;
3565+
detachedChild.sibling = null;
3566+
detachedChild = detachedSibling;
3567+
} while (detachedChild !== null);
3568+
}
3569+
}
3570+
}
3571+
}
3572+
3573+
function commitHookPassiveUnmountEffects(
3574+
finishedWork: Fiber,
3575+
nearestMountedAncestor,
3576+
hookFlags: HookFlags,
3577+
) {
3578+
if (
3579+
enableProfilerTimer &&
3580+
enableProfilerCommitHooks &&
3581+
finishedWork.mode & ProfileMode
3582+
) {
3583+
startPassiveEffectTimer();
3584+
commitHookEffectListUnmount(
3585+
hookFlags,
3586+
finishedWork,
3587+
nearestMountedAncestor,
3588+
);
3589+
recordPassiveEffectDuration(finishedWork);
3590+
} else {
3591+
commitHookEffectListUnmount(
3592+
hookFlags,
3593+
finishedWork,
3594+
nearestMountedAncestor,
3595+
);
3596+
}
3597+
}
3598+
35563599
function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void {
35573600
// Deletions effects can be scheduled on any fiber type. They need to happen
35583601
// before the children effects have fired.
@@ -3570,32 +3613,18 @@ function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void {
35703613
);
35713614
}
35723615
}
3573-
3574-
if (deletedTreeCleanUpLevel >= 1) {
3575-
// A fiber was deleted from this parent fiber, but it's still part of
3576-
// the previous (alternate) parent fiber's list of children. Because
3577-
// children are a linked list, an earlier sibling that's still alive
3578-
// will be connected to the deleted fiber via its `alternate`:
3579-
//
3580-
// live fiber
3581-
// --alternate--> previous live fiber
3582-
// --sibling--> deleted fiber
3583-
//
3584-
// We can't disconnect `alternate` on nodes that haven't been deleted
3585-
// yet, but we can disconnect the `sibling` and `child` pointers.
3586-
const previousFiber = parentFiber.alternate;
3587-
if (previousFiber !== null) {
3588-
let detachedChild = previousFiber.child;
3589-
if (detachedChild !== null) {
3590-
previousFiber.child = null;
3591-
do {
3592-
const detachedSibling = detachedChild.sibling;
3593-
detachedChild.sibling = null;
3594-
detachedChild = detachedSibling;
3595-
} while (detachedChild !== null);
3596-
}
3597-
}
3598-
}
3616+
// A fiber was deleted from this parent fiber, but it's still part of
3617+
// the previous (alternate) parent fiber's list of children. Because
3618+
// children are a linked list, an earlier sibling that's still alive
3619+
// will be connected to the deleted fiber via its `alternate`:
3620+
//
3621+
// live fiber
3622+
// --alternate--> previous live fiber
3623+
// --sibling--> deleted fiber
3624+
//
3625+
// We can't disconnect `alternate` on nodes that haven't been deleted
3626+
// yet, but we can disconnect the `sibling` and `child` pointers.
3627+
detachAlternateSiblings(parentFiber);
35993628
}
36003629

36013630
const prevDebugFiber = getCurrentDebugFiberInDEV();
@@ -3618,25 +3647,11 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void {
36183647
case SimpleMemoComponent: {
36193648
recursivelyTraversePassiveUnmountEffects(finishedWork);
36203649
if (finishedWork.flags & Passive) {
3621-
if (
3622-
enableProfilerTimer &&
3623-
enableProfilerCommitHooks &&
3624-
finishedWork.mode & ProfileMode
3625-
) {
3626-
startPassiveEffectTimer();
3627-
commitHookEffectListUnmount(
3628-
HookPassive | HookHasEffect,
3629-
finishedWork,
3630-
finishedWork.return,
3631-
);
3632-
recordPassiveEffectDuration(finishedWork);
3633-
} else {
3634-
commitHookEffectListUnmount(
3635-
HookPassive | HookHasEffect,
3636-
finishedWork,
3637-
finishedWork.return,
3638-
);
3639-
}
3650+
commitHookPassiveUnmountEffects(
3651+
finishedWork,
3652+
finishedWork.return,
3653+
HookPassive | HookHasEffect,
3654+
);
36403655
}
36413656
break;
36423657
}
@@ -3724,25 +3739,11 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
37243739
case FunctionComponent:
37253740
case ForwardRef:
37263741
case SimpleMemoComponent: {
3727-
if (
3728-
enableProfilerTimer &&
3729-
enableProfilerCommitHooks &&
3730-
current.mode & ProfileMode
3731-
) {
3732-
startPassiveEffectTimer();
3733-
commitHookEffectListUnmount(
3734-
HookPassive,
3735-
current,
3736-
nearestMountedAncestor,
3737-
);
3738-
recordPassiveEffectDuration(current);
3739-
} else {
3740-
commitHookEffectListUnmount(
3741-
HookPassive,
3742-
current,
3743-
nearestMountedAncestor,
3744-
);
3745-
}
3742+
commitHookPassiveUnmountEffects(
3743+
current,
3744+
nearestMountedAncestor,
3745+
HookPassive,
3746+
);
37463747
break;
37473748
}
37483749
// TODO: run passive unmount effects when unmounting a root.

‎packages/react-reconciler/src/ReactFiberCommitWork.old.js

Lines changed: 65 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -3553,6 +3553,49 @@ export function commitPassiveUnmountEffects(finishedWork: Fiber): void {
35533553
resetCurrentDebugFiberInDEV();
35543554
}
35553555

3556+
function detachAlternateSiblings(parentFiber: Fiber) {
3557+
if (deletedTreeCleanUpLevel >= 1) {
3558+
const previousFiber = parentFiber.alternate;
3559+
if (previousFiber !== null) {
3560+
let detachedChild = previousFiber.child;
3561+
if (detachedChild !== null) {
3562+
previousFiber.child = null;
3563+
do {
3564+
const detachedSibling = detachedChild.sibling;
3565+
detachedChild.sibling = null;
3566+
detachedChild = detachedSibling;
3567+
} while (detachedChild !== null);
3568+
}
3569+
}
3570+
}
3571+
}
3572+
3573+
function commitHookPassiveUnmountEffects(
3574+
finishedWork: Fiber,
3575+
nearestMountedAncestor,
3576+
hookFlags: HookFlags,
3577+
) {
3578+
if (
3579+
enableProfilerTimer &&
3580+
enableProfilerCommitHooks &&
3581+
finishedWork.mode & ProfileMode
3582+
) {
3583+
startPassiveEffectTimer();
3584+
commitHookEffectListUnmount(
3585+
hookFlags,
3586+
finishedWork,
3587+
nearestMountedAncestor,
3588+
);
3589+
recordPassiveEffectDuration(finishedWork);
3590+
} else {
3591+
commitHookEffectListUnmount(
3592+
hookFlags,
3593+
finishedWork,
3594+
nearestMountedAncestor,
3595+
);
3596+
}
3597+
}
3598+
35563599
function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void {
35573600
// Deletions effects can be scheduled on any fiber type. They need to happen
35583601
// before the children effects have fired.
@@ -3570,32 +3613,18 @@ function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void {
35703613
);
35713614
}
35723615
}
3573-
3574-
if (deletedTreeCleanUpLevel >= 1) {
3575-
// A fiber was deleted from this parent fiber, but it's still part of
3576-
// the previous (alternate) parent fiber's list of children. Because
3577-
// children are a linked list, an earlier sibling that's still alive
3578-
// will be connected to the deleted fiber via its `alternate`:
3579-
//
3580-
// live fiber
3581-
// --alternate--> previous live fiber
3582-
// --sibling--> deleted fiber
3583-
//
3584-
// We can't disconnect `alternate` on nodes that haven't been deleted
3585-
// yet, but we can disconnect the `sibling` and `child` pointers.
3586-
const previousFiber = parentFiber.alternate;
3587-
if (previousFiber !== null) {
3588-
let detachedChild = previousFiber.child;
3589-
if (detachedChild !== null) {
3590-
previousFiber.child = null;
3591-
do {
3592-
const detachedSibling = detachedChild.sibling;
3593-
detachedChild.sibling = null;
3594-
detachedChild = detachedSibling;
3595-
} while (detachedChild !== null);
3596-
}
3597-
}
3598-
}
3616+
// A fiber was deleted from this parent fiber, but it's still part of
3617+
// the previous (alternate) parent fiber's list of children. Because
3618+
// children are a linked list, an earlier sibling that's still alive
3619+
// will be connected to the deleted fiber via its `alternate`:
3620+
//
3621+
// live fiber
3622+
// --alternate--> previous live fiber
3623+
// --sibling--> deleted fiber
3624+
//
3625+
// We can't disconnect `alternate` on nodes that haven't been deleted
3626+
// yet, but we can disconnect the `sibling` and `child` pointers.
3627+
detachAlternateSiblings(parentFiber);
35993628
}
36003629

36013630
const prevDebugFiber = getCurrentDebugFiberInDEV();
@@ -3618,25 +3647,11 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void {
36183647
case SimpleMemoComponent: {
36193648
recursivelyTraversePassiveUnmountEffects(finishedWork);
36203649
if (finishedWork.flags & Passive) {
3621-
if (
3622-
enableProfilerTimer &&
3623-
enableProfilerCommitHooks &&
3624-
finishedWork.mode & ProfileMode
3625-
) {
3626-
startPassiveEffectTimer();
3627-
commitHookEffectListUnmount(
3628-
HookPassive | HookHasEffect,
3629-
finishedWork,
3630-
finishedWork.return,
3631-
);
3632-
recordPassiveEffectDuration(finishedWork);
3633-
} else {
3634-
commitHookEffectListUnmount(
3635-
HookPassive | HookHasEffect,
3636-
finishedWork,
3637-
finishedWork.return,
3638-
);
3639-
}
3650+
commitHookPassiveUnmountEffects(
3651+
finishedWork,
3652+
finishedWork.return,
3653+
HookPassive | HookHasEffect,
3654+
);
36403655
}
36413656
break;
36423657
}
@@ -3724,25 +3739,11 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
37243739
case FunctionComponent:
37253740
case ForwardRef:
37263741
case SimpleMemoComponent: {
3727-
if (
3728-
enableProfilerTimer &&
3729-
enableProfilerCommitHooks &&
3730-
current.mode & ProfileMode
3731-
) {
3732-
startPassiveEffectTimer();
3733-
commitHookEffectListUnmount(
3734-
HookPassive,
3735-
current,
3736-
nearestMountedAncestor,
3737-
);
3738-
recordPassiveEffectDuration(current);
3739-
} else {
3740-
commitHookEffectListUnmount(
3741-
HookPassive,
3742-
current,
3743-
nearestMountedAncestor,
3744-
);
3745-
}
3742+
commitHookPassiveUnmountEffects(
3743+
current,
3744+
nearestMountedAncestor,
3745+
HookPassive,
3746+
);
37463747
break;
37473748
}
37483749
// TODO: run passive unmount effects when unmounting a root.

0 commit comments

Comments
 (0)