fix: unstate object in Svelte 5 dev mode

[dummdidumm]

...before checking serializability
fixes #11363

---

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 detected

Latest commit: 33c944b

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	Patch

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

[benmccann]

I'd put these two lines up with the other svelte import just to keep things a little organized

[benmccann]

looks good to me once the test failures are addressed

[dummdidumm]

With sveltejs/svelte#11180 this will need a different approach, since we would need to use runes then.

[trueadm]

@dummdidumm Can you not import from svelte/internal?

[Rich-Harris]

Feels like there's a deeper question here: when would you ever use $state with pushState? That's not what it's for — it's for associating some current value with a history entry, not some reactive, changing-over-time value. If we're mandating that pushState should be called with serializable data, then that implies that the developer is responsible for calling unstate (or $state.raw or $state.snapshot or whatever it ends up being) themselves. Otherwise we're creating false expectations (that you can pass reactive state into pushState and it can change over time).

In #11363, the argument to pushState comes from preloadData. It feels like a bug that preloadData would return anything other than a static object.

[Rich-Harris]

Tinkered locally and I think the right fix for #11363 is to change this line:

kit/packages/kit/src/runtime/client/client.js

Line 561 in f71f381

data: data_changed ? data : page.data

-data: data_changed ? data : page.data
+data: data_changed ? Object.freeze(data) : page.data

$page.data should be immutable, and this conveniently solves the state problem.

[dummdidumm]

I think the underlying problem is that in legacy-client.js in Svelte 5, we're using a proxy to make the props reactive. That has unintended side effects for this.

Another way to solve this is to make devalue more robust against symbolic properties and only error when encountering symbolic properties that are not enumerable (after all, it also ignores those when they're regular string properties):

- if (Object.getOwnPropertySymbols(thing).length > 0) {
+ const enumerable_symbols = Object.getOwnPropertySymbols(thing).filter(s => Object.getOwnPropertyDescriptor(thing, s).enumerable);
+ if (enumerable_symbols.length > 0) {
	throw new DevalueError(
		`Cannot stringify POJOs with symbolic keys`,
		keys
	);
}

I don't think that people expect things added to pushState to be reactive - they will never be since they're serialized right away. So I think avoiding the need to call $state.snapshot where possible would be better DX.

[Rich-Harris]

Released a new version of devalue with that change

[benmccann]

I merged the rennovate PR to bump devalue to v5 here: #12141. I have not yet released it

[Rich-Harris]

I released it and confirmed that it fixes #11363, so I'm going to close this. We can revisit the Object.freeze(data) thing another time if necessary