-
Notifications
You must be signed in to change notification settings - Fork 90
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
Add Error Handler to Limit Exception Bubbling #1923
Conversation
Thanks for this PR @ProjectBarks. Sorry for the long delay, things got away on me a bit late last year 😅 Will have a look through the changes and will get back to you. Will bring the branch up-to-date etc, so there'll be some forced pushes |
src/ReactSVG.tsx
Outdated
if (!this._isMounted) { | ||
afterInjection(error) | ||
onError(error) |
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.
Don't think we can get away with this as it's going to result in setState
being called (L103) when the component is not mounted, which will throw an error. Will tweak things a bit.
Modified a few things and will merge now. Thanks for your contribution 🎉 |
Released in |
The idea is to limit the scope in which exceptions in the injection code can break the react component. This reduces the risk of total app failure.
Added
onError
handler because passing new errors intoafterInjection
felt like I was overloading the function.Example:
Before this would break your application, after the error is routed into
onError
and then defaulted to the fallback.