-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Log route module errors prior to reloading the page #8932
Conversation
🦋 Changeset detectedLatest commit: 045fae6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
console.error( | ||
`Error loading route module \`${route.module}\`, reloading page...` | ||
); | ||
console.error(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.
We've never logged this error before since it's swallowed on the reload and (I think) it was much harder to run into this for import issues in dev when using esbuild
. But using vite during dev, you don't find out about these module errors until runtime, so we can log them and they're available via Preserve log
I'm curious through if they could be surfaced through Vite's overlay? @pcattori @markdalgleish
throw error; | ||
} | ||
|
||
window.location.reload(); |
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.
We could also consider enhancing this behavior in the future too. Reloading the current page isn't necessarily what we want all the time. There's 2 usages of this method:
- prefetching - in that case there's no real reason to hard reload since the user may not click on the link, so we could just let the prefetch no-op
- navigating - ideally for the most seamless UX we'd hard-navigate to the next url instead of reloading the current URL in response to a clicked link. The issue here is that currently
route.lazy
(which calls this) isn't aware of the next location so we'd have to plumb that through in RR if we wanted to smooth out that flow.
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.
It has bugged me many times that the current page gets reloaded instead of the target URL.
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
Closes #8791, #8915