Batch updates within top-level unmount

Analogous change to facebook#2935.
sophiebits · Apr 30, 2015 · 36f3a2a · 36f3a2a
1 parent a092b47
commit 36f3a2a
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 23 deletions.
diff --git a/src/browser/ui/ReactMount.js b/src/browser/ui/ReactMount.js
@@ -306,6 +306,28 @@ function batchedMountComponentIntoNode(
   ReactUpdates.ReactReconcileTransaction.release(transaction);
 }
 
+/**
+ * Unmounts a component and removes it from the DOM.
+ *
+ * @param {ReactComponent} instance React component instance.
+ * @param {DOMElement} container DOM element to unmount from.
+ * @final
+ * @internal
+ * @see {ReactMount.unmountComponentAtNode}
+ */
+function unmountComponentFromNode(instance, container) {
+  ReactReconciler.unmountComponent(instance);
+
+  if (container.nodeType === DOC_NODE_TYPE) {
+    container = container.documentElement;
+  }
+
+  // http://jsperf.com/emptying-a-node
+  while (container.lastChild) {
+    container.removeChild(container.lastChild);
+  }
+}
+
 /**
  * Mounting is the process of initializing a React component by creating its
  * representative DOM elements and inserting them into a supplied `container`.
@@ -668,7 +690,11 @@ var ReactMount = {
     if (!component) {
       return false;
     }
-    ReactMount.unmountComponentFromNode(component, container);
+    ReactUpdates.batchedUpdates(
+      unmountComponentFromNode,
+      component,
+      container
+    );
     delete instancesByReactRootID[reactRootID];
     delete containersByReactRootID[reactRootID];
     if (__DEV__) {
@@ -677,28 +703,6 @@ var ReactMount = {
     return true;
   },
 
-  /**
-   * Unmounts a component and removes it from the DOM.
-   *
-   * @param {ReactComponent} instance React component instance.
-   * @param {DOMElement} container DOM element to unmount from.
-   * @final
-   * @internal
-   * @see {ReactMount.unmountComponentAtNode}
-   */
-  unmountComponentFromNode: function(instance, container) {
-    ReactReconciler.unmountComponent(instance);
-
-    if (container.nodeType === DOC_NODE_TYPE) {
-      container = container.documentElement;
-    }
-
-    // http://jsperf.com/emptying-a-node
-    while (container.lastChild) {
-      container.removeChild(container.lastChild);
-    }
-  },
-
   /**
    * Finds the container DOM element that contains React component to which the
    * supplied DOM `id` belongs.

diff --git a/src/core/__tests__/ReactCompositeComponentState-test.js b/src/core/__tests__/ReactCompositeComponentState-test.js
@@ -218,4 +218,32 @@ describe('ReactCompositeComponent-state', function() {
       ['componentWillUnmount', 'blue']
     ]);
   });
+
+  it('should batch unmounts', function() {
+    var outer;
+    var Inner = React.createClass({
+      render: function() {
+        return <div />;
+      },
+      componentWillUnmount: function() {
+        // This should get silently ignored (maybe with a warning), but it
+        // shouldn't break React.
+        outer.setState({showInner: false});
+      }
+    });
+    var Outer = React.createClass({
+      getInitialState: function() {
+        return {showInner: true};
+      },
+      render: function() {
+        return <div>{this.state.showInner && <Inner />}</div>;
+      }
+    });
+
+    var container = document.createElement('div');
+    outer = React.render(<Outer />, container);
+    expect(() => {
+      React.unmountComponentAtNode(container);
+    }).not.toThrow();
+  });
 });