Skip to content

Commit eae90cd

Browse files
Brian Vaughnlunaruan
Brian Vaughn
and
lunaruan
authored
Effects list refactor continued: passive effects traversal (facebook#19374)
* Adds new `Passive` subtree tag value. * Adds recursive traversal for passive effects (mounts and unmounts). * Removes `pendingPassiveHookEffectsMount` and `pendingPassiveHookEffectsUnmount` arrays from work loop. * Re-adds sibling and child pointer detaching (temporarily removed in previous PR). * Addresses some minor TODO comments left over from previous PRs. --- Co-authored-by: Luna Ruan <luna@fb.com>
1 parent 74cd7e5 commit eae90cd

11 files changed

+458
-274
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import type {Fiber} from './ReactInternalTypes';
1515
import type {Lanes} from './ReactFiberLane';
1616

1717
import getComponentName from 'shared/getComponentName';
18-
import {Placement, Deletion} from './ReactSideEffectTags';
18+
import {Deletion, Placement} from './ReactSideEffectTags';
1919
import {
2020
getIteratorFn,
2121
REACT_ELEMENT_TYPE,

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
enableScopeAPI,
3030
enableBlocksAPI,
3131
} from 'shared/ReactFeatureFlags';
32-
import {NoEffect, Placement} from './ReactSideEffectTags';
32+
import {NoEffect, Placement, StaticMask} from './ReactSideEffectTags';
3333
import {NoEffect as NoSubtreeEffect} from './ReactSubtreeTags';
3434
import {ConcurrentRoot, BlockingRoot} from './ReactRootTags';
3535
import {
@@ -288,8 +288,6 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
288288
workInProgress.type = current.type;
289289

290290
// We already have an alternate.
291-
// Reset the effect tag.
292-
workInProgress.effectTag = NoEffect;
293291
workInProgress.subtreeTag = NoSubtreeEffect;
294292
workInProgress.deletions = null;
295293

@@ -308,6 +306,9 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
308306
}
309307
}
310308

309+
// Reset all effects except static ones.
310+
// Static effects are not specific to a render.
311+
workInProgress.effectTag = current.effectTag & StaticMask;
311312
workInProgress.childLanes = current.childLanes;
312313
workInProgress.lanes = current.lanes;
313314

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2064,7 +2064,6 @@ function updateSuspensePrimaryChildren(
20642064
if (currentFallbackChildFragment !== null) {
20652065
// Delete the fallback child fragment
20662066
currentFallbackChildFragment.nextEffect = null;
2067-
currentFallbackChildFragment.effectTag = Deletion;
20682067
workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChildFragment;
20692068
const deletions = workInProgress.deletions;
20702069
if (deletions === null) {

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

Lines changed: 31 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ import {
6868
Placement,
6969
Snapshot,
7070
Update,
71+
Passive,
7172
} from './ReactSideEffectTags';
7273
import getComponentName from 'shared/getComponentName';
7374
import invariant from 'shared/invariant';
@@ -115,9 +116,8 @@ import {
115116
captureCommitPhaseError,
116117
resolveRetryWakeable,
117118
markCommitTimeOfFallback,
118-
enqueuePendingPassiveHookEffectMount,
119-
enqueuePendingPassiveHookEffectUnmount,
120119
enqueuePendingPassiveProfilerEffect,
120+
schedulePassiveEffectCallback,
121121
} from './ReactFiberWorkLoop.new';
122122
import {
123123
NoEffect as NoHookEffect,
@@ -130,6 +130,10 @@ import {
130130
updateDeprecatedEventListeners,
131131
unmountDeprecatedResponderListeners,
132132
} from './ReactFiberDeprecatedEvents.new';
133+
import {
134+
NoEffect as NoSubtreeTag,
135+
Passive as PassiveSubtreeTag,
136+
} from './ReactSubtreeTags';
133137

134138
let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
135139
if (__DEV__) {
@@ -381,26 +385,6 @@ function commitHookEffectListMount(tag: number, finishedWork: Fiber) {
381385
}
382386
}
383387

384-
function schedulePassiveEffects(finishedWork: Fiber) {
385-
const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any);
386-
const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null;
387-
if (lastEffect !== null) {
388-
const firstEffect = lastEffect.next;
389-
let effect = firstEffect;
390-
do {
391-
const {next, tag} = effect;
392-
if (
393-
(tag & HookPassive) !== NoHookEffect &&
394-
(tag & HookHasEffect) !== NoHookEffect
395-
) {
396-
enqueuePendingPassiveHookEffectUnmount(finishedWork, effect);
397-
enqueuePendingPassiveHookEffectMount(finishedWork, effect);
398-
}
399-
effect = next;
400-
} while (effect !== firstEffect);
401-
}
402-
}
403-
404388
export function commitPassiveEffectDurations(
405389
finishedRoot: FiberRoot,
406390
finishedWork: Fiber,
@@ -486,7 +470,9 @@ function commitLifeCycles(
486470
commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
487471
}
488472

489-
schedulePassiveEffects(finishedWork);
473+
if ((finishedWork.subtreeTag & PassiveSubtreeTag) !== NoSubtreeTag) {
474+
schedulePassiveEffectCallback();
475+
}
490476
return;
491477
}
492478
case ClassComponent: {
@@ -892,7 +878,12 @@ function commitUnmount(
892878
const {destroy, tag} = effect;
893879
if (destroy !== undefined) {
894880
if ((tag & HookPassive) !== NoHookEffect) {
895-
enqueuePendingPassiveHookEffectUnmount(current, effect);
881+
// TODO: Consider if we can move this block out of the synchronous commit phase
882+
effect.tag |= HookHasEffect;
883+
884+
current.effectTag |= Passive;
885+
886+
schedulePassiveEffectCallback();
896887
} else {
897888
if (
898889
enableProfilerTimer &&
@@ -1013,29 +1004,24 @@ function commitNestedUnmounts(
10131004
}
10141005

10151006
function detachFiberMutation(fiber: Fiber) {
1016-
// Cut off the return pointers to disconnect it from the tree. Ideally, we
1017-
// should clear the child pointer of the parent alternate to let this
1007+
// Cut off the return pointer to disconnect it from the tree.
1008+
// This enables us to detect and warn against state updates on an unmounted component.
1009+
// It also prevents events from bubbling from within disconnected components.
1010+
//
1011+
// Ideally, we should also clear the child pointer of the parent alternate to let this
10181012
// get GC:ed but we don't know which for sure which parent is the current
1019-
// one so we'll settle for GC:ing the subtree of this child. This child
1020-
// itself will be GC:ed when the parent updates the next time.
1021-
// Note: we cannot null out sibling here, otherwise it can cause issues
1022-
// with findDOMNode and how it requires the sibling field to carry out
1023-
// traversal in a later effect. See PR #16820. We now clear the sibling
1024-
// field after effects, see: detachFiberAfterEffects.
1025-
fiber.alternate = null;
1026-
fiber.child = null;
1027-
fiber.dependencies = null;
1028-
fiber.firstEffect = null;
1029-
fiber.lastEffect = null;
1030-
fiber.memoizedProps = null;
1031-
fiber.memoizedState = null;
1032-
fiber.pendingProps = null;
1033-
fiber.return = null;
1034-
fiber.stateNode = null;
1035-
fiber.updateQueue = null;
1036-
if (__DEV__) {
1037-
fiber._debugOwner = null;
1013+
// one so we'll settle for GC:ing the subtree of this child.
1014+
// This child itself will be GC:ed when the parent updates the next time.
1015+
//
1016+
// Note that we can't clear child or sibling pointers yet.
1017+
// They're needed for passive effects and for findDOMNode.
1018+
// We defer those fields, and all other cleanup, to the passive phase (see detachFiberAfterEffects).
1019+
const alternate = fiber.alternate;
1020+
if (alternate !== null) {
1021+
alternate.return = null;
1022+
fiber.alternate = null;
10381023
}
1024+
fiber.return = null;
10391025
}
10401026

10411027
function emptyPortalContainer(current: Fiber) {

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {createDeprecatedResponderListener} from './ReactFiberDeprecatedEvents.ne
5050
import {
5151
Update as UpdateEffect,
5252
Passive as PassiveEffect,
53+
PassiveStatic as PassiveStaticEffect,
5354
} from './ReactSideEffectTags';
5455
import {
5556
HasEffect as HookHasEffect,
@@ -1270,7 +1271,7 @@ function mountEffect(
12701271
}
12711272
}
12721273
return mountEffectImpl(
1273-
UpdateEffect | PassiveEffect,
1274+
UpdateEffect | PassiveEffect | PassiveStaticEffect,
12741275
HookPassive,
12751276
create,
12761277
deps,
@@ -1631,7 +1632,8 @@ function mountOpaqueIdentifier(): OpaqueIDType | void {
16311632
const setId = mountState(id)[1];
16321633

16331634
if ((currentlyRenderingFiber.mode & BlockingMode) === NoMode) {
1634-
currentlyRenderingFiber.effectTag |= UpdateEffect | PassiveEffect;
1635+
currentlyRenderingFiber.effectTag |=
1636+
UpdateEffect | PassiveEffect | PassiveStaticEffect;
16351637
pushEffect(
16361638
HookHasEffect | HookPassive,
16371639
() => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ function deleteHydratableInstance(
124124
const childToDelete = createFiberFromHostInstanceForDeletion();
125125
childToDelete.stateNode = instance;
126126
childToDelete.return = returnFiber;
127-
childToDelete.effectTag = Deletion;
127+
128128
const deletions = returnFiber.deletions;
129129
if (deletions === null) {
130130
returnFiber.deletions = [childToDelete];

0 commit comments

Comments
 (0)