Hooks design

[MartinKavik]

let (count, count_access) = use_state(|| 0);

1. Unnecessary cloning / Clone restrictions? i32 implements Copy, but what about String and other things that implement only Clone or does not implement either of them? (That's why you call .get and .update in my experiment.)

2. Unnecessary closure? I don't know the reasons for it in topo implementation, but I used PolyMap in my experiment and it works good.

---

OT notes:

• There is a naming convention in Elm to use NoOp (instead of DoNothing) (and C++ or jQuery guys already know it from other contexts).
• I think you can use () instead of Msg in this example if it makes sense to you.

[rebo]

Clone is perfectly fine Copy is not required. Currently need at least Clone for the stored type but I can look how that can be alleviated. Primarily this was for ease of implementation as getting a type back would be a clone/copy. It would be good to remove this restriction.

The closure is needed because you dont always want the default variable constructed, particularly if it is expensive. On the first run use_state would use the default but on future renders it would ignore it as it doesn't execute the closure.

It's a bit like unwrap_or vs unwrap_or_else in that unwrap_or 's argument is eagerly evaluated.

For instance you can actually cache entire Node<Msg> trees which is something you wouldn't want to reevaluate every render.

[MartinKavik]

at least Clone for the stored type but I can look how that can be alleviated

I meant this ^ Clone, nice.

The closure is needed because you dont always want the default variable constructed

Yeah, you are right, I forgot to take into account lazy / eager eval.

Thanks.

[rebo]

OT notes: There is a naming convention in Elm to use NoOp (instead of DoNothing) (and C++ or jQuery guys already know it from other contexts). I think you can use () instead of Msg in this example if it makes sense to you

Ok, changed to NoOp. I didn't use () because a Msg enum would be expected in most programs and its absence might be confusing.

[rebo]

After quite a bit of testing various approaches and having tested all combinations and configurations of Rc RefCell, OnceCell, and Cell I think the most ergonomic solution is as follows.

Clone not required for storing state. I.e set_state<T>(data:T) will not require Clone for T.

However the 'usual' api for retrieving state will be via a Clone. I.e. use_state will require Clone because it returns a T.

For those types where either Clone is not implemented or not preferred due to performance reasons then state can be accessed by removing it from the storage and when finished with it put it back again. I.e. this pattern.

let data = remove_state::<NonCloneType>();
do_something_with(data);
set_state(data);

In the very unusual case where Clone isn't wanted/needed and the user simply cannot temporarily remove the type from the store then they can store a RcRefCell and deal with the borrow conflicts directly.

The advantage of this approach is that there is no possibility of panic due to runtime borrow issues and both Clone and non-Clone types are supported. The api is therefore more predictable and will satisfy almost all use cases.

[MartinKavik]

Interesting idea. How would it look like in practice?

• A) How would you rewrite example below when count is NonCloneType?
• B) How would you rewrite example below when count implements Clone but not Copy?
• C) How would you rewrite example below when count implements Copy?

  fn counter() -> Node<Msg> {
  let count = use_state(3);
  div![
      button![ev(Ev::Click, enc!((count) move |_| { count.update(|v| *v -= 1); Msg::NoOp } )), "-"],
      div![count.get().to_string()],
      button![ev(Ev::Click, enc!((count) move |_| { count.update(|v| *v += 1); Msg::NoOp } )), "+"],
  ]
  }

[rebo]

B) (Clone) and C) (Copy) are identical. (also no need for enclose!)

fn counter() -> Node<Msg> {
    let (count , count_access) = use_state(||3);

    div![
        button![ev(Ev::Click, move |_| { count_access.update(|v| *v -= 1); Msg::NoOp } ), "-"],
        div![count.to_string()],
        button![ev(Ev::Click, move |_| { count_access.update(|v| *v += 1); Msg::NoOp } ), "+"],
    ]
}

If your type is NonClone then the following with_state (instead of use_state) pattern works quite well. with_state removes T from the store, you then do what you want with it and then it re-inserts it at the end of the with_state block.

// example non-Clone type
#[derive(Default)]
struct NonCloneI32(i32)

fn my_button() -> Node<Msg> {
    let (count_string, count_access) = with_state( NonCloneI32::default, 
        |item| { item.0.to_string()}
    );

    div![
        button![ev(Ev::Click, move |_| { count_access.update(|v| v.0 -= 1); Msg::NoOp } ), "-"],
        div![count_string],
        button![ev(Ev::Click, move |_| { count_access.update(|v| v.0 += 1); Msg::NoOp } ), "+"],
    ]]
}

As you can see it is fairly similar to the Clone version just a little extra ceremony to get at the type.

[rebo]

There is an example of this working in the comp_state_inc branch.

[MartinKavik]

What do you think about this:

Rewritten example 1:

fn counter() -> Node<Msg> {
    let count_access = use_state(||3);
    let count = count.get();  // get() or to_inner() or to_owned() returns copied or cloned value

    div![
        button![ev(Ev::Click, move |_| { count_access .update(|v| *v -= 1); Msg::NoOp } ), "-"],
        div![count.to_string()],
        button![ev(Ev::Click, move |_| { count_access .update(|v| *v += 1); Msg::NoOp } ), "+"],
    ]
}

OR

fn counter() -> Node<Msg> {
    let count = use_state(||3);

    div![
        button![ev(Ev::Click, move |_| { count.update(|v| *v -= 1); Msg::NoOp } ), "-"],
        div![count.get().to_string()],
        button![ev(Ev::Click, move |_| { count.update(|v| *v += 1); Msg::NoOp } ), "+"],
    ]
}

Rewritten example 2:

// example non-Clone type
#[derive(Default)]
struct NonCloneI32(i32)

fn my_button() -> Node<Msg> {
    let count = use_state(NonCloneI32::default);
    let count_string = count.get_with(|item| item.0.to_string())

    div![
        button![ev(Ev::Click, move |_| { count.update(|v| v.0 -= 1); Msg::NoOp } ), "-"],
        div![count_string],
        button![ev(Ev::Click, move |_| { count.update(|v| v.0 += 1); Msg::NoOp } ), "+"],
    ]]
}

[MartinKavik]

Note / tip to implementation: I see you use

thread_local! {
    static STORE: RefCell<Store> = RefCell::new(Store::new());
}

I don't know what's the difference from the perfomanc point of view, but maybe atomic_refcell can help clean implemenation a little bit.

[rebo]

Hi thanks for the thread local tip will explore that.

With ex1 that is basically the same as use_state returning the tuple of a cloned value plus the accessor struct. However I can see the advantage of a unified access function for both Clone and non clone. Currently it uses the tuple style mainly because that is what the React api does and almost always when using a hook you need the value and a way to access it.

That said I think just returning the state accessor makes sense because then users will be more inclined to use a fresh clone via get as opposed to a potentially stale one in a callback.

Yes I think I have convinced myself a change like this is the right choice. Will implement and update.

[rebo]

Ok think we are homing in on a consistent design. Here is the latest:

Clone/Copy:

fn my_button() -> Node<Msg> {
    let count = use_state(|| 3);
    div![
        button![ "-",
            mouse_ev(Ev::Click, move |_| {count.update(|v| *v -= 1); Msg::NoOp }),
        ],
        count.get().to_string(),
        button![ "+",
            mouse_ev(Ev::Click, move |_| {count.update(|v| *v += 1); Msg::NoOp }),
        ],
    ]
}

Non Clone:

#[derive(Default)]
struct NonCloneI32(i32);

#[topo::nested]
fn my_button_non_clone() -> Node<Msg> {
    let count = use_state(NonCloneI32::default);

    div![
        button![ "-",
            mouse_ev(Ev::Click, move |_| {count.update(|item| item.0 -= 1); Msg::NoOp }),
        ],
        count.get_with(|item| item.0.to_string()),
        button![ "+",
            mouse_ev(Ev::Click, move |_| { count.update(|item| item.0 += 1); Msg::NoOp }),
        ]
    ]
}

So all interaction through use_state(||...). Only difference is NonClone has to read via .get_with() , Clone can read via .get() or .get_with() as desired.

[MartinKavik]

Nice! I don't see any potential problems now. So, let's jump to the next problem - reusability & messages. The first idea:

fn counter<Ms: 'static + Default>() -> Node<Ms> {
    let count = use_state(|| 3);
    div![
        button![ "-",
            mouse_ev(Ev::Click, move |_| {count.update(|v| *v -= 1); Ms::default()}),
            // OR (if possible)
            mouse_ev(Ev::Click, move |_| count.update(|v| *v -= 1)),
        ],
        // ...
    ]
}

// parent

#[derive(SmartDefault)]
enum Msg {
    AMessage(String),
    #[default]
    NoOp,
}

See comments and links about Default for enums in this thread: https://internals.rust-lang.org/t/request-derive-enums-default/10576

[rebo]

Yes in my seed_comp_helpers crate i define a on_click helper to just accept a closure and auto wrap with Msg::default().

hence it is currently just:

 button![ 
    on_click( move |_| count_access.set(count + 1)),
     format!("Click Me × {}", count)
]

Now there are a lot of other events I just did that as it was the most common. Better would be for seeds mouse_ev function to accept an Ev:: and a closure as per your 'if possible' comment above.

Will read the internals about default.

[rebo]

Closing with resolution in #5