-
Notifications
You must be signed in to change notification settings - Fork 171
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
Ensure updating the remix router also updates the navigation data #4273
Conversation
Job #8337: Bundle Size — 62.7MiB (~+0.01%).
Warning Bundle contains 64 duplicate packages - View duplicate packages Bundle metrics (2 changes)
Bundle size by type (1 change)
View job #8337 report View fix/remix-404-sticking-bug branch activity |
( | ||
innerRouter: RouterType, | ||
location: Location, | ||
updateEntries: 'update-entries' | 'do-not-update-entries', |
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.
nitpick, but append-to-entries | replace-entries
would be much clearer form e
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.
I left a nit to pick
Problem:
When working on a Remix project, it is possible to get "stuck" on a 404 route (the navigation toolbars stop working) if a change is made to the project whilst one is showing
Fix:
The problem here is that a change to the project's routes will replace the
router
with a new one, but we weren't updating thenavigationData
when this happened, meaning the navigation functions would still be using the previousrouter
until the new one had triggered any form of navigation. This meant that hitting the "back" button would still call the loader function for the correct route, but then that route would never be rendered since this was happening via arouter
that wasn't being used as the actualRouterProvider
for the remix app.