sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
80.2k stars 4.27k forks source link

Update of $state variable overwrites it instead of update #13306

Open ronny-rentner opened 2 months ago

ronny-rentner commented 2 months ago

Describe the bug

Update of $state variable overwrites it instead of update when the state variable was returned by a function.

As you can see in the 2nd log line, count2 is not a state proxy anymore.

<script>
//App.svelte
    import { createStore } from './Stores.svelte.js';

    let count = $state({});
    let count2 = createStore();

    console.log('stores', count, count2);

    count = { some: 'value' };
    count2 = { some: 'value' };

    console.log('stores after', count, count2);

</script>
//Stores.svelte.js
export function createStore() {
  let store = $state({});
    return store;
}

Reproduction

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAA3WRTWrDMBCFrzKIghwINnhpp4WcIcu6C1cZFwVZMtIobRG6e-UfEps0aCFmnt684VNgnVToWPUemG57ZBU7DgPbM_odxsJdURGm2hlvxdg5OGHlQG-NTodkPxhLEEBYbAlPZCxChM6aHnheTLXL5yn5xfF6tikkEMZrgld4cZScWYi7eiOVSVtNzXaLVxjtjMJcma-MuymA72fLcpX3p3NEAGd6rIBfW-WRQ6xvavlM_j8J2o7QPssrihMifFrz7dDCYm_0oVgxSyx7c5adxDOryHqM-xv6Da77J1zc-gPwZ0LeeS1IGr1lBKHRACPDad9HvBbJWz2rqREf1_mIf_bmXW4VAgAA

Logs

- stores Proxy(Object) {} Proxy(Object) {}
- stores after Proxy(Object) {some: 'value'} {some: 'value'}

System Info

Svelte 5.0.0-next.251

Severity

blocking an upgrade

brunnerh commented 2 months ago

That is the expected behavior if you do not declare the local variable as $state. (And assignments will never affect the previous object; it just changes the reference to a different object.)

ronny-rentner commented 2 months ago

That's really confusing "magic" behavior. It's also not mentioned anywhere in the docs.

dummdidumm commented 2 months ago

In which way is it confusing? What was your expectation? That reassigning the value does mean that the store variable inside createStore is updated and therefore the new value is also proxified? In which way could we document that to make this more clear?

ronny-rentner commented 2 months ago

My expectation is that count2 does behave the same way as count when assigning a new value. After all they're the same before.

If I may quote https://svelte.dev/blog/runes: The reality is that as applications grow in complexity, figuring out which values are reactive and which aren't can get tricky.

So I am now in the same trap, figuring out which values are reactive and which ones not.

ronny-rentner commented 2 months ago

PS: I've been using writable() before and those behave the same, if you return them from a function or not.

dummdidumm commented 2 months ago

It sounds like there's a missing piece in the understanding of runes yet, which is that there is no value container, and as such whenever you return a value you "freeze" it at that point in time, just how regular JavaScript does it.

function foo() {
  let x = { y: 'z' }
  return x;
}

let x = foo();
x = { y: 'zzz' } // this will have no influence on x inside foo

The same semantics are true for runes. To keep them alive you need to use getters/setters - just like you would in regular JavaScript.

function foo() {
  let x = { y: 'z' }
  return { get x() { return x }, set x(v) { x = v } };
}

let value = foo();
value.x = { y: 'zzz' } // this write back to x inside foo
ronny-rentner commented 2 months ago

Well, if I do

    let proxy = new Proxy({}, {});
    proxy = { some: 'value' };

then the variable proxy is not a proxy anymore afterwards, same as count2. So far that's expected. Just that count continues to be a proxy after overwriting it, that's unexpected magic of Svelte.

I am not against the magic but it should happen consistently all the time, not just sometimes, or not at all.

Prinzhorn commented 2 months ago

Sorry for hijacking this issue, but I meant to open a discussion about this at some point anyway. And it fits :confetti_ball: perfectly :confetti_ball: here. @ronny-rentner's reaction and interpretation of the code is perfectly normal and I think the Svelte team might not see this any longer since they are already used to it.

In which way is it confusing?

It's confusing in the following fundamental way: $state is right-hand side but logically it's left-hand side. let count = $state({}); is actually more like state count = {};. I would assume everyone new to Svelte will find this confusing at first. Because you are not assigning $state({}) to count (which is happening from a syntax point of view) but you are telling the compiler to turn count into a special reactive sink. If you later do count = { some: 'value' }; it suddenly is an assignment but it doesn't undo the previous $state({}) one. That's what Svelte 4 reactivity got right by having $: on the left-hand side and by making all assignments reactive by default.

I assume the underlying issue is that Svelte still want's to be syntactically valid js? It was labels ($:) before and now it's dollar prefixed runes. I assume there aren't enough (elegant) options to make it left-hand side? Having $state on the right-hand side is IMO wrong and counterintuitive.

brunnerh commented 2 months ago

As far as I know options like using a $state: label have been considered but there were other issues with that approach.

Syntax being valid JS is a requirement to have all the existing tooling work correctly. Svelte could be much nicer with a custom domain-specific language, but this would hurt adoption and the editing experience massively.

brunnerh commented 2 months ago

I think the Svelte team might not see this any longer since they are already used to it.

That is also exactly why this is not a real problem: You get used to it.

It needs to be properly documented, though.

Prinzhorn commented 2 months ago

That is also exactly why this is not a real problem: You get used to it.

"You get used to it." is not really what I want to hear from the most basic building block of the most loved (including myself!) web framework :smile:. I can live with $state() but you have to admit it's a rough edge and goes against everything people know about how variables work in many languages.

ronny-rentner commented 2 months ago

"You get used to it." is a great argument for many problems to ignore them ;)

brunnerh commented 2 months ago

@Prinzhorn $: is also a basic building block of Svelte 3/4 and it's also weird in its own way that requires getting used to. Same for the whole reactivity system around stores. Or hacks like thing = thing dummy assignments used to force an update.

Svelte is not perfect and there will always be trade-offs. I was also very skeptical of the changes in Svelte 5 and still am to some degree. The learning curve for the new primitives might be a bit more steep and they come with a bunch of new edge cases but the fact that constructs like stores can be replaced and reactivity extended to plain JS files might be worth it in the end.

I find it hard to say without extensive user data on the subject.

@ronny-rentner Some people might call it "learning"

ronny-rentner commented 2 months ago

@brunnerh As you've said, "there will always be trade-offs". I think the trade-off is out of balance here. It should be corrected to a consistent and expectable behavior with fewer things to learn. After all that's one of the declared goals in the blog post https://svelte.dev/blog/runes .. "But newcomers won't need to learn all those things"

brunnerh commented 2 months ago

The system is consistent, it just has a different set of rules you need to learn. Runes are not regular functions so one needs to know what exactly they do and how variables annotated with them behave. It's more complicated than the previous system, but also more powerful and arguably more consistent (e.g. reactive let/$: only works at the top level and only within components, $state/$derived works everywhere).

Given that the Svelte 5 release is in its final phase with documentation being written and the last few bugs being resolved, fundamentals like its core syntax around $state are highly unlikely to be changed unless someone can propose a radically superior system. This is also unlikely since the team tested dozens of different alternatives before settling on the current system.

So as I see it, the best course of action is to explain things clearly, so people can use the new system with as little confusion as possible.

henrikvilhelmberglund commented 2 months ago

@brunnerh As you've said, "there will always be trade-offs". I think the trade-off is out of balance here. It should be corrected to a consistent and expectable behavior with fewer things to learn. After all that's one of the declared goals in the blog post https://svelte.dev/blog/runes .. "But newcomers won't need to learn all those things"

For me the trade-off is out of balance in a positive way, I can use reactivity in the same component just by adding $state() and even outside of the component by using a function (or class) that uses getters/setters. Since that works you don't really have to use stores so newcomers won't have to learn how to use those, meaning there are fewer things to learn.

After reading https://svelte-5-preview.vercel.app/docs/universal-reactivity (and the general rune introduction) I knew everything I needed to know so I don't really see how this is hard to learn.

FoHoOV commented 2 months ago

@brunnerh @dummdidumm I think you all are missing the point and derailing from what the OP said. THIS IS WHAT HE IS TALKING ABOUT:

It sounds like there's a missing piece in the understanding of runes yet, which is that there is no value container, and as such whenever you return a value you "freeze" it at that point in time, just how regular JavaScript does it.

function foo() {
  let x = { y: 'z' }
  return x;
}

let x = foo();
x = { y: 'zzz' } // this will have no influence on x inside foo

The same semantics are true for runes. To keep them alive you need to use getters/setters - just like you would in regular JavaScript.

AND

Well, if I do

  let proxy = new Proxy({}, {});
  proxy = { some: 'value' };

then the variable proxy is not a proxy anymore afterwards, same as count2. So far that's expected. Just that count continues to be a proxy after overwriting it, that's unexpected magic of Svelte.

I am not against the magic but it should happen consistently all the time, not just sometimes, or not at all.

I think OP is thinking $state is function call that returns X and when we do

let a = $state(x)
a = somethingelse;

the assignment should not cause reactivity cause of what @dummdidumm said himself. The fact that it causes reactivity in the same scope and doesnt when its cross boundry, is the incosistency that OP is talking about, at least this is what i think.

nmzein commented 2 months ago

I don't feel like this is an inconsistency though. That function returns a value not a reference. let x = foo() calls the function foo once and gets a value. This is just how JS works. It wouldn't make sense for that value to live past the lifetime of that function as state unless explicitly specified using get. I get that it might initially feel wrong but personally I think it would actually be diverging from JS if it were reactive. Happy to be wrong but I don't agree that this is a problem.

dummdidumm commented 2 months ago

The fact that it causes reactivity in the same scope and doesnt when its cross boundry, is the incosistency that OP is talking about, at least this is what i think.

This is my understanding aswell, and my response is "we need to properly explain this in the tutorial/documentation". Ultimately it's "just JS". On reassignment the value becomes something else. $state is really the exception here - and I can relate to the "that part is weird" feeling - because when you reassign the variable, it becomes something else, but the reactivity stays (I don't know how to better express this, but I think we're all on the same page by now).

Antonio-Bennett commented 2 months ago

The only way I can think is that in svelte files we are able to have a slackness of JS rules to an extent. Crossing file boundaries adheres to the rules of JS before svelte’s. So if you are crossing boundaries then yes you need getters and setters but locally declaring state in svelte files allows you to have a few niceties. I admit I reread this issue a few times to really grasp what’s being said but after coming to this mental model I think that as stated above it is one of the things that can be learned and is not too much of a burden in my opinion.