@samitouri / QOS-React-2 / commits / ab29695a05

Defer more field detachments to passive phase

This allows us to use those fields during passive unmount traversal.

Andrew Clark committed Dec 7, 2020 at 17:51 UTC ab29695a050b806492351250a24221d622d5e4cc
4 files changed +80 -46
packages/react-reconciler/src/ReactFiberCommitWork.new.js
+27 -15
@@ -1069,30 +1069,42 @@ function commitNestedUnmounts(
1069 }
1070
1071 function detachFiberMutation(fiber: Fiber) {
1072 - // Cut off the return pointers to disconnect it from the tree. Ideally, we
1073 - // should clear the child pointer of the parent alternate to let this
1072 + // Cut off the return pointer to disconnect it from the tree.
1073 + // This enables us to detect and warn against state updates on an unmounted component.
1074 + // It also prevents events from bubbling from within disconnected components.
1075 + //
1076 + // Ideally, we should also clear the child pointer of the parent alternate to let this
1077 // get GC:ed but we don't know which for sure which parent is the current
1075 - // one so we'll settle for GC:ing the subtree of this child. This child
1076 - // itself will be GC:ed when the parent updates the next time.
1077 - // Note: we cannot null out sibling here, otherwise it can cause issues
1078 - // with findDOMNode and how it requires the sibling field to carry out
1079 - // traversal in a later effect. See PR #16820. We now clear the sibling
1080 - // field after effects, see: detachFiberAfterEffects.
1078 + // one so we'll settle for GC:ing the subtree of this child.
1079 + // This child itself will be GC:ed when the parent updates the next time.
1080 //
1082 - // Don't disconnect stateNode now; it will be detached in detachFiberAfterEffects.
1083 - // It may be required if the current component is an error boundary,
1084 - // and one of its descendants throws while unmounting a passive effect.
1085 - fiber.alternate = null;
1081 + // Note that we can't clear child or sibling pointers yet.
1082 + // They're needed for passive effects and for findDOMNode.
1083 + // We defer those fields, and all other cleanup, to the passive phase (see detachFiberAfterEffects).
1084 + const alternate = fiber.alternate;
1085 + if (alternate !== null) {
1086 + alternate.return = null;
1087 + fiber.alternate = null;
1088 + }
1089 + fiber.return = null;
1090 +}
1091 +
1092 +export function detachFiberAfterEffects(fiber: Fiber): void {
1093 + // Null out fields to improve GC for references that may be lingering (e.g. DevTools).
1094 + // Note that we already cleared the return pointer in detachFiberMutation().
1095 fiber.child = null;
1096 fiber.deletions = null;
1097 fiber.dependencies = null;
1089 - fiber.firstEffect = null;
1090 - fiber.lastEffect = null;
1098 fiber.memoizedProps = null;
1099 fiber.memoizedState = null;
1100 fiber.pendingProps = null;
1094 - fiber.return = null;
1101 + fiber.sibling = null;
1102 + fiber.stateNode = null;
1103 fiber.updateQueue = null;
1104 + fiber.nextEffect = null;
1105 + fiber.firstEffect = null;
1106 + fiber.lastEffect = null;
1107 +
1108 if (__DEV__) {
1109 fiber._debugOwner = null;
1110 }
packages/react-reconciler/src/ReactFiberCommitWork.old.js
+27 -15
@@ -1069,30 +1069,42 @@ function commitNestedUnmounts(
1069 }
1070
1071 function detachFiberMutation(fiber: Fiber) {
1072 - // Cut off the return pointers to disconnect it from the tree. Ideally, we
1073 - // should clear the child pointer of the parent alternate to let this
1072 + // Cut off the return pointer to disconnect it from the tree.
1073 + // This enables us to detect and warn against state updates on an unmounted component.
1074 + // It also prevents events from bubbling from within disconnected components.
1075 + //
1076 + // Ideally, we should also clear the child pointer of the parent alternate to let this
1077 // get GC:ed but we don't know which for sure which parent is the current
1075 - // one so we'll settle for GC:ing the subtree of this child. This child
1076 - // itself will be GC:ed when the parent updates the next time.
1077 - // Note: we cannot null out sibling here, otherwise it can cause issues
1078 - // with findDOMNode and how it requires the sibling field to carry out
1079 - // traversal in a later effect. See PR #16820. We now clear the sibling
1080 - // field after effects, see: detachFiberAfterEffects.
1078 + // one so we'll settle for GC:ing the subtree of this child.
1079 + // This child itself will be GC:ed when the parent updates the next time.
1080 //
1082 - // Don't disconnect stateNode now; it will be detached in detachFiberAfterEffects.
1083 - // It may be required if the current component is an error boundary,
1084 - // and one of its descendants throws while unmounting a passive effect.
1085 - fiber.alternate = null;
1081 + // Note that we can't clear child or sibling pointers yet.
1082 + // They're needed for passive effects and for findDOMNode.
1083 + // We defer those fields, and all other cleanup, to the passive phase (see detachFiberAfterEffects).
1084 + const alternate = fiber.alternate;
1085 + if (alternate !== null) {
1086 + alternate.return = null;
1087 + fiber.alternate = null;
1088 + }
1089 + fiber.return = null;
1090 +}
1091 +
1092 +export function detachFiberAfterEffects(fiber: Fiber): void {
1093 + // Null out fields to improve GC for references that may be lingering (e.g. DevTools).
1094 + // Note that we already cleared the return pointer in detachFiberMutation().
1095 fiber.child = null;
1096 fiber.deletions = null;
1097 fiber.dependencies = null;
1089 - fiber.firstEffect = null;
1090 - fiber.lastEffect = null;
1098 fiber.memoizedProps = null;
1099 fiber.memoizedState = null;
1100 fiber.pendingProps = null;
1094 - fiber.return = null;
1101 + fiber.sibling = null;
1102 + fiber.stateNode = null;
1103 fiber.updateQueue = null;
1104 + fiber.nextEffect = null;
1105 + fiber.firstEffect = null;
1106 + fiber.lastEffect = null;
1107 +
1108 if (__DEV__) {
1109 fiber._debugOwner = null;
1110 }
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+13 -8
@@ -122,6 +122,7 @@ import {
122 Update,
123 PlacementAndUpdate,
124 Deletion,
125 + ChildDeletion,
126 Ref,
127 ContentReset,
128 Snapshot,
@@ -194,6 +195,7 @@ import {
195 commitResetTextContent,
196 isSuspenseBoundaryBeingHidden,
197 commitPassiveMountEffects,
198 + detachFiberAfterEffects,
199 } from './ReactFiberCommitWork.new';
200 import {enqueueUpdate} from './ReactUpdateQueue.new';
201 import {resetContextDependencies} from './ReactFiberNewContext.new';
@@ -2129,13 +2131,21 @@ function commitRootImpl(root, renderPriorityLevel) {
2131 } else {
2132 // We are done with the effect chain at this point so let's clear the
2133 // nextEffect pointers to assist with GC. If we have passive effects, we'll
2132 - // clear this in flushPassiveEffects.
2134 + // clear this in flushPassiveEffects
2135 + // TODO: We should always do this in the passive phase, by scheduling
2136 + // a passive callback for every deletion.
2137 nextEffect = firstEffect;
2138 while (nextEffect !== null) {
2139 const nextNextEffect = nextEffect.nextEffect;
2140 nextEffect.nextEffect = null;
2137 - if (nextEffect.flags & Deletion) {
2138 - detachFiberAfterEffects(nextEffect);
2141 + if (nextEffect.flags & ChildDeletion) {
2142 + const deletions = nextEffect.deletions;
2143 + if (deletions !== null) {
2144 + for (let i = 0; i < deletions.length; i++) {
2145 + const deletion = deletions[i];
2146 + detachFiberAfterEffects(deletion);
2147 + }
2148 + }
2149 }
2150 nextEffect = nextNextEffect;
2151 }
@@ -3708,8 +3718,3 @@ export function act(callback: () => Thenable<mixed>): Thenable<void> {
3718 };
3719 }
3720 }
3711 -
3712 -function detachFiberAfterEffects(fiber: Fiber): void {
3713 - fiber.sibling = null;
3714 - fiber.stateNode = null;
3715 -}
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+13 -8
@@ -122,6 +122,7 @@ import {
122 Update,
123 PlacementAndUpdate,
124 Deletion,
125 + ChildDeletion,
126 Ref,
127 ContentReset,
128 Snapshot,
@@ -194,6 +195,7 @@ import {
195 commitResetTextContent,
196 isSuspenseBoundaryBeingHidden,
197 commitPassiveMountEffects,
198 + detachFiberAfterEffects,
199 } from './ReactFiberCommitWork.old';
200 import {enqueueUpdate} from './ReactUpdateQueue.old';
201 import {resetContextDependencies} from './ReactFiberNewContext.old';
@@ -2129,13 +2131,21 @@ function commitRootImpl(root, renderPriorityLevel) {
2131 } else {
2132 // We are done with the effect chain at this point so let's clear the
2133 // nextEffect pointers to assist with GC. If we have passive effects, we'll
2132 - // clear this in flushPassiveEffects.
2134 + // clear this in flushPassiveEffects
2135 + // TODO: We should always do this in the passive phase, by scheduling
2136 + // a passive callback for every deletion.
2137 nextEffect = firstEffect;
2138 while (nextEffect !== null) {
2139 const nextNextEffect = nextEffect.nextEffect;
2140 nextEffect.nextEffect = null;
2137 - if (nextEffect.flags & Deletion) {
2138 - detachFiberAfterEffects(nextEffect);
2141 + if (nextEffect.flags & ChildDeletion) {
2142 + const deletions = nextEffect.deletions;
2143 + if (deletions !== null) {
2144 + for (let i = 0; i < deletions.length; i++) {
2145 + const deletion = deletions[i];
2146 + detachFiberAfterEffects(deletion);
2147 + }
2148 + }
2149 }
2150 nextEffect = nextNextEffect;
2151 }
@@ -3708,8 +3718,3 @@ export function act(callback: () => Thenable<mixed>): Thenable<void> {
3718 };
3719 }
3720 }
3711 -
3712 -function detachFiberAfterEffects(fiber: Fiber): void {
3713 - fiber.sibling = null;
3714 - fiber.stateNode = null;
3715 -}