From 2a5fe4c2b021ad9a67b904aceed6bb10fe160a79 Mon Sep 17 00:00:00 2001
From: Ben Alpert <ben@benalpert.com>
Date: Wed, 21 Dec 2016 13:51:59 -0800
Subject: [PATCH] Call refs and lifecycles on indeterminate components (#8614)

This was a bug because of the split between ClassComponent and IndeterminateComponent -- we didn't mark effects or push context when we come from an indeterminate component. Now they behave the same way.

The code is even clumsier than before but I'm pretty worried we'll screw these up in the future if we don't unify the paths.

Not many components exercise this path but Relay containers do so it's noticeable.
---
 scripts/fiber/tests-passing.txt               |  1 +
 .../shared/fiber/ReactFiberBeginWork.js       | 10 ++--
 .../__tests__/ReactComponentLifeCycle-test.js | 51 +++++++++++++++++++
 3 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt
index 2ab3530ca0..36bfeaa2c8 100644
--- a/scripts/fiber/tests-passing.txt
+++ b/scripts/fiber/tests-passing.txt
@@ -1343,6 +1343,7 @@ src/renderers/shared/shared/__tests__/ReactComponentLifeCycle-test.js
 * should not throw when updating an auxiliary component
 * should allow state updates in componentDidMount
 * should call nested lifecycle methods in the right order
+* calls effects on module-pattern component
 
 src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js
 * should support module pattern components
diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js
index a1cede0ee3..26bc381c06 100644
--- a/src/renderers/shared/fiber/ReactFiberBeginWork.js
+++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js
@@ -229,7 +229,10 @@ module.exports = function<T, P, I, TI, C, CX>(
     } else {
       shouldUpdate = updateClassInstance(current, workInProgress, priorityLevel);
     }
+    return finishClassComponent(current, workInProgress, shouldUpdate);
+  }
 
+  function finishClassComponent(current : ?Fiber, workInProgress : Fiber, shouldUpdate : boolean) {
     // Schedule side-effects
     if (shouldUpdate) {
       workInProgress.effectTag |= Update;
@@ -411,14 +414,13 @@ module.exports = function<T, P, I, TI, C, CX>(
       workInProgress.tag = ClassComponent;
       adoptClassInstance(workInProgress, value);
       mountClassInstance(workInProgress, priorityLevel);
-      ReactCurrentOwner.current = workInProgress;
-      value = value.render();
+      return finishClassComponent(current, workInProgress, true);
     } else {
       // Proceed under the assumption that this is a functional component
       workInProgress.tag = FunctionalComponent;
+      reconcileChildren(current, workInProgress, value);
+      return workInProgress.child;
     }
-    reconcileChildren(current, workInProgress, value);
-    return workInProgress.child;
   }
 
   function updateCoroutineComponent(current, workInProgress) {
diff --git a/src/renderers/shared/shared/__tests__/ReactComponentLifeCycle-test.js b/src/renderers/shared/shared/__tests__/ReactComponentLifeCycle-test.js
index 08687c0df2..b4a30aa034 100644
--- a/src/renderers/shared/shared/__tests__/ReactComponentLifeCycle-test.js
+++ b/src/renderers/shared/shared/__tests__/ReactComponentLifeCycle-test.js
@@ -594,4 +594,55 @@ describe('ReactComponentLifeCycle', () => {
       'inner componentWillUnmount',
     ]);
   });
+
+  it('calls effects on module-pattern component', function() {
+    const log = [];
+
+    function Parent() {
+      return {
+        render() {
+          expect(typeof this.props).toBe('object');
+          log.push('render');
+          return <Child />;
+        },
+        componentWillMount() {
+          log.push('will mount');
+        },
+        componentDidMount() {
+          log.push('did mount');
+        },
+        componentDidUpdate() {
+          log.push('did update');
+        },
+        getChildContext() {
+          return {x: 2};
+        },
+      };
+    }
+    Parent.childContextTypes = {
+      x: React.PropTypes.number,
+    };
+    function Child(props, context) {
+      expect(context.x).toBe(2);
+      return <div />;
+    }
+    Child.contextTypes = {
+      x: React.PropTypes.number,
+    };
+
+    const div = document.createElement('div');
+    ReactDOM.render(<Parent ref={(c) => c && log.push('ref')} />, div);
+    ReactDOM.render(<Parent ref={(c) => c && log.push('ref')} />, div);
+
+    expect(log).toEqual([
+      'will mount',
+      'render',
+      'did mount',
+      'ref',
+
+      'render',
+      'did update',
+      'ref',
+    ]);
+  });
 });