Skip to content

Commit e07542b

Browse files
committed
Should be able to unmount an error boundary before it is handled
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.
1 parent b7fc149 commit e07542b

File tree

2 files changed

+46
-8
lines changed

2 files changed

+46
-8
lines changed

src/renderers/shared/fiber/ReactFiberScheduler.js

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -761,7 +761,11 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
761761
// The loop stops once the children have unmounted and error lifecycles are
762762
// called. Then we return to the regular flow.
763763

764-
if (capturedErrors !== null && capturedErrors.size > 0) {
764+
if (
765+
capturedErrors !== null &&
766+
capturedErrors.size > 0 &&
767+
nextPriorityLevel === TaskPriority
768+
) {
765769
while (nextUnitOfWork !== null) {
766770
if (hasCapturedError(nextUnitOfWork)) {
767771
// Use a forked version of performUnitOfWork
@@ -780,19 +784,17 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
780784
commitAllWork(pendingCommit);
781785
priorityContext = nextPriorityLevel;
782786

783-
if (capturedErrors === null || capturedErrors.size === 0) {
787+
if (
788+
capturedErrors === null ||
789+
capturedErrors.size === 0 ||
790+
nextPriorityLevel !== TaskPriority
791+
) {
784792
// There are no more unhandled errors. We can exit this special
785793
// work loop. If there's still additional work, we'll perform it
786794
// using one of the normal work loops.
787795
break;
788796
}
789797
// The commit phase produced additional errors. Continue working.
790-
invariant(
791-
nextPriorityLevel === TaskPriority,
792-
'Commit phase errors should be scheduled to recover with task ' +
793-
'priority. This error is likely caused by a bug in React. ' +
794-
'Please file an issue.',
795-
);
796798
}
797799
}
798800
}

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,42 @@ describe('ReactIncrementalErrorHandling', () => {
512512
ReactNoop.render(<Parent />);
513513
});
514514

515+
it('can unmount an error boundary before it is handled', () => {
516+
let parent;
517+
518+
class Parent extends React.Component {
519+
state = {step: 0};
520+
render() {
521+
parent = this;
522+
return this.state.step === 0 ? <Boundary /> : null;
523+
}
524+
}
525+
526+
class Boundary extends React.Component {
527+
componentDidCatch() {}
528+
render() {
529+
return <Child />;
530+
}
531+
}
532+
533+
class Child extends React.Component {
534+
componentDidUpdate() {
535+
parent.setState({step: 1});
536+
throw new Error('update error');
537+
}
538+
render() {
539+
return null;
540+
}
541+
}
542+
543+
ReactNoop.render(<Parent />);
544+
ReactNoop.flush();
545+
546+
ReactNoop.flushSync(() => {
547+
ReactNoop.render(<Parent />);
548+
});
549+
});
550+
515551
it('continues work on other roots despite caught errors', () => {
516552
class ErrorBoundary extends React.Component {
517553
state = {error: null};

0 commit comments

Comments
 (0)