Detect subscriptions wrapped in startTransition (#22271)
* Detect subscriptions wrapped in startTransition
salazarm committed
Sep 8, 2021 at 17:01 UTC
a3fde2358896e32201f49bc81acd2f228951ca84
17 files changed
+184
-4
packages/react-reconciler/src/ReactFiberHooks.new.js
+18
@@ -116,6 +116,7 @@ import {
116
} from './ReactUpdateQueue.new';
117
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.new';
118
import {getIsStrictModeForDevtools} from './ReactFiberReconciler.new';
119
+import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';
120
121
const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
122
@@ -1861,6 +1862,23 @@ function startTransition(setPending, callback) {
1862
} finally {
1863
setCurrentUpdatePriority(previousPriority);
1864
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
+ }
1882
}
1883
}
1884
packages/react-reconciler/src/ReactFiberHooks.old.js
+18
@@ -116,6 +116,7 @@ import {
116
} from './ReactUpdateQueue.old';
117
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.old';
118
import {getIsStrictModeForDevtools} from './ReactFiberReconciler.old';
119
+import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';
120
121
const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
122
@@ -1861,6 +1862,23 @@ function startTransition(setPending, callback) {
1862
} finally {
1863
setCurrentUpdatePriority(previousPriority);
1864
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
+ }
1882
}
1883
}
1884
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+8
@@ -30,6 +30,7 @@ import {
30
enableStrictEffects,
31
skipUnmountedBoundaries,
32
enableUpdaterTracking,
33
+ warnOnSubscriptionInsideStartTransition,
34
} from 'shared/ReactFeatureFlags';
35
import ReactSharedInternals from 'shared/ReactSharedInternals';
36
import invariant from 'shared/invariant';
@@ -385,6 +386,13 @@ export function requestUpdateLane(fiber: Fiber): Lane {
386
387
const isTransition = requestCurrentTransition() !== NoTransition;
388
if (isTransition) {
389
+ if (
390
+ __DEV__ &&
391
+ warnOnSubscriptionInsideStartTransition &&
392
+ ReactCurrentBatchConfig._updatedFibers
393
+ ) {
394
+ ReactCurrentBatchConfig._updatedFibers.add(fiber);
395
+ }
396
// The algorithm for assigning an update to a lane should be stable for all
397
// updates at the same priority within the same event. To do this, the
398
// inputs to the algorithm must be the same.
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+8
@@ -30,6 +30,7 @@ import {
30
enableStrictEffects,
31
skipUnmountedBoundaries,
32
enableUpdaterTracking,
33
+ warnOnSubscriptionInsideStartTransition,
34
} from 'shared/ReactFeatureFlags';
35
import ReactSharedInternals from 'shared/ReactSharedInternals';
36
import invariant from 'shared/invariant';
@@ -385,6 +386,13 @@ export function requestUpdateLane(fiber: Fiber): Lane {
386
387
const isTransition = requestCurrentTransition() !== NoTransition;
388
if (isTransition) {
389
+ if (
390
+ __DEV__ &&
391
+ warnOnSubscriptionInsideStartTransition &&
392
+ ReactCurrentBatchConfig._updatedFibers
393
+ ) {
394
+ ReactCurrentBatchConfig._updatedFibers.add(fiber);
395
+ }
396
// The algorithm for assigning an update to a lane should be stable for all
397
// updates at the same priority within the same event. To do this, the
398
// inputs to the algorithm must be the same.
packages/react/src/ReactCurrentBatchConfig.js
+12
-2
@@ -7,12 +7,22 @@
7
* @flow
8
*/
9
10
+import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
11
+
12
+type BatchConfig = {
13
+ transition: number,
14
+ _updatedFibers?: Set<Fiber>,
15
+};
16
/**
17
* Keeps track of the current batch's configuration such as how long an update
18
* should suspend for if it needs to.
19
*/
14
-const ReactCurrentBatchConfig = {
15
- transition: (0: number),
20
+const ReactCurrentBatchConfig: BatchConfig = {
21
+ transition: 0,
22
};
23
24
+if (__DEV__) {
25
+ ReactCurrentBatchConfig._updatedFibers = new Set();
26
+}
27
+
28
export default ReactCurrentBatchConfig;
packages/react/src/ReactStartTransition.js
+18
@@ -8,6 +8,7 @@
8
*/
9
10
import ReactCurrentBatchConfig from './ReactCurrentBatchConfig';
11
+import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';
12
13
export function startTransition(scope: () => void) {
14
const prevTransition = ReactCurrentBatchConfig.transition;
@@ -16,5 +17,22 @@ export function startTransition(scope: () => void) {
17
scope();
18
} finally {
19
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
+ }
37
}
38
}
packages/react/src/__tests__/ReactStartTransition-test.js
new
+91
@@ -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
+2
@@ -99,6 +99,8 @@ export const enableTrustedTypesIntegration = false;
99
// a deprecated pattern we want to get rid of in the future
100
export const warnAboutSpreadingKeyToJSX = false;
101
102
+export const warnOnSubscriptionInsideStartTransition = false;
103
+
104
export const enableComponentStackLocations = true;
105
106
export const enableNewReconciler = false;
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -48,6 +48,7 @@ export const disableTextareaChildren = false;
48
export const disableModulePatternComponents = false;
49
export const warnUnstableRenderSubtreeIntoContainer = false;
50
export const warnAboutSpreadingKeyToJSX = false;
51
+export const warnOnSubscriptionInsideStartTransition = false;
52
export const enableComponentStackLocations = false;
53
export const enableLegacyFBSupport = false;
54
export const enableFilterEmptyStringAttributesDOM = false;
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
39
export const disableModulePatternComponents = false;
40
export const warnUnstableRenderSubtreeIntoContainer = false;
41
export const warnAboutSpreadingKeyToJSX = false;
42
+export const warnOnSubscriptionInsideStartTransition = false;
43
export const enableComponentStackLocations = false;
44
export const enableLegacyFBSupport = false;
45
export const enableFilterEmptyStringAttributesDOM = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
39
export const disableModulePatternComponents = false;
40
export const warnUnstableRenderSubtreeIntoContainer = false;
41
export const warnAboutSpreadingKeyToJSX = false;
42
+export const warnOnSubscriptionInsideStartTransition = false;
43
export const enableComponentStackLocations = true;
44
export const enableLegacyFBSupport = false;
45
export const enableFilterEmptyStringAttributesDOM = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
+1
-1
@@ -49,7 +49,7 @@ export const enableSuspenseLayoutEffectSemantics = false;
49
export const enableGetInspectorDataForInstanceInProduction = false;
50
export const enableNewReconciler = false;
51
export const deferRenderPhaseUpdateToNextBatch = false;
52
-
52
+export const warnOnSubscriptionInsideStartTransition = false;
53
export const enableStrictEffects = false;
54
export const createRootStrictEffectsByDefault = false;
55
export const enableUseRefAccessWarning = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
39
export const disableModulePatternComponents = true;
40
export const warnUnstableRenderSubtreeIntoContainer = false;
41
export const warnAboutSpreadingKeyToJSX = false;
42
+export const warnOnSubscriptionInsideStartTransition = false;
43
export const enableComponentStackLocations = true;
44
export const enableLegacyFBSupport = false;
45
export const enableFilterEmptyStringAttributesDOM = false;
packages/shared/forks/ReactFeatureFlags.testing.js
+1
@@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
39
export const disableModulePatternComponents = false;
40
export const warnUnstableRenderSubtreeIntoContainer = false;
41
export const warnAboutSpreadingKeyToJSX = false;
42
+export const warnOnSubscriptionInsideStartTransition = false;
43
export const enableComponentStackLocations = true;
44
export const enableLegacyFBSupport = false;
45
export const enableFilterEmptyStringAttributesDOM = false;
packages/shared/forks/ReactFeatureFlags.testing.www.js
+1
@@ -39,6 +39,7 @@ export const disableTextareaChildren = __EXPERIMENTAL__;
39
export const disableModulePatternComponents = true;
40
export const warnUnstableRenderSubtreeIntoContainer = false;
41
export const warnAboutSpreadingKeyToJSX = false;
42
+export const warnOnSubscriptionInsideStartTransition = false;
43
export const enableComponentStackLocations = true;
44
export const enableLegacyFBSupport = !__EXPERIMENTAL__;
45
export const enableFilterEmptyStringAttributesDOM = false;
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
+1
@@ -25,6 +25,7 @@ export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
25
export const enableLazyContextPropagation = __VARIANT__;
26
export const enableSyncDefaultUpdates = __VARIANT__;
27
export const consoleManagedByDevToolsDuringStrictMode = __VARIANT__;
28
+export const warnOnSubscriptionInsideStartTransition = __VARIANT__;
29
30
// Enable this flag to help with concurrent mode debugging.
31
// It logs information to the console about React scheduling, rendering, and commit phases.
packages/shared/forks/ReactFeatureFlags.www.js
+1
-1
@@ -31,6 +31,7 @@ export const {
31
disableSchedulerTimeoutInWorkLoop,
32
enableLazyContextPropagation,
33
enableSyncDefaultUpdates,
34
+ warnOnSubscriptionInsideStartTransition,
35
} = dynamicFeatureFlags;
36
37
// On WWW, __EXPERIMENTAL__ is used for a new modern build.
@@ -56,7 +57,6 @@ export const enableSchedulingProfiler =
57
// For now, we'll turn it on for everyone because it's *already* on for everyone in practice.
58
// At least this will let us stop shipping <Profiler> implementation to all users.
59
export const enableSchedulerDebugging = true;
59
-
60
export const warnAboutDeprecatedLifecycles = true;
61
export const disableLegacyContext = __EXPERIMENTAL__;
62
export const warnAboutStringRefs = false;