Skip to content

Commit

Permalink
Support passthrough updates for error boundaries (#7949)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@gaearon
gaearon authored Oct 15, 2016
1 parent 1b7066e commit f33f03e
Show file tree
Hide file tree
Showing 11 changed files with 1,574 additions and 248 deletions.
11 changes: 7 additions & 4 deletions src/renderers/dom/client/ReactMount.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -618,8 +622,7 @@ var ReactMount = {
ReactUpdates.batchedUpdates(
unmountComponentFromNode,
prevComponent,
container,
false
container
);
return true;
},
Expand Down
4 changes: 2 additions & 2 deletions src/renderers/dom/shared/ReactDOMComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1155,7 +1155,7 @@ ReactDOMComponent.Mixin = {
*
* @internal
*/
unmountComponent: function(safely) {
unmountComponent: function(safely, skipLifecycle) {
switch (this._tag) {
case 'audio':
case 'form':
Expand Down Expand Up @@ -1197,7 +1197,7 @@ ReactDOMComponent.Mixin = {
break;
}

this.unmountChildren(safely);
this.unmountChildren(safely, skipLifecycle);
ReactDOMComponentTree.uncacheNode(this);
EventPluginHub.deleteAllListeners(this);
this._rootNodeID = 0;
Expand Down
4 changes: 2 additions & 2 deletions src/renderers/native/ReactNativeBaseComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},

Expand Down
16 changes: 12 additions & 4 deletions src/renderers/shared/stack/reconciler/ReactChildReconciler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 */
);
}
}
},
Expand All @@ -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);
}
}
},
Expand Down
167 changes: 130 additions & 37 deletions src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand All @@ -492,6 +512,11 @@ var ReactCompositeComponent = {
);
this._renderedComponent = child;

var debugID = 0;
if (__DEV__) {
debugID = this._debugID;
}

var markup = ReactReconciler.mountComponent(
child,
transaction,
Expand Down Expand Up @@ -521,7 +546,7 @@ var ReactCompositeComponent = {
* @final
* @internal
*/
unmountComponent: function(safely) {
unmountComponent: function(safely, skipLifecycle) {
if (!this._renderedComponent) {
return;
}
Expand All @@ -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(
Expand All @@ -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;
Expand Down Expand Up @@ -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__) {
Expand All @@ -961,16 +996,70 @@ 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.
*
* @param {ReactReconcileTransaction} transaction
* @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__) {
Expand All @@ -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;
Expand Down
Loading

0 comments on commit f33f03e

Please sign in to comment.