Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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
3 changes: 3 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,9 @@ export function requestUpdateLane(fiber: Fiber): Lane {

const isTransition = requestCurrentTransition() !== NoTransition;
if (isTransition) {
if (__DEV__ && 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
3 changes: 3 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,9 @@ export function requestUpdateLane(fiber: Fiber): Lane {

const isTransition = requestCurrentTransition() !== NoTransition;
if (isTransition) {
if (__DEV__ && 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
7 changes: 7 additions & 0 deletions packages/react/src/ReactCurrentBatchConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,19 @@
* @flow
*/

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

/**
* 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),
_updatedFibers: (null: Set<Fiber> | null),
Comment thread
salazarm marked this conversation as resolved.
Outdated
};

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

export default ReactCurrentBatchConfig;
20 changes: 20 additions & 0 deletions packages/react/src/ReactStartTransition.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,32 @@

import ReactCurrentBatchConfig from './ReactCurrentBatchConfig';

/**
* If a certain number of fibers are updated inside startTransition then we suspect
* that a user-space defined subscription is being wrapped by startTransition.
* This is no good, they should instead use built-in APIs for concurrent mode to work correctly.
*/

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

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;
});

it('Warns if a suspicious number of fibers are updated inside startTransition', () => {
Comment thread
salazarm marked this conversation as resolved.
const subs = new Set();
const useUserSpaceSubscription = () => {
const [value, setState] = useState(0);
subs.add(setState);
return value;
};
const Component = ({level}) => {
useUserSpaceSubscription();
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 ' +
`(${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},
);
});
});