Remove unmountContainer

Instead, renderers should pass null to updateContainer.

scripts/fiber/tests-failing.txt

@@ -90,7 +90,6 @@ src/renderers/shared/shared/__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
-* unmounts and remounts a root in the same batch
 
 src/renderers/shared/shared/__tests__/refs-test.js
 * Should increase refs with an increase in divs

scripts/fiber/tests-passing.txt

@@ -1559,6 +1559,7 @@ src/renderers/shared/shared/__tests__/ReactUpdates-test.js
 * 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
+* unmounts and remounts a root in the same batch
 
 src/renderers/shared/shared/__tests__/refs-destruction-test.js
 * should remove refs when destroying the parent

src/renderers/art/ReactARTFiber.js

@@ -357,7 +357,11 @@ class Surface extends Component {
   }
 
   componentWillUnmount() {
-    ARTRenderer.unmountContainer(this._mountNode);
+    ARTRenderer.updateContainer(
+      null,
+      this._mountNode,
+      this,
+    );
   }
 
   render() {

src/renderers/dom/fiber/ReactDOMFiber.js

@@ -259,7 +259,7 @@ function warnAboutUnstableUse() {
   warned = true;
 }
 
-function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any, any>, element : ReactElement<any>, containerNode : DOMContainerElement | Document, callback: ?Function) {
+function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any, any>, children : ReactNodeList, containerNode : DOMContainerElement | Document, callback: ?Function) {
   let container : DOMContainerElement =
     containerNode.nodeType === DOCUMENT_NODE ? (containerNode : any).documentElement : (containerNode : any);
   let root;
@@ -268,9 +268,9 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any,
     while (container.lastChild) {
       container.removeChild(container.lastChild);
     }
-    root = container._reactRootContainer = DOMRenderer.mountContainer(element, container, parentComponent, callback);
+    root = container._reactRootContainer = DOMRenderer.mountContainer(children, container, parentComponent, callback);
   } else {
-    DOMRenderer.updateContainer(element, root = container._reactRootContainer, parentComponent, callback);
+    DOMRenderer.updateContainer(children, root = container._reactRootContainer, parentComponent, callback);
   }
   return DOMRenderer.getPublicRootInstance(root);
 }
@@ -292,13 +292,9 @@ var ReactDOM = {
 
   unmountComponentAtNode(container : DOMContainerElement) {
     warnAboutUnstableUse();
-    const root = container._reactRootContainer;
-    if (root) {
-      // TODO: Is it safe to reset this now or should I wait since this
-      // unmount could be deferred?
+    return renderSubtreeIntoContainer(null, null, container, () => {
       container._reactRootContainer = null;
-      DOMRenderer.unmountContainer(root);
-    }
+    });
   },
 
   findDOMNode: findDOMNode,

src/renderers/native/ReactNativeFiber.js

@@ -329,7 +329,7 @@ const NativeRenderer = ReactFiberReconciler({
     // But creates an additional child Fiber for raw text children.
     // No additional native views are created though.
     // It's not clear to me which is better so I'm deferring for now.
-    // More context @ github.com/facebook/react/pull/8560#discussion_r92111303 
+    // More context @ github.com/facebook/react/pull/8560#discussion_r92111303
     return false;
   },
 
@@ -375,8 +375,9 @@ const ReactNative = {
     const root = roots.get(containerTag);
     if (root) {
       // TODO: Is it safe to reset this now or should I wait since this unmount could be deferred?
-      roots.delete(containerTag);
-      NativeRenderer.unmountContainer(root);
+      NativeRenderer.updateContainer(null, root, null, () => {
+        roots.delete(containerTag);
+      });
     }
   },
 

src/renderers/noop/ReactNoop.js

@@ -198,10 +198,11 @@ var ReactNoop = {
 
   unmountRootWithID(rootID : string) {
     const root = roots.get(rootID);
-    roots.delete(rootID);
-    rootContainers.delete(rootID);
     if (root) {
-      NoopRenderer.unmountContainer(root);
+      NoopRenderer.updateContainer(null, root, null, () => {
+        roots.delete(rootID);
+        rootContainers.delete(rootID);
+      });
     }
   },
 

src/renderers/shared/fiber/ReactFiberReconciler.js

@@ -15,6 +15,7 @@
 import type { Fiber } from 'ReactFiber';
 import type { FiberRoot } from 'ReactFiberRoot';
 import type { PriorityLevel } from 'ReactPriorityLevel';
+import type { ReactNodeList } from 'ReactTypes';
 
 var {
   findCurrentUnmaskedContext,
@@ -70,9 +71,8 @@ export type HostConfig<T, P, I, TI, C, CX> = {
 };
 
 export type Reconciler<C, I, TI> = {
-  mountContainer(element : ReactElement<any>, containerInfo : C, parentComponent : ?ReactComponent<any, any, any>) : OpaqueNode,
-  updateContainer(element : ReactElement<any>, container : OpaqueNode, parentComponent : ?ReactComponent<any, any, any>) : void,
-  unmountContainer(container : OpaqueNode) : void,
+  mountContainer(element : ReactNodeList, containerInfo : C, parentComponent : ?ReactComponent<any, any, any>) : OpaqueNode,
+  updateContainer(element : ReactNodeList, container : OpaqueNode, parentComponent : ?ReactComponent<any, any, any>) : void,
   performWithPriority(priorityLevel : PriorityLevel, fn : Function) : void,
   /* eslint-disable no-undef */
   // FIXME: ESLint complains about type parameter
@@ -98,7 +98,7 @@ getContextForSubtree._injectFiber(function(fiber : Fiber) {
 module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C, CX>) : Reconciler<C, I, TI> {
 
   var {
-    scheduleSetState,
+    scheduleTopLevelSetState,
     scheduleUpdateCallback,
     performWithPriority,
     batchedUpdates,
@@ -108,12 +108,12 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
 
   return {
 
-    mountContainer(element : ReactElement<any>, containerInfo : C, parentComponent : ?ReactComponent<any, any, any>, callback: ?Function) : OpaqueNode {
+    mountContainer(element : ReactNodeList, containerInfo : C, parentComponent : ?ReactComponent<any, any, any>, callback: ?Function) : OpaqueNode {
       const context = getContextForSubtree(parentComponent);
       const root = createFiberRoot(containerInfo, context);
       const current = root.current;
 
-      scheduleSetState(current, { element });
+      scheduleTopLevelSetState(current, { element });
       if (callback) {
         scheduleUpdateCallback(current, callback);
       }
@@ -128,32 +128,27 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
       return current;
     },
 
-    updateContainer(element : ReactElement<any>, container : OpaqueNode, parentComponent : ?ReactComponent<any, any, any>, callback: ?Function) : void {
+    updateContainer(element : ReactNodeList, container : OpaqueNode, parentComponent : ?ReactComponent<any, any, any>, callback: ?Function) : void {
       // TODO: If this is a nested container, this won't be the root.
       const root : FiberRoot = (container.stateNode : any);
       const current = root.current;
 
       root.pendingContext = getContextForSubtree(parentComponent);
 
-      scheduleSetState(current, { element });
+      scheduleTopLevelSetState(current, { element });
       if (callback) {
         scheduleUpdateCallback(current, callback);
       }
 
-      if (__DEV__ && ReactFiberInstrumentation.debugTool) {
-        ReactFiberInstrumentation.debugTool.onUpdateContainer(root);
-      }
-    },
-
-    unmountContainer(container : OpaqueNode) : void {
-      // TODO: If this is a nested container, this won't be the root.
-      const root : FiberRoot = (container.stateNode : any);
-      const current = root.current;
-
-      scheduleSetState(current, { element: [] });
-
-      if (__DEV__ && ReactFiberInstrumentation.debugTool) {
-        ReactFiberInstrumentation.debugTool.onUnmountContainer(root);
+      if (__DEV__) {
+        if (ReactFiberInstrumentation.debugTool) {
+          if (element === null) {
+            ReactFiberInstrumentation.debugTool.onUpdateContainer(root);
+          } else {
+            // This is an unmount
+            ReactFiberInstrumentation.debugTool.onUnmountContainer(root);
+          }
+        }
       }
     },
 

src/renderers/shared/fiber/ReactFiberScheduler.js

@@ -59,6 +59,7 @@ var {
   addReplaceUpdate,
   addForceUpdate,
   addCallback,
+  addTopLevelUpdate,
 } = require('ReactFiberUpdateQueue');
 
 var {
@@ -1058,6 +1059,12 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
     scheduleUpdateAtPriority(fiber, priorityContext);
   }
 
+  // TODO: This indirection will be removed as part of #8585
+  function scheduleTopLevelSetState(fiber : Fiber, partialState : any) {
+    addTopLevelUpdate(fiber, partialState, priorityContext);
+    scheduleUpdateAtPriority(fiber, priorityContext);
+  }
+
   function performWithPriority(priorityLevel : PriorityLevel, fn : Function) {
     const previousPriorityContext = priorityContext;
     priorityContext = priorityLevel;
@@ -1105,7 +1112,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
   }
 
   return {
-    scheduleSetState: scheduleSetState,
+    scheduleTopLevelSetState: scheduleTopLevelSetState,
     scheduleUpdateCallback: scheduleUpdateCallback,
     performWithPriority: performWithPriority,
     batchedUpdates: batchedUpdates,

src/renderers/shared/fiber/ReactFiberUpdateQueue.js

@@ -39,6 +39,7 @@ type Update = {
   callback: Callback | null,
   isReplace: boolean,
   isForced: boolean,
+  isTopLevelUnmount: boolean,
   next: Update | null,
 };
 
@@ -141,6 +142,7 @@ function cloneUpdate(update : Update) : Update {
     callback: update.callback,
     isReplace: update.isReplace,
     isForced: update.isForced,
+    isTopLevelUnmount: update.isTopLevelUnmount,
     next: null,
   };
 }
@@ -209,7 +211,9 @@ function findInsertionPosition(queue, update) : Update | null {
 //
 // However, if incoming update is inserted into the same position of both lists,
 // we shouldn't make a copy.
-function insertUpdate(fiber : Fiber, update : Update, methodName : ?string) : void {
+//
+// If the update is cloned, it returns the cloned update.
+function insertUpdate(fiber : Fiber, update : Update, methodName : ?string) : Update | null {
   const queue1 = ensureUpdateQueue(fiber);
   const queue2 = fiber.alternate ? ensureUpdateQueue(fiber.alternate) : null;
 
@@ -244,7 +248,7 @@ function insertUpdate(fiber : Fiber, update : Update, methodName : ?string) : vo
   if (!queue2) {
     // If there's no alternate queue, there's nothing else to do but insert.
     insertUpdateIntoQueue(queue1, update, insertAfter1, insertBefore1);
-    return;
+    return null;
   }
 
   // If there is an alternate queue, find the insertion position.
@@ -260,6 +264,7 @@ function insertUpdate(fiber : Fiber, update : Update, methodName : ?string) : vo
     // insert the clone into the alternate queue.
     const update2 = cloneUpdate(update);
     insertUpdateIntoQueue(queue2, update2, insertAfter2, insertBefore2);
+    return update2;
   } else {
     // The insertion positions are the same, so when we inserted into the first
     // queue, it also inserted into the alternate. All we need to do is update
@@ -272,6 +277,8 @@ function insertUpdate(fiber : Fiber, update : Update, methodName : ?string) : vo
       queue2.last = null;
     }
   }
+
+  return null;
 }
 
 function addUpdate(
@@ -285,6 +292,7 @@ function addUpdate(
     callback: null,
     isReplace: false,
     isForced: false,
+    isTopLevelUnmount: false,
     next: null,
   };
   if (__DEV__) {
@@ -306,6 +314,7 @@ function addReplaceUpdate(
     callback: null,
     isReplace: true,
     isForced: false,
+    isTopLevelUnmount: false,
     next: null,
   };
 
@@ -324,6 +333,7 @@ function addForceUpdate(fiber : Fiber, priorityLevel : PriorityLevel) : void {
     callback: null,
     isReplace: false,
     isForced: true,
+    isTopLevelUnmount: false,
     next: null,
   };
   if (__DEV__) {
@@ -342,6 +352,7 @@ function addCallback(fiber : Fiber, callback: Callback, priorityLevel : Priority
     callback,
     isReplace: false,
     isForced: false,
+    isTopLevelUnmount: false,
     next: null,
   };
   insertUpdate(fiber, update);
@@ -353,6 +364,42 @@ function getPendingPriority(queue : UpdateQueue) : PriorityLevel {
 }
 exports.getPendingPriority = getPendingPriority;
 
+function addTopLevelUpdate(
+  fiber : Fiber,
+  partialState : PartialState<any, any>,
+  priorityLevel : PriorityLevel
+) : void {
+  const isTopLevelUnmount = partialState === null;
+
+  const update = {
+    priorityLevel,
+    partialState,
+    callback: null,
+    isReplace: false,
+    isForced: false,
+    isTopLevelUnmount,
+    next: null,
+  };
+  const update2 = insertUpdate(fiber, update);
+
+  if (isTopLevelUnmount) {
+    // Drop all updates that are lower-priority, so that the tree is not
+    // remounted. We need to do this for both queues.
+    const queue1 = fiber.updateQueue;
+    const queue2 = fiber.alternate && fiber.alternate.updateQueue;
+
+    if (queue1 && update.next) {
+      update.next = null;
+      queue1.last = update;
+    }
+    if (queue2 && update2 && update2.next) {
+      update2.next = null;
+      queue2.last = update;
+    }
+  }
+}
+exports.addTopLevelUpdate = addTopLevelUpdate;
+
 function getStateFromUpdate(update, instance, prevState, props) {
   const partialState = update.partialState;
   if (typeof partialState === 'function') {
@@ -412,12 +459,14 @@ function beginUpdateQueue(
     if (update.isForced) {
       queue.hasForceUpdate = true;
     }
-    if (update.callback) {
+    // Second condition ignores top-level unmount callbacks if they are not the
+    // last update in the queue, since a subsequent update will cause a remount.
+    if (update.callback && !(update.isTopLevelUnmount && update.next)) {
+      const callbackUpdate = cloneUpdate(update);
       if (callbackList && callbackList.last) {
-        callbackList.last.next = update;
-        callbackList.last = update;
+        callbackList.last.next = callbackUpdate;
+        callbackList.last = callbackUpdate;
       } else {
-        const callbackUpdate = cloneUpdate(update);
         callbackList = {
           first: callbackUpdate,
           last: callbackUpdate,

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

@@ -654,6 +654,7 @@ describe('ReactIncrementalErrorHandling', () => {
     ReactNoop.unmountRootWithID('d');
     ReactNoop.unmountRootWithID('e');
     ReactNoop.unmountRootWithID('f');
+    ReactNoop.flush();
     expect(ReactNoop.getChildren('a')).toEqual(null);
     expect(ReactNoop.getChildren('b')).toEqual(null);
     expect(ReactNoop.getChildren('c')).toEqual(null);