Skip to content

Commit

Permalink
offscreen double invoke effects (#19523)
Browse files Browse the repository at this point in the history 
This PR double invokes effects in __DEV__ mode.

We are thinking about unmounting layout and/or passive effects for a hidden tree. To understand potential issues with this, we want to double invoke effects. This PR changes the behavior in DEV when an effect runs from create() to create() -> destroy() -> create(). The effect cleanup function will still be called before the effect runs in both dev and prod. (Note: This change is purely for research for now as it is likely to break real code.)

**Note: The change is fully behind a flag and does not affect any of the code on npm.**
  • Loading branch information
@lunaruan
lunaruan committed Sep 24, 2020
1 parent a99bf5c commit c63741f
Show file tree
Hide file tree
Showing 17 changed files with 888 additions and 60 deletions.
27 changes: 22 additions & 5 deletions packages/react-reconciler/src/ReactFiberClassComponent.new.js
View file
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
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
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;
Loading

0 comments on commit c63741f

Please sign in to comment.