Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 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
4 changes: 4 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 {warnIfSubscriptionDetected} from 'react/src/ReactCurrentBatchConfig';

const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;

Expand Down Expand Up @@ -1861,6 +1862,9 @@ function startTransition(setPending, callback) {
} finally {
setCurrentUpdatePriority(previousPriority);
ReactCurrentBatchConfig.transition = prevTransition;
if (__DEV__) {
warnIfSubscriptionDetected();
}
}
}

Expand Down
4 changes: 4 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 {warnIfSubscriptionDetected} from 'react/src/ReactCurrentBatchConfig';

const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;

Expand Down Expand Up @@ -1861,6 +1862,9 @@ function startTransition(setPending, callback) {
} finally {
setCurrentUpdatePriority(previousPriority);
ReactCurrentBatchConfig.transition = prevTransition;
if (__DEV__) {
warnIfSubscriptionDetected();
}
}
}

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
36 changes: 34 additions & 2 deletions packages/react/src/ReactCurrentBatchConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,44 @@
* @flow
*/

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

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 function warnIfSubscriptionDetected() {
if (
__DEV__ &&
warnOnSubscriptionInsideStartTransition &&
ReactCurrentBatchConfig._updatedFibers
) {
const updatedFibersCount = ReactCurrentBatchConfig._updatedFibers.size;
if (updatedFibersCount > 10) {
if (__DEV__) {
console.warn(
'Detected a suspicious number of fibers being updated (10) inside startTransition. ' +
'If this is due to a user-space defined subscription please re-write ' +
'it to use React provided hooks. Otherwise concurrent mode guarantees ' +
'are off the table.',
);
}
}
ReactCurrentBatchConfig._updatedFibers.clear();
}
}

export default ReactCurrentBatchConfig;
7 changes: 6 additions & 1 deletion packages/react/src/ReactStartTransition.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
* @flow
*/

import ReactCurrentBatchConfig from './ReactCurrentBatchConfig';
import ReactCurrentBatchConfig, {
warnIfSubscriptionDetected,
} from './ReactCurrentBatchConfig';

Comment thread
salazarm marked this conversation as resolved.
export function startTransition(scope: () => void) {
const prevTransition = ReactCurrentBatchConfig.transition;
Expand All @@ -16,5 +18,8 @@ export function startTransition(scope: () => void) {
scope();
} finally {
ReactCurrentBatchConfig.transition = prevTransition;
if (__DEV__) {
warnIfSubscriptionDetected();
}
}
}
100 changes: 100 additions & 0 deletions packages/react/src/__tests__/ReactStartTransition-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**
* 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;
});

if (
require('shared/ReactFeatureFlags').warnOnSubscriptionInsideStartTransition
) {
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 suspicious number of fibers being updated ' +
Comment thread
salazarm marked this conversation as resolved.
Outdated
`(${SUSPICIOUS_NUMBER_OF_FIBERS_UPDATED}) inside startTransition. ` +
'If this is due to a user-space defined subscription please re-write ' +
Comment thread
salazarm marked this conversation as resolved.
Outdated
'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 suspicious number of fibers being updated ' +
`(${SUSPICIOUS_NUMBER_OF_FIBERS_UPDATED}) inside startTransition. ` +
'If this is due to a user-space defined subscription please re-write ' +
'it to use React provided hooks. Otherwise concurrent mode guarantees ' +
'are off the table.',
],
{withoutStack: true},
);
});
} else {
it('Dummy test so that jest doesnt complain about lack of tests in this file when the warning feature flag is off', () => {});
}
});
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 @@ -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
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ 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 warnOnSubscriptionInsideStartTransition = false;
Comment thread
salazarm marked this conversation as resolved.
Outdated
export const warnAboutDeprecatedLifecycles = true;
export const disableLegacyContext = __EXPERIMENTAL__;
export const warnAboutStringRefs = false;
Expand Down