-
-
Notifications
You must be signed in to change notification settings - Fork 574
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
fix(react-router): reset error boundary after the new matches are ready #2244
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 6591ebb. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
schiller-manuel
changed the title
fix(react-router)(issue-2162): reset error boundary after the new matches are ready
fix(react-router): reset error boundary after the new matches are ready
Sep 2, 2024
thanks a lot for your contribution! |
Thanks for the merge. I was quite busy today and didn't manage to create a unit test. Sorry about that. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the current version of the router, the error boundary is reset at the beginning of the loading phase of the new matching routes. This is a problem because the currently rendered routes can re-render while the new ones ar being in the loading process. If this is the case, the re-render will trigger the error boundary again and will void the reset.
I believe the reset should happen only after the new routes have finished loading. In this way, the reset will be in sync with the matched routes.
I don't know if this fix might have some side effects, but on our project it seems to work. If you think that this fix is not ok or you have a better solution, I would be happy to try it out.
Here is the issue I am referring to: #2162
fixes #2162
fixes #2043