Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Effects list refactor continued: passive effects traversal #19374

Merged
merged 11 commits into from
Jul 29, 2020
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactChildFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import type {Fiber} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane';

import getComponentName from 'shared/getComponentName';
import {Placement, Deletion} from './ReactSideEffectTags';
import {Deletion, Placement} from './ReactSideEffectTags';
import {
getIteratorFn,
REACT_ELEMENT_TYPE,
Expand Down
7 changes: 4 additions & 3 deletions packages/react-reconciler/src/ReactFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
enableScopeAPI,
enableBlocksAPI,
} from 'shared/ReactFeatureFlags';
import {NoEffect, Placement} from './ReactSideEffectTags';
import {NoEffect, Placement, StaticMask} from './ReactSideEffectTags';
import {NoEffect as NoSubtreeEffect} from './ReactSubtreeTags';
import {ConcurrentRoot, BlockingRoot} from './ReactRootTags';
import {
Expand Down Expand Up @@ -288,8 +288,6 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
workInProgress.type = current.type;

// We already have an alternate.
// Reset the effect tag.
workInProgress.effectTag = NoEffect;
workInProgress.subtreeTag = NoSubtreeEffect;
workInProgress.deletions = null;

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

// Reset all effects except static ones.
// Static effects are not specific to a render.
workInProgress.effectTag = current.effectTag & StaticMask;
workInProgress.childLanes = current.childLanes;
workInProgress.lanes = current.lanes;

Expand Down
1 change: 0 additions & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2064,7 +2064,6 @@ function updateSuspensePrimaryChildren(
if (currentFallbackChildFragment !== null) {
// Delete the fallback child fragment
currentFallbackChildFragment.nextEffect = null;
currentFallbackChildFragment.effectTag = Deletion;
workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChildFragment;
const deletions = workInProgress.deletions;
if (deletions === null) {
Expand Down
76 changes: 31 additions & 45 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import {
Placement,
Snapshot,
Update,
Passive,
} from './ReactSideEffectTags';
import getComponentName from 'shared/getComponentName';
import invariant from 'shared/invariant';
Expand Down Expand Up @@ -115,9 +116,8 @@ import {
captureCommitPhaseError,
resolveRetryWakeable,
markCommitTimeOfFallback,
enqueuePendingPassiveHookEffectMount,
enqueuePendingPassiveHookEffectUnmount,
enqueuePendingPassiveProfilerEffect,
schedulePassiveEffectCallback,
} from './ReactFiberWorkLoop.new';
import {
NoEffect as NoHookEffect,
Expand All @@ -130,6 +130,10 @@ import {
updateDeprecatedEventListeners,
unmountDeprecatedResponderListeners,
} from './ReactFiberDeprecatedEvents.new';
import {
NoEffect as NoSubtreeTag,
Passive as PassiveSubtreeTag,
} from './ReactSubtreeTags';

let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
if (__DEV__) {
Expand Down Expand Up @@ -381,26 +385,6 @@ function commitHookEffectListMount(tag: number, finishedWork: Fiber) {
}
}

function schedulePassiveEffects(finishedWork: Fiber) {
const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any);
const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null;
if (lastEffect !== null) {
const firstEffect = lastEffect.next;
let effect = firstEffect;
do {
const {next, tag} = effect;
if (
(tag & HookPassive) !== NoHookEffect &&
(tag & HookHasEffect) !== NoHookEffect
) {
enqueuePendingPassiveHookEffectUnmount(finishedWork, effect);
enqueuePendingPassiveHookEffectMount(finishedWork, effect);
}
effect = next;
} while (effect !== firstEffect);
}
}

export function commitPassiveEffectDurations(
finishedRoot: FiberRoot,
finishedWork: Fiber,
Expand Down Expand Up @@ -486,7 +470,9 @@ function commitLifeCycles(
commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
}

schedulePassiveEffects(finishedWork);
if ((finishedWork.subtreeTag & PassiveSubtreeTag) !== NoSubtreeTag) {
schedulePassiveEffectCallback();
}
return;
}
case ClassComponent: {
Expand Down Expand Up @@ -892,7 +878,12 @@ function commitUnmount(
const {destroy, tag} = effect;
if (destroy !== undefined) {
if ((tag & HookPassive) !== NoHookEffect) {
enqueuePendingPassiveHookEffectUnmount(current, effect);
// TODO: Consider if we can move this block out of the synchronous commit phase
effect.tag |= HookHasEffect;
bvaughn marked this conversation as resolved.
Show resolved Hide resolved

current.effectTag |= Passive;
bvaughn marked this conversation as resolved.
Show resolved Hide resolved

schedulePassiveEffectCallback();
bvaughn marked this conversation as resolved.
Show resolved Hide resolved
} else {
if (
enableProfilerTimer &&
Expand Down Expand Up @@ -1013,29 +1004,24 @@ function commitNestedUnmounts(
}

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
// Cut off the return pointer to disconnect it from the tree.
// This enables us to detect and warn against state updates on an unmounted component.
// It also prevents events from bubbling from within disconnected components.
//
// Ideally, we should also 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. We now clear the sibling
// field after effects, see: detachFiberAfterEffects.
fiber.alternate = null;
fiber.child = null;
fiber.dependencies = null;
fiber.firstEffect = null;
fiber.lastEffect = null;
fiber.memoizedProps = null;
fiber.memoizedState = null;
fiber.pendingProps = null;
fiber.return = null;
fiber.stateNode = null;
fiber.updateQueue = null;
if (__DEV__) {
fiber._debugOwner = null;
// 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 that we can't clear child or sibling pointers yet.
// They're needed for passive effects and for findDOMNode.
// We defer those fields, and all other cleanup, to the passive phase (see detachFiberAfterEffects).
bvaughn marked this conversation as resolved.
Show resolved Hide resolved
const alternate = fiber.alternate;
if (alternate !== null) {
alternate.return = null;
fiber.alternate = null;
}
fiber.return = null;
}

function emptyPortalContainer(current: Fiber) {
Expand Down
6 changes: 4 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {createDeprecatedResponderListener} from './ReactFiberDeprecatedEvents.ne
import {
Update as UpdateEffect,
Passive as PassiveEffect,
PassiveStatic as PassiveStaticEffect,
} from './ReactSideEffectTags';
import {
HasEffect as HookHasEffect,
Expand Down Expand Up @@ -1270,7 +1271,7 @@ function mountEffect(
}
}
return mountEffectImpl(
UpdateEffect | PassiveEffect,
UpdateEffect | PassiveEffect | PassiveStaticEffect,
bvaughn marked this conversation as resolved.
Show resolved Hide resolved
HookPassive,
create,
deps,
Expand Down Expand Up @@ -1631,7 +1632,8 @@ function mountOpaqueIdentifier(): OpaqueIDType | void {
const setId = mountState(id)[1];

if ((currentlyRenderingFiber.mode & BlockingMode) === NoMode) {
currentlyRenderingFiber.effectTag |= UpdateEffect | PassiveEffect;
currentlyRenderingFiber.effectTag |=
UpdateEffect | PassiveEffect | PassiveStaticEffect;
pushEffect(
HookHasEffect | HookPassive,
() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ function deleteHydratableInstance(
const childToDelete = createFiberFromHostInstanceForDeletion();
childToDelete.stateNode = instance;
childToDelete.return = returnFiber;
childToDelete.effectTag = Deletion;

const deletions = returnFiber.deletions;
if (deletions === null) {
returnFiber.deletions = [childToDelete];
Expand Down
Loading