@samitouri / QOS-React / commits / d153b1e27b

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.

Andrew Clark committed Oct 28, 2016 at 22:53 UTC d153b1e27b132c51fb33464fa1208a91a58ca1fe
4 files changed +73 -27
src/isomorphic/classic/class/ReactClass.js
+5
@@ -728,6 +728,11 @@ var ReactClassMixin = {
728 * type signature and the only use case for this, is to avoid that.
729 */
730 replaceState: function(newState, callback) {
731 + if (this.updater.isFiberUpdater) {
732 + this.updater.enqueueReplaceState(this, newState, callback);
733 + return;
734 + }
735 +
736 this.updater.enqueueReplaceState(this, newState);
737 if (callback) {
738 this.updater.enqueueCallback(this, callback, 'replaceState');
src/isomorphic/modern/class/ReactComponent.js
+11
@@ -65,6 +65,12 @@ ReactComponent.prototype.setState = function(partialState, callback) {
65 'setState(...): takes an object of state variables to update or a ' +
66 'function which returns an object of state variables.'
67 );
68 +
69 + if (this.updater.isFiberUpdater) {
70 + this.updater.enqueueSetState(this, partialState, callback);
71 + return;
72 + }
73 +
74 this.updater.enqueueSetState(this, partialState);
75 if (callback) {
76 this.updater.enqueueCallback(this, callback, 'setState');
@@ -86,6 +92,11 @@ ReactComponent.prototype.setState = function(partialState, callback) {
92 * @protected
93 */
94 ReactComponent.prototype.forceUpdate = function(callback) {
95 + if (this.updater.isFiberUpdater) {
96 + this.updater.enqueueForceUpdate(this, callback);
97 + return;
98 + }
99 +
100 this.updater.enqueueForceUpdate(this);
101 if (callback) {
102 this.updater.enqueueCallback(this, callback, 'forceUpdate');
src/renderers/shared/fiber/ReactFiberClassComponent.js
+13 -14
@@ -41,36 +41,35 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior
41 // Class component state updater
42 const updater = {
43 isMounted,
44 - enqueueSetState(instance, partialState) {
44 + enqueueSetState(instance, partialState, callback) {
45 const fiber = ReactInstanceMap.get(instance);
46 const updateQueue = fiber.updateQueue ?
47 addToQueue(fiber.updateQueue, partialState) :
48 createUpdateQueue(partialState);
49 + if (callback) {
50 + addCallbackToQueue(updateQueue, callback);
51 + }
52 scheduleUpdateQueue(fiber, updateQueue);
53 },
51 - enqueueReplaceState(instance, state) {
54 + enqueueReplaceState(instance, state, callback) {
55 const fiber = ReactInstanceMap.get(instance);
56 const updateQueue = createUpdateQueue(state);
57 updateQueue.isReplace = true;
58 + if (callback) {
59 + addCallbackToQueue(updateQueue, callback);
60 + }
61 scheduleUpdateQueue(fiber, updateQueue);
62 },
57 - enqueueForceUpdate(instance) {
63 + enqueueForceUpdate(instance, callback) {
64 const fiber = ReactInstanceMap.get(instance);
65 const updateQueue = fiber.updateQueue || createUpdateQueue(null);
66 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);
69 }
70 + scheduleUpdateQueue(fiber, updateQueue);
71 },
72 + isFiberUpdater: true,
73 };
74
75 function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState) {
src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js
+44 -13
@@ -11,6 +11,8 @@
11
12 'use strict';
13
14 +var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
15 +
16 var React;
17 var ReactDOM;
18
@@ -148,7 +150,7 @@ describe('ReactCompositeComponent-state', () => {
150
151 ReactDOM.unmountComponentAtNode(container);
152
151 - expect(stateListener.mock.calls.join('\n')).toEqual([
153 + let expected = [
154 // there is no state when getInitialState() is called
155 ['getInitialState', null],
156 ['componentWillMount-start', 'red'],
@@ -167,17 +169,44 @@ describe('ReactCompositeComponent-state', () => {
169 // componentDidMount() called setState({color:'yellow'}), which is async.
170 // The update doesn't happen until the next flush.
171 ['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'],
172 + ];
173 +
174 + if (ReactDOMFeatureFlags.useFiber) {
175 + // The setState callbacks in componentWillMount, and the initial callback
176 + // passed to ReactDOM.render, should be flushed right after component
177 + // did mount:
178 + expected.push(
179 + ['setState-sunrise', 'orange'], // 1
180 + ['setState-orange', 'orange'], // 2
181 + ['initial-callback', 'orange'], // 3
182 + ['shouldComponentUpdate-currentState', 'orange'],
183 + ['shouldComponentUpdate-nextState', 'yellow'],
184 + ['componentWillUpdate-currentState', 'orange'],
185 + ['componentWillUpdate-nextState', 'yellow'],
186 + ['render', 'yellow'],
187 + ['componentDidUpdate-currentState', 'yellow'],
188 + ['componentDidUpdate-prevState', 'orange'],
189 + ['setState-yellow', 'yellow'],
190 + );
191 + } else {
192 + // There is a bug in the stack reconciler where those callbacks are
193 + // enqueued, but aren't called until the next flush.
194 + expected.push(
195 + ['shouldComponentUpdate-currentState', 'orange'],
196 + ['shouldComponentUpdate-nextState', 'yellow'],
197 + ['componentWillUpdate-currentState', 'orange'],
198 + ['componentWillUpdate-nextState', 'yellow'],
199 + ['render', 'yellow'],
200 + ['componentDidUpdate-currentState', 'yellow'],
201 + ['componentDidUpdate-prevState', 'orange'],
202 + ['setState-sunrise', 'yellow'], // 1
203 + ['setState-orange', 'yellow'], // 2
204 + ['setState-yellow', 'yellow'],
205 + ['initial-callback', 'yellow'] // 3
206 + );
207 + }
208 +
209 + expected.push(
210 ['componentWillReceiveProps-start', 'yellow'],
211 // setState({color:'green'}) only enqueues a pending state.
212 ['componentWillReceiveProps-end', 'yellow'],
@@ -213,7 +242,9 @@ describe('ReactCompositeComponent-state', () => {
242 // unmountComponent()
243 // state is available within `componentWillUnmount()`
244 ['componentWillUnmount', 'blue'],
216 - ].join('\n'));
245 + );
246 +
247 + expect(stateListener.mock.calls.join('\n')).toEqual(expected.join('\n'));
248 });
249
250 it('should batch unmounts', () => {