breaking: remove unstate(), replace with $state.snapshot rune

[trueadm]

This PR removes unstate and replaces it with a new rune called $state.snapshot which works identically to how unstate worked. Closes #10421

[changeset-bot]

🦋 Changeset detected

Latest commit: 8ff9cce

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

[harrisi]

Is the name set in stone? $state.clean to me implies you're "un-dirtying" the state - which is pretty different. For one, it's a verb that makes it seem like it's mutating the actual variable to no longer be a signal, not returning a new object that isn't wrapped in a signal. Also, at least currently, it is also kind of confusing that it's a no-op for non-proxied non-signals.

So it's not really "cleaning", as much as "maybe-get-raw-value-from-signal"-ing. Or, more succinctly, toRaw. Or something.

[trueadm]

The naming for $state.clean easily allows us to add a matching $state.deepClean in the future too. $state.deepRaw sounds like something from the adult entertainment industry.

[Rich-Harris]

Definitely open to alternative naming. As mentioned $state.clean allows the slightly whimsical $state.deepClean recursive counterpart, though the point about 'un-dirtying' is fair.

Thinking about it further, I wonder if we actually need a shallow version. Where would that be useful? Certainly not in the structuredClone case, or indeed when logging values. If we only had a recursive version then it would give us more flexibility on the naming front.

[dummdidumm]

Yeah I can't really imagine a case where you would want a shallow version.

If we promise to deeply unwrap it, does this shift the expection to somehow transform classes with state objects on them, too? How would we do that if we are able to? We can't just create a POJO from a class, but we also can't reinstantiate a class. Is there some contract we need to define, i.e. "define this method and we'll call that to get the raw value"?

[harrisi]

I have the same issue with deepClean - I wouldn't think that it's returning a new object with reactive stuff removed. I think my main gripe with this is just that it's following historical JavaScript naming convention (sort, for example), while having modern semantics (toSorted). I'd rather have the modern naming and semantics, since it's more clear about what it's doing.

I don't really like toUnwrapped, because I don't think the object being "wrapped" is useful to think about since it's leaking implementation details (which may not be correct, even).

Maybe $state.cloneValue? That still leaves $state.deepCloneValue (if desired, depending on Rich's comment), and is less likely to lead to NSFW searches. It also is more familiar to think about, since it's related to structuredClone. It is slightly awkward to have structuredClone($state.cloneValue(obj)). Maybe just $state.value?

[harrisi]

Yeah I can't really imagine a case where you would want a shallow version.

If we promise to deeply unwrap it, does this shift the expection to somehow transform classes with state objects on them, too? How would we do that if we are able to? We can't just create a POJO from a class, but we also can't reinstantiate a class. Is there some contract we need to define, i.e. "define this method and we'll call that to get the raw value"?

I was trying to convey this in this comment, but it didn't seem there was much interest. I was going to comment about this in #10560, in response to this.

This is actually a pretty fundamental collection of issues, in my opinion. The whole idea of transferring between the Svelte world and not is really tricky and a bit inconsistent, currently. I think it's really important to get it correct.

I think some sort of contract that $state and $state.clean (or whatever) can utilize would be extremely powerful. I'm not sure what it should look like, but I'm partial to providing a known static property or some static methods or symbols or something (rather than subclassing some Reactive thing) would be ideal. That way, a user defined class never "is-a" reactive thing, but "has-a" reactive interface, when needed.

But I feel like I'm repeating myself and I'm not sure if this is what y'all are interested in. I'm still messing around with the idea, but maybe it's not right for Svelte.

[trueadm]

I renamed to rune to $state.raw. FWIW it does recursively remove state on the object deeply if all the properties are also state.

[dummdidumm]

Yes, but it does stop too early when there's a POJO at first - this was the concern about not doing the whole thing deeply.

[trueadm]

@dummdidumm That's fine – most state is deep state and so things will just work.

I really don't want us to deeply traverse every object, it's slow and a waste of CPU cycles as shown from all the DEV ownership stuff. It's just not a good strategy for something that isn't DEV only. We use this logic in $state.frozen too FWIW, so introducing that here will slow $state.frozen down.

[dummdidumm]

...which is why I was advocating for a second optional parameter to deeply traverse the whole object. There's code where you just don't know in advance whether or not you're going to encounter proxied state at some point. sveltejs/kit#12095 is such an example, where we have to traverse the whole history state object to remove any proxy state before handing it to the deserializer

[trueadm]

@dummdidumm I really don't want us to introduce a deep traverse from other objects – even with a second argument. It's very expensive to do and people will just run into performance issues, leading us to revert the change. If we are traversing other objects, it means we will also need to clone them too! Deep cloning this way is terribly expensive, which is why structuredClone exists (that's also expensive, but no where near as bad as this).

Worst case, if we traverse and clone a really big object to not find state in it, then we've just wasted many CPU cycles.

[dummdidumm]

We're already doing deep traversal ourselves internally in the code base here, and if we can't find a different solution to the deserialization problem in SvelteKit we'll have it there, too - so in the end we and others just reimplement it themselves where necessary anyway, with the same potentially imperformant outcome.

[trueadm]

We're already doing deep traversal ourselves internally in the code base here

We're not also cloning though. Deep cloning is the issue. If we need to have deep cloning, then we need to rearchitect how we proxy state today to not use the symbol property.

[Rich-Harris]

There's code where you just don't know in advance whether or not you're going to encounter proxied state at some point. sveltejs/kit#12095 is such an example

FWIW I think sveltejs/kit#12095 is probably a bad example — I think we're looking at it from the wrong angle. Commented on that thread.

[Rich-Harris]

I'm good with us not recursing into POJOs, as long as we've considered it and we're doing it on a principled basis. I do think we should handle classes-with-state-fields and svelte/reactivity though, whether in this PR or a follow-up

[trueadm]

Renamed to $state.snapshot.