SignalBundle derive macro

[jquesada2016]

Adds a derive macro for SignalBundle, closing #2042.

[jquesada2016]

@gbj I would like your feedback on a couple of things:

Macro Name

I like it a bit more as opposed to using something relating to solid's store, as it might confuse users thinking it has more to do with StoredValue.

Attribute arguments

Currently, I have the macro accepting 3 parameters, signal, rw_signal, store, and any combination of the above.

signal

This would be:

#[derive(SignalBundle)]
#[bundle(signal)]
struct Bundle {
  a: usize,
  b: usize,
  c: usize,
}

// generates

struct BundleRead {
  a: ReadSignal<usize>,
  b: ReadSignal<usize>,
  c: ReadSignal<usize>,
}

struct BundleWrite {
  a: WriteSignal<usize>,
  b: WriteSignal<usize>,
  c: WriteSignal<usize>,
}

// Can be used like:

let (read, write) = Bundle::default().into_signal_bundle();

// ...

rw_signal

#[derive(SignalBundle)]
#[bundle(rw_signall)]
struct Bundle {
  a: usize,
  b: usize,
  c: usize,
}

struct BundleRw {
  a: RwSignal<usize>,
  b: RwSignal<usize>,
  c: RwSignal<usize>,
}

store

This is the one I'm not sure about if we should even have as an option. since storing the entire struct might yield the same semantics. What do you think? Here's how it would expand:

#[derive(SignalBundle)]
#[bundle(store)]
struct Bundle {
  a: usize,
  b: usize,
  c: usize,
}

struct BundleStore {
  a: StoredValue<usize>,
  b: StoredValue<usize>,
  c: StoredValue<usize>,
}

[jquesada2016]

@gbj any thoughts on this PR?

[gbj]

Yeah, sorry for being so slow. I've started to write out a response a couple times and had to go back and think more.

There are a few reasons it is really useful to have something like a SolidJS "store," i.e., an object that supports nested reactivity:

1. Reactive structs, where you can track fine-grained updates to individual fields rather than the struct as a whole
2. Reactive collections, where you can have a Store<Vec<Todo>> and track access to, say, todos.get(0)
3. Data diffing: i.e., you have a Store<SomeData> and you can give it a new SomeData from the server and it will only notify on the fields that have actually changed

Unless I'm misunderstanding, this PR is a derive macro to do Part 1 by generating a separate struct with signals for each field. To me, this feels like a bandaid around the problem of not having real reactive stores.

Granted, a bandaid is very useful if you are bleeding. And this does solve the issue of "how do I make a struct of signals easily."

But in an ideal world I'd like to get to a place where it's possible to have a proper store which can do all of 1, 2, and 3. And so I'm a little hesitant to bring this into the core library, and then immediately break it. On the other hand, clearly I haven't actually built a true reactive store yet, so the alternative is totally hypothetical. It's just a matter of limited time for me right now.

I'd love to have feedback from others, and am very willing to yield to the wisdom of the community if merging the partial solution here sounds like a good idea to people.

(Also, if you rebase and run cargo fmt on leptos_macro that will help the CI.)

[jquesada2016]

@gbj no problem!

You bring up some really good points, although I'm not entirely sure how something like what you're describing would work in Rust. The only real blocker for doing so is not having good proxy types...one can simulate them with Deref and DerefMut, but it's a far cry from ideal. Especially when thinking of generic, non-specialized collection data structures.

Do you have anything in mind?

This PR does currently address point 1, and can easily be extended to support 3 as well. But point 2, would probably be the most ambitious.

[gbj]

@jquesada2016 The key thing is that if we create a getter/setter model for store fields, each field can be uniquely identified by the getter function's address. We can then create a table of internal signals of type () that are used to track subscription and updates to each field.

Here's a (working) example in 0.6:

#[derive(Store, Clone, Default)]
struct SomeStoreStruct {
    pub name: String,
    pub count: usize,
}

pub fn app() -> impl Render<Dom> {
    let store = Store::new(SomeStoreStruct {
        name: "Bob".to_string(),
        count: 37,
    });

    // effects are canceled on drop; TODO better API here
    mem::forget(Effect::new(move |_| {
        log(&format!("count is {:?}", store.at().count().get()));
    }));

    view! {
        <button on:click=move |_| {
            store.at_mut().count().update(|n| *n += 1);
        }>
            {move || store.at().count().get()}
        </button>
        {move ||  store.at().name().get()}
    }
}

(Ignore the weirdness of forgetting the effect, will iron that part out later)

Basically, at().name().get() says

1. is there a signal identified with the count field yet?
   a) if not, create it
   b) track that signal
   c) give me the value of that field

And at_mut().name().update() says

1. is there a signal identified with the count field yet?
   a) if not, create it
   b) notify that signal
   c) update the value of that field

The "signals" created and notified there have type (), and the .with() and .update() are applied to the subfield.

This should be able to basically replace the create_slice pattern, and also to serve the use case of SignalBundle.

I still have some work to do on Part 2 (reactive collections built on this) and part 3 (data diffing built on this) but I have a pretty clear sense of how to do each of those.

The Store derive macro here expands to a couple extension traits which it implements on ReadStoreField and WriteStoreField types, with a method per struct field.

trait SomeStoreStructReadStoreFields<OriginTy> {
    fn name(self) -> ReadStoreField<OriginTy, String>;
    fn count(self) -> ReadStoreField<OriginTy, usize>;
}
impl<OriginTy> SomeStoreStructReadStoreFields<OriginTy>
for ReadStoreField<OriginTy, SomeStoreStruct>
where
    OriginTy: 'static,
{
    fn name(self) -> ReadStoreField<OriginTy, String> {
        self.subfield(
            ReadStoreField::<
                OriginTy,
                SomeStoreStruct,
            >::name as usize,
            |prev| &prev.name,
        )
    }
    fn count(self) -> ReadStoreField<OriginTy, usize> {
        self.subfield(
            ReadStoreField::<
                OriginTy,
                SomeStoreStruct,
            >::count as usize,
            |prev| &prev.count,
        )
    }
}
trait SomeStoreStructWriteStoreFields<OriginTy> {
    fn name(self) -> WriteStoreField<OriginTy, String>;
    fn count(self) -> WriteStoreField<OriginTy, usize>;
}
impl<OriginTy> SomeStoreStructWriteStoreFields<OriginTy>
for WriteStoreField<OriginTy, SomeStoreStruct>
where
    OriginTy: 'static,
{
    fn name(self) -> WriteStoreField<OriginTy, String> {
        self.subfield(
            ReadStoreField::<
                OriginTy,
                SomeStoreStruct,
            >::name as usize,
            |prev| &mut prev.name,
        )
    }
    fn count(self) -> WriteStoreField<OriginTy, usize> {
        self.subfield(
            ReadStoreField::<
                OriginTy,
                SomeStoreStruct,
            >::count as usize,
            |prev| &mut prev.count,
        )
    }
}

The rest of it is handled in the reactive system.

[gbj]

P.S. I should clarify: by "in 0.6," I mean, "inspired by you pinging me on this PR, and reusing the macro you created, I implemented this in the new reactive system over the weekend." Totally inspired by your work, not a duplicate effort.

P.P.S. This model does in fact compose really well with sub-structs, as long as they also derive Store (as is the case for any derive right?) We can update a single field of an inner struct with no real fuss and without triggering any of the other fields on either the inner or outer struct

#[derive(Store, Clone, Default)]
struct Outer {
    pub outer_field: String,
    pub inner: Inner,
}

#[derive(Store, Clone, Default)]
struct Inner {
    pub name: String,
    pub count: usize,
}

pub fn app() -> impl Render<Dom> {
    let store = Store::new(Outer {
        outer_field: "Set once".to_string(),
        inner: Inner {
            name: "Bob".to_string(),
            count: 1,
        },
    });

    // effects are canceled on drop; TODO better API here
    mem::forget(Effect::new(move |_| {
        log(&format!(
            "outer_field is {:?}",
            store.at().outer_field().get()
        ));
    }));
    mem::forget(Effect::new(move |_| {
        log(&format!("name is {:?}", store.at().inner().name().get()));
    }));
    mem::forget(Effect::new(move |_| {
        log(&format!("count is {:?}", store.at().inner().count().get()));
    }));

    view! {
        <button on:click=move |_| {
            store.at_mut().inner().count().update(|n| *n += 1);
        }>
            {move || store.at().inner().count().get()}
        </button>
        <p>{move ||  store.at().outer_field().get()}</p>
        <p>{move ||  store.at().inner().name().get()}</p>
    }
}

[jquesada2016]

@gbj I really like the idea of encapsulating the type in a common Store rather than having StructRead and StructUpdate directly exposed to users. The biggest, and only disadvantage I can think of, is that destructuring won't work here.

[pscott31]

This looks great, I find myself doing this sort of stuff a lot:

    let user = create_rw_signal(NewUserForm::default());
    let (email, set_email) = create_slice(user, |u| u.email.clone(), |u, v| u.email = v);
    let (given_name, set_given_name) =
        create_slice(user, |u| u.given_name.clone(), |u, v| u.given_name = v);
    let (family_name, set_family_name) =
        create_slice(user, |u| u.family_name.clone(), |u, v| u.family_name = v);
    let (phone, set_phone) = create_slice(user, |u| u.phone.clone(), |u, v| u.phone = v);
    let (password1, set_password1) =
        create_slice(user, |u| u.password1.clone(), |u, v| u.password1 = v);
    let (password2, set_password2) =
        create_slice(user, |u| u.password2.clone(), |u, v| u.password2 = v);

and arrived at this issue googling around to see if anyone had made a macro for it. Regarding collections, here's my hacked up solution for a 'reactive list'; I am sure whatever you come up with will be much more elegant :)

use indexmap::IndexMap;
use leptos::*;
use uuid::Uuid;

pub type ReactiveList<T> = IndexMap<Uuid, RwSignal<T>>;

pub trait TrackableList<T> {
    fn tracked_push(&self, guest: T);
    fn tracked_remove(&self, uid: Uuid);
    fn tracked_insert(&self, uid: Uuid, new: T);
}

impl<S, T> TrackableList<T> for S
where
    S: SignalUpdate<Value = ReactiveList<T>>,
    T: 'static,
{
    fn tracked_push(&self, guest: T) {
        self.update(|gs| {
            gs.insert(Uuid::new_v4(), create_rw_signal::<T>(guest));
        });
    }

    fn tracked_remove(&self, uid: Uuid) {
        self.update(|gs| {
            gs.shift_remove(&uid);
        });
    }

    fn tracked_insert(&self, uid: Uuid, new: T) {
        self.update(|gs| {
            gs.insert(uid, create_rw_signal::<T>(new));
        });
    }
}

[jquesada2016]

@gbj, I've not visited this in a while so not sure if somethings changed, do you need me to do anything on this PR?

[gbj]

@jquesada2016 Thanks for checking in. I think my feelings are still the same as December, with the caveat that we did a separate 0.6 release for new server functions, so replace everywhere I said 0.6 with 0.7. I think this seems useful in the current state of things, and I also now have a working implementation for reactive stores that can achieve the same thing in the next version, so I'm hesitant to bring this into the core of the library and then immediately replace it in the next version.

Do you think it would make sense to publish as a separate crate? Or am I just being too conservative here about adding a useful feature to the library?

[jquesada2016]

Do you think it would make sense to publish as a separate crate? Or am I just being too conservative here about adding a useful feature to the library?

@gbj of course not! Having already a clear path forward, there is no need for this feature if it's going to be almost immediately replaced, nor do I see the need for this to be a seperate crate. It'll only add to technical debt of users if they decide to depend on it.

I would go ahead and close this PR. That's also why I wasn't sure if something changed; because it was still open.

[gbj]

Gotcha. Will close now then 😄