Skip to content

feat: use devalue for pushState/replaceState - #14129

Closed
GauBen wants to merge 6 commits into
sveltejs:mainfrom
GauBen:feat/pushstate-devalue
Closed

feat: use devalue for pushState/replaceState#14129
GauBen wants to merge 6 commits into
sveltejs:mainfrom
GauBen:feat/pushstate-devalue

Conversation

@GauBen

@GauBen GauBen commented Aug 6, 2025

Copy link
Copy Markdown
Contributor

Hi!

Closes #14128: pushState/replaceState now use devalue+transport to serialize custom objects

I kept it simple, not sure if the idea will be accepted


Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

@changeset-bot

changeset-bot Bot commented Aug 6, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5464cfe

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Minor

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

@svelte-docs-bot

Copy link
Copy Markdown

@GauBen
GauBen force-pushed the feat/pushstate-devalue branch from e47baa8 to ac02814 Compare August 6, 2025 21:04
@Rich-Harris

Rich-Harris commented Aug 7, 2025

Copy link
Copy Markdown
Member

I don't think we can do this. Today, if you try and do something like this...

<script>
  import { page } from '$app/state';
  import { pushState } from '$app/navigation';
</script>

<section>
  <button
    onclick={() => {
      let object = $state({ count: 0 });
      pushState('/', { object });
    }}
  >
    push state
  </button>

  {#if page.state?.object}
    <p>{page.state.object.count}</p>

    <button onclick={() => (page.state.object.count += 1)}>
      increment page.state.object.count
    </button>
  {/if}
</section>

...SvelteKit will yell at you, because you can't serialize a proxy. With this PR, if you click the first button, page.state.object becomes a reactive proxy. You can increment page.state.object.count by clicking the button. But if you navigate back, and then forward, object.count will be a non-reactive deserialized object. And so on. It's basically making a promise it can't keep. I think the current approach is probably better, even if (or maybe because) it's more restrictive

@GauBen

GauBen commented Aug 7, 2025

Copy link
Copy Markdown
Contributor Author

(Technically it's the browser that yells at me if I give it a proxy, devalue will happily serialize it)

Would it be acceptable to $state.snapshot the value given to pushState? For your specific example, the behavior would then be consistent before and after a history state pop (I guess)

In dev we can also add a warning that reactivity will be lost for history states if that makes the behavior less surprising

@GauBen
GauBen force-pushed the feat/pushstate-devalue branch from df5d01b to d75268c Compare August 13, 2025 12:59
Comment thread packages/kit/test/apps/basics/test/client.test.js Outdated
@GauBen

GauBen commented Aug 13, 2025

Copy link
Copy Markdown
Contributor Author

@Rich-Harris I fixed it the stupid way™, what do you think?

Well it's not that far from the fix in #14268, maybe it's not that dumb

@GauBen
GauBen force-pushed the feat/pushstate-devalue branch from 471e75e to d55c91e Compare August 17, 2025 20:24
@Rich-Harris Rich-Harris added the needs-decision Not sure if we want to do this yet, also design work needed label Aug 21, 2025
@GauBen
GauBen force-pushed the feat/pushstate-devalue branch from d55c91e to 656ff32 Compare September 30, 2025 21:34
@Rich-Harris

Copy link
Copy Markdown
Member

Quick (and long overdue — apologies) update: our latest thinking on this is that we should use IndexedDB instead of sessionStorage, since it handles these cases without needing any serialization at all (and gives us a much larger quota in the process). Groundwork in #16346

elliott-with-the-longest-name-on-github added a commit that referenced this pull request Aug 5, 2026
This is #16406 but takes the idea to its logical conclusion: by having
an internal module where this stuff is managed, we don't need to pass
anything around at all and can delete/simplify a bunch of stuff. Once
this is in, will rebase #14129 against `version-3` and update it to use
this.

Co-authored-by: Elliott Johnson <hello+github@ell.iott.dev>
@Rich-Harris

Copy link
Copy Markdown
Member

Alright, coming back to this now that we've a) deprecated pushState and replaceState in favour of goto and b) centralised the transport stuff. Closing in favour of #16662 — thank you!

@Rich-Harris Rich-Harris closed this Aug 5, 2026
Rich-Harris added a commit that referenced this pull request Aug 5, 2026
)

Supersedes #14129, closes #14128. Allows custom values to be part of
`page.state` and survive reloading.

---

### Please don't delete this checklist! Before submitting the PR, please
make sure you do the following:
- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] This message body should clearly illustrate what problems it
solves.
- [x] Ideally, include a test that fails without this PR but passes with
it.

### Tests
- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint` and `pnpm check`

### Changesets
- [x] If your PR makes a change that should be noted in one or more
packages' changelogs, generate a changeset by running `pnpm changeset`
and following the prompts. Changesets that add features should be
`minor` and those that fix bugs should be `patch`. Please prefix
changeset messages with `feat:`, `fix:`, or `chore:`.

### Edits

- [x] Please ensure that 'Allow edits from maintainers' is checked. PRs
without this option may be closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-decision Not sure if we want to do this yet, also design work needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow custom objects in page.state

2 participants