-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(react): Add Error Boundary component #2647
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Soooo good, so many tests ❤️
Small nits + don't forget Changelog
|
424ff83
to
b7d0a32
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥇
Hmm so I'm using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Are there plans to update our app usage of error boundaries?
@billyvg we will be updating the usage of |
52ef24a
to
110e338
Compare
The Profiler now has a mandatory name field, which describes what is being profiled
110e338
to
5174791
Compare
Following up #2631
Motivation
This PR implements an Error Boundary React component. See https://reactjs.org/docs/error-boundaries.html for more details.
This is an important API for us to support, as error boundaries are the defacto way to catch and deal with errors in React.
Implementation
The implementation was inspired by the way we do error boundaries at Sentry as well as bvaughn's excellent error boundary implementation
The error boundary provides the following options:
We recommend using
fallbackRender
to render the fallback component, as it uses a render props approach. This passesresetErrorBoundary
as render props, allowing the user to easily reset the error boundary.The error boundary is available as a standalone component with
or as a HOC
Testing
I switched to
@testing-library/react
, it's pretty good (and seems to be the standard way to do tests in React now). It's also 209.5 kb minified, while enzyme is 463.2 kb, and theyarn installs
definitely add up.Current test coverage
Future
After this gets merged in, we move on to creating a hooks based Profiler. I'm also looking to see how difficult it is to support
react-router
and similar.