-
-
Notifications
You must be signed in to change notification settings - Fork 916
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
Update history.pushState in navigation-events.js #1092
base: 5.x
Are you sure you want to change the base?
Conversation
In my Vue3 application, the router worked incorrectly if the parameters were null, null When following the recommendations, everything works well. https://developer.mozilla.org/en-US/docs/Web/API/History/pushState
File size impactdist (-32 bytes)Overall impact on dist files size
Detailed impact on dist files size
Impact on dist files cache8 files in you users cache are now outdated because their content have changed.
|
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.
This is the kind of change that could subtly alter behavior in weird ways. Ideally we'd test it in every supported browser - Internet Explorer especially.
@@ -58,7 +58,7 @@ export function navigateToUrl(obj) { | |||
window.location.hash = destination.hash; | |||
} else { | |||
// different path, host, or query params | |||
window.history.pushState(null, null, url); | |||
window.history.pushState({}, '', url); |
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 think this code might overwrite history.state
to {}
rather than keeping it at the previous value. I don't know if null
is any better, but would like to fully understand the implications of this change and ideally see tests for it.
In my Vue3 application, the router worked incorrectly if the parameters were null, null When following the recommendations, everything works well. https://developer.mozilla.org/en-US/docs/Web/API/History/pushState