-
-
Notifications
You must be signed in to change notification settings - Fork 227
fix: pass in full pathname + search in TSR to avoid trailing slashes #1217
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
SeanCassiere
left a comment
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.
The removal of from should be fine, since Tanner made some changes in router-core, during the big devinxi rewrite, to do more inference of the from value in router.buildLocation.
Its definitely weird that it is busting through the default of never for the trailingSlash option.
On the testing side of things, I'd make sure that your e2e testing covers the Router running at.
- At the apex -
localhost:3000?foo=bar - On a subpath -
localhost:3000/basepath?foo=bar - Using hash routing -
localhost:3000/#/?foo=bar
This testing might even make sense to be applied to the other adapters.
| // Note: we need to specify pathname + search here to avoid TSR appending | ||
| // a trailing slash to the pathname, see https://github.com/47ng/nuqs/issues/1215 | ||
| from: '/', | ||
| to: location.pathname + renderQueryString(search), |
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.
You might want to pull the pathname out using useLocation. I'd use a selector in there, for fine-grained subscriptions.
|
Thanks for your review @SeanCassiere (and good luck with the move!) 🫶 I doesn't work on the apex route indeed, and even this causes TSR to emit a trailing slash: navigate({ href: 'https://example.com?foo=bar' })
// navigates to https://example.com/?foo=barI'll look into the TSR code to see if this can be fixed upstream, even if we're not following the intended use of the API, Edit: actually it doesn't seem possible to not have a trailing slash for the apex, even this adds one: history.replaceState(null, '', 'https://example.com?foo=bar')
// navigates to https://example.com/?foo=bar |
Weird, do you think that is a bug with TSR? Should be possible |
|
🎉 This PR is included in version 2.8.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
No, this happens on https://example.com if you type Actually even loading |
|
TIL |
TBC if this method causes problems elsewhere (maybe running loaders when not needed?)
Also fixes persistence of custom
history.stateproperties.Closes #1215.