-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Open
Labels
Description
Describe the bug
If I want to export a shared state, the normal way is to do like this:
Option 1
// rune.svelte.ts
export const rune = <T>(initialValue: T) => {
let _rune = $state(initialValue);
return {
get value() {
return _rune;
},
set value(v: T) {
_rune = v;
}
};
};
Option 2
This also works:
// rune.svelte.ts
export const rune = <T>(initialValue: T) => {
const _rune = $state({ value: initialValue });
return _rune;
};
Option 3
However, if I simplify option 2, I get a compile error:
export const rune = <T>(initialValue: T) => $state({ value: initialValue });
// OR
export const rune = <T>(initialValue: T) => {
return $state({ value: initialValue });
};
I get the error:
$state(...)` can only be used as a variable declaration initializer or a class field
I would expect Option 3 to compile the exact same way as option 2 !?
Reproduction
Logs
Error message above.
System Info
Svelte 5.2.0
Severity
annoyance
Metadata
Metadata
Assignees
Labels
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
dummdidumm commentedon Nov 15, 2024
This is somewhat on purpose - we want to restrict the places where you can use runes to not make it look like it's something that it isn't. We're open to loosen that in the future if more use cases come up / we're confident that the rules of runes are well understood enough.
webJose commentedon Nov 15, 2024
Can this be made to work, @dummdidumm ? It is my understanding that $state is more an L-value than what it looks like (R-value).
trueadm commentedon Nov 15, 2024
What we could do is permit
return $state()
but add a runtime check in DEV that checks if the object is actually a proxy and throw an error/warning if not the case.Leonidaz commentedon Nov 15, 2024
I also found certain restrictions are overly strict, e.g.
Svelte compiler doesn't allow this for existing objects:
So, the workaround would have to be this but it's pretty cumbersome:
This works though since obj becomes a proxy but the whole object already has to be a state:
Incidentally, using
Object.defineProperty
on a proxified / state wrapped object doesn't work, in case you want to add a custom getter or setter, and results in an error:state_descriptors_fixed Property descriptors defined on `$state` objects must contain `value` and always be `enumerable`, `configurable` and `writable`.
if
writable: true
is added, the JS blows up as you can't specify bothwritable
and a setter:Invalid property descriptor. Cannot both specify accessors and a value or writable attribute
trueadm commentedon Nov 15, 2024
Just make the original object state, then you don’t need to create state to assign to it.
Leonidaz commentedon Nov 15, 2024
right, i have it outlined in my comment above. just saying that if there is a non-state object, it's cumbersome to add properties to it vs just a simple assignment. And, for a state object defining custom getters and setters for a property doesn't work.
trueadm commentedon Nov 15, 2024
You're trying to add a reactive state of
0
, that simply won't work. The value is a primitive, so it won't be reactive as there's no way for the runtime to capture the signal without encapsulation. Maybe we can loosen rules around this for proxied state though, as that can encapsulate reactivity on assignment.Please can you create a separate issue for that, seems like a bug.
Leonidaz commentedon Nov 16, 2024
Just for clarity, what I was aiming for is for svelte to perform a code transformation when assigning state directly.
assigning state with primitives:
svelte compiles into something like this:
and for assigning state with objects:
svelte compiles into something like this:
👍 done! #14320
trueadm commentedon Nov 16, 2024
That's far too magical and simply assumes too much. If
obj
already has a setter forobj.count
then that should be invoked rather than defining a new property. Not to mention if it was something like thisobj[i] = $state(…)
ifobj
is an array then you're going to cause all sorts of problems withlength
etc.Oh and
Object.defineProperty
is absolutely terrible for performance, so it's best to avoid in hot paths.jdgamble555 commentedon Nov 17, 2024
@dummdidumm @trueadm - On the original issue, is it possible to get the return value to work?
Thanks!
J
Leonidaz commentedon Dec 8, 2024
FWIW,
Object.defineProperty
seems to be outperforming setting properties directly on aProxy
. The fastest way is to set a direct property on an non-proxied object.https://www.measurethat.net/Benchmarks/Show/32930/5/objectdefineproperty-vs-objectassign-vs-assign-to-proxy
The
Object.defineProperties
(plural), on the other hand, yeah that one is slowtrueadm commentedon Dec 8, 2024
Direct object property is almost double the test for me
Leonidaz commentedon Dec 8, 2024
weird, on a Proxy?
This is what I'm getting on Chrome Version 131.0.6778.109 (Official Build) (x86_64)
Latest Safari seems to be faster but the results are pretty much the same:
Direct property on a POJO, just below these two, is definitely the fastest.
trueadm commentedon Dec 8, 2024
It appears you've not setup the Proxy properly in your benchmark, you're only handling
set
, but notdefineProperty
?Leonidaz commentedon Dec 8, 2024
I was just testing pass throughs without traps. But yeah, if we add a trap for
defineProperty
that will slow it down vs direct prop assignment on proxy that doesn't go through this trap. I'm not sure though why add a trap for defineProperty? Unless it's in dev for validation.with defineProperty, get, set traps:
https://www.measurethat.net/Benchmarks/Show/32930/6/objectdefineproperty-vs-objectassign-vs-assign-to-proxy
trueadm commentedon Dec 8, 2024
Our internal proxy implementation has it, as we don't allow modifications to the underlying target object.
jdgamble555 commentedon Jan 20, 2025
@trueadm - Any note on my original post?
Could we still permit
return $state()
to work?Thanks!
J
$state
in return statements #15589