Skip to content

Commit 2049e94

Browse files
acdlitekoto
authored andcommitted
[Bugfix] Prevent already-committed setState callback from firing again during a rebase (facebook#21498)
* Failing test: Class callback fired multiple times Happens during a rebase (low priority update followed by high priority update). The high priority callback gets fired twice. * Prevent setState callback firing during rebase Before enqueueing the effect, adds a guard to check if the update was already committed.
1 parent 212523d commit 2049e94

File tree

3 files changed

+59
-2
lines changed

3 files changed

+59
-2
lines changed

packages/react-reconciler/src/ReactUpdateQueue.new.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,12 @@ export function processUpdateQueue<State>(
580580
instance,
581581
);
582582
const callback = update.callback;
583-
if (callback !== null) {
583+
if (
584+
callback !== null &&
585+
// If the update was already committed, we should not queue its
586+
// callback again.
587+
update.lane !== NoLane
588+
) {
584589
workInProgress.flags |= Callback;
585590
const effects = queue.effects;
586591
if (effects === null) {

packages/react-reconciler/src/ReactUpdateQueue.old.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,12 @@ export function processUpdateQueue<State>(
580580
instance,
581581
);
582582
const callback = update.callback;
583-
if (callback !== null) {
583+
if (
584+
callback !== null &&
585+
// If the update was already committed, we should not queue its
586+
// callback again.
587+
update.lane !== NoLane
588+
) {
584589
workInProgress.flags |= Callback;
585590
const effects = queue.effects;
586591
if (effects === null) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
let React;
2+
let ReactNoop;
3+
let Scheduler;
4+
5+
describe('ReactClassSetStateCallback', () => {
6+
beforeEach(() => {
7+
jest.resetModules();
8+
9+
React = require('react');
10+
ReactNoop = require('react-noop-renderer');
11+
Scheduler = require('scheduler');
12+
});
13+
14+
function Text({text}) {
15+
Scheduler.unstable_yieldValue(text);
16+
return text;
17+
}
18+
19+
test('regression: setState callback (2nd arg) should only fire once, even after a rebase', async () => {
20+
let app;
21+
class App extends React.Component {
22+
state = {step: 0};
23+
render() {
24+
app = this;
25+
return <Text text={this.state.step} />;
26+
}
27+
}
28+
29+
const root = ReactNoop.createRoot();
30+
await ReactNoop.act(async () => {
31+
root.render(<App />);
32+
});
33+
expect(Scheduler).toHaveYielded([0]);
34+
35+
await ReactNoop.act(async () => {
36+
app.setState({step: 1}, () =>
37+
Scheduler.unstable_yieldValue('Callback 1'),
38+
);
39+
ReactNoop.flushSync(() => {
40+
app.setState({step: 2}, () =>
41+
Scheduler.unstable_yieldValue('Callback 2'),
42+
);
43+
});
44+
});
45+
expect(Scheduler).toHaveYielded([2, 'Callback 2', 2, 'Callback 1']);
46+
});
47+
});

0 commit comments

Comments
 (0)