Skip to content

Commit

Permalink
Should be able to unmount an error boundary before it is handled
Browse files Browse the repository at this point in the history
Fixes the case where an error boundary captures an error, but its
parent is unmounted before we can re-render it. componentDidCatch is
never called, and we don't remove the boundary from our set of
unhandled error boundaries.

We should not assume that if capturedErrors is non-null that we still
have unhandled errors.
  • Loading branch information
acdlite committed Aug 7, 2017
1 parent 5c803ea commit 894656c
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 8 deletions.
18 changes: 10 additions & 8 deletions src/renderers/shared/fiber/ReactFiberScheduler.js
Expand Up @@ -761,7 +761,11 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
// The loop stops once the children have unmounted and error lifecycles are
// called. Then we return to the regular flow.

if (capturedErrors !== null && capturedErrors.size > 0) {
if (
capturedErrors !== null &&
capturedErrors.size > 0 &&
nextPriorityLevel === TaskPriority
) {
while (nextUnitOfWork !== null) {
if (hasCapturedError(nextUnitOfWork)) {
// Use a forked version of performUnitOfWork
Expand All @@ -780,19 +784,17 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
commitAllWork(pendingCommit);
priorityContext = nextPriorityLevel;

if (capturedErrors === null || capturedErrors.size === 0) {
if (
capturedErrors === null ||
capturedErrors.size === 0 ||
nextPriorityLevel !== TaskPriority
) {
// There are no more unhandled errors. We can exit this special
// work loop. If there's still additional work, we'll perform it
// using one of the normal work loops.
break;
}
// The commit phase produced additional errors. Continue working.
invariant(
nextPriorityLevel === TaskPriority,
'Commit phase errors should be scheduled to recover with task ' +
'priority. This error is likely caused by a bug in React. ' +
'Please file an issue.',
);
}
}
}
Expand Down
Expand Up @@ -512,6 +512,42 @@ describe('ReactIncrementalErrorHandling', () => {
ReactNoop.render(<Parent />);
});

it('can unmount an error boundary before it is handled', () => {
let parent;

class Parent extends React.Component {
state = {step: 0};
render() {
parent = this;
return this.state.step === 0 ? <Boundary /> : null;
}
}

class Boundary extends React.Component {
componentDidCatch() {}
render() {
return <Child />;
}
}

class Child extends React.Component {
componentDidUpdate() {
parent.setState({step: 1});
throw new Error('update error');
}
render() {
return null;
}
}

ReactNoop.render(<Parent />);
ReactNoop.flush();

ReactNoop.flushSync(() => {
ReactNoop.render(<Parent />);
});
});

it('continues work on other roots despite caught errors', () => {
class ErrorBoundary extends React.Component {
state = {error: null};
Expand Down

0 comments on commit 894656c

Please sign in to comment.