Skip to content

Commit 3df17ba

Browse files
author
Brian Vaughn
committed
Warn if boundary with only componentDidCatch swallows error
1 parent f6fb111 commit 3df17ba

File tree

2 files changed

+84
-1
lines changed

2 files changed

+84
-1
lines changed

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

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2084,4 +2084,70 @@ describe('ReactErrorBoundaries', () => {
20842084
// Error should be the first thrown
20852085
expect(caughtError.message).toBe('child sad');
20862086
});
2087+
2088+
it('should warn if an error boundary with only componentDidCatch does not update state', () => {
2089+
class InvalidErrorBoundary extends React.Component {
2090+
componentDidCatch(error, info) {
2091+
// This component does not define getDerivedStateFromError().
2092+
// It also doesn't call setState().
2093+
// So it would swallow errors (which is probably unintentional).
2094+
}
2095+
render() {
2096+
return this.props.children;
2097+
}
2098+
}
2099+
2100+
const Throws = () => {
2101+
throw new Error('expected');
2102+
};
2103+
2104+
const container = document.createElement('div');
2105+
expect(() => {
2106+
ReactDOM.render(
2107+
<InvalidErrorBoundary>
2108+
<Throws />
2109+
</InvalidErrorBoundary>,
2110+
container,
2111+
);
2112+
}).toWarnDev(
2113+
'InvalidErrorBoundary: Error boundaries should implement getDerivedStateFromError(). ' +
2114+
'In that method, return a state update to display an error message or fallback UI, ' +
2115+
'or rethrow the error to let parent components handle it',
2116+
{withoutStack: true},
2117+
);
2118+
expect(container.textContent).toBe('');
2119+
});
2120+
2121+
it('should call both componentDidCatch and getDerivedStateFromError if both exist on a component', () => {
2122+
let componentDidCatchError, getDerivedStateFromErrorError;
2123+
class ErrorBoundaryWithBothMethods extends React.Component {
2124+
state = {error: null};
2125+
static getDerivedStateFromError(error) {
2126+
getDerivedStateFromErrorError = error;
2127+
return {error};
2128+
}
2129+
componentDidCatch(error, info) {
2130+
componentDidCatchError = error;
2131+
}
2132+
render() {
2133+
return this.state.error ? 'ErrorBoundary' : this.props.children;
2134+
}
2135+
}
2136+
2137+
const thrownError = new Error('expected');
2138+
const Throws = () => {
2139+
throw thrownError;
2140+
};
2141+
2142+
const container = document.createElement('div');
2143+
ReactDOM.render(
2144+
<ErrorBoundaryWithBothMethods>
2145+
<Throws />
2146+
</ErrorBoundaryWithBothMethods>,
2147+
container,
2148+
);
2149+
expect(container.textContent).toBe('ErrorBoundary');
2150+
expect(componentDidCatchError).toBe(thrownError);
2151+
expect(getDerivedStateFromErrorError).toBe(thrownError);
2152+
});
20872153
});

packages/react-reconciler/src/ReactFiberUnwindWork.js

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import type {CapturedValue} from './ReactCapturedValue';
1414
import type {Update} from './ReactUpdateQueue';
1515
import type {Thenable} from './ReactFiberScheduler';
1616

17+
import getComponentName from 'shared/getComponentName';
18+
import warningWithoutStack from 'shared/warningWithoutStack';
1719
import {
1820
IndeterminateComponent,
1921
FunctionalComponent,
@@ -111,7 +113,7 @@ function createClassErrorUpdate(
111113
const inst = fiber.stateNode;
112114
if (inst !== null && typeof inst.componentDidCatch === 'function') {
113115
update.callback = function callback() {
114-
if (getDerivedStateFromError !== 'function') {
116+
if (typeof getDerivedStateFromError === 'function') {
115117
// To preserve the preexisting retry behavior of error boundaries,
116118
// we keep track of which ones already failed during this batch.
117119
// This gets reset before we yield back to the browser.
@@ -125,6 +127,21 @@ function createClassErrorUpdate(
125127
this.componentDidCatch(error, {
126128
componentStack: stack !== null ? stack : '',
127129
});
130+
if (__DEV__) {
131+
if (typeof getDerivedStateFromError !== 'function') {
132+
// If componentDidCatch is the only error boundary method defined,
133+
// then it needs to call setState to recover from errors.
134+
// If no state update is scheduled then the boundary will swallow the error.
135+
const updateQueue = fiber.updateQueue;
136+
warningWithoutStack(
137+
updateQueue !== null && updateQueue.firstUpdate !== null,
138+
'%s: Error boundaries should implement getDerivedStateFromError(). ' +
139+
'In that method, return a state update to display an error message or fallback UI, ' +
140+
'or rethrow the error to let parent components handle it.',
141+
getComponentName(fiber.type) || 'Unknown',
142+
);
143+
}
144+
}
128145
};
129146
}
130147
return update;

0 commit comments

Comments
 (0)