feat: use goto for shallow routing, deprecate push/replaceState - #16449
Conversation
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/db68cbd1287bc3e05425d4d9e7e147a2e871827bOpen in |
🦋 Changeset detectedLatest commit: db68cbd 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
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?)
|
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
… reason to allow this, it will just break on reload
Rich-Harris
left a comment
There was a problem hiding this comment.
fantastic work. ready to merge as soon as we get a full bank of green lights — fingers crossed for this run
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to version-3, this PR will be updated.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ `version-3` is currently in **pre mode** so this branch has prereleases rather than normal releases. If you want to exit prereleases, run `changeset pre exit` on `version-3`.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ # Releases ## @sveltejs/adapter-vercel@7.0.0-next.4 ### Major Changes - breaking: remove support for edge and Node 20 runtimes ([#16520](#16520)) ### Patch Changes - chore: bump `@vercel/nft` dependency ([#16522](#16522)) - Updated dependencies [[`6942ddb`](6942ddb), [`68137d4`](68137d4), [`15dc603`](15dc603), [`a4f1521`](a4f1521), [`629b45d`](629b45d), [`7a7833a`](7a7833a), [`e6591a0`](e6591a0), [`7a7833a`](7a7833a), [`b196a39`](b196a39), [`66bf343`](66bf343), [`4bde76b`](4bde76b), [`d9f05cb`](d9f05cb), [`40efca1`](40efca1), [`8ef5548`](8ef5548), [`9efb8f2`](9efb8f2), [`6a670dc`](6a670dc), [`3416dcc`](3416dcc), [`6850fa5`](6850fa5), [`e6591a0`](e6591a0), [`688bead`](688bead), [`b596610`](b596610), [`0f26c13`](0f26c13), [`110a3db`](110a3db), [`665d467`](665d467), [`9c8c60c`](9c8c60c)]: - @sveltejs/kit@3.0.0-next.13 ## @sveltejs/kit@3.0.0-next.13 ### Major Changes - breaking: replace the `noScroll` and `keepFocus` options of `goto` with a single `reset` option, and the `data-sveltekit-noscroll` and `data-sveltekit-keepfocus` attributes with `data-sveltekit-reset` ([#16558](#16558)) - breaking: deprecate `error(status, {...})` in favour of `error(status, message, {...})` ([#16540](#16540)) ### Minor Changes - feat: add shallow routing to `goto` and deprecate `pushState` and `replaceState` ([#16449](#16449)) - feat: preserve page state set through `goto(..., { state, persistState: true })` across reloads ([#16449](#16449)) ### Patch Changes - chore: deduplicate repeated CSP directive handling ([#16498](#16498)) - fix: avoid Vite dev server reload on initial page request ([#16553](#16553)) - fix: don't set a `null` `accept-language` header on internal `fetch` sub-requests when the incoming request has none ([#16527](#16527)) - fix: correctly detect prerendered paths in server `fetch` when `paths.base` is set ([#16525](#16525)) - fix: don't duplicate remote modules in the generated manifest ([#16532](#16532)) - fix: exclude inlined files from the page's `$app/manifest` immutable list ([#16531](#16531)) - perf: avoid quadratic remote form issue merging ([#16493](#16493)) - fix: don't attempt to serialize fetch responses when the request body is not a string or TypedArray ([#16501](#16501)) - fix: read the error status of `App.Error` in the `prerender` and `query.live` remote functions ([#16529](#16529)) - fix: propagate errors from prerendered remote responses instead of re-running the function ([#16535](#16535)) - fix: respect the status returned from `handleError` on the fallback error page served to error-page sub-requests ([#16528](#16528)) - fix: skip unnecessary `version.json` checks if `updated.current` is already `true` ([#16518](#16518)) - fix: allow generation of $app/tsconfig without TypeScript installed ([#16534](#16534)) - perf: use a `Set` to check element ids when validating fragment links during prerendering ([#16494](#16494)) - fix: only include `_app/immutable` files in `$app/manifest`'s `immutable` in the service worker ([#16531](#16531)) - fix: log errors caught by the Vite dev server handler ([#16550](#16550)) - fix: resolve `root` per instance of the SvelteKit Vite plugin ([#16513](#16513)) - fix: ignore path casing differences when warning about overridden Vite config on Windows ([#16545](#16545)) - fix: preserve the current URL search parameters when submitting a remote form without JavaScript ([#16373](#16373)) - fix: treeshake prerendered remote functions in the right chunks ([#16533](#16533)) - fix: error during development if the adapter does not support instrumentation and it exists ([#16548](#16548)) ## @sveltejs/adapter-netlify@7.0.0-next.5 ### Patch Changes - chore: replace @netlify/functions with @netlify/types for the Context type ([#16542](#16542)) - Updated dependencies [[`6942ddb`](6942ddb), [`68137d4`](68137d4), [`15dc603`](15dc603), [`a4f1521`](a4f1521), [`629b45d`](629b45d), [`7a7833a`](7a7833a), [`e6591a0`](e6591a0), [`7a7833a`](7a7833a), [`b196a39`](b196a39), [`66bf343`](66bf343), [`4bde76b`](4bde76b), [`d9f05cb`](d9f05cb), [`40efca1`](40efca1), [`8ef5548`](8ef5548), [`9efb8f2`](9efb8f2), [`6a670dc`](6a670dc), [`3416dcc`](3416dcc), [`6850fa5`](6850fa5), [`e6591a0`](e6591a0), [`688bead`](688bead), [`b596610`](b596610), [`0f26c13`](0f26c13), [`110a3db`](110a3db), [`665d467`](665d467), [`9c8c60c`](9c8c60c)]: - @sveltejs/kit@3.0.0-next.13 ## @sveltejs/adapter-static@4.0.0-next.2 ### Patch Changes - fix: match adapter-vercel's prerendered redirect handling when deploying to Vercel ([#16497](#16497)) - Updated dependencies [[`6942ddb`](6942ddb), [`68137d4`](68137d4), [`15dc603`](15dc603), [`a4f1521`](a4f1521), [`629b45d`](629b45d), [`7a7833a`](7a7833a), [`e6591a0`](e6591a0), [`7a7833a`](7a7833a), [`b196a39`](b196a39), [`66bf343`](66bf343), [`4bde76b`](4bde76b), [`d9f05cb`](d9f05cb), [`40efca1`](40efca1), [`8ef5548`](8ef5548), [`9efb8f2`](9efb8f2), [`6a670dc`](6a670dc), [`3416dcc`](3416dcc), [`6850fa5`](6850fa5), [`e6591a0`](e6591a0), [`688bead`](688bead), [`b596610`](b596610), [`0f26c13`](0f26c13), [`110a3db`](110a3db), [`665d467`](665d467), [`9c8c60c`](9c8c60c)]: - @sveltejs/kit@3.0.0-next.13 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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