Skip to content

Commit

Permalink
Clear fiber.sibling field when clearing nextEffect (#18970)
Browse files Browse the repository at this point in the history
* Clear fiber.sibling field when clearing nextEffect
  • Loading branch information
trueadm authored May 21, 2020
1 parent c93a6cb commit 730ae7a
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 8 deletions.
9 changes: 5 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -1124,15 +1124,16 @@ function commitNestedUnmounts(
}
}

function detachFiber(fiber: Fiber) {
function detachFiberMutation(fiber: Fiber) {
// Cut off the return pointers to disconnect it from the tree. Ideally, we
// should clear the child pointer of the parent alternate to let this
// get GC:ed but we don't know which for sure which parent is the current
// one so we'll settle for GC:ing the subtree of this child. This child
// itself will be GC:ed when the parent updates the next time.
// Note: we cannot null out sibling here, otherwise it can cause issues
// with findDOMNode and how it requires the sibling field to carry out
// traversal in a later effect. See PR #16820.
// traversal in a later effect. See PR #16820. We now clear the sibling
// field after effects, see: detachFiberAfterEffects.
fiber.alternate = null;
fiber.child = null;
fiber.dependencies_new = null;
Expand Down Expand Up @@ -1543,9 +1544,9 @@ function commitDeletion(
commitNestedUnmounts(finishedRoot, current, renderPriorityLevel);
}
const alternate = current.alternate;
detachFiber(current);
detachFiberMutation(current);
if (alternate !== null) {
detachFiber(alternate);
detachFiberMutation(alternate);
}
}

Expand Down
9 changes: 5 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -1122,15 +1122,16 @@ function commitNestedUnmounts(
}
}

function detachFiber(fiber: Fiber) {
function detachFiberMutation(fiber: Fiber) {
// Cut off the return pointers to disconnect it from the tree. Ideally, we
// should clear the child pointer of the parent alternate to let this
// get GC:ed but we don't know which for sure which parent is the current
// one so we'll settle for GC:ing the subtree of this child. This child
// itself will be GC:ed when the parent updates the next time.
// Note: we cannot null out sibling here, otherwise it can cause issues
// with findDOMNode and how it requires the sibling field to carry out
// traversal in a later effect. See PR #16820.
// traversal in a later effect. See PR #16820. We now clear the sibling
// field after effects, see: detachFiberAfterEffects.
fiber.alternate = null;
fiber.child = null;
fiber.dependencies_old = null;
Expand Down Expand Up @@ -1541,9 +1542,9 @@ function commitDeletion(
commitNestedUnmounts(finishedRoot, current, renderPriorityLevel);
}
const alternate = current.alternate;
detachFiber(current);
detachFiberMutation(current);
if (alternate !== null) {
detachFiber(alternate);
detachFiberMutation(alternate);
}
}

Expand Down
10 changes: 10 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -1993,6 +1993,9 @@ function commitRootImpl(root, renderPriorityLevel) {
while (nextEffect !== null) {
const nextNextEffect = nextEffect.nextEffect;
nextEffect.nextEffect = null;
if (nextEffect.effectTag & Deletion) {
detachFiberAfterEffects(nextEffect);
}
nextEffect = nextNextEffect;
}
}
Expand Down Expand Up @@ -2447,6 +2450,9 @@ function flushPassiveEffectsImpl() {
const nextNextEffect = effect.nextEffect;
// Remove nextEffect pointer to assist GC
effect.nextEffect = null;
if (effect.effectTag & Deletion) {
detachFiberAfterEffects(effect);
}
effect = nextNextEffect;
}

Expand Down Expand Up @@ -3549,3 +3555,7 @@ export function act(callback: () => Thenable<mixed>): Thenable<void> {
};
}
}

function detachFiberAfterEffects(fiber: Fiber): void {
fiber.sibling = null;
}
10 changes: 10 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -2101,6 +2101,9 @@ function commitRootImpl(root, renderPriorityLevel) {
while (nextEffect !== null) {
const nextNextEffect = nextEffect.nextEffect;
nextEffect.nextEffect = null;
if (nextEffect.effectTag & Deletion) {
detachFiberAfterEffects(nextEffect);
}
nextEffect = nextNextEffect;
}
}
Expand Down Expand Up @@ -2595,6 +2598,9 @@ function flushPassiveEffectsImpl() {
const nextNextEffect = effect.nextEffect;
// Remove nextEffect pointer to assist GC
effect.nextEffect = null;
if (effect.effectTag & Deletion) {
detachFiberAfterEffects(effect);
}
effect = nextNextEffect;
}

Expand Down Expand Up @@ -3712,3 +3718,7 @@ export function act(callback: () => Thenable<mixed>): Thenable<void> {
};
}
}

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

0 comments on commit 730ae7a

Please sign in to comment.