Skip to content

Commit a3fde23

Browse files
salazarmsalazarm
authored
Detect subscriptions wrapped in startTransition (#22271)
* Detect subscriptions wrapped in startTransition
1 parent 95d762e commit a3fde23

17 files changed

+184
-4
lines changed

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ import {
116116
} from './ReactUpdateQueue.new';
117117
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.new';
118118
import {getIsStrictModeForDevtools} from './ReactFiberReconciler.new';
119+
import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';
119120

120121
const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
121122

@@ -1861,6 +1862,23 @@ function startTransition(setPending, callback) {
18611862
} finally {
18621863
setCurrentUpdatePriority(previousPriority);
18631864
ReactCurrentBatchConfig.transition = prevTransition;
1865+
if (__DEV__) {
1866+
if (
1867+
prevTransition !== 1 &&
1868+
warnOnSubscriptionInsideStartTransition &&
1869+
ReactCurrentBatchConfig._updatedFibers
1870+
) {
1871+
const updatedFibersCount = ReactCurrentBatchConfig._updatedFibers.size;
1872+
if (updatedFibersCount > 10) {
1873+
console.warn(
1874+
'Detected a large number of updates inside startTransition. ' +
1875+
'If this is due to a subscription please re-write it to use React provided hooks. ' +
1876+
'Otherwise concurrent mode guarantees are off the table.',
1877+
);
1878+
}
1879+
ReactCurrentBatchConfig._updatedFibers.clear();
1880+
}
1881+
}
18641882
}
18651883
}
18661884

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ import {
116116
} from './ReactUpdateQueue.old';
117117
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.old';
118118
import {getIsStrictModeForDevtools} from './ReactFiberReconciler.old';
119+
import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';
119120

120121
const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
121122

@@ -1861,6 +1862,23 @@ function startTransition(setPending, callback) {
18611862
} finally {
18621863
setCurrentUpdatePriority(previousPriority);
18631864
ReactCurrentBatchConfig.transition = prevTransition;
1865+
if (__DEV__) {
1866+
if (
1867+
prevTransition !== 1 &&
1868+
warnOnSubscriptionInsideStartTransition &&
1869+
ReactCurrentBatchConfig._updatedFibers
1870+
) {
1871+
const updatedFibersCount = ReactCurrentBatchConfig._updatedFibers.size;
1872+
if (updatedFibersCount > 10) {
1873+
console.warn(
1874+
'Detected a large number of updates inside startTransition. ' +
1875+
'If this is due to a subscription please re-write it to use React provided hooks. ' +
1876+
'Otherwise concurrent mode guarantees are off the table.',
1877+
);
1878+
}
1879+
ReactCurrentBatchConfig._updatedFibers.clear();
1880+
}
1881+
}
18641882
}
18651883
}
18661884

‎packages/react-reconciler/src/ReactFiberWorkLoop.new.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
enableStrictEffects,
3131
skipUnmountedBoundaries,
3232
enableUpdaterTracking,
33+
warnOnSubscriptionInsideStartTransition,
3334
} from 'shared/ReactFeatureFlags';
3435
import ReactSharedInternals from 'shared/ReactSharedInternals';
3536
import invariant from 'shared/invariant';
@@ -385,6 +386,13 @@ export function requestUpdateLane(fiber: Fiber): Lane {
385386

386387
const isTransition = requestCurrentTransition() !== NoTransition;
387388
if (isTransition) {
389+
if (
390+
__DEV__ &&
391+
warnOnSubscriptionInsideStartTransition &&
392+
ReactCurrentBatchConfig._updatedFibers
393+
) {
394+
ReactCurrentBatchConfig._updatedFibers.add(fiber);
395+
}
388396
// The algorithm for assigning an update to a lane should be stable for all
389397
// updates at the same priority within the same event. To do this, the
390398
// inputs to the algorithm must be the same.

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
enableStrictEffects,
3131
skipUnmountedBoundaries,
3232
enableUpdaterTracking,
33+
warnOnSubscriptionInsideStartTransition,
3334
} from 'shared/ReactFeatureFlags';
3435
import ReactSharedInternals from 'shared/ReactSharedInternals';
3536
import invariant from 'shared/invariant';
@@ -385,6 +386,13 @@ export function requestUpdateLane(fiber: Fiber): Lane {
385386

386387
const isTransition = requestCurrentTransition() !== NoTransition;
387388
if (isTransition) {
389+
if (
390+
__DEV__ &&
391+
warnOnSubscriptionInsideStartTransition &&
392+
ReactCurrentBatchConfig._updatedFibers
393+
) {
394+
ReactCurrentBatchConfig._updatedFibers.add(fiber);
395+
}
388396
// The algorithm for assigning an update to a lane should be stable for all
389397
// updates at the same priority within the same event. To do this, the
390398
// inputs to the algorithm must be the same.

‎packages/react/src/ReactCurrentBatchConfig.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,22 @@
77
* @flow
88
*/
99

10+
import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
11+
12+
type BatchConfig = {
13+
transition: number,
14+
_updatedFibers?: Set<Fiber>,
15+
};
1016
/**
1117
* Keeps track of the current batch's configuration such as how long an update
1218
* should suspend for if it needs to.
1319
*/
14-
const ReactCurrentBatchConfig = {
15-
transition: (0: number),
20+
const ReactCurrentBatchConfig: BatchConfig = {
21+
transition: 0,
1622
};
1723

24+
if (__DEV__) {
25+
ReactCurrentBatchConfig._updatedFibers = new Set();
26+
}
27+
1828
export default ReactCurrentBatchConfig;

‎packages/react/src/ReactStartTransition.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99

1010
import ReactCurrentBatchConfig from './ReactCurrentBatchConfig';
11+
import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';
1112

1213
export function startTransition(scope: () => void) {
1314
const prevTransition = ReactCurrentBatchConfig.transition;
@@ -16,5 +17,22 @@ export function startTransition(scope: () => void) {
1617
scope();
1718
} finally {
1819
ReactCurrentBatchConfig.transition = prevTransition;
20+
if (__DEV__) {
21+
if (
22+
prevTransition !== 1 &&
23+
warnOnSubscriptionInsideStartTransition &&
24+
ReactCurrentBatchConfig._updatedFibers
25+
) {
26+
const updatedFibersCount = ReactCurrentBatchConfig._updatedFibers.size;
27+
if (updatedFibersCount > 10) {
28+
console.warn(
29+
'Detected a large number of updates inside startTransition. ' +
30+
'If this is due to a subscription please re-write it to use React provided hooks. ' +
31+
'Otherwise concurrent mode guarantees are off the table.',
32+
);
33+
}
34+
ReactCurrentBatchConfig._updatedFibers.clear();
35+
}
36+
}
1937
}
2038
}

‎packages/react/src/__tests__/ReactStartTransition-test.js

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @emails react-core
8+
*/
9+
10+
'use strict';
11+
12+
let React;
13+
let ReactTestRenderer;
14+
let act;
15+
let useState;
16+
let useTransition;
17+
18+
const SUSPICIOUS_NUMBER_OF_FIBERS_UPDATED = 10;
19+
20+
describe('ReactStartTransition', () => {
21+
beforeEach(() => {
22+
jest.resetModules();
23+
React = require('react');
24+
ReactTestRenderer = require('react-test-renderer');
25+
act = require('jest-react').act;
26+
useState = React.useState;
27+
useTransition = React.useTransition;
28+
});
29+
30+
// @gate warnOnSubscriptionInsideStartTransition || !__DEV__
31+
it('Warns if a suspicious number of fibers are updated inside startTransition', () => {
32+
const subs = new Set();
33+
const useUserSpaceSubscription = () => {
34+
const setState = useState(0)[1];
35+
subs.add(setState);
36+
};
37+
38+
let triggerHookTransition;
39+
40+
const Component = ({level}) => {
41+
useUserSpaceSubscription();
42+
if (level === 0) {
43+
triggerHookTransition = useTransition()[1];
44+
}
45+
if (level < SUSPICIOUS_NUMBER_OF_FIBERS_UPDATED) {
46+
return <Component level={level + 1} />;
47+
}
48+
return null;
49+
};
50+
51+
act(() => {
52+
ReactTestRenderer.create(<Component level={0} />, {
53+
unstable_isConcurrent: true,
54+
});
55+
});
56+
57+
expect(() => {
58+
act(() => {
59+
React.startTransition(() => {
60+
subs.forEach(setState => {
61+
setState(state => state + 1);
62+
});
63+
});
64+
});
65+
}).toWarnDev(
66+
[
67+
'Detected a large number of updates inside startTransition. ' +
68+
'If this is due to a subscription please re-write it to use React provided hooks. ' +
69+
'Otherwise concurrent mode guarantees are off the table.',
70+
],
71+
{withoutStack: true},
72+
);
73+
74+
expect(() => {
75+
act(() => {
76+
triggerHookTransition(() => {
77+
subs.forEach(setState => {
78+
setState(state => state + 1);
79+
});
80+
});
81+
});
82+
}).toWarnDev(
83+
[
84+
'Detected a large number of updates inside startTransition. ' +
85+
'If this is due to a subscription please re-write it to use React provided hooks. ' +
86+
'Otherwise concurrent mode guarantees are off the table.',
87+
],
88+
{withoutStack: true},
89+
);
90+
});
91+
});

‎packages/shared/ReactFeatureFlags.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ export const enableTrustedTypesIntegration = false;
9999
// a deprecated pattern we want to get rid of in the future
100100
export const warnAboutSpreadingKeyToJSX = false;
101101

102+
export const warnOnSubscriptionInsideStartTransition = false;
103+
102104
export const enableComponentStackLocations = true;
103105

104106
export const enableNewReconciler = false;

‎packages/shared/forks/ReactFeatureFlags.native-fb.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export const disableTextareaChildren = false;
4848
export const disableModulePatternComponents = false;
4949
export const warnUnstableRenderSubtreeIntoContainer = false;
5050
export const warnAboutSpreadingKeyToJSX = false;
51+
export const warnOnSubscriptionInsideStartTransition = false;
5152
export const enableComponentStackLocations = false;
5253
export const enableLegacyFBSupport = false;
5354
export const enableFilterEmptyStringAttributesDOM = false;

‎packages/shared/forks/ReactFeatureFlags.native-oss.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
3939
export const disableModulePatternComponents = false;
4040
export const warnUnstableRenderSubtreeIntoContainer = false;
4141
export const warnAboutSpreadingKeyToJSX = false;
42+
export const warnOnSubscriptionInsideStartTransition = false;
4243
export const enableComponentStackLocations = false;
4344
export const enableLegacyFBSupport = false;
4445
export const enableFilterEmptyStringAttributesDOM = false;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
3939
export const disableModulePatternComponents = false;
4040
export const warnUnstableRenderSubtreeIntoContainer = false;
4141
export const warnAboutSpreadingKeyToJSX = false;
42+
export const warnOnSubscriptionInsideStartTransition = false;
4243
export const enableComponentStackLocations = true;
4344
export const enableLegacyFBSupport = false;
4445
export const enableFilterEmptyStringAttributesDOM = false;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.native.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export const enableSuspenseLayoutEffectSemantics = false;
4949
export const enableGetInspectorDataForInstanceInProduction = false;
5050
export const enableNewReconciler = false;
5151
export const deferRenderPhaseUpdateToNextBatch = false;
52-
52+
export const warnOnSubscriptionInsideStartTransition = false;
5353
export const enableStrictEffects = false;
5454
export const createRootStrictEffectsByDefault = false;
5555
export const enableUseRefAccessWarning = false;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
3939
export const disableModulePatternComponents = true;
4040
export const warnUnstableRenderSubtreeIntoContainer = false;
4141
export const warnAboutSpreadingKeyToJSX = false;
42+
export const warnOnSubscriptionInsideStartTransition = false;
4243
export const enableComponentStackLocations = true;
4344
export const enableLegacyFBSupport = false;
4445
export const enableFilterEmptyStringAttributesDOM = false;

‎packages/shared/forks/ReactFeatureFlags.testing.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
3939
export const disableModulePatternComponents = false;
4040
export const warnUnstableRenderSubtreeIntoContainer = false;
4141
export const warnAboutSpreadingKeyToJSX = false;
42+
export const warnOnSubscriptionInsideStartTransition = false;
4243
export const enableComponentStackLocations = true;
4344
export const enableLegacyFBSupport = false;
4445
export const enableFilterEmptyStringAttributesDOM = false;

‎packages/shared/forks/ReactFeatureFlags.testing.www.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export const disableTextareaChildren = __EXPERIMENTAL__;
3939
export const disableModulePatternComponents = true;
4040
export const warnUnstableRenderSubtreeIntoContainer = false;
4141
export const warnAboutSpreadingKeyToJSX = false;
42+
export const warnOnSubscriptionInsideStartTransition = false;
4243
export const enableComponentStackLocations = true;
4344
export const enableLegacyFBSupport = !__EXPERIMENTAL__;
4445
export const enableFilterEmptyStringAttributesDOM = false;

‎packages/shared/forks/ReactFeatureFlags.www-dynamic.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
2525
export const enableLazyContextPropagation = __VARIANT__;
2626
export const enableSyncDefaultUpdates = __VARIANT__;
2727
export const consoleManagedByDevToolsDuringStrictMode = __VARIANT__;
28+
export const warnOnSubscriptionInsideStartTransition = __VARIANT__;
2829

2930
// Enable this flag to help with concurrent mode debugging.
3031
// It logs information to the console about React scheduling, rendering, and commit phases.

‎packages/shared/forks/ReactFeatureFlags.www.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export const {
3131
disableSchedulerTimeoutInWorkLoop,
3232
enableLazyContextPropagation,
3333
enableSyncDefaultUpdates,
34+
warnOnSubscriptionInsideStartTransition,
3435
} = dynamicFeatureFlags;
3536

3637
// On WWW, __EXPERIMENTAL__ is used for a new modern build.
@@ -56,7 +57,6 @@ export const enableSchedulingProfiler =
5657
// For now, we'll turn it on for everyone because it's *already* on for everyone in practice.
5758
// At least this will let us stop shipping <Profiler> implementation to all users.
5859
export const enableSchedulerDebugging = true;
59-
6060
export const warnAboutDeprecatedLifecycles = true;
6161
export const disableLegacyContext = __EXPERIMENTAL__;
6262
export const warnAboutStringRefs = false;

0 commit comments

Comments
 (0)