Skip to content

Commit 2994868

Browse files
acdliteacdlite
committed
Enqueue update and callback simultaneously
Without this fix, in non-batched mode, the update is scheduled first and synchronously flushed before the callback is added to the queue. The callback isn't called until the next flush.
1 parent ffc832e commit 2994868

File tree

4 files changed

+171
-80
lines changed

4 files changed

+171
-80
lines changed

‎src/isomorphic/classic/class/ReactClass.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,11 @@ var ReactClassMixin = {
728728
* type signature and the only use case for this, is to avoid that.
729729
*/
730730
replaceState: function(newState, callback) {
731+
if (this.updater.isFiberUpdater) {
732+
this.updater.enqueueReplaceState(this, newState, callback);
733+
return;
734+
}
735+
731736
this.updater.enqueueReplaceState(this, newState);
732737
if (callback) {
733738
this.updater.enqueueCallback(this, callback, 'replaceState');

‎src/isomorphic/modern/class/ReactComponent.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,12 @@ ReactComponent.prototype.setState = function(partialState, callback) {
6565
'setState(...): takes an object of state variables to update or a ' +
6666
'function which returns an object of state variables.'
6767
);
68+
69+
if (this.updater.isFiberUpdater) {
70+
this.updater.enqueueSetState(this, partialState, callback);
71+
return;
72+
}
73+
6874
this.updater.enqueueSetState(this, partialState);
6975
if (callback) {
7076
this.updater.enqueueCallback(this, callback, 'setState');
@@ -86,6 +92,11 @@ ReactComponent.prototype.setState = function(partialState, callback) {
8692
* @protected
8793
*/
8894
ReactComponent.prototype.forceUpdate = function(callback) {
95+
if (this.updater.isFiberUpdater) {
96+
this.updater.enqueueForceUpdate(this, callback);
97+
return;
98+
}
99+
89100
this.updater.enqueueForceUpdate(this);
90101
if (callback) {
91102
this.updater.enqueueCallback(this, callback, 'forceUpdate');

‎src/renderers/shared/fiber/ReactFiberClassComponent.js‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,36 +41,35 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior
4141
// Class component state updater
4242
const updater = {
4343
isMounted,
44-
enqueueSetState(instance, partialState) {
44+
enqueueSetState(instance, partialState, callback) {
4545
const fiber = ReactInstanceMap.get(instance);
4646
const updateQueue = fiber.updateQueue ?
4747
addToQueue(fiber.updateQueue, partialState) :
4848
createUpdateQueue(partialState);
49+
if (callback) {
50+
addCallbackToQueue(updateQueue, callback);
51+
}
4952
scheduleUpdateQueue(fiber, updateQueue);
5053
},
51-
enqueueReplaceState(instance, state) {
54+
enqueueReplaceState(instance, state, callback) {
5255
const fiber = ReactInstanceMap.get(instance);
5356
const updateQueue = createUpdateQueue(state);
5457
updateQueue.isReplace = true;
58+
if (callback) {
59+
addCallbackToQueue(updateQueue, callback);
60+
}
5561
scheduleUpdateQueue(fiber, updateQueue);
5662
},
57-
enqueueForceUpdate(instance) {
63+
enqueueForceUpdate(instance, callback) {
5864
const fiber = ReactInstanceMap.get(instance);
5965
const updateQueue = fiber.updateQueue || createUpdateQueue(null);
6066
updateQueue.isForced = true;
61-
scheduleUpdateQueue(fiber, updateQueue);
62-
},
63-
enqueueCallback(instance, callback) {
64-
const fiber = ReactInstanceMap.get(instance);
65-
let updateQueue = fiber.updateQueue ?
66-
fiber.updateQueue :
67-
createUpdateQueue(null);
68-
addCallbackToQueue(updateQueue, callback);
69-
fiber.updateQueue = updateQueue;
70-
if (fiber.alternate) {
71-
fiber.alternate.updateQueue = updateQueue;
67+
if (callback) {
68+
addCallbackToQueue(updateQueue, callback);
7269
}
70+
scheduleUpdateQueue(fiber, updateQueue);
7371
},
72+
isFiberUpdater: true,
7473
};
7574

7675
function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState) {

‎src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js‎

Lines changed: 142 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
'use strict';
1313

14+
var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
15+
1416
var React;
1517
var ReactDOM;
1618

@@ -148,72 +150,146 @@ describe('ReactCompositeComponent-state', () => {
148150

149151
ReactDOM.unmountComponentAtNode(container);
150152

151-
expect(stateListener.mock.calls.join('\n')).toEqual([
152-
// there is no state when getInitialState() is called
153-
['getInitialState', null],
154-
['componentWillMount-start', 'red'],
155-
// setState()'s only enqueue pending states.
156-
['componentWillMount-after-sunrise', 'red'],
157-
['componentWillMount-end', 'red'],
158-
// pending state queue is processed
159-
['before-setState-sunrise', 'red'],
160-
['after-setState-sunrise', 'sunrise'],
161-
['after-setState-orange', 'orange'],
162-
// pending state has been applied
163-
['render', 'orange'],
164-
['componentDidMount-start', 'orange'],
165-
// setState-sunrise and setState-orange should be called here,
166-
// after the bug in #1740
167-
// componentDidMount() called setState({color:'yellow'}), which is async.
168-
// The update doesn't happen until the next flush.
169-
['componentDidMount-end', 'orange'],
170-
['shouldComponentUpdate-currentState', 'orange'],
171-
['shouldComponentUpdate-nextState', 'yellow'],
172-
['componentWillUpdate-currentState', 'orange'],
173-
['componentWillUpdate-nextState', 'yellow'],
174-
['render', 'yellow'],
175-
['componentDidUpdate-currentState', 'yellow'],
176-
['componentDidUpdate-prevState', 'orange'],
177-
['setState-sunrise', 'yellow'],
178-
['setState-orange', 'yellow'],
179-
['setState-yellow', 'yellow'],
180-
['initial-callback', 'yellow'],
181-
['componentWillReceiveProps-start', 'yellow'],
182-
// setState({color:'green'}) only enqueues a pending state.
183-
['componentWillReceiveProps-end', 'yellow'],
184-
// pending state queue is processed
185-
// before-setState-receiveProps never called, due to replaceState.
186-
['before-setState-again-receiveProps', undefined],
187-
['after-setState-receiveProps', 'green'],
188-
['shouldComponentUpdate-currentState', 'yellow'],
189-
['shouldComponentUpdate-nextState', 'green'],
190-
['componentWillUpdate-currentState', 'yellow'],
191-
['componentWillUpdate-nextState', 'green'],
192-
['render', 'green'],
193-
['componentDidUpdate-currentState', 'green'],
194-
['componentDidUpdate-prevState', 'yellow'],
195-
['setState-receiveProps', 'green'],
196-
['setProps', 'green'],
197-
// setFavoriteColor('blue')
198-
['shouldComponentUpdate-currentState', 'green'],
199-
['shouldComponentUpdate-nextState', 'blue'],
200-
['componentWillUpdate-currentState', 'green'],
201-
['componentWillUpdate-nextState', 'blue'],
202-
['render', 'blue'],
203-
['componentDidUpdate-currentState', 'blue'],
204-
['componentDidUpdate-prevState', 'green'],
205-
['setFavoriteColor', 'blue'],
206-
// forceUpdate()
207-
['componentWillUpdate-currentState', 'blue'],
208-
['componentWillUpdate-nextState', 'blue'],
209-
['render', 'blue'],
210-
['componentDidUpdate-currentState', 'blue'],
211-
['componentDidUpdate-prevState', 'blue'],
212-
['forceUpdate', 'blue'],
213-
// unmountComponent()
214-
// state is available within `componentWillUnmount()`
215-
['componentWillUnmount', 'blue'],
216-
].join('\n'));
153+
let expected;
154+
if (ReactDOMFeatureFlags.useFiber) {
155+
expected = [
156+
// there is no state when getInitialState() is called
157+
['getInitialState', null],
158+
['componentWillMount-start', 'red'],
159+
// setState()'s only enqueue pending states.
160+
['componentWillMount-after-sunrise', 'red'],
161+
['componentWillMount-end', 'red'],
162+
// pending state queue is processed
163+
['before-setState-sunrise', 'red'],
164+
['after-setState-sunrise', 'sunrise'],
165+
['after-setState-orange', 'orange'],
166+
// pending state has been applied
167+
['render', 'orange'],
168+
['componentDidMount-start', 'orange'],
169+
// setState-sunrise and setState-orange should be called here,
170+
// after the bug in #1740
171+
// componentDidMount() called setState({color:'yellow'}), which is async.
172+
// The update doesn't happen until the next flush.
173+
['componentDidMount-end', 'orange'],
174+
['setState-sunrise', 'orange'],
175+
['setState-orange', 'orange'],
176+
['initial-callback', 'orange'],
177+
['shouldComponentUpdate-currentState', 'orange'],
178+
['shouldComponentUpdate-nextState', 'yellow'],
179+
['componentWillUpdate-currentState', 'orange'],
180+
['componentWillUpdate-nextState', 'yellow'],
181+
['render', 'yellow'],
182+
['componentDidUpdate-currentState', 'yellow'],
183+
['componentDidUpdate-prevState', 'orange'],
184+
['setState-yellow', 'yellow'],
185+
['componentWillReceiveProps-start', 'yellow'],
186+
// setState({color:'green'}) only enqueues a pending state.
187+
['componentWillReceiveProps-end', 'yellow'],
188+
// pending state queue is processed
189+
// before-setState-receiveProps never called, due to replaceState.
190+
['before-setState-again-receiveProps', undefined],
191+
['after-setState-receiveProps', 'green'],
192+
['shouldComponentUpdate-currentState', 'yellow'],
193+
['shouldComponentUpdate-nextState', 'green'],
194+
['componentWillUpdate-currentState', 'yellow'],
195+
['componentWillUpdate-nextState', 'green'],
196+
['render', 'green'],
197+
['componentDidUpdate-currentState', 'green'],
198+
['componentDidUpdate-prevState', 'yellow'],
199+
['setState-receiveProps', 'green'],
200+
['setProps', 'green'],
201+
// setFavoriteColor('blue')
202+
['shouldComponentUpdate-currentState', 'green'],
203+
['shouldComponentUpdate-nextState', 'blue'],
204+
['componentWillUpdate-currentState', 'green'],
205+
['componentWillUpdate-nextState', 'blue'],
206+
['render', 'blue'],
207+
['componentDidUpdate-currentState', 'blue'],
208+
['componentDidUpdate-prevState', 'green'],
209+
['setFavoriteColor', 'blue'],
210+
// forceUpdate()
211+
['componentWillUpdate-currentState', 'blue'],
212+
['componentWillUpdate-nextState', 'blue'],
213+
['render', 'blue'],
214+
['componentDidUpdate-currentState', 'blue'],
215+
['componentDidUpdate-prevState', 'blue'],
216+
['forceUpdate', 'blue'],
217+
// unmountComponent()
218+
// state is available within `componentWillUnmount()`
219+
['componentWillUnmount', 'blue'],
220+
];
221+
} else {
222+
// There's a bug in the stack reconciler where setState callbacks inside
223+
// componentWillMount aren't flushed properly
224+
expected = [
225+
// there is no state when getInitialState() is called
226+
['getInitialState', null],
227+
['componentWillMount-start', 'red'],
228+
// setState()'s only enqueue pending states.
229+
['componentWillMount-after-sunrise', 'red'],
230+
['componentWillMount-end', 'red'],
231+
// pending state queue is processed
232+
['before-setState-sunrise', 'red'],
233+
['after-setState-sunrise', 'sunrise'],
234+
['after-setState-orange', 'orange'],
235+
// pending state has been applied
236+
['render', 'orange'],
237+
['componentDidMount-start', 'orange'],
238+
// setState-sunrise and setState-orange should be called here,
239+
// after the bug in #1740
240+
// componentDidMount() called setState({color:'yellow'}), which is async.
241+
// The update doesn't happen until the next flush.
242+
['componentDidMount-end', 'orange'],
243+
['shouldComponentUpdate-currentState', 'orange'],
244+
['shouldComponentUpdate-nextState', 'yellow'],
245+
['componentWillUpdate-currentState', 'orange'],
246+
['componentWillUpdate-nextState', 'yellow'],
247+
['render', 'yellow'],
248+
['componentDidUpdate-currentState', 'yellow'],
249+
['componentDidUpdate-prevState', 'orange'],
250+
['setState-sunrise', 'yellow'],
251+
['setState-orange', 'yellow'],
252+
['setState-yellow', 'yellow'],
253+
['initial-callback', 'yellow'],
254+
['componentWillReceiveProps-start', 'yellow'],
255+
// setState({color:'green'}) only enqueues a pending state.
256+
['componentWillReceiveProps-end', 'yellow'],
257+
// pending state queue is processed
258+
// before-setState-receiveProps never called, due to replaceState.
259+
['before-setState-again-receiveProps', undefined],
260+
['after-setState-receiveProps', 'green'],
261+
['shouldComponentUpdate-currentState', 'yellow'],
262+
['shouldComponentUpdate-nextState', 'green'],
263+
['componentWillUpdate-currentState', 'yellow'],
264+
['componentWillUpdate-nextState', 'green'],
265+
['render', 'green'],
266+
['componentDidUpdate-currentState', 'green'],
267+
['componentDidUpdate-prevState', 'yellow'],
268+
['setState-receiveProps', 'green'],
269+
['setProps', 'green'],
270+
// setFavoriteColor('blue')
271+
['shouldComponentUpdate-currentState', 'green'],
272+
['shouldComponentUpdate-nextState', 'blue'],
273+
['componentWillUpdate-currentState', 'green'],
274+
['componentWillUpdate-nextState', 'blue'],
275+
['render', 'blue'],
276+
['componentDidUpdate-currentState', 'blue'],
277+
['componentDidUpdate-prevState', 'green'],
278+
['setFavoriteColor', 'blue'],
279+
// forceUpdate()
280+
['componentWillUpdate-currentState', 'blue'],
281+
['componentWillUpdate-nextState', 'blue'],
282+
['render', 'blue'],
283+
['componentDidUpdate-currentState', 'blue'],
284+
['componentDidUpdate-prevState', 'blue'],
285+
['forceUpdate', 'blue'],
286+
// unmountComponent()
287+
// state is available within `componentWillUnmount()`
288+
['componentWillUnmount', 'blue'],
289+
];
290+
}
291+
292+
expect(stateListener.mock.calls.join('\n')).toEqual(expected.join('\n'));
217293
});
218294

219295
it('should batch unmounts', () => {

0 commit comments

Comments
 (0)