Skip to content

Commit 31e1977

Browse files
acdliteacdlite
committed
Re-use existing recursive clean up path
We already have a recursive loop that visits every deleted fiber. We can re-use that one for clean up instead of adding another one.
1 parent f73ae7e commit 31e1977

File tree

2 files changed

+50
-40
lines changed

2 files changed

+50
-40
lines changed

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

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,8 +1228,7 @@ function detachFiberAfterEffects(fiber: Fiber) {
12281228
// Note: Defensively using negation instead of < in case
12291229
// `deletedTreeCleanUpLevel` is undefined.
12301230
if (!(deletedTreeCleanUpLevel >= 2)) {
1231-
// This is the default branch (level 0). We do not recursively clear all the
1232-
// fiber fields. Only the root of the deleted subtree.
1231+
// This is the default branch (level 0).
12331232
fiber.child = null;
12341233
fiber.deletions = null;
12351234
fiber.dependencies = null;
@@ -1244,14 +1243,6 @@ function detachFiberAfterEffects(fiber: Fiber) {
12441243
fiber._debugOwner = null;
12451244
}
12461245
} else {
1247-
// Recursively traverse the entire deleted tree and clean up fiber fields.
1248-
// This is more aggressive than ideal, and the long term goal is to only
1249-
// have to detach the deleted tree at the root.
1250-
let child = fiber.child;
1251-
while (child !== null) {
1252-
detachFiberAfterEffects(child);
1253-
child = child.sibling;
1254-
}
12551246
// Clear cyclical Fiber fields. This level alone is designed to roughly
12561247
// approximate the planned Fiber refactor. In that world, `setState` will be
12571248
// bound to a special "instance" object instead of a Fiber. The Instance
@@ -2365,9 +2356,6 @@ function commitPassiveUnmountEffects_begin() {
23652356
fiberToDelete,
23662357
fiber,
23672358
);
2368-
2369-
// Now that passive effects have been processed, it's safe to detach lingering pointers.
2370-
detachFiberAfterEffects(fiberToDelete);
23712359
}
23722360

23732361
if (deletedTreeCleanUpLevel >= 1) {
@@ -2472,7 +2460,8 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
24722460
resetCurrentDebugFiberInDEV();
24732461

24742462
const child = fiber.child;
2475-
// TODO: Only traverse subtree if it has a PassiveStatic flag
2463+
// TODO: Only traverse subtree if it has a PassiveStatic flag. (But, if we
2464+
// do this, still need to handle `deletedTreeCleanUpLevel` correctly.)
24762465
if (child !== null) {
24772466
ensureCorrectReturnPointer(child, fiber);
24782467
nextEffect = child;
@@ -2489,19 +2478,35 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete(
24892478
) {
24902479
while (nextEffect !== null) {
24912480
const fiber = nextEffect;
2492-
if (fiber === deletedSubtreeRoot) {
2493-
nextEffect = null;
2494-
return;
2481+
const sibling = fiber.sibling;
2482+
const returnFiber = fiber.return;
2483+
2484+
if (deletedTreeCleanUpLevel >= 2) {
2485+
// Recursively traverse the entire deleted tree and clean up fiber fields.
2486+
// This is more aggressive than ideal, and the long term goal is to only
2487+
// have to detach the deleted tree at the root.
2488+
detachFiberAfterEffects(fiber);
2489+
if (fiber === deletedSubtreeRoot) {
2490+
nextEffect = null;
2491+
return;
2492+
}
2493+
} else {
2494+
// This is the default branch (level 0). We do not recursively clear all
2495+
// the fiber fields. Only the root of the deleted subtree.
2496+
if (fiber === deletedSubtreeRoot) {
2497+
detachFiberAfterEffects(fiber);
2498+
nextEffect = null;
2499+
return;
2500+
}
24952501
}
24962502

2497-
const sibling = fiber.sibling;
24982503
if (sibling !== null) {
2499-
ensureCorrectReturnPointer(sibling, fiber.return);
2504+
ensureCorrectReturnPointer(sibling, returnFiber);
25002505
nextEffect = sibling;
25012506
return;
25022507
}
25032508

2504-
nextEffect = fiber.return;
2509+
nextEffect = returnFiber;
25052510
}
25062511
}
25072512

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

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,8 +1228,7 @@ function detachFiberAfterEffects(fiber: Fiber) {
12281228
// Note: Defensively using negation instead of < in case
12291229
// `deletedTreeCleanUpLevel` is undefined.
12301230
if (!(deletedTreeCleanUpLevel >= 2)) {
1231-
// This is the default branch (level 0). We do not recursively clear all the
1232-
// fiber fields. Only the root of the deleted subtree.
1231+
// This is the default branch (level 0).
12331232
fiber.child = null;
12341233
fiber.deletions = null;
12351234
fiber.dependencies = null;
@@ -1244,14 +1243,6 @@ function detachFiberAfterEffects(fiber: Fiber) {
12441243
fiber._debugOwner = null;
12451244
}
12461245
} else {
1247-
// Recursively traverse the entire deleted tree and clean up fiber fields.
1248-
// This is more aggressive than ideal, and the long term goal is to only
1249-
// have to detach the deleted tree at the root.
1250-
let child = fiber.child;
1251-
while (child !== null) {
1252-
detachFiberAfterEffects(child);
1253-
child = child.sibling;
1254-
}
12551246
// Clear cyclical Fiber fields. This level alone is designed to roughly
12561247
// approximate the planned Fiber refactor. In that world, `setState` will be
12571248
// bound to a special "instance" object instead of a Fiber. The Instance
@@ -2365,9 +2356,6 @@ function commitPassiveUnmountEffects_begin() {
23652356
fiberToDelete,
23662357
fiber,
23672358
);
2368-
2369-
// Now that passive effects have been processed, it's safe to detach lingering pointers.
2370-
detachFiberAfterEffects(fiberToDelete);
23712359
}
23722360

23732361
if (deletedTreeCleanUpLevel >= 1) {
@@ -2472,7 +2460,8 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
24722460
resetCurrentDebugFiberInDEV();
24732461

24742462
const child = fiber.child;
2475-
// TODO: Only traverse subtree if it has a PassiveStatic flag
2463+
// TODO: Only traverse subtree if it has a PassiveStatic flag. (But, if we
2464+
// do this, still need to handle `deletedTreeCleanUpLevel` correctly.)
24762465
if (child !== null) {
24772466
ensureCorrectReturnPointer(child, fiber);
24782467
nextEffect = child;
@@ -2489,19 +2478,35 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete(
24892478
) {
24902479
while (nextEffect !== null) {
24912480
const fiber = nextEffect;
2492-
if (fiber === deletedSubtreeRoot) {
2493-
nextEffect = null;
2494-
return;
2481+
const sibling = fiber.sibling;
2482+
const returnFiber = fiber.return;
2483+
2484+
if (deletedTreeCleanUpLevel >= 2) {
2485+
// Recursively traverse the entire deleted tree and clean up fiber fields.
2486+
// This is more aggressive than ideal, and the long term goal is to only
2487+
// have to detach the deleted tree at the root.
2488+
detachFiberAfterEffects(fiber);
2489+
if (fiber === deletedSubtreeRoot) {
2490+
nextEffect = null;
2491+
return;
2492+
}
2493+
} else {
2494+
// This is the default branch (level 0). We do not recursively clear all
2495+
// the fiber fields. Only the root of the deleted subtree.
2496+
if (fiber === deletedSubtreeRoot) {
2497+
detachFiberAfterEffects(fiber);
2498+
nextEffect = null;
2499+
return;
2500+
}
24952501
}
24962502

2497-
const sibling = fiber.sibling;
24982503
if (sibling !== null) {
2499-
ensureCorrectReturnPointer(sibling, fiber.return);
2504+
ensureCorrectReturnPointer(sibling, returnFiber);
25002505
nextEffect = sibling;
25012506
return;
25022507
}
25032508

2504-
nextEffect = fiber.return;
2509+
nextEffect = returnFiber;
25052510
}
25062511
}
25072512

0 commit comments

Comments
 (0)