[Bugfix] Prevent already-committed setState callback from firing again during a rebase (#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.
Andrew Clark committed
May 12, 2021 at 10:40 UTC
46491dce96e6a151706f51494edfd3cb7f09ed7f
3 files changed
+59
-2
packages/react-reconciler/src/ReactUpdateQueue.new.js
+6
-1
@@ -580,7 +580,12 @@ export function processUpdateQueue<State>(
580
instance,
581
);
582
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
+ ) {
589
workInProgress.flags |= Callback;
590
const effects = queue.effects;
591
if (effects === null) {
packages/react-reconciler/src/ReactUpdateQueue.old.js
+6
-1
@@ -580,7 +580,12 @@ export function processUpdateQueue<State>(
580
instance,
581
);
582
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
+ ) {
589
workInProgress.flags |= Callback;
590
const effects = queue.effects;
591
if (effects === null) {
packages/react-reconciler/src/__tests__/ReactClassSetStateCallback-test.js
new
+47
@@ -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
+});