feat: use goto for shallow routing, deprecate push/replaceState#16449
feat: use goto for shallow routing, deprecate push/replaceState#16449dummdidumm wants to merge 18 commits into
goto for shallow routing, deprecate push/replaceState#16449Conversation
There have been a few confusions with how shallow routing and `pushState/replaceState` work. It was several parts, which are all addressed in this PR: - `page.url/params/route` are not updated. This still is that way, but the new `page.shallow.url/params/route` object now gives you that information. Closes #10661 - people want to preserve history state across full page reloads. This was previously not done because `pushState/replaceState` were only concerned with shallow routing. The case of "I just wanna set some history state" was basically overlooked. This changes: `pushState/replaceState('', ...)` does _not_ enter shallow routing mode if you are not already in it. And a new third argument to these two functions makes it possible to preserve state across reloads, e.g. `pushState('', { survives: 'yay' }, { persist: true })`. `goto` will always persist state. Closes #13293, closes #11956 - Shallow routing did not trigger navigation hooks (before/after/onNavigate). Now they do. If you don't want that because you just wanna set some state, use `pushState('', ...)`. Closes #11759, closes #11776 - As a an additional DX-win, you can now also do `pushState/replaceState(null, ...)` which basically means "end shallow routing mode and revert the visible url to what it was before"
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
|
Install the latest version of pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/d33a324df5db9cf7f08468c708d706041ba8c9dbOpen in |
🦋 Changeset detectedLatest commit: d33a324 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
|
||
| Shallow routing is a feature that requires JavaScript to work. Be mindful when using it and try to think of sensible fallback behavior in case JavaScript isn't available. | ||
|
|
||
| If you navigate to another page via shallow routing, reloading on that route will not start the app in shallow routing mode. Instead the actual page on that URL is loaded. On the server this is unavoidable (because history state isn't available at that point), and hence it would mean too much of a UI flicker if it would change the page and enter shallow routing once JavaScript is loaded. |
There was a problem hiding this comment.
this is duplicative with the stuff above, we could streamline it. probably best to tweak the API and then come back to the docs though
There was a problem hiding this comment.
This is not duplicative it isn't mentioned anywhere else at the moment. persistState also does not change this; we don't swap out the server-loaded page for the real page once we know we're actually shallow-routing, we only persist state (as the option name says).
| /** If `true`, preserves the browser's scroll position. */ | ||
| noScroll?: boolean; |
There was a problem hiding this comment.
| /** If `true`, preserves the browser's scroll position. */ | |
| noScroll?: boolean; | |
| /** | |
| * If `true`, scrolls to the top of the page, otherwise preserves the current scroll position. | |
| * @default true | |
| */ | |
| resetScroll?: boolean; | |
| /** @deprecated Use `resetScroll` instead */ | |
| noScroll?: boolean; |
| /** If `true`, keeps the currently focused element focused. */ | ||
| keepFocus?: boolean; |
There was a problem hiding this comment.
| /** If `true`, keeps the currently focused element focused. */ | |
| keepFocus?: boolean; | |
| /** | |
| * If `true`, resets focus to the `<body>` element, otherwise allows the current `document.activeElement` to remain focused. | |
| * @default true | |
| */ | |
| resetFocus?: boolean; | |
| /** @deprecated Use `resetFocus` instead */ | |
| keepFocus?: boolean; |
There was a problem hiding this comment.
let's keep the deprecation discussion out of this for now - I don't see a big reason to change this (and an argument against is that now everything's false by default whereas with your change some would be true by default), and we would have to adjust the data-sveltekit-... attributes as well
There was a problem hiding this comment.
We can tackle it in a follow-up PR but I do think it needs to change. keepFocus is just misleading in the case where something is focused as a result of the navigation, which is extremely likely in the case of a modal (which should update the SFNSP and trap focus) — we're not keeping the previously focused element, we're just not actively resetting it. resetFocus is a much more accurate description of what's happening.
This was kind of already true but is more acute in the shallow routing case, because we probably need to switch the default in the shallow: true case to keepFocus: true (or resetFocus: false) — would feel weird to focus the <body> as a result of a shallow navigation.
some would be true by default
There's nothing wrong with an option defaulting to true — see e.g. config.csrf.checkOrigin or config.paths.relative or config.prerender.crawl.
Rich-Harris
left a comment
Rich-Harris
left a comment
There was a problem hiding this comment.
As mentioned inline: I don't think goto(null) should be a thing, and I certainly don't think it should be a way to set state without navigating, because shallow: true is the way to set state without navigating.
For me the only question is whether relative URLs, including the empty string, are resolved against page.url or page.shallow.url ?? page.url, aka window.location. I incline towards the latter (which I think is what's already happening?)
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| } | ||
|
|
||
| interface Window { | ||
| shallow_navigation_log: Array<{ |
There was a problem hiding this comment.
side-note, would be cool if we could just log stuff and read the logs back like we do in the Svelte test suite. something for future us to consider
|
Playing with this locally, so far so good. Really happy with how this is shaping up. One thought I have, which isn't new to this PR but maybe worth thinking about since we're currently deep in this topic: if I perform this sequence of actions...
...what should happen? Currently, step 4 will shallowly navigate me to I tried to see what Instagram does in this situation, and the answer is 'it breaks'. (Maybe Meta needs to pay engineers more.) Unsplash does the second one — modal on click, full page on reload, full page on back-then-forward. The question is: should this behaviour always apply? Should it vary on |
|
Actually now that I think about it... would we need to upgrade the entire history in that case? Is it weird if you're thumbing through a bunch of photos 1, 2, 3, 4, 5, then go back to 3, reload to a full page, go back to a modal, then go forward to a full page and forward to another modal? That's what Unsplash does. Genuinely no idea if there's a right answer here: unsplash.mov |
|
Some bugginess around scroll handling: if I shallowly navigate from A to B, the document correctly doesn't scroll. If I scroll, then hit back, the document does scroll, incorrectly. If I then hit forward, it again scrolls incorrectly. Related: I think the solution is conceptually fairly simple: we preserve those options with the history entry, so the entry representing B would keep its When we navigate back from B to A, we use B's |
|
Opened #16480 for the 'what should happen when we reload?' question |
…to account for shallow navigations and default them to true Fixes #11452
2 participants
There have been a few confusions with how shallow routing and
pushState/replaceStatework. That's why we're deprecating these two methods in favor ofgoto, which gets new features, which addresses these confusions/requests:page.url/params/routeare not updated. This still is that way, but the newpage.shallow.url/params/routeobject now gives you the information about the user-visible url. Closes$pagestore doesn't update when you set search params in the URL usinghistory#10661, closes page.url does not update when pushState is called #13569pushState/replaceStatewere only concerned with shallow routing, and when you reload you likely land on a different page where the history state is unfitting. The case of "I just wanna set some history state" was basically overlooked. This changes:goto(null, ...)does not enter shallow routing mode if you are not already in it. And a new option ofgotomakes it possible to preserve state across reloads:goto(null, { state: { survives: 'yay' }, persistState: true }). Closes Keephistory.statein sync with$page.state#13293, closes$page.stateis lost after page refresh #11956goto(null, ...). Closes Make easier to use View Transitions with shallow routing #11759, closes pushState/replaceState don't trigger beforeNavigate #11776Also fixes #16457 / fixes #15618 / fixes #11452