sveltejs / kit

web development, streamlined
https://svelte.dev/docs/kit
MIT License
18.78k stars 1.96k forks source link

`$page.state` is lost after page refresh #11956

Open mpost opened 8 months ago

mpost commented 8 months ago

Describe the bug

When refreshing a page (or submitting a form) with a PageState, the PageState is lost. However, when checking the history.state['sveltekit:states'], we see that the expected state is still attached to the current history entry.

Reproduction

The following +page.svelte demonstrates the issue:

<script lang="ts">
  import { browser } from '$app/environment'
  import { replaceState } from '$app/navigation'
  import { page } from '$app/stores'

  let historyState = browser && history.state['sveltekit:states']
</script>

<div>App PageState: {$page.state.filter}</div>
<div>History state: {JSON.stringify(historyState)}</div>

<hr />
<button on:click={() => replaceState('', { filter: 'banana' })}>Replace state with banana</button>
<button on:click={() => (historyState = history.state['sveltekit:states'])}>
  Refresh from state.history
</button>

After initial loading the $page.state.filter is undefined

  1. Click on "Replace state with banana => PageState shows {"filter":"banana"}
  2. Click on "Refresh from state.history" => PageState and history state show {"filter":"banana"}
  3. Refresh the browser page => PageState shows undefined
  4. Click on "Refresh from state.history" => history state shows {"filter":"banana"} but page state remains undefined

Logs

No response

System Info

System:
    OS: Windows 11 10.0.26063
    CPU: (12) x64 Intel(R) Core(TM) i7-9850H CPU @ 2.60GHz
    Memory: 13.65 GB / 31.71 GB
  Binaries:
    Node: 21.1.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.2.3 - C:\Program Files\nodejs\npm.CMD
    pnpm: 8.6.7 - ~\AppData\Local\pnpm\pnpm.CMD
  Browsers:
    Chrome: 122.0.6261.112
    Edge: Chromium (123.0.2420.10)
    Internet Explorer: 11.0.26063.1
  npmPackages:
    @sveltejs/adapter-node: ^4.0.1 => 4.0.1 
    @sveltejs/kit: ^2.5.2 => 2.5.2 
    @sveltejs/vite-plugin-svelte: ^3.0.2 => 3.0.2 
    svelte: ^4.2.10 => 4.2.10 
    vite: ^5.1.2 => 5.1.2

Severity

serious, but I can work around it

Additional Information

No response

rChaoz commented 8 months ago

Just encountered this myself, took a while to debug. I just switched all of my modals to use shallow routing, as suggested by the docs. But because of this issue it seems like I'll have to abandon this work and keep using stores instead, since some of my modals contain forms, which close the dialog when submitted. Additionally, hitting the back button after this happens does nothing, because it pops the state from the history, but since $page.state was already undefined, nothing happens.

Additionally, I've encountered a bug where closing the model sometimes causes the whole page to go back. Which means that, sometimes, $page.state is set even when it shouldn't be anymore (back navigation already happened). I'm not sure how to reproduce this but it seems to happen quite often.

All in all, it seems that $page.state is generally out of sync with history.state.

eltigerchino commented 8 months ago

Seems like this is noted as a caveat in the docs. I can't explain why though. cc: @Rich-Harris

During server-side rendering, $page.state is always an empty object. The same is true for the first page the user lands on — if the user reloads the page (or returns from another document), state will not be applied until they navigate. https://kit.svelte.dev/docs/shallow-routing#caveats

rChaoz commented 8 months ago

That's very interesting. Maybe Svelte 5/associated SvelteKit major version will change this behaviour. I find it very difficult to find even a single use case of shallow routing that doesn't break with those "caveats".

mpost commented 8 months ago

I read those lines in the docs as well, but thought that it applied to server side rendering only.

If these caveats also apply on the client, shallow routing would only be useful in the simpler cases like a navigating to a dialog.

mpost commented 8 months ago

Chrome just added support for the new NavigationActivation api. This sounds like a accurate callback based api to get insights into the current PageState. Alas, it is chrome only atm.

rChaoz commented 8 months ago

Workaround:

import { derived } from "svelte/store"
import { page } from "$app/stores"
import { browser } from "$app/environment"
import deepEquals from "fast-deep-equal"

let oldState: App.PageState = {}

export const pageState = derived<typeof page, App.PageState>(page, (_, set) => {
    if (!browser) return
    setTimeout(() => {
        const newState = history.state["sveltekit:states"] ?? {}
        if (!deepEquals(oldState, newState)) {
            oldState = newState
            set(newState)
        }
    })
}, {})

Use this instead of $page.state. This correctly loads the state after a page reload and does not remove it on form submission. Additionally, it doesn't update the store when the state doesn't change but $page would update without the state changing.

eltigerchino commented 7 months ago

Going to keep this open while I don't know the reason for $page.state being reset on browser refresh https://github.com/sveltejs/kit/issues/11956#issuecomment-2004266264 However, for issues related to $page.state being lost after an enhanced form submission (which invokes invalidateAll) see https://github.com/sveltejs/kit/issues/11783

Rich-Harris commented 7 months ago

Consider one of the primary use cases that shallow routing was added to solve: Instagram-style navigation, where clicking on a photo on /[user] opens a preview page but also updates the URL.

If you reload the page, you'll get a server rendered page for a different route — /[user]/[photo]. The previous $page.state just doesn't apply here, and it could even be detrimental (depending on what you do with it). You certainly don't want to navigate from /[user]/[photo] back to /[user] (where the $page.state does apply) upon hydration, because that would cause flashing.

And in general that applies to any use of $page.state — because it's not available during SSR, it would at minimum cause some flashing and weirdness if it was applied post-hydration.

It's definitely possible that there are some cases where you do want to apply state on hydration, flickering be damned. Perhaps an option would make sense for that:

pushState('', state, {
  hydrate: true
});

I don't think it should be the default though.

rChaoz commented 7 months ago

I believe an issue I have with this system is that the state isn't completely reset on re-load, but I'm not sure if this is fixable:

Page A -> pushState -> Reload

This results in a state where the back button does nothing (it pops history.state, but this doesn't affect $page.state as it was already). I don't remember but I think then hitting forward does apply the state which wasn't applied initially.

yahao87 commented 5 months ago

This issue is critical for those who want to display the popup again upon refreshing, as it disregards the default state behavior of the browser. Is there any solution for this?

yahao87 commented 5 months ago

Consider one of the primary use cases that shallow routing was added to solve: Instagram-style navigation, where clicking on a photo on /[user] opens a preview page but also updates the URL.

If you reload the page, you'll get a server rendered page for a different route — /[user]/[photo]. The previous $page.state just doesn't apply here, and it could even be detrimental (depending on what you do with it). You certainly don't want to navigate from /[user]/[photo] back to /[user] (where the $page.state does apply) upon hydration, because that would cause flashing.

And in general that applies to any use of $page.state — because it's not available during SSR, it would at minimum cause some flashing and weirdness if it was applied post-hydration.

It's definitely possible that there are some cases where you do want to apply state on hydration, flickering be damned. Perhaps an option would make sense for that:

pushState('', state, {
  hydrate: true
});

I don't think it should be the default though.

The behavior of SSR hydrating should be a concern for developers implementing SSR, not an issue to be solved by blocking actual browser functionality. Isn't it natural for SSR operations to ignore history.state by default? Why should the existence of SSR prevent the use of history.state in the browser entirely?

Do other frameworks have implementations that completely block basic browser functionality? I think it's a serious problem for a framework to prevent the use of the browser's basic functionality.