diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index df1a13cfd4fb4..be8545fcace05 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -761,7 +761,11 @@ module.exports = function( // 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 @@ -780,19 +784,17 @@ module.exports = function( 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.', - ); } } } @@ -1129,7 +1131,7 @@ module.exports = function( willRetry = true; } } else if (node.tag === HostRoot) { - // Treat the root like a no-op error boundary. + // Treat the root like a no-op error boundary boundary = node; } @@ -1349,6 +1351,14 @@ module.exports = function( } function scheduleUpdate(fiber: Fiber, priorityLevel: PriorityLevel) { + return scheduleUpdateImpl(fiber, priorityLevel, false); + } + + function scheduleUpdateImpl( + fiber: Fiber, + priorityLevel: PriorityLevel, + isErrorRecovery: boolean, + ) { if (__DEV__) { recordScheduleUpdate(); } @@ -1372,7 +1382,7 @@ module.exports = function( } if (__DEV__) { - if (fiber.tag === ClassComponent) { + if (!isErrorRecovery && fiber.tag === ClassComponent) { const instance = fiber.stateNode; warnAboutInvalidUpdates(instance); } @@ -1438,7 +1448,7 @@ module.exports = function( } } else { if (__DEV__) { - if (fiber.tag === ClassComponent) { + if (!isErrorRecovery && fiber.tag === ClassComponent) { warnAboutUpdateOnUnmounted(fiber.stateNode); } } @@ -1478,7 +1488,7 @@ module.exports = function( } function scheduleErrorRecovery(fiber: Fiber) { - scheduleUpdate(fiber, TaskPriority); + scheduleUpdateImpl(fiber, TaskPriority, true); } function performWithPriority(priorityLevel: PriorityLevel, fn: Function) { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js index 83b1ad07b168e..95e862867033e 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js @@ -177,11 +177,7 @@ describe('ReactIncrementalErrorHandling', () => { } ReactNoop.flushSync(() => { - ReactNoop.render( - - Before the storm. - , - ); + ReactNoop.render(Before the storm.); ReactNoop.render( @@ -327,9 +323,7 @@ describe('ReactIncrementalErrorHandling', () => { expect(() => { ReactNoop.flushSync(() => { ReactNoop.render( - - Before the storm. - , + Before the storm., ); ReactNoop.render( @@ -478,6 +472,82 @@ describe('ReactIncrementalErrorHandling', () => { expect(ops).toEqual(['Foo']); }); + it('should not attempt to recover an unmounting error boundary', () => { + class Parent extends React.Component { + componentWillUnmount() { + ReactNoop.yield('Parent componentWillUnmount'); + } + render() { + return ; + } + } + + class Boundary extends React.Component { + componentDidCatch(e) { + ReactNoop.yield(`Caught error: ${e.message}`); + } + render() { + return ; + } + } + + class ThrowsOnUnmount extends React.Component { + componentWillUnmount() { + ReactNoop.yield('ThrowsOnUnmount componentWillUnmount'); + throw new Error('unmount error'); + } + render() { + return null; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + ReactNoop.render(null); + expect(ReactNoop.flush()).toEqual([ + // Parent unmounts before the error is thrown. + 'Parent componentWillUnmount', + 'ThrowsOnUnmount componentWillUnmount', + ]); + ReactNoop.render(); + }); + + 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 ? : null; + } + } + + class Boundary extends React.Component { + componentDidCatch() {} + render() { + return ; + } + } + + class Child extends React.Component { + componentDidUpdate() { + parent.setState({step: 1}); + throw new Error('update error'); + } + render() { + return null; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + + ReactNoop.flushSync(() => { + ReactNoop.render(); + }); + }); + it('continues work on other roots despite caught errors', () => { class ErrorBoundary extends React.Component { state = {error: null}; @@ -896,7 +966,13 @@ describe('ReactIncrementalErrorHandling', () => { } try { - ReactNoop.render(
); + ReactNoop.render( +
+ + + +
, + ); ReactNoop.flushDeferredPri(); } catch (error) {} @@ -927,7 +1003,13 @@ describe('ReactIncrementalErrorHandling', () => { } try { - ReactNoop.render(
); + ReactNoop.render( +
+ + + +
, + ); ReactNoop.flushDeferredPri(); } catch (error) {} @@ -965,7 +1047,13 @@ describe('ReactIncrementalErrorHandling', () => { ); try { - ReactNoop.render(
); + ReactNoop.render( +
+ + + +
, + ); ReactNoop.flushDeferredPri(); } catch (error) {}