sveltejs / svelte

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

Spreading in the template isn't a potentially reactive operation #13922

Open 7nik opened 1 week ago

7nik commented 1 week ago

Describe the bug

<span>{ [...array] }</span>

is compiled as static text when the variable isn't marked as reactive.

Though I doubt people use it in the prod (at least it should be array.join(", ")), they can do it during development.

Reproduction

REPL

Logs

No response

System Info

Svelte 5.1.2

Severity

annoyance

webJose commented 1 week ago

Unsure what you want to happen. Do you want arr2 to behave the same as arr1? If yes, your code needs to use $state() for arr2 as well.

<script>
    let arr1 = $state([]);
    let arr2 = $state(arr1);
</script>

The above works.

7nik commented 1 week ago

You don't need to statify a stateful value. Also, in the original case, it was

let arr = getContect("myarr"); // returns stateful array

and it can be any other function.

webJose commented 1 week ago

Ah, I think I see what confuses you. Assigning a stateful value to a variable doesn't carry the same effect. You see, runes tend to fool us by looking exactly like functions, but they don't work like functions. The reactive nature of a value is not carried over or copied by virtue of the assignment operator.

Someone at some point suggested to remove runes and instead do this syntax:

let state arr1 = []; // let arr1 = $state([]);
let arr2 = arr1; // What you have, but do you see the difference?

The $state() rune, no matter how identical-looking to a regular function seems to be, is not a function and does not produce an R-value. The effect of using $state is more of an L-value.

Prinzhorn commented 1 week ago

In addition to what @webJose said see https://github.com/sveltejs/svelte/issues/13306 for related discussion (https://github.com/sveltejs/svelte/issues/13306#issuecomment-2358591827)

"Solution" is to use $derived https://svelte.dev/playground/hello-world?version=5.1.2#H4sIAAAAAAAAA22PwQqDMBBEfyWEHhRKpB6tCv2OmEPUbQ2NMSSrtEj-vcYe6qGXZXfezMCu9K40eFrwleLbAi12gZ6pkWO8btYyv4DGqLXSwz-9mwyCwa2Glr5zymLdmAY1IJHOXUhFTh4lQsJFej2QPJIenFqgT6Iz0jL7VZiynREnQybTadU9qzVJSVXvrczOfthTTIN54JCGWvY9UQhjmX1zdWxw24y2gvCVM8biLoI4oPyA8h1tTyG8kBboZggifABrk329KAEAAA==

Prinzhorn commented 1 week ago

~Never mind, this actually looks like a bug:~ See edit

https://svelte.dev/playground/hello-world?version=5.1.2#H4sIAAAAAAAAA22PwQrCMBBEfyUEDy1I1B5rW_A7Yg5pXbWYpiHZihLy7yYtYg9eltmZtwPr6bVX4GjJPcW3AVrOBt1SLYe0nYxh7gkKk9dKB__8btQIGmMNrVxne4PNWZ9RARJp7YHUZONQImRc5MdVUsQkAdGrdr9DXbUT4qjJqDvVd4_aZzmpmxllZnL3bFYK9A3veWjk5UJ6hKHaLXdNarBxJqwk3HPGWNIiCOKT4HsRVlCxgoovVCxQfA_hhbREO0EQ4QNj2GMXMgEAAA==

Why would adding {arr2[0]} make the entire arr2 reactive?

Edit: OHHHHHHH, because it's the same text node. So it's just a side-effect:

https://svelte.dev/playground/hello-world?version=5.1.2#H4sIAAAAAAAAA3WQ0WrEIBBFf0WkDwkU0-YxNcJ-h_HBTaZdadaITpYW8d-rlqWBti_DnTtnrjKRvpoVAh1kpPjpgA7VoI_U6mvpTs6xcIMVi3fWAf7y580iWMwxlIfZG4dishOugER7_0xG8hBQIzRStS-HSZ8nBcge734WLT_viJslm51XM7-PsWnJKCrK3B4uTVUr2De8tEnoZSEG4cq77z1REnyuBRuIjJIxVrRKisQi5JNKd4gv5ibKZw5kfyf7QvKuIDXuf-jw5i-IB6etOOTVfrL5dAgfSAf0OySVvgDKhG7AjgEAAA==

7nik commented 1 week ago

The problem is that both arr1 and arr2 contain a reactive value (though arr1 is additionally marked as reactive because it is defined with a rune), but the compiler compiles

arr1: [{[...arr1]}]
<br>
arr2: [{[...arr2]}]

as

$.template_effect(() => $.set_text(text, ` arr1: [${[...arr1] ?? ""}] `)); // reactive
text_1.nodeValue = ` arr2: [${[...arr2] ?? ""}]`; // static

I assume this happens because, for performance reasons, printing a bare variable (<span>{foo}</span>) is considered reactive only if the variable is defined with a rune. And a bare spreading falls in this category, but I think it shouldn't.

Prinzhorn commented 1 week ago

but I think it shouldn't.

I think that's fundamentally how runes work, from what I understand. Compiler does not know arr2 is reactive. You need to mark it as such, e.g. using $derived.

7nik commented 1 week ago

Yes, it doesn't know, but the spreading should be considered potentially reactive, like reading a prop {foo.bar}.

webJose commented 1 week ago

Yes, it doesn't know, but the spreading should be considered potentially reactive, like reading a prop {foo.bar}.

Props are reactive by definition, as I understand things, so I don't think it's the same thing. Just because props is spreaded, doesn't mean that anything that spreads needs to get the same treatment.

In other words: Spreading props get the dynamic treatment because they are reactive, not because they have the potential to be spreaded.

7nik commented 1 week ago

I meant an object's prop but not a component prop. My bad. Reading an object's property will always result in the dynamic output, even if it is obviously static.

Spreading a bare variable in the template is rare, so making it reactive will have almost no effect on performance and bundle size, so I don't see reasons not to do so.

Prinzhorn commented 1 week ago

Your REPL is not in runes mode, I think this makes a big difference when discussing reactive behavior.

7nik commented 1 week ago

If you mean REPL with static prop - yes, though the output is the same in the rune mode.

trueadm commented 1 week ago

We do this intentionally, if you want to mark a variable something as reactive, it must be $state.