From af7ae1be91563014bcd8bf7c9c77af04c8e8c1c9 Mon Sep 17 00:00:00 2001
From: Andrew Clark <acdlite@fb.com>
Date: Tue, 31 Jan 2017 14:39:52 -0800
Subject: [PATCH] Don't swap the trees until after the first pass of the commit
 phase, where componentWillUnmount is called, but before the second pass,
 where componentDidMount/Update is called.

---
 .../shared/fiber/ReactFiberScheduler.js       |  7 +++++-
 .../ReactIncrementalReflection-test.js        | 24 ++++++++++++++++---
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js
index cb4ab209c1..aa01b76637 100644
--- a/src/renderers/shared/fiber/ReactFiberScheduler.js
+++ b/src/renderers/shared/fiber/ReactFiberScheduler.js
@@ -346,7 +346,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(config : HostConfig<T, P,
         'related to the return field.'
       );
     }
-    root.current = finishedWork;
 
     // Updates that occur during the commit phase should have Task priority
     const previousPriorityContext = priorityContext;
@@ -392,6 +391,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(config : HostConfig<T, P,
 
     resetAfterCommit(commitInfo);
 
+    // The work-in-progress tree is now the current tree. This must come after
+    // the first pass of the commit phase, so that the previous tree is still
+    // current during componentWillUnmount, but before the second pass, so that
+    // the finished work is current during componentDidMount/Update.
+    root.current = finishedWork;
+
     // In the second pass we'll perform all life-cycles and ref callbacks.
     // Life-cycles happen as a separate pass so that all placements, updates,
     // and deletions in the entire tree have already been invoked.
diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js
index 470d0e278c..80ff14643d 100644
--- a/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js
+++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js
@@ -148,12 +148,19 @@ describe('ReactIncrementalReflection', () => {
       componentDidUpdate() {
         ops.push('componentDidUpdate', ReactNoop.findInstance(this));
       }
+      componentWillUnmount() {
+        ops.push('componentWillUnmount', ReactNoop.findInstance(this));
+      }
       render() {
         ops.push('render');
         return this.props.step < 2 ?
           <span ref={ref => this.span = ref} /> :
           this.props.step === 2 ?
           <div ref={ref => this.div = ref} /> :
+          this.props.step === 3 ?
+          null :
+          this.props.step === 4 ?
+          <div ref={ref => this.span = ref} /> :
           null;
       }
     }
@@ -250,7 +257,7 @@ describe('ReactIncrementalReflection', () => {
     // We should now find the new host node.
     expect(ReactNoop.findInstance(classInstance)).toBe(hostDiv);
 
-    // Finally we will render to null but not yet commit it.
+    // Render to null but don't commit it yet.
     ReactNoop.render(<Foo step={3} />);
     ReactNoop.flushDeferredPri(25);
 
@@ -273,7 +280,18 @@ describe('ReactIncrementalReflection', () => {
 
     // This should still be the host div since the deletion is not committed.
     expect(ReactNoop.findInstance(classInstance)).toBe(null);
+
+    // Render a div again
+    ReactNoop.render(<Foo step={4} />);
+    ReactNoop.flush();
+
+    ops = [];
+
+    // Unmount the component.
+    ReactNoop.render([]);
+    ReactNoop.flush();
+    expect(ops).toEqual([
+      'componentWillUnmount', hostDiv,
+    ]);
   });
-
-
 });