Skip to content

Commit 9072299

Browse files
author
Brian Vaughn
committed
Double Invoke Effects in __DEV__ (in old reconciler fork)
We originally added a new DEV behavior of double-invoking effects during mount to our new reconciler fork in PRs #19523 and #19935 and later refined it to only affect modern roots in PR #20028. This PR adds that behavior to the old reconciler fork with a small twist– the behavior applies to StrictMode subtrees, regardless of the root type. This commit also adds a few additional tests that weren't in the original commits.
1 parent cdae31a commit 9072299

File tree

7 files changed

+1099
-50
lines changed

7 files changed

+1099
-50
lines changed

‎packages/react-reconciler/src/ReactFiberClassComponent.old.js

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,14 @@ import type {Lanes} from './ReactFiberLane.old';
1212
import type {UpdateQueue} from './ReactUpdateQueue.old';
1313

1414
import * as React from 'react';
15-
import {Update, Snapshot} from './ReactFiberFlags';
15+
import {Update, Snapshot, MountLayoutDev} from './ReactFiberFlags';
1616
import {
1717
debugRenderPhaseSideEffectsForStrictMode,
1818
disableLegacyContext,
1919
enableDebugTracing,
2020
enableSchedulingProfiler,
2121
warnAboutDeprecatedLifecycles,
22+
enableDoubleInvokingEffects,
2223
} from 'shared/ReactFeatureFlags';
2324
import ReactStrictModeWarnings from './ReactStrictModeWarnings.old';
2425
import {isMounted} from './ReactFiberTreeReflection';
@@ -29,7 +30,7 @@ import invariant from 'shared/invariant';
2930
import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols';
3031

3132
import {resolveDefaultProps} from './ReactFiberLazyComponent.old';
32-
import {DebugTracingMode, StrictMode} from './ReactTypeOfMode';
33+
import {DebugTracingMode, NoMode, StrictMode} from './ReactTypeOfMode';
3334

3435
import {
3536
enqueueUpdate,
@@ -890,7 +891,15 @@ function mountClassInstance(
890891
}
891892

892893
if (typeof instance.componentDidMount === 'function') {
893-
workInProgress.flags |= Update;
894+
if (
895+
__DEV__ &&
896+
enableDoubleInvokingEffects &&
897+
(workInProgress.mode & StrictMode) !== NoMode
898+
) {
899+
workInProgress.flags |= MountLayoutDev | Update;
900+
} else {
901+
workInProgress.flags |= Update;
902+
}
894903
}
895904
}
896905

@@ -960,7 +969,15 @@ function resumeMountClassInstance(
960969
// If an update was already in progress, we should schedule an Update
961970
// effect even though we're bailing out, so that cWU/cDU are called.
962971
if (typeof instance.componentDidMount === 'function') {
963-
workInProgress.flags |= Update;
972+
if (
973+
__DEV__ &&
974+
enableDoubleInvokingEffects &&
975+
(workInProgress.mode & StrictMode) !== NoMode
976+
) {
977+
workInProgress.flags |= MountLayoutDev | Update;
978+
} else {
979+
workInProgress.flags |= Update;
980+
}
964981
}
965982
return false;
966983
}
@@ -1003,13 +1020,29 @@ function resumeMountClassInstance(
10031020
}
10041021
}
10051022
if (typeof instance.componentDidMount === 'function') {
1006-
workInProgress.flags |= Update;
1023+
if (
1024+
__DEV__ &&
1025+
enableDoubleInvokingEffects &&
1026+
(workInProgress.mode & StrictMode) !== NoMode
1027+
) {
1028+
workInProgress.flags |= MountLayoutDev | Update;
1029+
} else {
1030+
workInProgress.flags |= Update;
1031+
}
10071032
}
10081033
} else {
10091034
// If an update was already in progress, we should schedule an Update
10101035
// effect even though we're bailing out, so that cWU/cDU are called.
10111036
if (typeof instance.componentDidMount === 'function') {
1012-
workInProgress.flags |= Update;
1037+
if (
1038+
__DEV__ &&
1039+
enableDoubleInvokingEffects &&
1040+
(workInProgress.mode & StrictMode) !== NoMode
1041+
) {
1042+
workInProgress.flags |= MountLayoutDev | Update;
1043+
} else {
1044+
workInProgress.flags |= Update;
1045+
}
10131046
}
10141047

10151048
// If shouldComponentUpdate returned false, we should still update the

‎packages/react-reconciler/src/ReactFiberCommitWork.old.js

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
enableFundamentalAPI,
3636
enableSuspenseCallback,
3737
enableScopeAPI,
38+
enableDoubleInvokingEffects,
3839
} from 'shared/ReactFeatureFlags';
3940
import {
4041
FunctionComponent,
@@ -1797,6 +1798,116 @@ function commitResetTextContent(current: Fiber) {
17971798
resetTextContent(current.stateNode);
17981799
}
17991800

1801+
function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
1802+
if (__DEV__ && enableDoubleInvokingEffects) {
1803+
switch (fiber.tag) {
1804+
case FunctionComponent:
1805+
case ForwardRef:
1806+
case SimpleMemoComponent: {
1807+
invokeGuardedCallback(
1808+
null,
1809+
commitHookEffectListMount,
1810+
null,
1811+
HookLayout | HookHasEffect,
1812+
fiber,
1813+
);
1814+
if (hasCaughtError()) {
1815+
const mountError = clearCaughtError();
1816+
captureCommitPhaseError(fiber, mountError);
1817+
}
1818+
break;
1819+
}
1820+
case ClassComponent: {
1821+
const instance = fiber.stateNode;
1822+
invokeGuardedCallback(null, instance.componentDidMount, instance);
1823+
if (hasCaughtError()) {
1824+
const mountError = clearCaughtError();
1825+
captureCommitPhaseError(fiber, mountError);
1826+
}
1827+
break;
1828+
}
1829+
}
1830+
}
1831+
}
1832+
1833+
function invokePassiveEffectMountInDEV(fiber: Fiber): void {
1834+
if (__DEV__ && enableDoubleInvokingEffects) {
1835+
switch (fiber.tag) {
1836+
case FunctionComponent:
1837+
case ForwardRef:
1838+
case SimpleMemoComponent: {
1839+
invokeGuardedCallback(
1840+
null,
1841+
commitHookEffectListMount,
1842+
null,
1843+
HookPassive | HookHasEffect,
1844+
fiber,
1845+
);
1846+
if (hasCaughtError()) {
1847+
const mountError = clearCaughtError();
1848+
captureCommitPhaseError(fiber, mountError);
1849+
}
1850+
break;
1851+
}
1852+
}
1853+
}
1854+
}
1855+
1856+
function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void {
1857+
if (__DEV__ && enableDoubleInvokingEffects) {
1858+
switch (fiber.tag) {
1859+
case FunctionComponent:
1860+
case ForwardRef:
1861+
case SimpleMemoComponent: {
1862+
invokeGuardedCallback(
1863+
null,
1864+
commitHookEffectListUnmount,
1865+
null,
1866+
HookLayout | HookHasEffect,
1867+
fiber,
1868+
fiber.return,
1869+
);
1870+
if (hasCaughtError()) {
1871+
const unmountError = clearCaughtError();
1872+
captureCommitPhaseError(fiber, unmountError);
1873+
}
1874+
break;
1875+
}
1876+
case ClassComponent: {
1877+
const instance = fiber.stateNode;
1878+
if (typeof instance.componentWillUnmount === 'function') {
1879+
safelyCallComponentWillUnmount(fiber, instance);
1880+
}
1881+
break;
1882+
}
1883+
}
1884+
}
1885+
}
1886+
1887+
function invokePassiveEffectUnmountInDEV(fiber: Fiber): void {
1888+
if (__DEV__ && enableDoubleInvokingEffects) {
1889+
switch (fiber.tag) {
1890+
case FunctionComponent:
1891+
case ForwardRef:
1892+
case SimpleMemoComponent: {
1893+
invokeGuardedCallback(
1894+
null,
1895+
commitHookEffectListUnmount,
1896+
null,
1897+
HookPassive | HookHasEffect,
1898+
fiber,
1899+
fiber.return,
1900+
);
1901+
if (hasCaughtError()) {
1902+
const unmountError = clearCaughtError();
1903+
captureCommitPhaseError(fiber, unmountError);
1904+
}
1905+
break;
1906+
}
1907+
}
1908+
}
1909+
}
1910+
18001911
export {
18011912
commitBeforeMutationLifeCycles,
18021913
commitResetTextContent,
@@ -1806,4 +1917,8 @@ export {
18061917
commitLifeCycles,
18071918
commitAttachRef,
18081919
commitDetachRef,
1920+
invokeLayoutEffectMountInDEV,
1921+
invokeLayoutEffectUnmountInDEV,
1922+
invokePassiveEffectMountInDEV,
1923+
invokePassiveEffectUnmountInDEV,
18091924
};

‎packages/react-reconciler/src/ReactFiberHooks.old.js

Lines changed: 87 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,15 @@ import {
2828
enableCache,
2929
decoupleUpdatePriorityFromScheduler,
3030
enableUseRefAccessWarning,
31+
enableDoubleInvokingEffects,
3132
} from 'shared/ReactFeatureFlags';
3233

33-
import {NoMode, BlockingMode, DebugTracingMode} from './ReactTypeOfMode';
34+
import {
35+
NoMode,
36+
BlockingMode,
37+
DebugTracingMode,
38+
StrictMode,
39+
} from './ReactTypeOfMode';
3440
import {
3541
NoLane,
3642
NoLanes,
@@ -49,6 +55,8 @@ import {readContext} from './ReactFiberNewContext.old';
4955
import {
5056
Update as UpdateEffect,
5157
Passive as PassiveEffect,
58+
MountLayoutDev as MountLayoutDevEffect,
59+
MountPassiveDev as MountPassiveDevEffect,
5260
} from './ReactFiberFlags';
5361
import {
5462
HasEffect as HookHasEffect,
@@ -467,7 +475,20 @@ export function bailoutHooks(
467475
lanes: Lanes,
468476
) {
469477
workInProgress.updateQueue = current.updateQueue;
470-
workInProgress.flags &= ~(PassiveEffect | UpdateEffect);
478+
if (
479+
__DEV__ &&
480+
enableDoubleInvokingEffects &&
481+
(workInProgress.mode & StrictMode) !== NoMode
482+
) {
483+
workInProgress.flags &= ~(
484+
MountLayoutDevEffect |
485+
MountPassiveDevEffect |
486+
PassiveEffect |
487+
UpdateEffect
488+
);
489+
} else {
490+
workInProgress.flags &= ~(PassiveEffect | UpdateEffect);
491+
}
471492
current.lanes = removeLanes(current.lanes, lanes);
472493
}
473494

@@ -1303,12 +1324,26 @@ function mountEffect(
13031324
warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
13041325
}
13051326
}
1306-
return mountEffectImpl(
1307-
UpdateEffect | PassiveEffect,
1308-
HookPassive,
1309-
create,
1310-
deps,
1311-
);
1327+
1328+
if (
1329+
__DEV__ &&
1330+
enableDoubleInvokingEffects &&
1331+
(currentlyRenderingFiber.mode & StrictMode) !== NoMode
1332+
) {
1333+
return mountEffectImpl(
1334+
UpdateEffect | PassiveEffect | MountPassiveDevEffect,
1335+
HookPassive,
1336+
create,
1337+
deps,
1338+
);
1339+
} else {
1340+
return mountEffectImpl(
1341+
UpdateEffect | PassiveEffect,
1342+
HookPassive,
1343+
create,
1344+
deps,
1345+
);
1346+
}
13121347
}
13131348

13141349
function updateEffect(
@@ -1333,7 +1368,20 @@ function mountLayoutEffect(
13331368
create: () => (() => void) | void,
13341369
deps: Array<mixed> | void | null,
13351370
): void {
1336-
return mountEffectImpl(UpdateEffect, HookLayout, create, deps);
1371+
if (
1372+
__DEV__ &&
1373+
enableDoubleInvokingEffects &&
1374+
(currentlyRenderingFiber.mode & StrictMode) !== NoMode
1375+
) {
1376+
return mountEffectImpl(
1377+
UpdateEffect | MountLayoutDevEffect,
1378+
HookLayout,
1379+
create,
1380+
deps,
1381+
);
1382+
} else {
1383+
return mountEffectImpl(UpdateEffect, HookLayout, create, deps);
1384+
}
13371385
}
13381386

13391387
function updateLayoutEffect(
@@ -1392,12 +1440,25 @@ function mountImperativeHandle<T>(
13921440
const effectDeps =
13931441
deps !== null && deps !== undefined ? deps.concat([ref]) : null;
13941442

1395-
return mountEffectImpl(
1396-
UpdateEffect,
1397-
HookLayout,
1398-
imperativeHandleEffect.bind(null, create, ref),
1399-
effectDeps,
1400-
);
1443+
if (
1444+
__DEV__ &&
1445+
enableDoubleInvokingEffects &&
1446+
(currentlyRenderingFiber.mode & StrictMode) !== NoMode
1447+
) {
1448+
return mountEffectImpl(
1449+
UpdateEffect | MountLayoutDevEffect,
1450+
HookLayout,
1451+
imperativeHandleEffect.bind(null, create, ref),
1452+
effectDeps,
1453+
);
1454+
} else {
1455+
return mountEffectImpl(
1456+
UpdateEffect,
1457+
HookLayout,
1458+
imperativeHandleEffect.bind(null, create, ref),
1459+
effectDeps,
1460+
);
1461+
}
14011462
}
14021463

14031464
function updateImperativeHandle<T>(
@@ -1678,7 +1739,17 @@ function mountOpaqueIdentifier(): OpaqueIDType | void {
16781739
const setId = mountState(id)[1];
16791740

16801741
if ((currentlyRenderingFiber.mode & BlockingMode) === NoMode) {
1681-
currentlyRenderingFiber.flags |= UpdateEffect | PassiveEffect;
1742+
if (
1743+
__DEV__ &&
1744+
enableDoubleInvokingEffects &&
1745+
(currentlyRenderingFiber.mode & StrictMode) !== NoMode
1746+
) {
1747+
currentlyRenderingFiber.flags |=
1748+
UpdateEffect | PassiveEffect | MountPassiveDevEffect;
1749+
} else {
1750+
currentlyRenderingFiber.flags |= UpdateEffect | PassiveEffect;
1751+
}
1752+
16821753
pushEffect(
16831754
HookHasEffect | HookPassive,
16841755
() => {

0 commit comments

Comments
 (0)