sveltejs / svelte

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

Svelte 5 Exporting $state compile problem #14316

Open jdgamble555 opened 4 hours ago

jdgamble555 commented 4 hours ago

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

REPL

Logs

Error message above.

System Info

Svelte 5.2.0

Severity

annoyance

dummdidumm commented 3 hours ago

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 commented 2 hours ago

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 commented 1 hour ago

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 commented 1 hour ago

I also found certain restrictions are overly strict, e.g.

Svelte compiler doesn't allow this for existing objects:

    const obj = {};

    obj.count = $state(0);

So, the workaround would have to be this but it's pretty cumbersome:

    const obj = {};
    let count = $state(0);
    Object.defineProperty(obj, 'count', {
        get: function() { return count },
        set: function(v) { count = v},
        enumerable: true,
    });

This works though since obj becomes a proxy but the whole object already has to be a state:

    const obj = $state({});

    obj.count = 0;

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 both writable and a setter: Invalid property descriptor. Cannot both specify accessors and a value or writable attribute

    const obj = $state({});
    let count = $state(0);
    Object.defineProperty(obj, 'count', {
        get: function() { return count },
        set: function(v) { count = v},
        enumerable: true,
                configurable: true,
    });
trueadm commented 24 minutes ago

Just make the original object state, then you don’t need to create state to assign to it.