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.
facebook · Oct 29, 2016 · 2994868 · 2994868
1 parent ffc832e
commit 2994868
Show file tree

Hide file tree

Showing 4 changed files with 171 additions and 80 deletions.
diff --git a/src/isomorphic/classic/class/ReactClass.js b/src/isomorphic/classic/class/ReactClass.js
@@ -728,6 +728,11 @@ var ReactClassMixin = {
    * type signature and the only use case for this, is to avoid that.
    */
   replaceState: function(newState, callback) {
+    if (this.updater.isFiberUpdater) {
+      this.updater.enqueueReplaceState(this, newState, callback);
+      return;
+    }
+
     this.updater.enqueueReplaceState(this, newState);
     if (callback) {
       this.updater.enqueueCallback(this, callback, 'replaceState');

diff --git a/src/isomorphic/modern/class/ReactComponent.js b/src/isomorphic/modern/class/ReactComponent.js
@@ -65,6 +65,12 @@ ReactComponent.prototype.setState = function(partialState, callback) {
     'setState(...): takes an object of state variables to update or a ' +
     'function which returns an object of state variables.'
   );
+
+  if (this.updater.isFiberUpdater) {
+    this.updater.enqueueSetState(this, partialState, callback);
+    return;
+  }
+
   this.updater.enqueueSetState(this, partialState);
   if (callback) {
     this.updater.enqueueCallback(this, callback, 'setState');
@@ -86,6 +92,11 @@ ReactComponent.prototype.setState = function(partialState, callback) {
  * @protected
  */
 ReactComponent.prototype.forceUpdate = function(callback) {
+  if (this.updater.isFiberUpdater) {
+    this.updater.enqueueForceUpdate(this, callback);
+    return;
+  }
+
   this.updater.enqueueForceUpdate(this);
   if (callback) {
     this.updater.enqueueCallback(this, callback, 'forceUpdate');

diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js
@@ -41,36 +41,35 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior
   // Class component state updater
   const updater = {
     isMounted,
-    enqueueSetState(instance, partialState) {
+    enqueueSetState(instance, partialState, callback) {
       const fiber = ReactInstanceMap.get(instance);
       const updateQueue = fiber.updateQueue ?
         addToQueue(fiber.updateQueue, partialState) :
         createUpdateQueue(partialState);
+      if (callback) {
+        addCallbackToQueue(updateQueue, callback);
+      }
       scheduleUpdateQueue(fiber, updateQueue);
     },
-    enqueueReplaceState(instance, state) {
+    enqueueReplaceState(instance, state, callback) {
       const fiber = ReactInstanceMap.get(instance);
       const updateQueue = createUpdateQueue(state);
       updateQueue.isReplace = true;
+      if (callback) {
+        addCallbackToQueue(updateQueue, callback);
+      }
       scheduleUpdateQueue(fiber, updateQueue);
     },
-    enqueueForceUpdate(instance) {
+    enqueueForceUpdate(instance, callback) {
       const fiber = ReactInstanceMap.get(instance);
       const updateQueue = fiber.updateQueue || createUpdateQueue(null);
       updateQueue.isForced = true;
-      scheduleUpdateQueue(fiber, updateQueue);
-    },
-    enqueueCallback(instance, callback) {
-      const fiber = ReactInstanceMap.get(instance);
-      let updateQueue = fiber.updateQueue ?
-        fiber.updateQueue :
-        createUpdateQueue(null);
-      addCallbackToQueue(updateQueue, callback);
-      fiber.updateQueue = updateQueue;
-      if (fiber.alternate) {
-        fiber.alternate.updateQueue = updateQueue;
+      if (callback) {
+        addCallbackToQueue(updateQueue, callback);
       }
+      scheduleUpdateQueue(fiber, updateQueue);
     },
+    isFiberUpdater: true,
   };
 
   function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState) {

diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js
@@ -11,6 +11,8 @@
 
 'use strict';
 
+var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
+
 var React;
 var ReactDOM;
 
@@ -148,72 +150,146 @@ describe('ReactCompositeComponent-state', () => {
 
     ReactDOM.unmountComponentAtNode(container);
 
-    expect(stateListener.mock.calls.join('\n')).toEqual([
-      // there is no state when getInitialState() is called
-      ['getInitialState', null],
-      ['componentWillMount-start', 'red'],
-      // setState()'s only enqueue pending states.
-      ['componentWillMount-after-sunrise', 'red'],
-      ['componentWillMount-end', 'red'],
-      // pending state queue is processed
-      ['before-setState-sunrise', 'red'],
-      ['after-setState-sunrise', 'sunrise'],
-      ['after-setState-orange', 'orange'],
-      // pending state has been applied
-      ['render', 'orange'],
-      ['componentDidMount-start', 'orange'],
-      // setState-sunrise and setState-orange should be called here,
-      // after the bug in #1740
-      // componentDidMount() called setState({color:'yellow'}), which is async.
-      // The update doesn't happen until the next flush.
-      ['componentDidMount-end', 'orange'],
-      ['shouldComponentUpdate-currentState', 'orange'],
-      ['shouldComponentUpdate-nextState', 'yellow'],
-      ['componentWillUpdate-currentState', 'orange'],
-      ['componentWillUpdate-nextState', 'yellow'],
-      ['render', 'yellow'],
-      ['componentDidUpdate-currentState', 'yellow'],
-      ['componentDidUpdate-prevState', 'orange'],
-      ['setState-sunrise', 'yellow'],
-      ['setState-orange', 'yellow'],
-      ['setState-yellow', 'yellow'],
-      ['initial-callback', 'yellow'],
-      ['componentWillReceiveProps-start', 'yellow'],
-      // setState({color:'green'}) only enqueues a pending state.
-      ['componentWillReceiveProps-end', 'yellow'],
-      // pending state queue is processed
-      // before-setState-receiveProps never called, due to replaceState.
-      ['before-setState-again-receiveProps', undefined],
-      ['after-setState-receiveProps', 'green'],
-      ['shouldComponentUpdate-currentState', 'yellow'],
-      ['shouldComponentUpdate-nextState', 'green'],
-      ['componentWillUpdate-currentState', 'yellow'],
-      ['componentWillUpdate-nextState', 'green'],
-      ['render', 'green'],
-      ['componentDidUpdate-currentState', 'green'],
-      ['componentDidUpdate-prevState', 'yellow'],
-      ['setState-receiveProps', 'green'],
-      ['setProps', 'green'],
-      // setFavoriteColor('blue')
-      ['shouldComponentUpdate-currentState', 'green'],
-      ['shouldComponentUpdate-nextState', 'blue'],
-      ['componentWillUpdate-currentState', 'green'],
-      ['componentWillUpdate-nextState', 'blue'],
-      ['render', 'blue'],
-      ['componentDidUpdate-currentState', 'blue'],
-      ['componentDidUpdate-prevState', 'green'],
-      ['setFavoriteColor', 'blue'],
-      // forceUpdate()
-      ['componentWillUpdate-currentState', 'blue'],
-      ['componentWillUpdate-nextState', 'blue'],
-      ['render', 'blue'],
-      ['componentDidUpdate-currentState', 'blue'],
-      ['componentDidUpdate-prevState', 'blue'],
-      ['forceUpdate', 'blue'],
-      // unmountComponent()
-      // state is available within `componentWillUnmount()`
-      ['componentWillUnmount', 'blue'],
-    ].join('\n'));
+    let expected;
+    if (ReactDOMFeatureFlags.useFiber) {
+      expected = [
+        // there is no state when getInitialState() is called
+        ['getInitialState', null],
+        ['componentWillMount-start', 'red'],
+        // setState()'s only enqueue pending states.
+        ['componentWillMount-after-sunrise', 'red'],
+        ['componentWillMount-end', 'red'],
+        // pending state queue is processed
+        ['before-setState-sunrise', 'red'],
+        ['after-setState-sunrise', 'sunrise'],
+        ['after-setState-orange', 'orange'],
+        // pending state has been applied
+        ['render', 'orange'],
+        ['componentDidMount-start', 'orange'],
+        // setState-sunrise and setState-orange should be called here,
+        // after the bug in #1740
+        // componentDidMount() called setState({color:'yellow'}), which is async.
+        // The update doesn't happen until the next flush.
+        ['componentDidMount-end', 'orange'],
+        ['setState-sunrise', 'orange'],
+        ['setState-orange', 'orange'],
+        ['initial-callback', 'orange'],
+        ['shouldComponentUpdate-currentState', 'orange'],
+        ['shouldComponentUpdate-nextState', 'yellow'],
+        ['componentWillUpdate-currentState', 'orange'],
+        ['componentWillUpdate-nextState', 'yellow'],
+        ['render', 'yellow'],
+        ['componentDidUpdate-currentState', 'yellow'],
+        ['componentDidUpdate-prevState', 'orange'],
+        ['setState-yellow', 'yellow'],
+        ['componentWillReceiveProps-start', 'yellow'],
+        // setState({color:'green'}) only enqueues a pending state.
+        ['componentWillReceiveProps-end', 'yellow'],
+        // pending state queue is processed
+        // before-setState-receiveProps never called, due to replaceState.
+        ['before-setState-again-receiveProps', undefined],
+        ['after-setState-receiveProps', 'green'],
+        ['shouldComponentUpdate-currentState', 'yellow'],
+        ['shouldComponentUpdate-nextState', 'green'],
+        ['componentWillUpdate-currentState', 'yellow'],
+        ['componentWillUpdate-nextState', 'green'],
+        ['render', 'green'],
+        ['componentDidUpdate-currentState', 'green'],
+        ['componentDidUpdate-prevState', 'yellow'],
+        ['setState-receiveProps', 'green'],
+        ['setProps', 'green'],
+        // setFavoriteColor('blue')
+        ['shouldComponentUpdate-currentState', 'green'],
+        ['shouldComponentUpdate-nextState', 'blue'],
+        ['componentWillUpdate-currentState', 'green'],
+        ['componentWillUpdate-nextState', 'blue'],
+        ['render', 'blue'],
+        ['componentDidUpdate-currentState', 'blue'],
+        ['componentDidUpdate-prevState', 'green'],
+        ['setFavoriteColor', 'blue'],
+        // forceUpdate()
+        ['componentWillUpdate-currentState', 'blue'],
+        ['componentWillUpdate-nextState', 'blue'],
+        ['render', 'blue'],
+        ['componentDidUpdate-currentState', 'blue'],
+        ['componentDidUpdate-prevState', 'blue'],
+        ['forceUpdate', 'blue'],
+        // unmountComponent()
+        // state is available within `componentWillUnmount()`
+        ['componentWillUnmount', 'blue'],
+      ];
+    } else {
+      // There's a bug in the stack reconciler where setState callbacks inside
+      // componentWillMount aren't flushed properly
+      expected = [
+        // there is no state when getInitialState() is called
+        ['getInitialState', null],
+        ['componentWillMount-start', 'red'],
+        // setState()'s only enqueue pending states.
+        ['componentWillMount-after-sunrise', 'red'],
+        ['componentWillMount-end', 'red'],
+        // pending state queue is processed
+        ['before-setState-sunrise', 'red'],
+        ['after-setState-sunrise', 'sunrise'],
+        ['after-setState-orange', 'orange'],
+        // pending state has been applied
+        ['render', 'orange'],
+        ['componentDidMount-start', 'orange'],
+        // setState-sunrise and setState-orange should be called here,
+        // after the bug in #1740
+        // componentDidMount() called setState({color:'yellow'}), which is async.
+        // The update doesn't happen until the next flush.
+        ['componentDidMount-end', 'orange'],
+        ['shouldComponentUpdate-currentState', 'orange'],
+        ['shouldComponentUpdate-nextState', 'yellow'],
+        ['componentWillUpdate-currentState', 'orange'],
+        ['componentWillUpdate-nextState', 'yellow'],
+        ['render', 'yellow'],
+        ['componentDidUpdate-currentState', 'yellow'],
+        ['componentDidUpdate-prevState', 'orange'],
+        ['setState-sunrise', 'yellow'],
+        ['setState-orange', 'yellow'],
+        ['setState-yellow', 'yellow'],
+        ['initial-callback', 'yellow'],
+        ['componentWillReceiveProps-start', 'yellow'],
+        // setState({color:'green'}) only enqueues a pending state.
+        ['componentWillReceiveProps-end', 'yellow'],
+        // pending state queue is processed
+        // before-setState-receiveProps never called, due to replaceState.
+        ['before-setState-again-receiveProps', undefined],
+        ['after-setState-receiveProps', 'green'],
+        ['shouldComponentUpdate-currentState', 'yellow'],
+        ['shouldComponentUpdate-nextState', 'green'],
+        ['componentWillUpdate-currentState', 'yellow'],
+        ['componentWillUpdate-nextState', 'green'],
+        ['render', 'green'],
+        ['componentDidUpdate-currentState', 'green'],
+        ['componentDidUpdate-prevState', 'yellow'],
+        ['setState-receiveProps', 'green'],
+        ['setProps', 'green'],
+        // setFavoriteColor('blue')
+        ['shouldComponentUpdate-currentState', 'green'],
+        ['shouldComponentUpdate-nextState', 'blue'],
+        ['componentWillUpdate-currentState', 'green'],
+        ['componentWillUpdate-nextState', 'blue'],
+        ['render', 'blue'],
+        ['componentDidUpdate-currentState', 'blue'],
+        ['componentDidUpdate-prevState', 'green'],
+        ['setFavoriteColor', 'blue'],
+        // forceUpdate()
+        ['componentWillUpdate-currentState', 'blue'],
+        ['componentWillUpdate-nextState', 'blue'],
+        ['render', 'blue'],
+        ['componentDidUpdate-currentState', 'blue'],
+        ['componentDidUpdate-prevState', 'blue'],
+        ['forceUpdate', 'blue'],
+        // unmountComponent()
+        // state is available within `componentWillUnmount()`
+        ['componentWillUnmount', 'blue'],
+      ];
+    }
+
+    expect(stateListener.mock.calls.join('\n')).toEqual(expected.join('\n'));
   });
 
   it('should batch unmounts', () => {