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.

Sign up for GitHub

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

Double Invoke Effects in __DEV__ #19523

Merged
merged 1 commit into from
Sep 24, 2020
Merged

Double Invoke Effects in __DEV__ #19523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 22 additions & 5 deletions packages/react-reconciler/src/ReactFiberClassComponent.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ import type {Lanes} from './ReactFiberLane';
import type {UpdateQueue} from './ReactUpdateQueue.new';

import * as React from 'react';
import {Update, Snapshot} from './ReactFiberFlags';
import {Update, Snapshot, MountLayoutDev} from './ReactFiberFlags';
import {
debugRenderPhaseSideEffectsForStrictMode,
disableLegacyContext,
enableDebugTracing,
enableSchedulingProfiler,
warnAboutDeprecatedLifecycles,
enableDoubleInvokingEffects,
} from 'shared/ReactFeatureFlags';
import ReactStrictModeWarnings from './ReactStrictModeWarnings.new';
import {isMounted} from './ReactFiberTreeReflection';
Expand Down Expand Up @@ -890,7 +891,11 @@ function mountClassInstance(
}

if (typeof instance.componentDidMount === 'function') {
workInProgress.flags |= Update;
if (__DEV__ && enableDoubleInvokingEffects) {
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
}
}

Expand Down Expand Up @@ -960,7 +965,11 @@ function resumeMountClassInstance(
// If an update was already in progress, we should schedule an Update
// effect even though we're bailing out, so that cWU/cDU are called.
if (typeof instance.componentDidMount === 'function') {
workInProgress.flags |= Update;
if (__DEV__ && enableDoubleInvokingEffects) {
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
}
return false;
}
Expand Down Expand Up @@ -1003,13 +1012,21 @@ function resumeMountClassInstance(
}
}
if (typeof instance.componentDidMount === 'function') {
workInProgress.flags |= Update;
if (__DEV__ && enableDoubleInvokingEffects) {
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
}
} else {
// If an update was already in progress, we should schedule an Update
// effect even though we're bailing out, so that cWU/cDU are called.
if (typeof instance.componentDidMount === 'function') {
workInProgress.flags |= Update;
if (__DEV__ && enableDoubleInvokingEffects) {
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
}

// If shouldComponentUpdate returned false, we should still update the
Expand Down
140 changes: 135 additions & 5 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
enableFundamentalAPI,
enableSuspenseCallback,
enableScopeAPI,
enableDoubleInvokingEffects,
} from 'shared/ReactFeatureFlags';
import {
FunctionComponent,
Expand Down Expand Up @@ -159,7 +160,7 @@ const callComponentWillUnmountWithTimer = function(current, instance) {
function safelyCallComponentWillUnmount(
current: Fiber,
instance: any,
nearestMountedAncestor: Fiber,
nearestMountedAncestor: Fiber | null,
) {
if (__DEV__) {
invokeGuardedCallback(
Expand Down Expand Up @@ -318,7 +319,7 @@ function commitBeforeMutationLifeCycles(
}

function commitHookEffectListUnmount(
tag: HookFlags,
flags: HookFlags,
finishedWork: Fiber,
nearestMountedAncestor: Fiber | null,
) {
Expand All @@ -328,7 +329,7 @@ function commitHookEffectListUnmount(
const firstEffect = lastEffect.next;
let effect = firstEffect;
do {
if ((effect.tag & tag) === tag) {
if ((effect.tag & flags) === flags) {
// Unmount
const destroy = effect.destroy;
effect.destroy = undefined;
Expand All @@ -341,14 +342,14 @@ function commitHookEffectListUnmount(
}
}

function commitHookEffectListMount(tag: HookFlags, finishedWork: Fiber) {
function commitHookEffectListMount(flags: HookFlags, 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 {
if ((effect.tag & tag) === tag) {
if ((effect.tag & flags) === flags) {
// Mount
const create = effect.create;
effect.destroy = create();
Expand Down Expand Up @@ -1884,6 +1885,131 @@ function commitPassiveMount(
}
}

function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent:
case Block: {
invokeGuardedCallback(
null,
commitHookEffectListMount,
null,
HookLayout | HookHasEffect,
fiber,
);
if (hasCaughtError()) {
const mountError = clearCaughtError();
captureCommitPhaseError(fiber, fiber.return, mountError);
}
break;
}
case ClassComponent: {
const instance = fiber.stateNode;
invokeGuardedCallback(null, instance.componentDidMount, null);
if (hasCaughtError()) {
const mountError = clearCaughtError();
captureCommitPhaseError(fiber, fiber.return, mountError);
}
break;
}
}
}
}

function invokePassiveEffectMountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent:
case Block: {
invokeGuardedCallback(
null,
commitHookEffectListMount,
null,
HookPassive | HookHasEffect,
fiber,
);
if (hasCaughtError()) {
const mountError = clearCaughtError();
captureCommitPhaseError(fiber, fiber.return, mountError);
}
break;
}
}
}
}

function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent:
case Block: {
invokeGuardedCallback(
null,
commitHookEffectListUnmount,
null,
HookLayout | HookHasEffect,
fiber,
fiber.return,
);
if (hasCaughtError()) {
const unmountError = clearCaughtError();
captureCommitPhaseError(fiber, fiber.return, unmountError);
}
break;
}
case ClassComponent: {
const instance = fiber.stateNode;
if (typeof instance.componentWillUnmount === 'function') {
invokeGuardedCallback(
null,
safelyCallComponentWillUnmount,
null,
fiber,
instance,
fiber.return,
);
if (hasCaughtError()) {
const unmountError = clearCaughtError();
captureCommitPhaseError(fiber, fiber.return, unmountError);
}
}
break;
}
}
}
}

function invokePassiveEffectUnmountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent:
case Block: {
invokeGuardedCallback(
null,
commitHookEffectListUnmount,
null,
HookPassive | HookHasEffect,
fiber,
fiber.return,
);
if (hasCaughtError()) {
const unmountError = clearCaughtError();
captureCommitPhaseError(fiber, fiber.return, unmountError);
}
break;
}
}
}
}

export {
commitBeforeMutationLifeCycles,
commitResetTextContent,
Expand All @@ -1896,4 +2022,8 @@ export {
commitPassiveUnmount,
commitPassiveUnmountInsideDeletedTree,
commitPassiveMount,
invokeLayoutEffectMountInDEV,
invokeLayoutEffectUnmountInDEV,
invokePassiveEffectMountInDEV,
invokePassiveEffectUnmountInDEV,
};
58 changes: 32 additions & 26 deletions packages/react-reconciler/src/ReactFiberFlags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,50 +10,56 @@
export type Flags = number;

// Don't change these two values. They're used by React Dev Tools.
export const NoFlags = /* */ 0b0000000000000000;
export const PerformedWork = /* */ 0b0000000000000001;
export const NoFlags = /* */ 0b000000000000000000;
export const PerformedWork = /* */ 0b000000000000000001;

// You can change the rest (and add more).
export const Placement = /* */ 0b0000000000000010;
export const Update = /* */ 0b0000000000000100;
export const PlacementAndUpdate = /* */ 0b0000000000000110;
export const Deletion = /* */ 0b0000000000001000;
export const ContentReset = /* */ 0b0000000000010000;
export const Callback = /* */ 0b0000000000100000;
export const DidCapture = /* */ 0b0000000001000000;
export const Ref = /* */ 0b0000000010000000;
export const Snapshot = /* */ 0b0000000100000000;
export const Passive = /* */ 0b0000001000000000;
export const Placement = /* */ 0b000000000000000010;
export const Update = /* */ 0b000000000000000100;
export const PlacementAndUpdate = /* */ 0b000000000000000110;
export const Deletion = /* */ 0b000000000000001000;
export const ContentReset = /* */ 0b000000000000010000;
export const Callback = /* */ 0b000000000000100000;
export const DidCapture = /* */ 0b000000000001000000;
export const Ref = /* */ 0b000000000010000000;
export const Snapshot = /* */ 0b000000000100000000;
export const Passive = /* */ 0b000000001000000000;
// TODO (effects) Remove this bit once the new reconciler is synced to the old.
export const PassiveUnmountPendingDev = /* */ 0b0010000000000000;
export const Hydrating = /* */ 0b0000010000000000;
export const HydratingAndUpdate = /* */ 0b0000010000000100;
export const PassiveUnmountPendingDev = /* */ 0b000010000000000000;
export const Hydrating = /* */ 0b000000010000000000;
export const HydratingAndUpdate = /* */ 0b000000010000000100;

// Passive & Update & Callback & Ref & Snapshot
export const LifecycleEffectMask = /* */ 0b0000001110100100;
export const LifecycleEffectMask = /* */ 0b000000001110100100;

// Union of all host effects
export const HostEffectMask = /* */ 0b0000011111111111;
export const HostEffectMask = /* */ 0b000000011111111111;

// These are not really side effects, but we still reuse this field.
export const Incomplete = /* */ 0b0000100000000000;
export const ShouldCapture = /* */ 0b0001000000000000;
export const ForceUpdateForLegacySuspense = /* */ 0b0100000000000000;
export const Incomplete = /* */ 0b000000100000000000;
export const ShouldCapture = /* */ 0b000001000000000000;
export const ForceUpdateForLegacySuspense = /* */ 0b000100000000000000;

// Static tags describe aspects of a fiber that are not specific to a render,
// e.g. a fiber uses a passive effect (even if there are no updates on this particular render).
// This enables us to defer more work in the unmount case,
// since we can defer traversing the tree during layout to look for Passive effects,
// and instead rely on the static flag as a signal that there may be cleanup work.
export const PassiveStatic = /* */ 0b1000000000000000;
export const PassiveStatic = /* */ 0b001000000000000000;

// Union of side effect groupings as pertains to subtreeFlags
export const BeforeMutationMask = /* */ 0b0000001100001010;
export const MutationMask = /* */ 0b0000010010011110;
export const LayoutMask = /* */ 0b0000000010100100;
export const PassiveMask = /* */ 0b0000001000001000;
export const BeforeMutationMask = /* */ 0b000000001100001010;
export const MutationMask = /* */ 0b000000010010011110;
export const LayoutMask = /* */ 0b000000000010100100;
export const PassiveMask = /* */ 0b000000001000001000;

// Union of tags that don't get reset on clones.
// This allows certain concepts to persist without recalculting them,
// e.g. whether a subtree contains passive effects or portals.
export const StaticMask = /* */ 0b1000000000000000;
export const StaticMask = /* */ 0b001000000000000000;

// These flags allow us to traverse to fibers that have effects on mount
// without traversing the entire tree after every commit for
// double invoking
export const MountLayoutDev = /* */ 0b010000000000000000;
export const MountPassiveDev = /* */ 0b100000000000000000;
Comment on lines +64 to +65
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, I found myself wondering why these two flags were necessary. I think what they enable us to do is to avoid traversing the entire tree after each commit just to see if any of the nested components were newly mounted. (Checking the Update or Passive effects alone wouldn't tell us enough info.)

Maybe we can add a small explanatory comment above the flags in case anyone else wonders about this?

Loading