fix: ensure $state.snapshot never errors

[dummdidumm]

Snapshotting can error on un-cloneable objects. It's not practical to error in this case; often there's no way out of this for users, so it makes sense to return the original value in that case, and warn in dev mode about it.

closes #12438

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
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests and linting

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

[changeset-bot]

🦋 Changeset detected

Latest commit: faf89f7

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

This PR includes changesets to release 1 package

Name	Type
svelte	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

[trueadm]

I'm not sure we want to return the original value. If I snapshot something and then pass that on to somewhere with the expectation that it's a snapshot of the original, I would be horrified if it somehow wasn't. If anything, we should throw an error explaining that the snapshot failed due to a specific reason. The problem is that this API then becomes less useful, hmm.

However, I don't think we should be using structuredClone for $inspect at all. That seems wrong.

[dummdidumm]

If you error on $state.frozen that you can't pass a proxified value, but then also get an error using $state.snapshot, then you're in a impossible-to-resolve state.

[trueadm]

@dummdidumm You'd still error, as the snapshot would just return the original value in the case it errors, meaning the $state.frozen would also fail as the value passed to the snapshot was proxied – like where we have a condition = $bindable() we can check the object at that point in DEV and throw a warning if it's in the WeakMap.

[dummdidumm]

$state.frozen will not error in that case, because if you return the original value, that means you have a non-proxifiable object at hand, so there will be no proxy symbol on that object. Conversely, if that object has the proxy symbol, it will be able to get snapshotted, but its inner contents might not be able to get snapshotted correctly.

[Rich-Harris]

I think this is probably a pragmatic thing to do. Though the text of the warning is arguably a little confusing — to me it suggests that the $state.snapshot argument is the thing being returned as-is, rather than one of its properties. And if you try to snapshot an object like this...

const value = {
  a: () => {},
  b: () => {},
  c: () => {}
};

...you get three separate warnings. It's not necessarily obvious which properties were uncloneable.

So even though it's probably a bit more work, I wonder if we should track those properties and issue a single warning per $state.snapshot call:

The following properties cannot be cloned with $state.snapshot — the return value contains the originals:

• value.a
• value.b
• value.c

Of course if the argument itself can't be cloned, we probably want something more like this:

Value cannot be cloned with $state.snapshot — the original value was returned: