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

Schedule passive phase whenever there's a deletion #20624

Merged
merged 1 commit into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 1 addition & 27 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,6 @@ import {
captureCommitPhaseError,
resolveRetryWakeable,
markCommitTimeOfFallback,
enqueuePendingPassiveHookEffectMount,
enqueuePendingPassiveHookEffectUnmount,
enqueuePendingPassiveProfilerEffect,
} from './ReactFiberWorkLoop.new';
import {
Expand Down Expand Up @@ -533,26 +531,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 @@ -639,8 +617,6 @@ function commitLayoutEffectOnFiber(
} else {
commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
}

schedulePassiveEffects(finishedWork);
break;
}
case ClassComponent: {
Expand Down Expand Up @@ -1091,9 +1067,7 @@ function commitUnmount(
do {
const {destroy, tag} = effect;
if (destroy !== undefined) {
if ((tag & HookPassive) !== NoHookEffect) {
enqueuePendingPassiveHookEffectUnmount(current, effect);
} else {
if ((tag & HookLayout) !== NoHookEffect) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export const MutationMask =
Hydrating |
Visibility;
export const LayoutMask = Update | Callback | Ref;

// TODO: Split into PassiveMountMask and PassiveUnmountMask
export const PassiveMask = Passive | ChildDeletion;

// Union of tags that don't get reset on clones.
Expand Down
13 changes: 5 additions & 8 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,8 @@ export function bailoutHooks(
lanes: Lanes,
) {
workInProgress.updateQueue = current.updateQueue;
// TODO: Don't need to reset the flags here, because they're reset in the
// complete phase (bubbleProperties).
workInProgress.flags &= ~(PassiveEffect | UpdateEffect);
current.lanes = removeLanes(current.lanes, lanes);
}
Expand Down Expand Up @@ -1309,7 +1311,7 @@ function mountEffect(
}
}
return mountEffectImpl(
UpdateEffect | PassiveEffect | PassiveStaticEffect,
PassiveEffect | PassiveStaticEffect,
HookPassive,
create,
deps,
Expand All @@ -1326,12 +1328,7 @@ function updateEffect(
warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
}
}
return updateEffectImpl(
UpdateEffect | PassiveEffect,
HookPassive,
create,
deps,
);
return updateEffectImpl(PassiveEffect, HookPassive, create, deps);
}

function mountLayoutEffect(
Expand Down Expand Up @@ -1683,7 +1680,7 @@ function mountOpaqueIdentifier(): OpaqueIDType | void {
const setId = mountState(id)[1];

if ((currentlyRenderingFiber.mode & BlockingMode) === NoMode) {
currentlyRenderingFiber.flags |= UpdateEffect | PassiveEffect;
currentlyRenderingFiber.flags |= PassiveEffect;
pushEffect(
HookHasEffect | HookPassive,
() => {
Expand Down
61 changes: 3 additions & 58 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import type {Lanes, Lane} from './ReactFiberLane.new';
import type {ReactPriorityLevel} from './ReactInternalTypes';
import type {Interaction} from 'scheduler/src/Tracing';
import type {SuspenseState} from './ReactFiberSuspenseComponent.new';
import type {Effect as HookEffect} from './ReactFiberHooks.new';
import type {StackCursor} from './ReactFiberStack.new';
import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new';

Expand Down Expand Up @@ -120,12 +119,11 @@ import {
NoFlags,
PerformedWork,
Placement,
ChildDeletion,
Passive,
PassiveStatic,
Incomplete,
HostEffectMask,
Hydrating,
PassiveMask,
StaticMask,
} from './ReactFiberFlags';
import {
Expand Down Expand Up @@ -183,7 +181,6 @@ import {
commitPassiveEffectDurations,
commitPassiveMountEffects,
commitPassiveUnmountEffects,
detachFiberAfterEffects,
} from './ReactFiberCommitWork.new';
import {enqueueUpdate} from './ReactUpdateQueue.new';
import {resetContextDependencies} from './ReactFiberNewContext.new';
Expand Down Expand Up @@ -314,7 +311,6 @@ export function getRenderTargetTime(): number {
return workInProgressRootRenderTargetTime;
}

let nextEffect: Fiber | null = null;
let hasUncaughtError = false;
let firstUncaughtError = null;
let legacyErrorBoundariesThatAlreadyFailed: Set<mixed> | null = null;
Expand Down Expand Up @@ -1938,8 +1934,8 @@ function commitRootImpl(root, renderPriorityLevel) {
// TODO: Delete all other places that schedule the passive effect callback
// They're redundant.
if (
(finishedWork.subtreeFlags & Passive) !== NoFlags ||
(finishedWork.flags & Passive) !== NoFlags
(finishedWork.subtreeFlags & PassiveMask) !== NoFlags ||
(finishedWork.flags & PassiveMask) !== NoFlags
) {
if (!rootDoesHavePassiveEffects) {
rootDoesHavePassiveEffects = true;
Expand Down Expand Up @@ -2061,31 +2057,6 @@ function commitRootImpl(root, renderPriorityLevel) {
rootWithPendingPassiveEffects = root;
pendingPassiveEffectsLanes = lanes;
pendingPassiveEffectsRenderPriority = renderPriorityLevel;
} else {
// We are done with the effect chain at this point so let's clear the
// nextEffect pointers to assist with GC. If we have passive effects, we'll
// clear this in flushPassiveEffects
// TODO: We should always do this in the passive phase, by scheduling
// a passive callback for every deletion.
nextEffect = firstEffect;
while (nextEffect !== null) {
const nextNextEffect = nextEffect.nextEffect;
nextEffect.nextEffect = null;
if (nextEffect.flags & ChildDeletion) {
const deletions = nextEffect.deletions;
if (deletions !== null) {
for (let i = 0; i < deletions.length; i++) {
const deletion = deletions[i];
const alternate = deletion.alternate;
detachFiberAfterEffects(deletion);
if (alternate !== null) {
detachFiberAfterEffects(alternate);
}
}
}
}
nextEffect = nextNextEffect;
}
}

// Read this again, since an effect might have updated it
Expand Down Expand Up @@ -2229,32 +2200,6 @@ export function enqueuePendingPassiveProfilerEffect(fiber: Fiber): void {
}
}

export function enqueuePendingPassiveHookEffectMount(
fiber: Fiber,
effect: HookEffect,
): void {
if (!rootDoesHavePassiveEffects) {
rootDoesHavePassiveEffects = true;
scheduleCallback(NormalSchedulerPriority, () => {
flushPassiveEffects();
return null;
});
}
}

export function enqueuePendingPassiveHookEffectUnmount(
fiber: Fiber,
effect: HookEffect,
): void {
if (!rootDoesHavePassiveEffects) {
rootDoesHavePassiveEffects = true;
scheduleCallback(NormalSchedulerPriority, () => {
flushPassiveEffects();
return null;
});
}
}

function flushPassiveEffectsImpl() {
if (rootWithPendingPassiveEffects === null) {
return false;
Expand Down