Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: ErrorBoundary remounts children when errors are caught #26259

Open
fatton139 opened this issue Feb 28, 2023 · 10 comments
Open

Bug: ErrorBoundary remounts children when errors are caught #26259

fatton139 opened this issue Feb 28, 2023 · 10 comments

Comments

@fatton139
Copy link

In the render block of a ErrorBoundary component the props.children is remounted when getDerivedStateFromError derives a new state (or componentDidCatch sets a state).

Is this a special case ? Setting the state in any other manner (e.g componentDidUpdate doesn't have this behavior). I don't see this behavior documented in the React docs.

Thanks!

React version: 18.2.0 (Happens on 17.0.2 as well).

Steps To Reproduce

  1. In the code example increment the counter past 5 and the child component unmounts and mounts, the child state also resets

Link to code example: https://codesandbox.io/s/nifty-platform-wub9wn?file=/src/App.tsx

The current behavior

props.children unmounts and mounts.

The expected behavior

props.children rerenders.

@fatton139 fatton139 added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Feb 28, 2023
@Twipped
Copy link

Twipped commented Mar 2, 2023

I am experiencing this with componentDidUpdate, as well. It gets even weirder, because the ErrorBoundary doesn't receive all of the thrown errors.

My own example: https://codesandbox.io/s/bold-brook-63i6vm?file=/src/App.js
Notice that the console shows the error occurred four times, but the Boundary only received the third instance, and the overlay caught the second. Errors 1 and 2 just vanished into the ether.

@Twipped
Copy link

Twipped commented Mar 2, 2023

@fatton139 the reason yours is being remounted is because you're using StrictMode. That causes every component to mount twice.

@fatton139
Copy link
Author

fatton139 commented Mar 2, 2023

I am experiencing this with componentDidUpdate, as well. It gets even weirder, because the ErrorBoundary doesn't receive all of the thrown errors.

My own example: https://codesandbox.io/s/bold-brook-63i6vm?file=/src/App.js Notice that the console shows the error occurred four times, but the Boundary only received the third instance, and the overlay caught the second. Errors 1 and 2 just vanished into the ether.

In your example the overlay catches the second and fourth instance for me, the third is handled by the error boundary so only Errors 0 has vanished for me. Could React internally be doing some sort of batching?

@fatton139 the reason yours is being remounted is because you're using StrictMode. That causes every component to mount twice.

Not sure if I've done it correctly, but I removed <StrictMode> from root.render and the component is still remounting.

@Werter12
Copy link

Werter12 commented Mar 5, 2023

Hi @fatton139
As I can see according to the docs we shouldn't render props.children when the error occurred. When I am rendering fallback when error occurred unmount- is only called once. Maybe when we keeping rendering components with error - there is some mechanism which prevents component keep throwing errors and finally shows error ui.

{this.state?.hasError ? <div>error!</div> : this.props.children}

@fatton139
Copy link
Author

Hi @fatton139 As I can see according to the docs we shouldn't render props.children when the error occurred. When I am rendering fallback when error occurred unmount- is only called once. Maybe when we keeping rendering components with error - there is some mechanism which prevents component keep throwing errors and finally shows error ui.

{this.state?.hasError ? <div>error!</div> : this.props.children}

Thanks for the reply @Werter12
I just find it odd that the docs don't mention anything regarding this behavior if I choose to keep the children rendered (Since nothing is stopping me from using children that way).

I would've expected something like the Suspense API where you would do<Suspense fallback={<FallbackComponent />} /> if it's intended that we must unmount the children on error.

@eps1lon
Copy link
Collaborator

eps1lon commented Mar 7, 2023

I think there's a case for not resetting state. An error boundary that just renders children anyway is like rethrowing an error. But there's also a case for resetting state: It likely recovers the UI. Either way, this should be documented if the current behavior is intended.

@fatton139 could you check if error boundaries always behaved that way by checking if the behavior is the same in React 16?

@fatton139
Copy link
Author

fatton139 commented Mar 10, 2023

I think there's a case for not resetting state. An error boundary that just renders children anyway is like rethrowing an error. But there's also a case for resetting state: It likely recovers the UI. Either way, this should be documented if the current behavior is intended.

@fatton139 could you check if error boundaries always behaved that way by checking if the behavior is the same in React 16?

Hi @eps1lon, I checked on React 16.14.0 the behavior seems to be the same. Here is the playground: https://codesandbox.io/s/recursing-stonebraker-ki0yz3?file=/src/App.jsx

Regardless of how we proceed, the documentation of the intended behavior would be good.

Our use case is for not resetting the state, our child components throws various errors, some are fatal where the child cannot be rendered, others are non fatal where there is sufficient information to render the child + error message. The current solution is to throw errors if the error is fatal, otherwise manually set a state, ideally we'd like to abstract away the "set a state" part.

@pakomarov
Copy link

Our use case is for not resetting the state, our child components throws various errors, some are fatal and where the child cannot be rendered, others are non fatal where there is sufficient information to render the child + error message. The current solution is to throw errors if the error is fatal, otherwise manually set a state, ideally we'd like to abstract away the "set a state" part.

Have exactly the same problem. In a small app I thought it would be convenient to handle errors in one place, in ErrorBoundary, and just throw everything there. Bumped into the fact that ErrorBoundary remounts children. I wanted to show little toasts, but instead all my state is gone and my mounting effects are relaunched. Super unintuitive behaviour. The interface implies that I deal with an error myself and ErrorBoundary just catches it for me. Remounting children is too big of a thing to not mention it.

Copy link

github-actions bot commented Apr 9, 2024

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Apr 9, 2024
@eps1lon eps1lon added Type: Discussion Component: Reconciler and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Resolution: Stale Automatically closed due to inactivity labels Apr 9, 2024
@eps1lon
Copy link
Collaborator

eps1lon commented Apr 28, 2024

This is intended behavior. We're resetting state to attempt to recover from the error. The idea being that the error might be based on the state being invalid/corrupted.

Otherwise we'd enter an infinite loop since a re-render with the same state/props would produce the same result which is throwing an error in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants