Fiber: Fix reentrant mounting in synchronous mode (facebook#8623)

Author: sophiebits

scripts/fiber/tests-passing.txt

@@ -1568,6 +1568,7 @@ src/renderers/shared/shared/__tests__/ReactUpdates-test.js
 * does not update one component twice in a batch (#6371)
 * unstable_batchedUpdates should return value from a callback
 * unmounts and remounts a root in the same batch
+* handles reentrant mounting in synchronous mode
 
 src/renderers/shared/shared/__tests__/refs-destruction-test.js
 * should remove refs when destroying the parent

src/renderers/art/ReactARTFiber.js

@@ -328,9 +328,10 @@ class Surface extends Component {
 
     this._surface = Mode.Surface(+width, +height, this._tagRef);
 
-    this._mountNode = ARTRenderer.mountContainer(
+    this._mountNode = ARTRenderer.createContainer(this._surface);
+    ARTRenderer.updateContainer(
       this.props.children,
-      this._surface,
+      this._mountNode,
       this,
     );
   }

src/renderers/dom/fiber/ReactDOMFiber.js

@@ -289,16 +289,15 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any,
 
   let container : DOMContainerElement =
     containerNode.nodeType === DOCUMENT_NODE ? (containerNode : any).documentElement : (containerNode : any);
-  let root;
-  if (!container._reactRootContainer) {
+  let root = container._reactRootContainer;
+  if (!root) {
     // First clear any existing content.
     while (container.lastChild) {
       container.removeChild(container.lastChild);
     }
-    root = container._reactRootContainer = DOMRenderer.mountContainer(children, container, parentComponent, callback);
-  } else {
-    DOMRenderer.updateContainer(children, root = container._reactRootContainer, parentComponent, callback);
+    root = container._reactRootContainer = DOMRenderer.createContainer(container);
   }
+  DOMRenderer.updateContainer(children, root, parentComponent, callback);
   return DOMRenderer.getPublicRootInstance(root);
 }
 

src/renderers/native/ReactNativeFiber.js

@@ -361,12 +361,10 @@ const ReactNative = {
     if (!root) {
       // TODO (bvaughn): If we decide to keep the wrapper component,
       // We could create a wrapper for containerTag as well to reduce special casing.
-      root = NativeRenderer.mountContainer(element, containerTag, null, callback);
-
+      root = NativeRenderer.createContainer(containerTag);
       roots.set(containerTag, root);
-    } else {
-      NativeRenderer.updateContainer(element, root, null, callback);
     }
+    NativeRenderer.updateContainer(element, root, null, callback);
 
     return NativeRenderer.getPublicRootInstance(root);
   },

src/renderers/noop/ReactNoop.js

@@ -188,17 +188,14 @@ var ReactNoop = {
   },
 
   renderToRootWithID(element : ReactElement<any>, rootID : string, callback : ?Function) {
-    if (!roots.has(rootID)) {
+    let root = roots.get(rootID);
+    if (!root) {
       const container = { rootID: rootID, children: [] };
       rootContainers.set(rootID, container);
-      const root = NoopRenderer.mountContainer(element, container, null, callback);
+      root = NoopRenderer.createContainer(container);
       roots.set(rootID, root);
-    } else {
-      const root = roots.get(rootID);
-      if (root) {
-        NoopRenderer.updateContainer(element, root, null, callback);
-      }
     }
+    NoopRenderer.updateContainer(element, root, null, callback);
   },
 
   unmountRootWithID(rootID : string) {

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -280,7 +280,8 @@ module.exports = function<T, P, I, TI, C, CX>(
         root.pendingContext,
         root.pendingContext !== root.context
       );
-    } else {
+    } else if (root.context) {
+      // Should always be set
       pushTopLevelContextObject(
         workInProgress,
         root.context,

src/renderers/shared/fiber/ReactFiberReconciler.js

@@ -75,7 +75,7 @@ export type HostConfig<T, P, I, TI, C, CX> = {
 };
 
 export type Reconciler<C, I, TI> = {
-  mountContainer(element : ReactNodeList, containerInfo : C, parentComponent : ?ReactComponent<any, any, any>) : OpaqueNode,
+  createContainer(containerInfo : C) : OpaqueNode,
   updateContainer(element : ReactNodeList, container : OpaqueNode, parentComponent : ?ReactComponent<any, any, any>) : void,
   performWithPriority(priorityLevel : PriorityLevel, fn : Function) : void,
   /* eslint-disable no-undef */
@@ -119,17 +119,10 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
 
   return {
 
-    mountContainer(element : ReactNodeList, containerInfo : C, parentComponent : ?ReactComponent<any, any, any>, callback: ?Function) : OpaqueNode {
-      const context = getContextForSubtree(parentComponent);
-      const root = createFiberRoot(containerInfo, context);
+    createContainer(containerInfo : C) : OpaqueNode {
+      const root = createFiberRoot(containerInfo);
       const current = root.current;
 
-      scheduleTopLevelUpdate(current, element, callback);
-
-      if (__DEV__ && ReactFiberInstrumentation.debugTool) {
-        ReactFiberInstrumentation.debugTool.onMountContainer(root);
-      }
-
       // It may seem strange that we don't return the root here, but that will
       // allow us to have containers that are in the middle of the tree instead
       // of being roots.
@@ -141,19 +134,26 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
       const root : FiberRoot = (container.stateNode : any);
       const current = root.current;
 
-      root.pendingContext = getContextForSubtree(parentComponent);
-
-      scheduleTopLevelUpdate(current, element, callback);
-
       if (__DEV__) {
         if (ReactFiberInstrumentation.debugTool) {
-          if (element === null) {
+          if (current.alternate === null) {
+            ReactFiberInstrumentation.debugTool.onMountContainer(root);
+          } else if (element === null) {
             ReactFiberInstrumentation.debugTool.onUnmountContainer(root);
           } else {
             ReactFiberInstrumentation.debugTool.onUpdateContainer(root);
           }
         }
       }
+
+      const context = getContextForSubtree(parentComponent);
+      if (root.context === null) {
+        root.context = context;
+      } else {
+        root.pendingContext = context;
+      }
+
+      scheduleTopLevelUpdate(current, element, context, callback);
     },
 
     performWithPriority,

src/renderers/shared/fiber/ReactFiberRoot.js

@@ -26,11 +26,11 @@ export type FiberRoot = {
   // The work schedule is a linked list.
   nextScheduledRoot: ?FiberRoot,
   // Top context object, used by renderSubtreeIntoContainer
-  context: Object,
+  context: ?Object,
   pendingContext: ?Object,
 };
 
-exports.createFiberRoot = function(containerInfo : any, context : Object) : FiberRoot {
+exports.createFiberRoot = function(containerInfo : any) : FiberRoot {
   // Cyclic construction. This cheats the type system right now because
   // stateNode is any.
   const uninitializedFiber = createHostRootFiber();
@@ -40,7 +40,7 @@ exports.createFiberRoot = function(containerInfo : any, context : Object) : Fibe
     isScheduled: false,
     nextScheduledRoot: null,
     callbackList: null,
-    context: context,
+    context: null,
     pendingContext: null,
   };
   uninitializedFiber.stateNode = root;

src/renderers/shared/shared/__tests__/ReactUpdates-test.js

@@ -1105,4 +1105,41 @@ describe('ReactUpdates', () => {
     });
     expect(container.textContent).toBe('b');
   });
+
+  it('handles reentrant mounting in synchronous mode', () => {
+    var mounts = 0;
+    class Editor extends React.Component {
+      render() {
+        return <div>{this.props.text}</div>;
+      }
+      componentDidMount() {
+        mounts++;
+        // This should be called only once but we guard just in case.
+        if (!this.props.rendered) {
+          this.props.onChange({rendered: true});
+        }
+      }
+    }
+
+    var container = document.createElement('div');
+    function render() {
+      ReactDOM.render(
+        <Editor
+          onChange={(newProps) => {
+            props = {...props, ...newProps};
+            render();
+          }}
+          {...props}
+        />,
+        container
+      );
+    }
+
+    var props = {text: 'hello', rendered: false};
+    render();
+    props = {...props, text: 'goodbye'};
+    render();
+    expect(container.textContent).toBe('goodbye');
+    expect(mounts).toBe(1);
+  });
 });