Skip to content

Commit

Permalink
Re-add "strict effects mode" for legacy roots only (facebook#20639)
Browse files Browse the repository at this point in the history 
This combines changes originally made in facebook#19523, facebook#20028, and facebook#20415 but with slightly different semantics: "strict effects" mode is enabled only for the experimental root APIs (never for legacy render, regardless of <StrictMode> usage). These semantics may change slightly in the future.
  • Loading branch information
@koto
Brian Vaughn authored and koto committed Jun 15, 2021
1 parent 0b22b27 commit 4a2dc76
Show file tree
Hide file tree
Showing 10 changed files with 707 additions and 102 deletions.
55 changes: 49 additions & 6 deletions packages/react-reconciler/src/ReactFiberClassComponent.new.js
View file
Expand Up @@ -12,13 +12,14 @@ import type {Lanes} from './ReactFiberLane.new';
import type {UpdateQueue} from './ReactUpdateQueue.new';

import * as React from 'react';
import {Update, Snapshot} from './ReactFiberFlags';
import {MountLayoutDev, Update, Snapshot} from './ReactFiberFlags';
import {
debugRenderPhaseSideEffectsForStrictMode,
disableLegacyContext,
enableDebugTracing,
enableSchedulingProfiler,
warnAboutDeprecatedLifecycles,
enableDoubleInvokingEffects,
} from 'shared/ReactFeatureFlags';
import ReactStrictModeWarnings from './ReactStrictModeWarnings.new';
import {isMounted} from './ReactFiberTreeReflection';
Expand All @@ -29,7 +30,13 @@ import invariant from 'shared/invariant';
import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols';

import {resolveDefaultProps} from './ReactFiberLazyComponent.new';
import {DebugTracingMode, StrictMode} from './ReactTypeOfMode';
import {
BlockingMode,
ConcurrentMode,
DebugTracingMode,
NoMode,
StrictMode,
} from './ReactTypeOfMode';

import {
enqueueUpdate,
Expand Down Expand Up @@ -890,7 +897,16 @@ function mountClassInstance(
}

if (typeof instance.componentDidMount === 'function') {
workInProgress.flags |= Update;
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(workInProgress.mode & (BlockingMode | ConcurrentMode)) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
}
}

Expand Down Expand Up @@ -960,7 +976,16 @@ 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.mode & (BlockingMode | ConcurrentMode)) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
}
return false;
}
Expand Down Expand Up @@ -1003,13 +1028,31 @@ function resumeMountClassInstance(
}
}
if (typeof instance.componentDidMount === 'function') {
workInProgress.flags |= Update;
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(workInProgress.mode & (BlockingMode | ConcurrentMode)) !== NoMode
) {
// Never double-invoke effects for legacy roots.
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.mode & (BlockingMode | ConcurrentMode)) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
}

// If shouldComponentUpdate returned false, we should still update the
Expand Down
55 changes: 49 additions & 6 deletions packages/react-reconciler/src/ReactFiberClassComponent.old.js
View file
Expand Up @@ -12,13 +12,14 @@ import type {Lanes} from './ReactFiberLane.old';
import type {UpdateQueue} from './ReactUpdateQueue.old';

import * as React from 'react';
import {Update, Snapshot} from './ReactFiberFlags';
import {MountLayoutDev, Update, Snapshot} from './ReactFiberFlags';
import {
debugRenderPhaseSideEffectsForStrictMode,
disableLegacyContext,
enableDebugTracing,
enableSchedulingProfiler,
warnAboutDeprecatedLifecycles,
enableDoubleInvokingEffects,
} from 'shared/ReactFeatureFlags';
import ReactStrictModeWarnings from './ReactStrictModeWarnings.old';
import {isMounted} from './ReactFiberTreeReflection';
Expand All @@ -29,7 +30,13 @@ import invariant from 'shared/invariant';
import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols';

import {resolveDefaultProps} from './ReactFiberLazyComponent.old';
import {DebugTracingMode, StrictMode} from './ReactTypeOfMode';
import {
BlockingMode,
ConcurrentMode,
DebugTracingMode,
NoMode,
StrictMode,
} from './ReactTypeOfMode';

import {
enqueueUpdate,
Expand Down Expand Up @@ -890,7 +897,16 @@ function mountClassInstance(
}

if (typeof instance.componentDidMount === 'function') {
workInProgress.flags |= Update;
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(workInProgress.mode & (BlockingMode | ConcurrentMode)) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
}
}

Expand Down Expand Up @@ -960,7 +976,16 @@ 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.mode & (BlockingMode | ConcurrentMode)) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
}
return false;
}
Expand Down Expand Up @@ -1003,13 +1028,31 @@ function resumeMountClassInstance(
}
}
if (typeof instance.componentDidMount === 'function') {
workInProgress.flags |= Update;
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(workInProgress.mode & (BlockingMode | ConcurrentMode)) !== NoMode
) {
// Never double-invoke effects for legacy roots.
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.mode & (BlockingMode | ConcurrentMode)) !== NoMode
) {
// Never double-invoke effects for legacy roots.
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
}

// If shouldComponentUpdate returned false, we should still update the
Expand Down
134 changes: 134 additions & 0 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Expand Up @@ -36,6 +36,7 @@ import {
enableFundamentalAPI,
enableSuspenseCallback,
enableScopeAPI,
enableDoubleInvokingEffects,
} from 'shared/ReactFeatureFlags';
import {
FunctionComponent,
Expand Down Expand Up @@ -2436,11 +2437,144 @@ function ensureCorrectReturnPointer(fiber, expectedReturnFiber) {
fiber.return = expectedReturnFiber;
}

function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
// We don't need to re-check for legacy roots here.
// This function will not be called within legacy roots.
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent: {
invokeGuardedCallback(
null,
commitHookEffectListMount,
null,
HookLayout | HookHasEffect,
fiber,
);
if (hasCaughtError()) {
const mountError = clearCaughtError();
captureCommitPhaseError(fiber, mountError);
}
break;
}
case ClassComponent: {
const instance = fiber.stateNode;
invokeGuardedCallback(null, instance.componentDidMount, instance);
if (hasCaughtError()) {
const mountError = clearCaughtError();
captureCommitPhaseError(fiber, mountError);
}
break;
}
}
}
}

function invokePassiveEffectMountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
// We don't need to re-check for legacy roots here.
// This function will not be called within legacy roots.
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent: {
invokeGuardedCallback(
null,
commitHookEffectListMount,
null,
HookPassive | HookHasEffect,
fiber,
);
if (hasCaughtError()) {
const mountError = clearCaughtError();
captureCommitPhaseError(fiber, mountError);
}
break;
}
}
}
}

function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
// We don't need to re-check for legacy roots here.
// This function will not be called within legacy roots.
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent: {
invokeGuardedCallback(
null,
commitHookEffectListUnmount,
null,
HookLayout | HookHasEffect,
fiber,
fiber.return,
);
if (hasCaughtError()) {
const unmountError = clearCaughtError();
captureCommitPhaseError(fiber, 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, unmountError);
}
}
break;
}
}
}
}

function invokePassiveEffectUnmountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
// We don't need to re-check for legacy roots here.
// This function will not be called within legacy roots.
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent: {
invokeGuardedCallback(
null,
commitHookEffectListUnmount,
null,
HookPassive | HookHasEffect,
fiber,
fiber.return,
);
if (hasCaughtError()) {
const unmountError = clearCaughtError();
captureCommitPhaseError(fiber, unmountError);
}
break;
}
}
}
}

export {
commitResetTextContent,
commitPlacement,
commitDeletion,
commitWork,
commitAttachRef,
commitDetachRef,
invokeLayoutEffectMountInDEV,
invokeLayoutEffectUnmountInDEV,
invokePassiveEffectMountInDEV,
invokePassiveEffectUnmountInDEV,
};

0 comments on commit 4a2dc76

Please sign in to comment.