Skip to content

Detect subscriptions wrapped in startTransition #22271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Sep 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ import {
} from './ReactUpdateQueue.new';
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.new';
import {getIsStrictModeForDevtools} from './ReactFiberReconciler.new';
import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';

const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;

Expand Down Expand Up @@ -1861,6 +1862,23 @@ function startTransition(setPending, callback) {
} finally {
setCurrentUpdatePriority(previousPriority);
ReactCurrentBatchConfig.transition = prevTransition;
if (__DEV__) {
if (
prevTransition !== 1 &&
warnOnSubscriptionInsideStartTransition &&
ReactCurrentBatchConfig._updatedFibers
) {
const updatedFibersCount = ReactCurrentBatchConfig._updatedFibers.size;
if (updatedFibersCount > 10) {
console.warn(
'Detected a large number of updates inside startTransition. ' +
'If this is due to a subscription please re-write it to use React provided hooks. ' +
'Otherwise concurrent mode guarantees are off the table.',
);
}
ReactCurrentBatchConfig._updatedFibers.clear();
}
}
}
}

Expand Down
18 changes: 18 additions & 0 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ import {
} from './ReactUpdateQueue.old';
import {pushInterleavedQueue} from './ReactFiberInterleavedUpdates.old';
import {getIsStrictModeForDevtools} from './ReactFiberReconciler.old';
import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';

const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;

Expand Down Expand Up @@ -1861,6 +1862,23 @@ function startTransition(setPending, callback) {
} finally {
setCurrentUpdatePriority(previousPriority);
ReactCurrentBatchConfig.transition = prevTransition;
if (__DEV__) {
if (
prevTransition !== 1 &&
warnOnSubscriptionInsideStartTransition &&
ReactCurrentBatchConfig._updatedFibers
) {
const updatedFibersCount = ReactCurrentBatchConfig._updatedFibers.size;
if (updatedFibersCount > 10) {
console.warn(
'Detected a large number of updates inside startTransition. ' +
'If this is due to a subscription please re-write it to use React provided hooks. ' +
'Otherwise concurrent mode guarantees are off the table.',
);
}
ReactCurrentBatchConfig._updatedFibers.clear();
}
}
}
}

Expand Down
8 changes: 8 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
enableStrictEffects,
skipUnmountedBoundaries,
enableUpdaterTracking,
warnOnSubscriptionInsideStartTransition,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import invariant from 'shared/invariant';
Expand Down Expand Up @@ -385,6 +386,13 @@ export function requestUpdateLane(fiber: Fiber): Lane {

const isTransition = requestCurrentTransition() !== NoTransition;
if (isTransition) {
if (
__DEV__ &&
warnOnSubscriptionInsideStartTransition &&
ReactCurrentBatchConfig._updatedFibers
) {
ReactCurrentBatchConfig._updatedFibers.add(fiber);
}
// The algorithm for assigning an update to a lane should be stable for all
// updates at the same priority within the same event. To do this, the
// inputs to the algorithm must be the same.
Expand Down
8 changes: 8 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
enableStrictEffects,
skipUnmountedBoundaries,
enableUpdaterTracking,
warnOnSubscriptionInsideStartTransition,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import invariant from 'shared/invariant';
Expand Down Expand Up @@ -385,6 +386,13 @@ export function requestUpdateLane(fiber: Fiber): Lane {

const isTransition = requestCurrentTransition() !== NoTransition;
if (isTransition) {
if (
__DEV__ &&
warnOnSubscriptionInsideStartTransition &&
ReactCurrentBatchConfig._updatedFibers
) {
ReactCurrentBatchConfig._updatedFibers.add(fiber);
}
// The algorithm for assigning an update to a lane should be stable for all
// updates at the same priority within the same event. To do this, the
// inputs to the algorithm must be the same.
Expand Down
14 changes: 12 additions & 2 deletions packages/react/src/ReactCurrentBatchConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,22 @@
* @flow
*/

import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';

type BatchConfig = {
transition: number,
_updatedFibers?: Set<Fiber>,
};
/**
* Keeps track of the current batch's configuration such as how long an update
* should suspend for if it needs to.
*/
const ReactCurrentBatchConfig = {
transition: (0: number),
const ReactCurrentBatchConfig: BatchConfig = {
transition: 0,
};

if (__DEV__) {
ReactCurrentBatchConfig._updatedFibers = new Set();
}

export default ReactCurrentBatchConfig;
18 changes: 18 additions & 0 deletions packages/react/src/ReactStartTransition.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import ReactCurrentBatchConfig from './ReactCurrentBatchConfig';
import {warnOnSubscriptionInsideStartTransition} from 'shared/ReactFeatureFlags';

export function startTransition(scope: () => void) {
const prevTransition = ReactCurrentBatchConfig.transition;
Expand All @@ -16,5 +17,22 @@ export function startTransition(scope: () => void) {
scope();
} finally {
ReactCurrentBatchConfig.transition = prevTransition;
if (__DEV__) {
if (
prevTransition !== 1 &&
warnOnSubscriptionInsideStartTransition &&
ReactCurrentBatchConfig._updatedFibers
) {
const updatedFibersCount = ReactCurrentBatchConfig._updatedFibers.size;
if (updatedFibersCount > 10) {
console.warn(
'Detected a large number of updates inside startTransition. ' +
'If this is due to a subscription please re-write it to use React provided hooks. ' +
'Otherwise concurrent mode guarantees are off the table.',
);
}
ReactCurrentBatchConfig._updatedFibers.clear();
}
}
}
}
91 changes: 91 additions & 0 deletions packages/react/src/__tests__/ReactStartTransition-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
*/

'use strict';

let React;
let ReactTestRenderer;
let act;
let useState;
let useTransition;

const SUSPICIOUS_NUMBER_OF_FIBERS_UPDATED = 10;

describe('ReactStartTransition', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactTestRenderer = require('react-test-renderer');
act = require('jest-react').act;
useState = React.useState;
useTransition = React.useTransition;
});

// @gate warnOnSubscriptionInsideStartTransition || !__DEV__
it('Warns if a suspicious number of fibers are updated inside startTransition', () => {
const subs = new Set();
const useUserSpaceSubscription = () => {
const setState = useState(0)[1];
subs.add(setState);
};

let triggerHookTransition;

const Component = ({level}) => {
useUserSpaceSubscription();
if (level === 0) {
triggerHookTransition = useTransition()[1];
}
if (level < SUSPICIOUS_NUMBER_OF_FIBERS_UPDATED) {
return <Component level={level + 1} />;
}
return null;
};

act(() => {
ReactTestRenderer.create(<Component level={0} />, {
unstable_isConcurrent: true,
});
});

expect(() => {
act(() => {
React.startTransition(() => {
subs.forEach(setState => {
setState(state => state + 1);
});
});
});
}).toWarnDev(
[
'Detected a large number of updates inside startTransition. ' +
'If this is due to a subscription please re-write it to use React provided hooks. ' +
'Otherwise concurrent mode guarantees are off the table.',
],
{withoutStack: true},
);

expect(() => {
act(() => {
triggerHookTransition(() => {
subs.forEach(setState => {
setState(state => state + 1);
});
});
});
}).toWarnDev(
[
'Detected a large number of updates inside startTransition. ' +
'If this is due to a subscription please re-write it to use React provided hooks. ' +
'Otherwise concurrent mode guarantees are off the table.',
],
{withoutStack: true},
);
});
});
2 changes: 2 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ export const enableTrustedTypesIntegration = false;
// a deprecated pattern we want to get rid of in the future
export const warnAboutSpreadingKeyToJSX = false;

export const warnOnSubscriptionInsideStartTransition = false;

export const enableComponentStackLocations = true;

export const enableNewReconciler = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const disableTextareaChildren = false;
export const disableModulePatternComponents = false;
export const warnUnstableRenderSubtreeIntoContainer = false;
export const warnAboutSpreadingKeyToJSX = false;
export const warnOnSubscriptionInsideStartTransition = false;
export const enableComponentStackLocations = false;
export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
export const disableModulePatternComponents = false;
export const warnUnstableRenderSubtreeIntoContainer = false;
export const warnAboutSpreadingKeyToJSX = false;
export const warnOnSubscriptionInsideStartTransition = false;
export const enableComponentStackLocations = false;
export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
export const disableModulePatternComponents = false;
export const warnUnstableRenderSubtreeIntoContainer = false;
export const warnAboutSpreadingKeyToJSX = false;
export const warnOnSubscriptionInsideStartTransition = false;
export const enableComponentStackLocations = true;
export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export const enableSuspenseLayoutEffectSemantics = false;
export const enableGetInspectorDataForInstanceInProduction = false;
export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = false;

export const warnOnSubscriptionInsideStartTransition = false;
export const enableStrictEffects = false;
export const createRootStrictEffectsByDefault = false;
export const enableUseRefAccessWarning = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
export const disableModulePatternComponents = true;
export const warnUnstableRenderSubtreeIntoContainer = false;
export const warnAboutSpreadingKeyToJSX = false;
export const warnOnSubscriptionInsideStartTransition = false;
export const enableComponentStackLocations = true;
export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const disableTextareaChildren = false;
export const disableModulePatternComponents = false;
export const warnUnstableRenderSubtreeIntoContainer = false;
export const warnAboutSpreadingKeyToJSX = false;
export const warnOnSubscriptionInsideStartTransition = false;
export const enableComponentStackLocations = true;
export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const disableTextareaChildren = __EXPERIMENTAL__;
export const disableModulePatternComponents = true;
export const warnUnstableRenderSubtreeIntoContainer = false;
export const warnAboutSpreadingKeyToJSX = false;
export const warnOnSubscriptionInsideStartTransition = false;
export const enableComponentStackLocations = true;
export const enableLegacyFBSupport = !__EXPERIMENTAL__;
export const enableFilterEmptyStringAttributesDOM = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
export const enableLazyContextPropagation = __VARIANT__;
export const enableSyncDefaultUpdates = __VARIANT__;
export const consoleManagedByDevToolsDuringStrictMode = __VARIANT__;
export const warnOnSubscriptionInsideStartTransition = __VARIANT__;

// Enable this flag to help with concurrent mode debugging.
// It logs information to the console about React scheduling, rendering, and commit phases.
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const {
disableSchedulerTimeoutInWorkLoop,
enableLazyContextPropagation,
enableSyncDefaultUpdates,
warnOnSubscriptionInsideStartTransition,
} = dynamicFeatureFlags;

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

export const warnAboutDeprecatedLifecycles = true;
export const disableLegacyContext = __EXPERIMENTAL__;
export const warnAboutStringRefs = false;
Expand Down