react.dev icon indicating copy to clipboard operation
react.dev copied to clipboard

Unexpected warning for error boundaries without getDerivedStateFromError

Open conradreuter opened this issue 4 years ago • 5 comments

(Note: I dismissed the idea described here by now, but I still felt like reporting my findings.)

Steps to reproduce

I create a reusable error boundary component, ErrorCatcher, in order to deduplicate across different error boundary components.

class ErrorCatcher extends React.Component {
  componentDidCatch(error, info) {
    this.props.onError(error);
  }

  render() {
    return this.props.children;
  }
}

I then create new error boundary on top of that. For example:

function ErrorBoundary({children}) {
  const [error, setError] = useState(null)

  if (error) {
    return <ErrorMessage error={error} />
  }

  return (
    <ErrorCatcher
      onError={error => setError(error)}}
    >
      {children}
    </ErrorCatcher>
  )
}

For convenience, check out this Codesandbox for a live example.

Actual Behavior

Testing it out, I notice that React prints the following warning to the console:

Warning: ErrorCatcher: Error boundaries should implement getDerivedStateFromError(). In that method, return a state update to display an error message or fallback UI.

Expected Behavior

Checking the documentation, I find the following (emphasis mine):

A class component becomes an error boundary if it defines either (or both) of the lifecycle methods [...]

That leaves me a bit puzzled, since on the one hand the documentation is offering me to define either, and on the other I get a warning. I want the documentation to be explicit about eventual error and warning messages.

If you see some value in enabling users to create ErrorCatcher, without being bugged by the warning, I could see other solutions: get rid of the message, or make it optional.

Kindest regards 🙃

conradreuter avatar Jun 08 '20 12:06 conradreuter

To prevent the errors in jest, I had to do something like this (unfortunately):

const spy = jest.spyOn(console, 'error')
describe('Error boundary', () => {
    beforeAll(() => {
        spy.mockImplementation(() => {
            // Errors are intentional in this test, so we prevent them from showing in the console
        })
    })
    afterAll(() => {
        spy.mockRestore()
    })

BernardoFBBraga avatar Sep 30 '20 14:09 BernardoFBBraga

I just learned that calling setState will also silence the message:

class ErrorCatcher extends React.Component {
  componentDidCatch(error, info) {
    this.setState(null)
    this.props.onError(error);
  }

  render() {
    return this.props.children;
  }
}

conradreuter avatar Feb 26 '21 11:02 conradreuter

These are both unfortunate hacks.

prometheas avatar Aug 23 '21 17:08 prometheas

Unfortunately this.setState(null) didn't silent the warning for me @conradreuter. Do you know if the latest version of React throw the warning even though reseting the state?

BrTkCa avatar Aug 10 '22 11:08 BrTkCa

Still an issue 5 years after the feature shipped. Either it is a doc issue or a implementation issue. Either way an issue.

fatso83 avatar Mar 15 '23 14:03 fatso83