-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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: router changes the URL prematurely #2917
Conversation
|
Currently, the client-side navigation doesn't comply with the server-side navigation standard. We should change the URL path after the component or page is rendered. This is a fix for issue #2870, but I think I need to include more test cases. |
Thank you, but the current behaviour is intentional, and changing it would require a thorough discussion. There's no universally accepted 'right' way to address this. This poll gives a slight edge to the behaviour in this PR... ...but the following thread gives some very convincing reasons why changing the URL at the start of the navigation makes more sense: Personally I think delaying the change is worse UX — it robs the user of valuable feedback that their click was registered, and makes error recovery harder since they can't simply refresh the page if client-side navigation fails for some reason. I'm also persuaded by the argument that state should follow URL, not vice versa. This position is supported by the behaviour of widely-used SPAs like YouTube and Instagram. Delaying the update until after a successful navigation would therefore violate the expectations that users have developed through using these sites. |
Closing in favour of #3241 which has a simpler implementation, in case we were to decide to go ahead with this change |
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpx changeset
and following the prompts. All changesets should bepatch
until SvelteKit 1.0