Skip to content

Commit 0220609

Browse files
acdliteacdlite
authored
Use recursion to traverse during passive unmount phase (#24918)
This converts the layout phase to iterate over its effects recursively instead of iteratively. This makes it easier to track contextual information, like whether a fiber is inside a hidden tree. We already made this change for several other phases, like mutation and layout mount. See 481dece for more context.
1 parent d54880d commit 0220609

File tree

2 files changed

+154
-166
lines changed

2 files changed

+154
-166
lines changed

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

Lines changed: 77 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -3144,113 +3144,107 @@ function commitPassiveMountOnFiber(
31443144
}
31453145
}
31463146

3147-
export function commitPassiveUnmountEffects(firstChild: Fiber): void {
3148-
nextEffect = firstChild;
3149-
commitPassiveUnmountEffects_begin();
3147+
export function commitPassiveUnmountEffects(finishedWork: Fiber): void {
3148+
setCurrentDebugFiberInDEV(finishedWork);
3149+
commitPassiveUnmountOnFiber(finishedWork);
3150+
resetCurrentDebugFiberInDEV();
31503151
}
31513152

3152-
function commitPassiveUnmountEffects_begin() {
3153-
while (nextEffect !== null) {
3154-
const fiber = nextEffect;
3155-
const child = fiber.child;
3153+
function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void {
3154+
// Deletions effects can be scheduled on any fiber type. They need to happen
3155+
// before the children effects have fired.
3156+
const deletions = parentFiber.deletions;
31563157

3157-
if ((nextEffect.flags & ChildDeletion) !== NoFlags) {
3158-
const deletions = fiber.deletions;
3159-
if (deletions !== null) {
3160-
for (let i = 0; i < deletions.length; i++) {
3161-
const fiberToDelete = deletions[i];
3162-
nextEffect = fiberToDelete;
3158+
if ((parentFiber.flags & ChildDeletion) !== NoFlags) {
3159+
if (deletions !== null) {
3160+
for (let i = 0; i < deletions.length; i++) {
3161+
const childToDelete = deletions[i];
3162+
try {
3163+
// TODO: Convert this to use recursion
3164+
nextEffect = childToDelete;
31633165
commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
3164-
fiberToDelete,
3165-
fiber,
3166+
childToDelete,
3167+
parentFiber,
31663168
);
3169+
} catch (error) {
3170+
captureCommitPhaseError(childToDelete, parentFiber, error);
31673171
}
3168-
3169-
if (deletedTreeCleanUpLevel >= 1) {
3170-
// A fiber was deleted from this parent fiber, but it's still part of
3171-
// the previous (alternate) parent fiber's list of children. Because
3172-
// children are a linked list, an earlier sibling that's still alive
3173-
// will be connected to the deleted fiber via its `alternate`:
3174-
//
3175-
// live fiber
3176-
// --alternate--> previous live fiber
3177-
// --sibling--> deleted fiber
3178-
//
3179-
// We can't disconnect `alternate` on nodes that haven't been deleted
3180-
// yet, but we can disconnect the `sibling` and `child` pointers.
3181-
const previousFiber = fiber.alternate;
3182-
if (previousFiber !== null) {
3183-
let detachedChild = previousFiber.child;
3184-
if (detachedChild !== null) {
3185-
previousFiber.child = null;
3186-
do {
3187-
const detachedSibling = detachedChild.sibling;
3188-
detachedChild.sibling = null;
3189-
detachedChild = detachedSibling;
3190-
} while (detachedChild !== null);
3191-
}
3192-
}
3193-
}
3194-
3195-
nextEffect = fiber;
31963172
}
31973173
}
31983174

3199-
if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && child !== null) {
3200-
child.return = fiber;
3201-
nextEffect = child;
3202-
} else {
3203-
commitPassiveUnmountEffects_complete();
3175+
if (deletedTreeCleanUpLevel >= 1) {
3176+
// A fiber was deleted from this parent fiber, but it's still part of
3177+
// the previous (alternate) parent fiber's list of children. Because
3178+
// children are a linked list, an earlier sibling that's still alive
3179+
// will be connected to the deleted fiber via its `alternate`:
3180+
//
3181+
// live fiber
3182+
// --alternate--> previous live fiber
3183+
// --sibling--> deleted fiber
3184+
//
3185+
// We can't disconnect `alternate` on nodes that haven't been deleted
3186+
// yet, but we can disconnect the `sibling` and `child` pointers.
3187+
const previousFiber = parentFiber.alternate;
3188+
if (previousFiber !== null) {
3189+
let detachedChild = previousFiber.child;
3190+
if (detachedChild !== null) {
3191+
previousFiber.child = null;
3192+
do {
3193+
const detachedSibling = detachedChild.sibling;
3194+
detachedChild.sibling = null;
3195+
detachedChild = detachedSibling;
3196+
} while (detachedChild !== null);
3197+
}
3198+
}
32043199
}
32053200
}
3206-
}
3207-
3208-
function commitPassiveUnmountEffects_complete() {
3209-
while (nextEffect !== null) {
3210-
const fiber = nextEffect;
3211-
if ((fiber.flags & Passive) !== NoFlags) {
3212-
setCurrentDebugFiberInDEV(fiber);
3213-
commitPassiveUnmountOnFiber(fiber);
3214-
resetCurrentDebugFiberInDEV();
3215-
}
32163201

3217-
const sibling = fiber.sibling;
3218-
if (sibling !== null) {
3219-
sibling.return = fiber.return;
3220-
nextEffect = sibling;
3221-
return;
3202+
const prevDebugFiber = getCurrentDebugFiberInDEV();
3203+
// TODO: Split PassiveMask into separate masks for mount and unmount?
3204+
if (parentFiber.subtreeFlags & PassiveMask) {
3205+
let child = parentFiber.child;
3206+
while (child !== null) {
3207+
setCurrentDebugFiberInDEV(child);
3208+
commitPassiveUnmountOnFiber(child);
3209+
child = child.sibling;
32223210
}
3223-
3224-
nextEffect = fiber.return;
32253211
}
3212+
setCurrentDebugFiberInDEV(prevDebugFiber);
32263213
}
32273214

32283215
function commitPassiveUnmountOnFiber(finishedWork: Fiber): void {
32293216
switch (finishedWork.tag) {
32303217
case FunctionComponent:
32313218
case ForwardRef:
32323219
case SimpleMemoComponent: {
3233-
if (
3234-
enableProfilerTimer &&
3235-
enableProfilerCommitHooks &&
3236-
finishedWork.mode & ProfileMode
3237-
) {
3238-
startPassiveEffectTimer();
3239-
commitHookEffectListUnmount(
3240-
HookPassive | HookHasEffect,
3241-
finishedWork,
3242-
finishedWork.return,
3243-
);
3244-
recordPassiveEffectDuration(finishedWork);
3245-
} else {
3246-
commitHookEffectListUnmount(
3247-
HookPassive | HookHasEffect,
3248-
finishedWork,
3249-
finishedWork.return,
3250-
);
3220+
recursivelyTraversePassiveUnmountEffects(finishedWork);
3221+
if (finishedWork.flags & Passive) {
3222+
if (
3223+
enableProfilerTimer &&
3224+
enableProfilerCommitHooks &&
3225+
finishedWork.mode & ProfileMode
3226+
) {
3227+
startPassiveEffectTimer();
3228+
commitHookEffectListUnmount(
3229+
HookPassive | HookHasEffect,
3230+
finishedWork,
3231+
finishedWork.return,
3232+
);
3233+
recordPassiveEffectDuration(finishedWork);
3234+
} else {
3235+
commitHookEffectListUnmount(
3236+
HookPassive | HookHasEffect,
3237+
finishedWork,
3238+
finishedWork.return,
3239+
);
3240+
}
32513241
}
32523242
break;
32533243
}
3244+
default: {
3245+
recursivelyTraversePassiveUnmountEffects(finishedWork);
3246+
break;
3247+
}
32543248
}
32553249
}
32563250

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

Lines changed: 77 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -3144,113 +3144,107 @@ function commitPassiveMountOnFiber(
31443144
}
31453145
}
31463146

3147-
export function commitPassiveUnmountEffects(firstChild: Fiber): void {
3148-
nextEffect = firstChild;
3149-
commitPassiveUnmountEffects_begin();
3147+
export function commitPassiveUnmountEffects(finishedWork: Fiber): void {
3148+
setCurrentDebugFiberInDEV(finishedWork);
3149+
commitPassiveUnmountOnFiber(finishedWork);
3150+
resetCurrentDebugFiberInDEV();
31503151
}
31513152

3152-
function commitPassiveUnmountEffects_begin() {
3153-
while (nextEffect !== null) {
3154-
const fiber = nextEffect;
3155-
const child = fiber.child;
3153+
function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void {
3154+
// Deletions effects can be scheduled on any fiber type. They need to happen
3155+
// before the children effects have fired.
3156+
const deletions = parentFiber.deletions;
31563157

3157-
if ((nextEffect.flags & ChildDeletion) !== NoFlags) {
3158-
const deletions = fiber.deletions;
3159-
if (deletions !== null) {
3160-
for (let i = 0; i < deletions.length; i++) {
3161-
const fiberToDelete = deletions[i];
3162-
nextEffect = fiberToDelete;
3158+
if ((parentFiber.flags & ChildDeletion) !== NoFlags) {
3159+
if (deletions !== null) {
3160+
for (let i = 0; i < deletions.length; i++) {
3161+
const childToDelete = deletions[i];
3162+
try {
3163+
// TODO: Convert this to use recursion
3164+
nextEffect = childToDelete;
31633165
commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
3164-
fiberToDelete,
3165-
fiber,
3166+
childToDelete,
3167+
parentFiber,
31663168
);
3169+
} catch (error) {
3170+
captureCommitPhaseError(childToDelete, parentFiber, error);
31673171
}
3168-
3169-
if (deletedTreeCleanUpLevel >= 1) {
3170-
// A fiber was deleted from this parent fiber, but it's still part of
3171-
// the previous (alternate) parent fiber's list of children. Because
3172-
// children are a linked list, an earlier sibling that's still alive
3173-
// will be connected to the deleted fiber via its `alternate`:
3174-
//
3175-
// live fiber
3176-
// --alternate--> previous live fiber
3177-
// --sibling--> deleted fiber
3178-
//
3179-
// We can't disconnect `alternate` on nodes that haven't been deleted
3180-
// yet, but we can disconnect the `sibling` and `child` pointers.
3181-
const previousFiber = fiber.alternate;
3182-
if (previousFiber !== null) {
3183-
let detachedChild = previousFiber.child;
3184-
if (detachedChild !== null) {
3185-
previousFiber.child = null;
3186-
do {
3187-
const detachedSibling = detachedChild.sibling;
3188-
detachedChild.sibling = null;
3189-
detachedChild = detachedSibling;
3190-
} while (detachedChild !== null);
3191-
}
3192-
}
3193-
}
3194-
3195-
nextEffect = fiber;
31963172
}
31973173
}
31983174

3199-
if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && child !== null) {
3200-
child.return = fiber;
3201-
nextEffect = child;
3202-
} else {
3203-
commitPassiveUnmountEffects_complete();
3175+
if (deletedTreeCleanUpLevel >= 1) {
3176+
// A fiber was deleted from this parent fiber, but it's still part of
3177+
// the previous (alternate) parent fiber's list of children. Because
3178+
// children are a linked list, an earlier sibling that's still alive
3179+
// will be connected to the deleted fiber via its `alternate`:
3180+
//
3181+
// live fiber
3182+
// --alternate--> previous live fiber
3183+
// --sibling--> deleted fiber
3184+
//
3185+
// We can't disconnect `alternate` on nodes that haven't been deleted
3186+
// yet, but we can disconnect the `sibling` and `child` pointers.
3187+
const previousFiber = parentFiber.alternate;
3188+
if (previousFiber !== null) {
3189+
let detachedChild = previousFiber.child;
3190+
if (detachedChild !== null) {
3191+
previousFiber.child = null;
3192+
do {
3193+
const detachedSibling = detachedChild.sibling;
3194+
detachedChild.sibling = null;
3195+
detachedChild = detachedSibling;
3196+
} while (detachedChild !== null);
3197+
}
3198+
}
32043199
}
32053200
}
3206-
}
3207-
3208-
function commitPassiveUnmountEffects_complete() {
3209-
while (nextEffect !== null) {
3210-
const fiber = nextEffect;
3211-
if ((fiber.flags & Passive) !== NoFlags) {
3212-
setCurrentDebugFiberInDEV(fiber);
3213-
commitPassiveUnmountOnFiber(fiber);
3214-
resetCurrentDebugFiberInDEV();
3215-
}
32163201

3217-
const sibling = fiber.sibling;
3218-
if (sibling !== null) {
3219-
sibling.return = fiber.return;
3220-
nextEffect = sibling;
3221-
return;
3202+
const prevDebugFiber = getCurrentDebugFiberInDEV();
3203+
// TODO: Split PassiveMask into separate masks for mount and unmount?
3204+
if (parentFiber.subtreeFlags & PassiveMask) {
3205+
let child = parentFiber.child;
3206+
while (child !== null) {
3207+
setCurrentDebugFiberInDEV(child);
3208+
commitPassiveUnmountOnFiber(child);
3209+
child = child.sibling;
32223210
}
3223-
3224-
nextEffect = fiber.return;
32253211
}
3212+
setCurrentDebugFiberInDEV(prevDebugFiber);
32263213
}
32273214

32283215
function commitPassiveUnmountOnFiber(finishedWork: Fiber): void {
32293216
switch (finishedWork.tag) {
32303217
case FunctionComponent:
32313218
case ForwardRef:
32323219
case SimpleMemoComponent: {
3233-
if (
3234-
enableProfilerTimer &&
3235-
enableProfilerCommitHooks &&
3236-
finishedWork.mode & ProfileMode
3237-
) {
3238-
startPassiveEffectTimer();
3239-
commitHookEffectListUnmount(
3240-
HookPassive | HookHasEffect,
3241-
finishedWork,
3242-
finishedWork.return,
3243-
);
3244-
recordPassiveEffectDuration(finishedWork);
3245-
} else {
3246-
commitHookEffectListUnmount(
3247-
HookPassive | HookHasEffect,
3248-
finishedWork,
3249-
finishedWork.return,
3250-
);
3220+
recursivelyTraversePassiveUnmountEffects(finishedWork);
3221+
if (finishedWork.flags & Passive) {
3222+
if (
3223+
enableProfilerTimer &&
3224+
enableProfilerCommitHooks &&
3225+
finishedWork.mode & ProfileMode
3226+
) {
3227+
startPassiveEffectTimer();
3228+
commitHookEffectListUnmount(
3229+
HookPassive | HookHasEffect,
3230+
finishedWork,
3231+
finishedWork.return,
3232+
);
3233+
recordPassiveEffectDuration(finishedWork);
3234+
} else {
3235+
commitHookEffectListUnmount(
3236+
HookPassive | HookHasEffect,
3237+
finishedWork,
3238+
finishedWork.return,
3239+
);
3240+
}
32513241
}
32523242
break;
32533243
}
3244+
default: {
3245+
recursivelyTraversePassiveUnmountEffects(finishedWork);
3246+
break;
3247+
}
32543248
}
32553249
}
32563250

0 commit comments

Comments
 (0)