Support passthrough updates for error boundaries (#7949)

* Initial pass at the easy case of updates (updates that start at the root).

* Don't expect an extra componentWillUnmount call

It was fixed in #6613.

* Remove duplicate expectations from the test

* Fix style issues

* Make naming consistent throughout the tests

* receiveComponent() does not accept safely argument

* Assert that lifecycle and refs fire for error message

* Add more tests for mounting

* Do not call componentWillMount twice on error boundary

* Document more of existing behavior in tests

* Do not call componentWillUnmount() when aborting mounting

Previously, we would call componentWillUnmount() safely on the tree whenever we abort mounting it. However this is likely risky because the tree was never mounted in the first place.

People shouldn't hold resources in componentWillMount() so it's safe to say that we can skip componentWillUnmount() if componentDidMount() was never called.

Here, we introduce a new flag. If we abort during mounting, we will not call componentWillUnmount(). However if we abort during an update, it is safe to call componentWillUnmount() because the previous tree has been mounted by now.

* Consistently display error messages in tests

* Add more logging to tests and remove redundant one

* Refactor tests

* Split complicated tests into smaller ones

* Assert clean unmounting

* Add assertions about update hooks

* Add more tests to document existing behavior and remove irrelevant details

* Verify we can recover from error state

* Fix lint

* Error in boundary’s componentWillMount should propagate up

This test is currently failing.

* Move calling componentWillMount() into mountComponent()

This removes the unnecessary non-recursive skipLifecycle check.
It fixes the previously failing test that verifies that if a boundary throws in its own componentWillMount(), the error will propagate.

* Remove extra whitespace

src/renderers/dom/client/ReactMount.js

@@ -170,11 +170,15 @@ function batchedMountComponentIntoNode(
  * @internal
  * @see {ReactMount.unmountComponentAtNode}
  */
-function unmountComponentFromNode(instance, container, safely) {
+function unmountComponentFromNode(instance, container) {
   if (__DEV__) {
     ReactInstrumentation.debugTool.onBeginFlush();
   }
-  ReactReconciler.unmountComponent(instance, safely);
+  ReactReconciler.unmountComponent(
+    instance,
+    false /* safely */,
+    false /* skipLifecycle */
+  );
   if (__DEV__) {
     ReactInstrumentation.debugTool.onEndFlush();
   }
@@ -618,8 +622,7 @@ var ReactMount = {
     ReactUpdates.batchedUpdates(
       unmountComponentFromNode,
       prevComponent,
-      container,
-      false
+      container
     );
     return true;
   },

src/renderers/dom/shared/ReactDOMComponent.js

@@ -1155,7 +1155,7 @@ ReactDOMComponent.Mixin = {
    *
    * @internal
    */
-  unmountComponent: function(safely) {
+  unmountComponent: function(safely, skipLifecycle) {
     switch (this._tag) {
       case 'audio':
       case 'form':
@@ -1197,7 +1197,7 @@ ReactDOMComponent.Mixin = {
         break;
     }
 
-    this.unmountChildren(safely);
+    this.unmountChildren(safely, skipLifecycle);
     ReactDOMComponentTree.uncacheNode(this);
     EventPluginHub.deleteAllListeners(this);
     this._rootNodeID = 0;

src/renderers/native/ReactNativeBaseComponent.js

@@ -55,10 +55,10 @@ ReactNativeBaseComponent.Mixin = {
     return this;
   },
 
-  unmountComponent: function() {
+  unmountComponent: function(safely, skipLifecycle) {
     ReactNativeComponentTree.uncacheNode(this);
     deleteAllListeners(this);
-    this.unmountChildren();
+    this.unmountChildren(safely, skipLifecycle);
     this._rootNodeID = 0;
   },
 

src/renderers/shared/stack/reconciler/ReactChildReconciler.js

@@ -146,7 +146,11 @@ var ReactChildReconciler = {
       } else {
         if (prevChild) {
           removedNodes[name] = ReactReconciler.getHostNode(prevChild);
-          ReactReconciler.unmountComponent(prevChild, false);
+          ReactReconciler.unmountComponent(
+            prevChild,
+            false, /* safely */
+            false /* skipLifecycle */
+          );
         }
         // The child must be instantiated before it's mounted.
         var nextChildInstance = instantiateReactComponent(nextElement, true);
@@ -170,7 +174,11 @@ var ReactChildReconciler = {
           !(nextChildren && nextChildren.hasOwnProperty(name))) {
         prevChild = prevChildren[name];
         removedNodes[name] = ReactReconciler.getHostNode(prevChild);
-        ReactReconciler.unmountComponent(prevChild, false);
+        ReactReconciler.unmountComponent(
+          prevChild,
+          false, /* safely */
+          false /* skipLifecycle */
+        );
       }
     }
   },
@@ -182,11 +190,11 @@ var ReactChildReconciler = {
    * @param {?object} renderedChildren Previously initialized set of children.
    * @internal
    */
-  unmountChildren: function(renderedChildren, safely) {
+  unmountChildren: function(renderedChildren, safely, skipLifecycle) {
     for (var name in renderedChildren) {
       if (renderedChildren.hasOwnProperty(name)) {
         var renderedChild = renderedChildren[name];
-        ReactReconciler.unmountComponent(renderedChild, safely);
+        ReactReconciler.unmountComponent(renderedChild, safely, skipLifecycle);
       }
     }
   },

src/renderers/shared/stack/reconciler/ReactCompositeComponent.js

@@ -333,6 +333,23 @@ var ReactCompositeComponent = {
     this._pendingReplaceState = false;
     this._pendingForceUpdate = false;
 
+    if (inst.componentWillMount) {
+      if (__DEV__) {
+        measureLifeCyclePerf(
+          () => inst.componentWillMount(),
+          this._debugID,
+          'componentWillMount'
+        );
+      } else {
+        inst.componentWillMount();
+      }
+      // When mounting, calls to `setState` by `componentWillMount` will set
+      // `this._pendingStateQueue` without triggering a re-render.
+      if (this._pendingStateQueue) {
+        inst.state = this._processPendingState(inst.props, inst.context);
+      }
+    }
+
     var markup;
     if (inst.unstable_handleError) {
       markup = this.performInitialMountWithErrorHandling(
@@ -343,7 +360,13 @@ var ReactCompositeComponent = {
         context
       );
     } else {
-      markup = this.performInitialMount(renderedElement, hostParent, hostContainerInfo, transaction, context);
+      markup = this.performInitialMount(
+        renderedElement,
+        hostParent,
+        hostContainerInfo,
+        transaction,
+        context
+      );
     }
 
     if (inst.componentDidMount) {
@@ -434,7 +457,13 @@ var ReactCompositeComponent = {
     var markup;
     var checkpoint = transaction.checkpoint();
     try {
-      markup = this.performInitialMount(renderedElement, hostParent, hostContainerInfo, transaction, context);
+      markup = this.performInitialMount(
+        renderedElement,
+        hostParent,
+        hostContainerInfo,
+        transaction,
+        context
+      );
     } catch (e) {
       // Roll back to checkpoint, handle error (which may add items to the transaction), and take a new checkpoint
       transaction.rollback(checkpoint);
@@ -443,42 +472,33 @@ var ReactCompositeComponent = {
         this._instance.state = this._processPendingState(this._instance.props, this._instance.context);
       }
       checkpoint = transaction.checkpoint();
-
-      this._renderedComponent.unmountComponent(true);
+      this._renderedComponent.unmountComponent(
+        true, /* safely */
+        // Don't call componentWillUnmount() because they never fully mounted:
+        true /* skipLifecyle */
+      );
       transaction.rollback(checkpoint);
 
       // Try again - we've informed the component about the error, so they can render an error message this time.
       // If this throws again, the error will bubble up (and can be caught by a higher error boundary).
-      markup = this.performInitialMount(renderedElement, hostParent, hostContainerInfo, transaction, context);
+      markup = this.performInitialMount(
+        renderedElement,
+        hostParent,
+        hostContainerInfo,
+        transaction,
+        context
+      );
     }
     return markup;
   },
 
-  performInitialMount: function(renderedElement, hostParent, hostContainerInfo, transaction, context) {
-    var inst = this._instance;
-
-    var debugID = 0;
-    if (__DEV__) {
-      debugID = this._debugID;
-    }
-
-    if (inst.componentWillMount) {
-      if (__DEV__) {
-        measureLifeCyclePerf(
-          () => inst.componentWillMount(),
-          debugID,
-          'componentWillMount'
-        );
-      } else {
-        inst.componentWillMount();
-      }
-      // When mounting, calls to `setState` by `componentWillMount` will set
-      // `this._pendingStateQueue` without triggering a re-render.
-      if (this._pendingStateQueue) {
-        inst.state = this._processPendingState(inst.props, inst.context);
-      }
-    }
-
+  performInitialMount: function(
+    renderedElement,
+    hostParent,
+    hostContainerInfo,
+    transaction,
+    context
+  ) {
     // If not a stateless component, we now render
     if (renderedElement === undefined) {
       renderedElement = this._renderValidatedComponent();
@@ -492,6 +512,11 @@ var ReactCompositeComponent = {
     );
     this._renderedComponent = child;
 
+    var debugID = 0;
+    if (__DEV__) {
+      debugID = this._debugID;
+    }
+
     var markup = ReactReconciler.mountComponent(
       child,
       transaction,
@@ -521,7 +546,7 @@ var ReactCompositeComponent = {
    * @final
    * @internal
    */
-  unmountComponent: function(safely) {
+  unmountComponent: function(safely, skipLifecycle) {
     if (!this._renderedComponent) {
       return;
     }
@@ -532,8 +557,10 @@ var ReactCompositeComponent = {
       inst._calledComponentWillUnmount = true;
 
       if (safely) {
-        var name = this.getName() + '.componentWillUnmount()';
-        ReactErrorUtils.invokeGuardedCallback(name, inst.componentWillUnmount.bind(inst));
+        if (!skipLifecycle) {
+          var name = this.getName() + '.componentWillUnmount()';
+          ReactErrorUtils.invokeGuardedCallback(name, inst.componentWillUnmount.bind(inst));
+        }
       } else {
         if (__DEV__) {
           measureLifeCyclePerf(
@@ -548,7 +575,11 @@ var ReactCompositeComponent = {
     }
 
     if (this._renderedComponent) {
-      ReactReconciler.unmountComponent(this._renderedComponent, safely);
+      ReactReconciler.unmountComponent(
+        this._renderedComponent,
+        safely,
+        skipLifecycle
+      );
       this._renderedNodeType = null;
       this._renderedComponent = null;
       this._instance = null;
@@ -941,7 +972,11 @@ var ReactCompositeComponent = {
     inst.state = nextState;
     inst.context = nextContext;
 
-    this._updateRenderedComponent(transaction, unmaskedContext);
+    if (inst.unstable_handleError) {
+      this._updateRenderedComponentWithErrorHandling(transaction, unmaskedContext);
+    } else {
+      this._updateRenderedComponent(transaction, unmaskedContext);
+    }
 
     if (hasComponentDidUpdate) {
       if (__DEV__) {
@@ -961,6 +996,40 @@ var ReactCompositeComponent = {
     }
   },
 
+  /**
+   * Call the component's `render` method and update the DOM accordingly.
+   *
+   * @param {ReactReconcileTransaction} transaction
+   * @internal
+   */
+  _updateRenderedComponentWithErrorHandling: function(transaction, context) {
+    var checkpoint = transaction.checkpoint();
+    try {
+      this._updateRenderedComponent(transaction, context);
+    } catch (e) {
+      // Roll back to checkpoint, handle error (which may add items to the transaction),
+      // and take a new checkpoint
+      transaction.rollback(checkpoint);
+      this._instance.unstable_handleError(e);
+      if (this._pendingStateQueue) {
+        this._instance.state = this._processPendingState(this._instance.props, this._instance.context);
+      }
+      checkpoint = transaction.checkpoint();
+
+      // Gracefully update to a clean state
+      this._updateRenderedComponentWithNextElement(
+        transaction,
+        context,
+        null,
+        true /* safely */
+      );
+
+      // Try again - we've informed the component about the error, so they can render an error message this time.
+      // If this throws again, the error will bubble up (and can be caught by a higher error boundary).
+      this._updateRenderedComponent(transaction, context);
+    }
+  },
+
   /**
    * Call the component's `render` method and update the DOM accordingly.
    *
@@ -968,9 +1037,29 @@ var ReactCompositeComponent = {
    * @internal
    */
   _updateRenderedComponent: function(transaction, context) {
+    var nextRenderedElement = this._renderValidatedComponent();
+    this._updateRenderedComponentWithNextElement(
+      transaction,
+      context,
+      nextRenderedElement,
+      false /* safely */
+    );
+  },
+
+  /**
+   * Call the component's `render` method and update the DOM accordingly.
+   *
+   * @param {ReactReconcileTransaction} transaction
+   * @internal
+   */
+  _updateRenderedComponentWithNextElement: function(
+    transaction,
+    context,
+    nextRenderedElement,
+    safely
+  ) {
     var prevComponentInstance = this._renderedComponent;
     var prevRenderedElement = prevComponentInstance._currentElement;
-    var nextRenderedElement = this._renderValidatedComponent();
 
     var debugID = 0;
     if (__DEV__) {
@@ -986,7 +1075,11 @@ var ReactCompositeComponent = {
       );
     } else {
       var oldHostNode = ReactReconciler.getHostNode(prevComponentInstance);
-      ReactReconciler.unmountComponent(prevComponentInstance, false);
+      ReactReconciler.unmountComponent(
+        prevComponentInstance,
+        safely,
+        false /* skipLifecycle */
+      );
 
       var nodeType = ReactNodeTypes.getType(nextRenderedElement);
       this._renderedNodeType = nodeType;

src/renderers/shared/stack/reconciler/ReactMultiChild.js

@@ -289,7 +289,11 @@ var ReactMultiChild = {
   updateTextContent: function(nextContent) {
     var prevChildren = this._renderedChildren;
     // Remove any rendered children.
-    ReactChildReconciler.unmountChildren(prevChildren, false);
+    ReactChildReconciler.unmountChildren(
+      prevChildren,
+      false, /* safely */
+      false /* skipLifecycle */
+    );
     for (var name in prevChildren) {
       if (prevChildren.hasOwnProperty(name)) {
         invariant(false, 'updateTextContent called on non-empty component.');
@@ -309,7 +313,11 @@ var ReactMultiChild = {
   updateMarkup: function(nextMarkup) {
     var prevChildren = this._renderedChildren;
     // Remove any rendered children.
-    ReactChildReconciler.unmountChildren(prevChildren, false);
+    ReactChildReconciler.unmountChildren(
+      prevChildren,
+      false, /* safely */
+      false /* skipLifecycle */
+    );
     for (var name in prevChildren) {
       if (prevChildren.hasOwnProperty(name)) {
         invariant(false, 'updateTextContent called on non-empty component.');
@@ -423,9 +431,13 @@ var ReactMultiChild = {
    *
    * @internal
    */
-  unmountChildren: function(safely) {
+  unmountChildren: function(safely, skipLifecycle) {
     var renderedChildren = this._renderedChildren;
-    ReactChildReconciler.unmountChildren(renderedChildren, safely);
+    ReactChildReconciler.unmountChildren(
+      renderedChildren,
+      safely,
+      skipLifecycle
+    );
     this._renderedChildren = null;
   },
 

src/renderers/shared/stack/reconciler/ReactReconciler.js

@@ -93,7 +93,7 @@ var ReactReconciler = {
    * @final
    * @internal
    */
-  unmountComponent: function(internalInstance, safely) {
+  unmountComponent: function(internalInstance, safely, skipLifecycle) {
     if (__DEV__) {
       if (internalInstance._debugID !== 0) {
         ReactInstrumentation.debugTool.onBeforeUnmountComponent(
@@ -102,7 +102,7 @@ var ReactReconciler = {
       }
     }
     ReactRef.detachRefs(internalInstance, internalInstance._currentElement);
-    internalInstance.unmountComponent(safely);
+    internalInstance.unmountComponent(safely, skipLifecycle);
     if (__DEV__) {
       if (internalInstance._debugID !== 0) {
         ReactInstrumentation.debugTool.onUnmountComponent(

src/renderers/shared/stack/reconciler/ReactSimpleEmptyComponent.js

@@ -40,8 +40,12 @@ Object.assign(ReactSimpleEmptyComponent.prototype, {
   getHostNode: function() {
     return ReactReconciler.getHostNode(this._renderedComponent);
   },
-  unmountComponent: function() {
-    ReactReconciler.unmountComponent(this._renderedComponent);
+  unmountComponent: function(safely, skipLifecycle) {
+    ReactReconciler.unmountComponent(
+      this._renderedComponent,
+      safely,
+      skipLifecycle
+    );
     this._renderedComponent = null;
   },
 });

src/renderers/testing/ReactTestMount.js

@@ -128,7 +128,8 @@ ReactTestInstance.prototype.unmount = function(nextElement) {
     transaction.perform(function() {
       ReactReconciler.unmountComponent(
         component,
-        false
+        false, /* safely */
+        false /* skipLifecycle */
       );
     });
     ReactUpdates.ReactReconcileTransaction.release(transaction);

src/test/ReactShallowRenderer.js

@@ -117,7 +117,11 @@ class ReactShallowRenderer {
   }
   unmount() {
     if (this._instance) {
-      ReactReconciler.unmountComponent(this._instance, false);
+      ReactReconciler.unmountComponent(
+        this._instance,
+        false, /* safely */
+        false /* skipLifecycle */
+      );
     }
   }
   _render(element, transaction, context) {