New effect type: Callback

acdlite · acdlite · commit d3ee9c65457f · 2016-11-02T10:46:15.000-07:00
This solves the problem I had with enqueueSetState and enqueueCallback
being separate.
diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt
@@ -87,6 +87,9 @@ src/renderers/art/__tests__/ReactART-test.js
 * resolves refs before componentDidMount
 * resolves refs before componentDidUpdate
 
+src/renderers/dom/__tests__/ReactDOMProduction-test.js
+* should throw with an error code in production
+
 src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
 * should set style attribute when styles exist
 * should warn when using hyphenated style names
@@ -368,7 +371,6 @@ src/renderers/dom/stack/client/__tests__/ReactMount-test.js
 * should account for escaping on a checksum mismatch
 * should warn if render removes React-rendered children
 * should warn if the unmounted node was rendered by another copy of React
-* passes the correct callback context
 * tracks root instances
 * marks top-level mounts
 
@@ -607,8 +609,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js
 * throws in setState if the update callback is not a function
 * throws in replaceState if the update callback is not a function
 * throws in forceUpdate if the update callback is not a function
-* does not update one component twice in a batch (#2410)
-* unstable_batchedUpdates should return value from a callback
 
 src/renderers/shared/stack/reconciler/__tests__/refs-test.js
 * Should increase refs with an increase in divs
diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt
@@ -438,7 +438,6 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js
 * should use prod React
 * should handle a simple flow
 * should call lifecycle methods
-* should throw with an error code in production
 
 src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
 * should render strings as children
@@ -692,6 +691,7 @@ src/renderers/dom/stack/client/__tests__/ReactMount-test.js
 * should warn if mounting into dirty rendered markup
 * should not warn if mounting into non-empty node
 * should warn when mounting into document.body
+* passes the correct callback context
 
 src/renderers/dom/stack/client/__tests__/ReactMountDestruction-test.js
 * should destroy a react root upon request
@@ -788,6 +788,8 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
 * calls componentWill* twice if an update render is aborted
 * does not call componentWillReceiveProps for state-only updates
 * skips will/DidUpdate when bailing unless an update was already in progress
+* performs batched updates at the end of the batch
+* can nest batchedUpdates
 
 src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js
 * handles isMounted even when the initial render is deferred
@@ -815,6 +817,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
 * can defer side-effects and reuse them later - complex
 * deprioritizes setStates that happens within a deprioritized tree
 * calls callback after update is flushed
+* calls setState callback even if component bails out
 * calls componentWillUnmount after a deletion, even if nested
 * calls componentDidMount/Update after insertion/update
 * invokes ref callbacks after insertion/update/unmount
@@ -1106,7 +1109,9 @@ src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js
 * should not reconcile children passed via props
 * should queue nested updates
 * calls componentWillReceiveProps setState callback properly
+* does not update one component twice in a batch (#2410)
 * does not update one component twice in a batch (#6371)
+* unstable_batchedUpdates should return value from a callback
 
 src/renderers/shared/stack/reconciler/__tests__/Transaction-test.js
 * should invoke closers with/only-with init returns
diff --git a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js
@@ -97,10 +97,10 @@ describe('ReactTransitionGroup', () => {
       expect(log).toEqual(['didMount', 'willEnter', 'didEnter']);
 
       log = [];
-      instance.setState({count: 1}, function() {
-        expect(log).toEqual(['willLeave', 'didLeave', 'willUnmount']);
-      });
+      instance.setState({count: 1});
     });
+
+    expect(log).toEqual(['willLeave', 'didLeave', 'willUnmount']);
   });
 
   it('should handle enter/leave/enter/leave correctly', () => {
diff --git a/src/isomorphic/classic/class/ReactClass.js b/src/isomorphic/classic/class/ReactClass.js
@@ -728,11 +728,6 @@ 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,12 +65,6 @@ 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');
@@ -92,11 +86,6 @@ 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/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js
@@ -48,7 +48,7 @@ var ReactFiberClassComponent = require('ReactFiberClassComponent');
 
 module.exports = function<T, P, I, TI, C>(
   config : HostConfig<T, P, I, TI, C>,
-  scheduleUpdate : (fiber: Fiber, priorityLevel : ?PriorityLevel) => void
+  scheduleUpdate : (fiber: Fiber) => void
 ) {
 
   const {
diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js
@@ -26,7 +26,7 @@ var { isMounted } = require('ReactFiberTreeReflection');
 var ReactInstanceMap = require('ReactInstanceMap');
 var shallowEqual = require('shallowEqual');
 
-module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?PriorityLevel) => void) {
+module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
 
   function scheduleUpdateQueue(fiber: Fiber, updateQueue: UpdateQueue) {
     fiber.updateQueue = updateQueue;
@@ -41,35 +41,33 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior
   // Class component state updater
   const updater = {
     isMounted,
-    enqueueSetState(instance, partialState, callback) {
+    enqueueSetState(instance, partialState) {
       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, callback) {
+    enqueueReplaceState(instance, state) {
       const fiber = ReactInstanceMap.get(instance);
       const updateQueue = createUpdateQueue(state);
       updateQueue.isReplace = true;
-      if (callback) {
-        addCallbackToQueue(updateQueue, callback);
-      }
       scheduleUpdateQueue(fiber, updateQueue);
     },
-    enqueueForceUpdate(instance, callback) {
+    enqueueForceUpdate(instance) {
       const fiber = ReactInstanceMap.get(instance);
       const updateQueue = fiber.updateQueue || createUpdateQueue(null);
       updateQueue.isForced = true;
-      if (callback) {
-        addCallbackToQueue(updateQueue, callback);
-      }
       scheduleUpdateQueue(fiber, updateQueue);
     },
-    isFiberUpdater: true,
+    enqueueCallback(instance, callback) {
+      const fiber = ReactInstanceMap.get(instance);
+      let updateQueue = fiber.updateQueue ?
+        fiber.updateQueue :
+        createUpdateQueue(null);
+      addCallbackToQueue(updateQueue, callback);
+      scheduleUpdateQueue(fiber, updateQueue);
+    },
   };
 
   function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState) {
@@ -210,11 +208,21 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior
     // TODO: Previous state can be null.
     let newState;
     if (updateQueue) {
-      newState = mergeUpdateQueue(updateQueue, instance, previousState, newProps);
+      if (!updateQueue.hasUpdate) {
+        newState = previousState;
+      } else {
+        newState = mergeUpdateQueue(updateQueue, instance, previousState, newProps);
+      }
     } else {
       newState = previousState;
     }
 
+    if (oldProps === newProps &&
+        previousState === newState &&
+        updateQueue && !updateQueue.isForced) {
+      return false;
+    }
+
     if (!checkShouldComponentUpdate(
       workInProgress,
       oldProps,
diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js
@@ -29,7 +29,8 @@ var { callCallbacks } = require('ReactFiberUpdateQueue');
 
 var {
   Placement,
-  PlacementAndUpdate,
+  Update,
+  Callback,
 } = require('ReactTypeOfSideEffect');
 
 module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
@@ -102,8 +103,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
         // If it is not host node and, we might have a host node inside it.
         // Try to search down until we find one.
         // TODO: For coroutines, this will have to search the stateNode.
-        if (node.effectTag === Placement ||
-          node.effectTag === PlacementAndUpdate) {
+        if (node.effectTag & Placement) {
           // If we don't have a child, try the siblings instead.
           continue siblings;
         }
@@ -114,8 +114,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
         }
       }
       // Check if this host node is stable or about to be placed.
-      if (node.effectTag !== Placement &&
-        node.effectTag !== PlacementAndUpdate) {
+      if (!(node.effectTag & Placement)) {
         // Found it!
         return node.stateNode;
       }
@@ -329,41 +328,41 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
       case ClassComponent: {
         const instance = finishedWork.stateNode;
         let error = null;
-        if (!current) {
-          if (typeof instance.componentDidMount === 'function') {
-            error = tryCallComponentDidMount(instance);
-          }
-        } else {
-          if (typeof instance.componentDidUpdate === 'function') {
-            const prevProps = current.memoizedProps;
-            const prevState = current.memoizedState;
-            error = tryCallComponentDidUpdate(instance, prevProps, prevState);
+        if (finishedWork.effectTag & Update) {
+          if (!current) {
+            if (typeof instance.componentDidMount === 'function') {
+              error = tryCallComponentDidMount(instance);
+            }
+          } else {
+            if (typeof instance.componentDidUpdate === 'function') {
+              const prevProps = current.memoizedProps;
+              const prevState = current.memoizedState;
+              error = tryCallComponentDidUpdate(instance, prevProps, prevState);
+            }
           }
+          attachRef(current, finishedWork, instance);
         }
-        // Clear updates from current fiber. This must go before the callbacks
-        // are reset, in case an update is triggered from inside a callback. Is
-        // this safe? Relies on the assumption that work is only committed if
-        // the update queue is empty.
+        // Clear updates from current fiber.
         if (finishedWork.alternate) {
           finishedWork.alternate.updateQueue = null;
         }
-        if (finishedWork.callbackList) {
-          const { callbackList } = finishedWork;
-          finishedWork.callbackList = null;
-          callCallbacks(callbackList, instance);
+        if (finishedWork.effectTag & Callback) {
+          if (finishedWork.callbackList) {
+            callCallbacks(finishedWork.callbackList, instance);
+            finishedWork.callbackList = null;
+          }
         }
-        attachRef(current, finishedWork, instance);
         if (error) {
           return trapError(finishedWork, error);
         }
         return null;
       }
       case HostContainer: {
-        const instance = finishedWork.stateNode;
-        if (instance.callbackList) {
-          const { callbackList } = instance;
-          instance.callbackList = null;
-          callCallbacks(callbackList, instance.current.child.stateNode);
+        const rootFiber = finishedWork.stateNode;
+        if (rootFiber.callbackList) {
+          const { callbackList } = rootFiber;
+          rootFiber.callbackList = null;
+          callCallbacks(callbackList, rootFiber.current.child.stateNode);
         }
       }
       case HostComponent: {
diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js
@@ -34,6 +34,7 @@ var {
 } = ReactTypeOfWork;
 var {
   Update,
+  Callback,
 } = ReactTypeOfSideEffect;
 
 module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
@@ -48,6 +49,11 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
     workInProgress.effectTag |= Update;
   }
 
+  function markCallback(workInProgress : Fiber) {
+    // Tag the fiber with a callback effect.
+    workInProgress.effectTag |= Callback;
+  }
+
   function transferOutput(child : ?Fiber, returnFiber : Fiber) {
     // If we have a single result, we just pass that through as the output to
     // avoid unnecessary traversal. When we have multiple output, we just pass
@@ -124,19 +130,24 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
         // Also need to transfer the props, because pendingProps will be null
         // in the case of an update
         const { state, props } = workInProgress.stateNode;
+        const updateQueue = workInProgress.updateQueue;
         workInProgress.memoizedState = state;
         workInProgress.memoizedProps = props;
-        // Transfer update queue to callbackList field so callbacks can be
-        // called during commit phase.
-        workInProgress.callbackList = workInProgress.updateQueue;
         if (current) {
           if (current.memoizedProps !== workInProgress.memoizedProps ||
-              current.memoizedState !== workInProgress.memoizedState) {
+              current.memoizedState !== workInProgress.memoizedState ||
+              updateQueue && updateQueue.isForced) {
             markUpdate(workInProgress);
           }
         } else {
           markUpdate(workInProgress);
         }
+        if (updateQueue && updateQueue.hasCallback) {
+          // Transfer update queue to callbackList field so callbacks can be
+          // called during commit phase.
+          workInProgress.callbackList = updateQueue;
+          markCallback(workInProgress);
+        }
         return null;
       case HostContainer:
         transferOutput(workInProgress.child, workInProgress);
diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js
diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js
diff --git a/src/renderers/shared/fiber/ReactTypeOfSideEffect.js b/src/renderers/shared/fiber/ReactTypeOfSideEffect.js
diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
