Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Runes: $member rune to reduce getters and setters boilerplate #9289

Closed
JakeBeaver opened this issue Oct 3, 2023 · 24 comments
Closed

Runes: $member rune to reduce getters and setters boilerplate #9289

JakeBeaver opened this issue Oct 3, 2023 · 24 comments
Labels

Comments

@JakeBeaver
Copy link
Contributor

JakeBeaver commented Oct 3, 2023

Describe the problem

I love how runes can massively reduce the global state complexity.
I am also fully on board with using getters and setters so as to simplyfy the usage of the code we write.

However, because I expect we'll be writing a lot of getters and setters, I feel there should be a way, in true svelte fashion, to compile away this all too common boilerplate.

Describe the proposed solution

While playing with the syntax in the preview playground, I came across this idea:

We could write this - note the $object() call:

//// written code
let text = $state("");

export const todo = $object({
    text,
    done: $state(false)
});

And before the compiler transforms the $state rune into signals, the $object rune could get lowered, filling in the getters and the setters.

//// lowered code
let text = $state("");

// generated properties
let _done = $state(false);

export const todo = {
    get text() { return text; },
    set text(v) { text = v; },
    get done() { return _done; },
    set done(v) { _done = v; }
};

This would still allow for extending the objects with $derived signals and, still, getters and setters

such that this:

//// written code
let text = $state("");

export const todo = $object({
    text,
    done: $state(false),
 
    upperCaseText: $derived(text.toUpperCase()),
    get lowerCaseText() { return text.toLowerCase(); },
});

becomes this:

//// lowered code
let text = $state("");

// generated properties
let _done = $state(false);
let _upperCaseText = $derived(text.toUpperCase())

export const todo = {
    get text() { return text; },
    set text(v) { text = v; },
    get done() { return _done },
    set done(v) { _done = v; },
	
    get upperCaseText() { return _upperCaseText; },
    get lowerCaseText() { return text.toLowerCase(); },
};

I don't know too much about the under the hood of compiling runes, so I'm not sure about how hard the implementation would be, but because it's mainly code lowering, seems ok-ish?

I'd love have someone smarter disilusion me of this idea!

Alternatives considered

c# like private int a { get; set; } auto property syntax - not possible with standard js
plain js functions for creating compound objects - still seems boilerplatey and complicated to use

Importance

nice to have

@SBHattarj
Copy link

You can check my solution here, I personally think that $expose is a better syntax, but that might just be my opinion. I don't really like the idea of creating state in an object, but can see the apeal for it.

@JakeBeaver
Copy link
Contributor Author

Oh, this is really cool, I didn't think about modifying a function on the fly, I might try prototyping my idea using this.

Regarding the $expose rune, it's cool that it saves having to write out both getter and setter, but I'd also love to be able to define state on the fly, like:

export const counter = {
   count: $expose($state(0))
};

And in this case, I think putting a rune before the full object is a little more readable

export const counter = $object({
   count: $state(0)
});

The perfect DX I think lies somewhere in between

@JakeBeaver JakeBeaver changed the title Idea: $object rune Runes: $object rune to reduce getters and setters boilerplate Oct 4, 2023
@dominikg dominikg added the runes label Oct 4, 2023
@SBHattarj
Copy link

SBHattarj commented Oct 4, 2023

If I could directly modify what svelte is doing or add a preprocessor I can probably get $object to work, otherwise it's tricky, in the repl you can't use $state inside an object (probably because that could potentially expose a raw signal, if not handled properly). I'm not entirely sold on $object, and I think rather than $expose($state(...)) it'd be better to have something like $stateMember, I think it's best to have $state exclusive to assignments. Similarly would be $derivedMember, and then there could be an object version of them as $stateMembers and derivedMembers. Then rather than $object which to me is very vague we can have $exposeMembers or something similar (I'm not good at naming.

Then some object might be created as follows

let state1 = $stare()
let stare2 = $state()
let someOther = $state()
// only state
export const stateObj = {
    someOther: $expose(someOther),
    singleState: $stateMember(0),
    // here we should be able to use any reactive value
    dirivedSingle: $derivedMember(state1 * state2),
   // these may also be allowed to be standalone runes, in which case the output might change a little
    ...$exposeMembers({
        state1,
        state2
    }),
    ...$stateMembers({
        state3: 0,
        state4: 1,
    }),
    // here we should be able to use any reactive value
    ...$derivedMembers({
        state1PState2: state1+state2,
        state1MState2: state1-state2
    })
}

which then compiles to equivalent of

let state1 = $stare()
let stare2 = $state()
let someOther = $state()
let singleState_randomstr = $state(0)
let state3_randomstr = $state(0)
let state4_randomstr = $state(1)
// only state
export const stateObj = {
    get someOther() {
        return someOther
    },
    set someOther(someOtherNew) {
       someOther = someOtherNew
    },
    get singleState() {
        return singleState_randomstr
    },
    set singleState(singleStateNew) {
        return singleState_randomstr = singleStateNew
    },
    // here we should be able to use any reactive value
    get derivedSingle() {
       return state1 * state2
    },
   // these may also be allowed to be standalone runes, in which case the output might change a little
    get state1() {
        return state1
    },
    set state1(state1New) {
        return state1 = state1New
    },
    get state2() {
        return state2
    },
    set state2(state2New) {
        return state2 = state2New
    },
    get state3() {
        return state3_randomstr
    },
    set state3(state3New) {
        return state3_randomstr = state3New
    },
    get state4() {
        return state4_randomstr
    },
    set state4(state4New) {
        return state4_randomstr = state4New
    },
    // here we should be able to use any reactive value
    get state1PState2() {
        return state1+state2
    },
    get state1MState2() {
        return state1-state2
    },
}

How does this look?

If this looks like it has potential I can try to get this to work.

@JakeBeaver
Copy link
Contributor Author

The concept looks really good actually!

I'd only be worried that we'd be proposing many new runes

But I do love a dedicated rune for creating state ad hoc. So maybe it would be doable to add only one rune, but a special one that is an object containing functions, rather than being just a function itself - this I would imagine could be used for intellisense down the line

I was thinking $prop, but that is part of some other discussions and probably will be too easily confused with $props, so $member sounds good to me.

$member rune syntax proof of concept:

let count = $state(0)
// only state
export const stateObj = {
    countReadWrite: $member.of(count),
    countReadOnly: $member.get.of(count),
    // this one maybe should error if count was a derived?
    countWriteOnly: $member.set.of(count),

    stateReadWrite: $member.state(0),
    stateReadOnly: $member.get.state(0),
    stateWriteOnly: $member.set.state(0),

    // derived would never expose a set I don't think
    derivedReadOnly: $member.derived(0)
}

This kind of feels good to write. I by no means presume there isn't room for improvement though!

The destructuring also looks interesting, although feels like kind of a bother to implement. Maybe it could fit in like ...$member.list.of({})?

@SBHattarj
Copy link

SBHattarj commented Oct 4, 2023

That actually seem nice. I'd personally not have .of, but do $member() instead. Other changes would be as follows:

  1. Similarly to the first point then we will only have .get() and .set().
  2. .state() only, having only a getter or setter of a state which is never accessed else where seems redundant (only having accessor mean it would never change, and only having setter means it can be never read)
  3. I was never particularly fond of having multiple properties defined (I think I was clear that I didn't like the idea much, I guess my wording wasn't good), but if necessary I'll much more prefer $members, this will be inline with $prop and $props if implemented (I like the idea of $prop personally btw), where $props is already there for multiple prop, while $prop is being proposed for singular prop. I don't think the destructuring will be that much of an bother honestly, if the singular prop is implemented correctly, the destructured props can potentially be about 20 lines of code, maybe a little more or even a little less.

Hope my points makes sense, for a better understanding look at the code bellow:

let count = $state(0)

export const statObj = {
    countReadWrite: $member(count),
    countReadOnly: $member.get(count),
    countWriteOnly: $member.set(count),
    state: $member.state(0),
    stateReadOnly: $member.state.get(0),
    // The above is never gonna change so might as well just have it be a static value
    stateWriteOnly: $member.state.set(0),
    // This can never be read, setting this variable allows you to do a 
    // grand total of nothing through assignments to this member other than 
    // waste a few lines of code
    

    // obviously you want the derived variable to always be in sync with it's dependant,
    // if you are allowed to change the value that will mean it gets desynced 
    // for some amount of time until one of it's dependent change, 
    // which can be really confusing
    // That's why it can and will only define a getter
    derived: $member.derived(2 * count)
    // the destructured one
    ...$members({count})
    // or maybe
    ...$member.multiple({count})
    // maybe even make it .multy
}

@JakeBeaver
Copy link
Contributor Author

Absolutely love what you've got there!

Haha, I totally missed how pointless $member.state.get() is, 100% agreed.

$members({count}) also sounds great. I was more wondering about the destructured equivalent of $member.state, but with what you've written, doing full count: $member.state(0), for each property is actually less code, and destructuring an aggregate of multiple external signals is quite enough.

Last thing I'm wondering about is what if you'd want to do more logic in the setter. I'm thinking either a mapping function as an optional second parameter of $member.set. Or just fall back to normal setters? Not sure. I think I kind of like the parameter, makes things a bit more consistent, no jumping between mental abstraction layers

let count = $state(0)

export const statObj = {
    // $member rune vs just a setter to compare
    countStr: $member.set(count, (v) => +v),
    set countStr2(v) { count = +v; },

    // and fully on board with the rest
    countReadWrite: $member(count),
    countReadOnly: $member.get(count),
    countWriteOnly: $member.set(count),
    state: $member.state(0),
    derived: $member.derived(2 * count),
    ...$members({count})
}

@JakeBeaver JakeBeaver changed the title Runes: $object rune to reduce getters and setters boilerplate Runes: $member rune to reduce getters and setters boilerplate Oct 4, 2023
@brunnerh
Copy link
Member

brunnerh commented Oct 4, 2023

Related, where I proposed something analogous to $exposeMembers:

Currently know of no reason why $state could not just be used on object/class properties (see this comment). Introducing new runes just for that seems a bit overkill.

@SBHattarj
Copy link

SBHattarj commented Oct 4, 2023

The problem with using $state in an object that I see is that, in object it acts completely differently when compared to outside an object, given that it converts to a getter, setter. That to me would be confusing, otherwise, it'll expose a signal to the developer, which svelte seemingly doesn't want.

@JakeBeaver
Copy link
Contributor Author

Related, where I proposed something analogous to $exposeMembers:

Currently know of no reason why $state could not just be used on object/class properties (see this comment). Introducing new runes just for that seems a bit overkill.

Using $state for property initializers might actually have something. If we kept with the last snippet and lowering, we could arrive at something like this?

let count = $state(0)

export const statObj = {
    // cool shorthand
    state: $state(0),
    derived: $derived(2 * count),

    // but we'd still need these, no?
    countReadWrite: $member(count),
    countReadOnly: $member.get(count),
    countWriteOnly: $member.set(count),
    ...$members({count})
}

@JakeBeaver
Copy link
Contributor Author

JakeBeaver commented Oct 4, 2023

also maybe we use named properties in an object approach?

let count = $state(0)

export const statObj = {
    countReadWrite: $member(count),
    countReadOnly: $member(count, {get: count}),
    countWriteOnly: $member(count, {set: v=>count=v}),

    doubleReadWrite: $member(count, {get: count*2, set: v=>v/2}),
    doubleReadOnly: $member(count, {get: count*2}),
    doubleWriteOnly: $member(count, {set: v => v/2}),
}

or something like discards for getters and setters?

let count = $state(0)

export const statObj = {
    countReadWrite: $member(count),
    countReadOnly: $member(count, _),
    countWriteOnly: $member(_, v => count = v),

    doubleReadWrite: $member(count*2, v => count = v/2}),
    doubleReadOnly: $member(count*2, _),
    doubleWriteOnly: $member(_, v => count = v/2}),
}

doesn't seem to flow...

@SBHattarj
Copy link

I think $member.set, $member.get is just better in this case

@SBHattarj
Copy link

We could have the object for modification of behaviour in case of a $member, rather than to define only getter or only setter

@JakeBeaver
Copy link
Contributor Author

I think $member.set, $member.get is just better in this case

looks much better, agreed

We could have the object for modification of behaviour in case of a $member, rather than to define only getter or only setter

Sorry, I don't quite understand what you mean, is this about a way around mapping functions in getters and setters?

@SBHattarj
Copy link

We could have the object for modification of behaviour in case of a $member, rather than to define only getter or only setter

Sorry, I don't quite understand what you mean, is this about a way around mapping functions in getters and setters?

Yes. Something like this:

{
    countStr: $member(count, {
        // even if you only define one of these both are defined just without a mapper function
        get(count) { return count.toString() },
        set(count) { return +count }
    })
}

@brunnerh
Copy link
Member

brunnerh commented Oct 4, 2023

Using $state for property initializers might actually have something. If we kept with the last snippet and lowering, we could arrive at something like this?

let count = $state(0)

export const statObj = {
    // cool shorthand
    state: $state(0),
    derived: $derived(2 * count),

    // but we'd still need these, no?
    countReadWrite: $member(count),
    countReadOnly: $member.get(count),
    countWriteOnly: $member.set(count),
    ...$members({count})
}

This looks pretty good to me. For completeness $members could be extended with analogous $members.set and $members.get that expose multiple getters/setters.

Also, renaming the resulting properties could potentially be implemented as well:

let localName = $state();
return {
   ...$members({ propName: localName })
}

@JakeBeaver
Copy link
Contributor Author

How's this?

let count = $state(0)

export const statObj = {
    state: $state(0),
    derived: $derived(2 * count),

    countReadWrite: $member(count),
    countReadOnly: $member.get(count),
    countWriteOnly: $member.set(count),

    // optional second parameter
    doubleReadWrite: $member(count, {
        get: (c) => c * 2, 
        set: (v) => v / 2}),
    doubleReadOnly: $member.get(count, (c) => c * 2),
    doubleWriteOnly: $member.set(count, (v) => v / 2),

    // and best not have behavior change for multiples, would get confusing fast
    ...$members({count})
    ...$members.get({readonlyCount: count})
    ...$members.set({writeonlyCount: count})
}

@JakeBeaver
Copy link
Contributor Author

JakeBeaver commented Oct 4, 2023

maybe we shouldn't worry about modified $member.get because its actually covered by a $derived?

edit: or the whole modify getter setter behavior is out the window, because it's probably going to break the signal reactivity?

@brunnerh
Copy link
Member

brunnerh commented Oct 4, 2023

Should not be an issue for signals. Anything that uses the properties will trigger the state signal, you can just test this.

The single $member.get/set with modification is so long, that you might as well just completely drop it, though:

{
  doubleReadOnly: $member.get(count, c => c * 2),
  // vs
  get doubleReadOnly() { return count * 2 }
}

@JakeBeaver
Copy link
Contributor Author

JakeBeaver commented Oct 4, 2023

Too true. Also, for getters, it's even easier to go with $derived

{
  doubleReadOnly: $member.get(count * 2)
  // vs
  get doubleReadOnly() { return count * 2 }
  // vs
  doubleReadOnly: $derived(count * 2)
  // im also wondering about changing $member to $field
  doubleReadOnly: $field.get(count * 2)
}

That does however leave me still with some desire for consistency

const a = {
    text: $member(text), 
    textReadOnly: $member.get(text),
    textWriteOnly: $member.set(text),

    count: $state(0),
    double: derived(count * 2),
    set triple(v) { count = v / 3 },
    get quad() { return count * 4;},
    set quad(v) { count = v / 4; },
}
// vs
const b = {
    text: $member(text), 
    textReadOnly: $member.get(text),
    textWriteOnly: $member.set(text),

    count: $state(0),
    double: $member.get(count * 2),
    triple: $member.set(v => count = v / 3 ),
    quad: $member
        .get(count * 4)
        .set(v => count = v / 4),
}

I think I would still love to have this as an option.

Though I'm not so sure about overloading $member.set() to accept either a value to assign to, or an assigning function

@JakeBeaver
Copy link
Contributor Author

JakeBeaver commented Oct 4, 2023

or maybe a $get rune and a $set rune that can be chained to $get().set()?

const count = $state(0);

const a = {
    count,
    count2 = count,
/// get countReadOnly(): { return count; },
/// countReadOnly: $derived(count),
    countReadOnly: $get(count),
/// set countWriteOnly(v): { count = v; },
    countWriteOnly: $set(count),

    double: $get(count * 2).set(v => text = v),
/// get doubleReadOnly(): { return count * 2; },
    doubleReadOnly: $get(count * 2),
/// set doubleWriteOnly(v): { count = v / 2; },
    doubleWriteOnly: $set(v => count = v / 2),
}

still the same overload problem though

edit: and dropping $member(count) could lead to compilation problems. could be solved by $get(count).set(count), but that's a bit ugly

@SBHattarj
Copy link

I don't think $member.get(...).set(...) or $get(...).set(...) work with ts. Because get must return the type of it's argument, to work without issues. So please do consider this moving forward on this discussion.

@JakeBeaver
Copy link
Contributor Author

ah yes, that's absolutely correct.

and truth is $effect and $effect.pre are also treated as separate runes, so we're introducing multiple runes anyway.

something like this then maybe:

const count = $state(0);

const a = {
    count: $member(count),
    countReadOnly: $get(count),
    countWriteOnly: $set(count),

    double: $member(count * 2, v => text = v),
    doubleReadOnly: $get(count * 2),
    doubleWriteOnly: $set(v => count = v / 2),
}

any ideas on avoiding $set() overloading?

@JakeBeaver
Copy link
Contributor Author

JakeBeaver commented Oct 4, 2023

I'm thinking maybe just functions for all setters? Not much win with respect to amount of code, but flows better than native setters and stays consistent

const count = $state(0);

const a = {
    count: $member(count),
    countReadOnly: $get(count),
    countWriteOnly: $set(v => count = v),

    double: $member(count * 2, v => count = v / 2),
    doubleReadOnly: $get(count * 2),
    doubleWriteOnly: $set(v => count = v / 2),
}

@7nik
Copy link

7nik commented Oct 25, 2023

Speaking of typing, both

const a = {
    countReadOnly: $get(count),
};

and

const a = {
    ...$members.get({countReadOnly: count})
};

suffer from the fact that a.countReadOnly will have a writable type, and this makes these approaches in fact not compatible with TS. TS Playground.
And we cannot use readonly modifier on object litterals.

@JakeBeaver JakeBeaver closed this as not planned Won't fix, can't repro, duplicate, stale Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants