Skip to content

Commit bcca5a6

Browse files
author
Brian Vaughn
authored
Always skip unmounted/unmounting error boundaries (#19627)
The behavior of error boundaries for passive effects that throw during cleanup was recently changed so that React ignores boundaries which are also unmounting in favor of still-mounted boundaries. This commit implements that same behavior for layout effects (useLayoutEffect, componentWillUnmount, and ref-detachment). The new, skip-unmounting-boundaries behavior is behind a feature flag (`skipUnmountedBoundaries`).
1 parent 1a41a19 commit bcca5a6

17 files changed

+480
-73
lines changed

packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js

+171
Original file line numberDiff line numberDiff line change
@@ -2473,4 +2473,175 @@ describe('ReactErrorBoundaries', () => {
24732473
'Caught an error: gotta catch em all.',
24742474
);
24752475
});
2476+
2477+
// @gate skipUnmountedBoundaries
2478+
it('catches errors thrown in componentWillUnmount', () => {
2479+
class LocalErrorBoundary extends React.Component {
2480+
state = {error: null};
2481+
static getDerivedStateFromError(error) {
2482+
Scheduler.unstable_yieldValue(
2483+
`ErrorBoundary static getDerivedStateFromError`,
2484+
);
2485+
return {error};
2486+
}
2487+
render() {
2488+
const {children, id, fallbackID} = this.props;
2489+
const {error} = this.state;
2490+
if (error) {
2491+
Scheduler.unstable_yieldValue(`${id} render error`);
2492+
return <Component id={fallbackID} />;
2493+
}
2494+
Scheduler.unstable_yieldValue(`${id} render success`);
2495+
return children || null;
2496+
}
2497+
}
2498+
2499+
class Component extends React.Component {
2500+
render() {
2501+
const {id} = this.props;
2502+
Scheduler.unstable_yieldValue('Component render ' + id);
2503+
return id;
2504+
}
2505+
}
2506+
2507+
class LocalBrokenComponentWillUnmount extends React.Component {
2508+
componentWillUnmount() {
2509+
Scheduler.unstable_yieldValue(
2510+
'BrokenComponentWillUnmount componentWillUnmount',
2511+
);
2512+
throw Error('Expected');
2513+
}
2514+
2515+
render() {
2516+
Scheduler.unstable_yieldValue('BrokenComponentWillUnmount render');
2517+
return 'broken';
2518+
}
2519+
}
2520+
2521+
const container = document.createElement('div');
2522+
2523+
ReactDOM.render(
2524+
<LocalErrorBoundary id="OuterBoundary" fallbackID="OuterFallback">
2525+
<Component id="sibling" />
2526+
<LocalErrorBoundary id="InnerBoundary" fallbackID="InnerFallback">
2527+
<LocalBrokenComponentWillUnmount />
2528+
</LocalErrorBoundary>
2529+
</LocalErrorBoundary>,
2530+
container,
2531+
);
2532+
2533+
expect(container.firstChild.textContent).toBe('sibling');
2534+
expect(container.lastChild.textContent).toBe('broken');
2535+
expect(Scheduler).toHaveYielded([
2536+
'OuterBoundary render success',
2537+
'Component render sibling',
2538+
'InnerBoundary render success',
2539+
'BrokenComponentWillUnmount render',
2540+
]);
2541+
2542+
ReactDOM.render(
2543+
<LocalErrorBoundary id="OuterBoundary" fallbackID="OuterFallback">
2544+
<Component id="sibling" />
2545+
</LocalErrorBoundary>,
2546+
container,
2547+
);
2548+
2549+
// React should skip over the unmounting boundary and find the nearest still-mounted boundary.
2550+
expect(container.firstChild.textContent).toBe('OuterFallback');
2551+
expect(container.lastChild.textContent).toBe('OuterFallback');
2552+
expect(Scheduler).toHaveYielded([
2553+
'OuterBoundary render success',
2554+
'Component render sibling',
2555+
'BrokenComponentWillUnmount componentWillUnmount',
2556+
'ErrorBoundary static getDerivedStateFromError',
2557+
'OuterBoundary render error',
2558+
'Component render OuterFallback',
2559+
]);
2560+
});
2561+
2562+
// @gate skipUnmountedBoundaries
2563+
it('catches errors thrown while detaching refs', () => {
2564+
class LocalErrorBoundary extends React.Component {
2565+
state = {error: null};
2566+
static getDerivedStateFromError(error) {
2567+
Scheduler.unstable_yieldValue(
2568+
`ErrorBoundary static getDerivedStateFromError`,
2569+
);
2570+
return {error};
2571+
}
2572+
render() {
2573+
const {children, id, fallbackID} = this.props;
2574+
const {error} = this.state;
2575+
if (error) {
2576+
Scheduler.unstable_yieldValue(`${id} render error`);
2577+
return <Component id={fallbackID} />;
2578+
}
2579+
Scheduler.unstable_yieldValue(`${id} render success`);
2580+
return children || null;
2581+
}
2582+
}
2583+
2584+
class Component extends React.Component {
2585+
render() {
2586+
const {id} = this.props;
2587+
Scheduler.unstable_yieldValue('Component render ' + id);
2588+
return id;
2589+
}
2590+
}
2591+
2592+
class LocalBrokenCallbackRef extends React.Component {
2593+
_ref = ref => {
2594+
Scheduler.unstable_yieldValue('LocalBrokenCallbackRef ref ' + !!ref);
2595+
if (ref === null) {
2596+
throw Error('Expected');
2597+
}
2598+
};
2599+
2600+
render() {
2601+
Scheduler.unstable_yieldValue('LocalBrokenCallbackRef render');
2602+
return <div ref={this._ref}>ref</div>;
2603+
}
2604+
}
2605+
2606+
const container = document.createElement('div');
2607+
2608+
ReactDOM.render(
2609+
<LocalErrorBoundary id="OuterBoundary" fallbackID="OuterFallback">
2610+
<Component id="sibling" />
2611+
<LocalErrorBoundary id="InnerBoundary" fallbackID="InnerFallback">
2612+
<LocalBrokenCallbackRef />
2613+
</LocalErrorBoundary>
2614+
</LocalErrorBoundary>,
2615+
container,
2616+
);
2617+
2618+
expect(container.firstChild.textContent).toBe('sibling');
2619+
expect(container.lastChild.textContent).toBe('ref');
2620+
expect(Scheduler).toHaveYielded([
2621+
'OuterBoundary render success',
2622+
'Component render sibling',
2623+
'InnerBoundary render success',
2624+
'LocalBrokenCallbackRef render',
2625+
'LocalBrokenCallbackRef ref true',
2626+
]);
2627+
2628+
ReactDOM.render(
2629+
<LocalErrorBoundary id="OuterBoundary" fallbackID="OuterFallback">
2630+
<Component id="sibling" />
2631+
</LocalErrorBoundary>,
2632+
container,
2633+
);
2634+
2635+
// React should skip over the unmounting boundary and find the nearest still-mounted boundary.
2636+
expect(container.firstChild.textContent).toBe('OuterFallback');
2637+
expect(container.lastChild.textContent).toBe('OuterFallback');
2638+
expect(Scheduler).toHaveYielded([
2639+
'OuterBoundary render success',
2640+
'Component render sibling',
2641+
'LocalBrokenCallbackRef ref false',
2642+
'ErrorBoundary static getDerivedStateFromError',
2643+
'OuterBoundary render error',
2644+
'Component render OuterFallback',
2645+
]);
2646+
});
24762647
});

0 commit comments

Comments
 (0)