Invoke all null ref calls before any new ref calls

This reorganizes the two commit passes so that all host
environment mutations happens before any life-cycles. That means
that the tree is guaranteed to be in a consistent state at that
point so that you can read layout etc.

This also lets us to detach all refs in the same pass so that
when they get invoked with new instances, that happens after it
has been reset.

src/renderers/shared/fiber/ReactFiberCommitWork.js

@@ -47,18 +47,18 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
     }
   }
 
-  function attachRef(current : ?Fiber, finishedWork : Fiber, instance : any) {
-    const ref = finishedWork.ref;
+  function detachRefIfNeeded(current : ?Fiber, finishedWork : Fiber) {
     if (current) {
       const currentRef = current.ref;
-      if (currentRef && currentRef !== ref) {
-        // TODO: This needs to be done in a separate pass before any other refs
-        // gets resolved. Otherwise we might invoke them in the wrong order
-        // when the same ref switches between two components.
+      if (currentRef && currentRef !== finishedWork.ref) {
         currentRef(null);
       }
     }
-    if (ref) {
+  }
+
+  function attachRef(current : ?Fiber, finishedWork : Fiber, instance : any) {
+    const ref = finishedWork.ref;
+    if (ref && (!current || current.ref !== ref)) {
       ref(instance);
     }
   }
@@ -239,6 +239,46 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
   }
 
   function commitWork(current : ?Fiber, finishedWork : Fiber) : void {
+    switch (finishedWork.tag) {
+      case ClassComponent: {
+        detachRefIfNeeded(current, finishedWork);
+        return;
+      }
+      case HostContainer: {
+        // TODO: Attach children to root container.
+        const children = finishedWork.output;
+        const root : FiberRoot = finishedWork.stateNode;
+        const containerInfo : C = root.containerInfo;
+        updateContainer(containerInfo, children);
+        return;
+      }
+      case HostComponent: {
+        const instance : I = finishedWork.stateNode;
+        if (instance != null && current) {
+          // Commit the work prepared earlier.
+          const newProps = finishedWork.memoizedProps;
+          const oldProps = current.memoizedProps;
+          commitUpdate(instance, oldProps, newProps);
+        }
+        detachRefIfNeeded(current, finishedWork);
+        return;
+      }
+      case HostText: {
+        if (finishedWork.stateNode == null || !current) {
+          throw new Error('This should only be done during updates.');
+        }
+        const textInstance : TI = finishedWork.stateNode;
+        const newText : string = finishedWork.memoizedProps;
+        const oldText : string = current.memoizedProps;
+        commitTextUpdate(textInstance, oldText, newText);
+        return;
+      }
+      default:
+        throw new Error('This unit of work tag should not have side-effects.');
+    }
+  }
+
+  function commitLifeCycles(current : ?Fiber, finishedWork : Fiber) : void {
     switch (finishedWork.tag) {
       case ClassComponent: {
         const instance = finishedWork.stateNode;
@@ -270,33 +310,13 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
         attachRef(current, finishedWork, instance);
         return;
       }
-      case HostContainer: {
-        // TODO: Attach children to root container.
-        const children = finishedWork.output;
-        const root : FiberRoot = finishedWork.stateNode;
-        const containerInfo : C = root.containerInfo;
-        updateContainer(containerInfo, children);
-        return;
-      }
       case HostComponent: {
         const instance : I = finishedWork.stateNode;
-        if (instance != null && current) {
-          // Commit the work prepared earlier.
-          const newProps = finishedWork.memoizedProps;
-          const oldProps = current.memoizedProps;
-          commitUpdate(instance, oldProps, newProps);
-        }
         attachRef(current, finishedWork, instance);
         return;
       }
       case HostText: {
-        if (finishedWork.stateNode == null || !current) {
-          throw new Error('This should only be done during updates.');
-        }
-        const textInstance : TI = finishedWork.stateNode;
-        const newText : string = finishedWork.memoizedProps;
-        const oldText : string = current.memoizedProps;
-        commitTextUpdate(textInstance, oldText, newText);
+        // We have no life-cycles associated with text.
         return;
       }
       default:
@@ -308,6 +328,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
     commitInsertion,
     commitDeletion,
     commitWork,
+    commitLifeCycles,
   };
 
 };

src/renderers/shared/fiber/ReactFiberScheduler.js

@@ -54,7 +54,8 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
 
   const { beginWork } = ReactFiberBeginWork(config, getScheduler);
   const { completeWork } = ReactFiberCompleteWork(config);
-  const { commitInsertion, commitDeletion, commitWork } = ReactFiberCommitWork(config);
+  const { commitInsertion, commitDeletion, commitWork, commitLifeCycles } =
+    ReactFiberCommitWork(config);
 
   const scheduleAnimationCallback = config.scheduleAnimationCallback;
   const scheduleDeferredCallback = config.scheduleDeferredCallback;
@@ -112,31 +113,43 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
     // Commit all the side-effects within a tree.
     // TODO: Error handling.
 
-    // First, we'll perform all the host insertion and deletion effects.
+    // First, we'll perform all the host insertions, updates, deletions and
+    // ref unmounts.
     let effectfulFiber = finishedWork.firstEffect;
     while (effectfulFiber) {
       switch (effectfulFiber.effectTag) {
-        // TODO: Should we commit host updates here? Otherwise update to parent
-        // host components are not visible in life-cycles. Such as when you read
-        // layout information.
-        case Placement:
-        case PlacementAndUpdate:
+        case Placement: {
           commitInsertion(effectfulFiber);
           break;
-        case Deletion:
+        }
+        case PlacementAndUpdate: {
+          commitInsertion(effectfulFiber);
+          const current = effectfulFiber.alternate;
+          commitWork(current, effectfulFiber);
+          break;
+        }
+        case Update: {
+          const current = effectfulFiber.alternate;
+          commitWork(current, effectfulFiber);
+          break;
+        }
+        case Deletion: {
           commitDeletion(effectfulFiber);
           break;
+        }
       }
       effectfulFiber = effectfulFiber.nextEffect;
     }
 
-    // Next, we'll perform all other effects.
+    // Next, we'll perform all life-cycles and ref callbacks. Life-cycles
+    // happens as a separate pass so that all effects in the entire tree have
+    // already been invoked.
     effectfulFiber = finishedWork.firstEffect;
     while (effectfulFiber) {
-      const current = effectfulFiber.alternate;
       if (effectfulFiber.effectTag === Update ||
           effectfulFiber.effectTag === PlacementAndUpdate) {
-        commitWork(current, effectfulFiber);
+        const current = effectfulFiber.alternate;
+        commitLifeCycles(current, effectfulFiber);
       }
       const next = effectfulFiber.nextEffect;
       // Ensure that we clean these up so that we don't accidentally keep them.

src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js

@@ -680,14 +680,11 @@ describe('ReactIncrementalSideEffects', () => {
     ReactNoop.render(<Foo show={true} />);
     ReactNoop.flush();
     expect(ops).toEqual([
-      // TODO: All detach should happen first. Currently they're interleaved.
-      // detach
+      // detach all refs that switched handlers first.
       null,
-      // reattach
+      null,
+      // reattach as a separate phase
       classInstance,
-      // detach
-      null,
-      // reattach
       div(),
     ]);