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

use_state setter is *highly* unergonomic #1505

Closed
jkelleyrtp opened this issue Aug 19, 2020 · 13 comments · Fixed by #1630
Closed

use_state setter is *highly* unergonomic #1505

jkelleyrtp opened this issue Aug 19, 2020 · 13 comments · Fixed by #1630

Comments

@jkelleyrtp
Copy link
Contributor

There's definitely some rough edges around the functional API, with some good ideas on how to make it better.

image

A very common use case in React-land is to use the same setter in two different locations in the component. When use_state returns a box, this is impossible. It's required to wrap the setter in an Rc, and then manually clone it into every scope.

To get the comfiness of JS/TS, whatever is returned by use_state and co. need to return some clever pointer type, whether it be Rc or something else that implements deref_mut. Ideally, they would be auto-cloned into closure scopes so that cloning the Rc into closures wouldn't be an issue either.

@jkelleyrtp jkelleyrtp added the bug label Aug 19, 2020
@jstarry
Copy link
Member

jstarry commented Aug 19, 2020

Yeah, that's definitely a rough edge. Thanks for bringing this up, I agree with your conclusion

@lukechu10
Copy link
Contributor

Hmm, would it be possible to have a smart pointer that implements Copy? Ideally, the state (searchinput) could use this too to prevent clone().

@lukechu10
Copy link
Contributor

Ideally, they would be auto-cloned into closure scopes so that cloning the Rc into closures wouldn't be an issue either.

That's the main problem. Rc cannot implement Copy because it also implements Drop.

@mattferrin
Copy link
Contributor

TypeScript dev here whose Rust experience is mostly limited to exercism.io toy problems, but typing .clone() doesn't bother me too much. It might even help readability to have to write:

let set_more_specific_name = set_less_specific_name.clone();

I don't have enough knowledge yet to know if the preceding:

let set_more_specific_name = Rc::new(set_more_specific_name);

line can be abstracted away though, but it would be nice.

mattferrin pushed a commit to mattferrin/yew that referenced this issue Oct 25, 2020
jstarry pushed a commit that referenced this issue Oct 25, 2020
* Makes calling use_state setter from multiple locations more ergonomic, #1505

* Tests more than one use_state setter call location, #1505

Co-authored-by: Matthew Ferrin <ferrin@MacBook-Pro-2.home>
@lukechu10
Copy link
Contributor

Even though #1630 is definitely an improvement, I don't think it completely solves the problem. React hooks in JS/TS is still a lot more concise and eloquent than the current way in Rust in my opinion.

The problem here is that what ever smart pointer cannot be implicitly "cloned" into the closure because it cannot implement Copy: https://stackoverflow.com/questions/40014703/why-is-stdrcrc-not-copy

Maybe a simple macro could solve this by cloning the hooks in a new scope. Something like this:

let (counter, set_counter) = use_state(|| 0);

// capture! is a vararg macro. The last arg is the closure.
let increment = capture!(counter, set_counter, move |_| set_counter(*counter + 1));

which expands to this:

let (counter, set_counter) = use_state(|| 0);

// capture! is a vararg macro. The last arg is the closure.
let increment = {
    let counter = counter.clone();
    let set_counter = set_counter.clone();
    Callback::from(move |_| set_counter(*counter + 1))
};

@mattferrin
Copy link
Contributor

I suspect we're getting into the weeds of what makes Rust Rust, compiler enforced memory management. F# and ReasonML/OCaml are both languages whose ecosystems fully support React hooks and require less minutia (at the cost of greater memory consumption and garbage collection). If Rust can do what you are hoping for as a low-cost abstraction, I would be interested in achieving that level of enlightenment.

(I did experiment with your code snippet without success.)

@lukechu10
Copy link
Contributor

lukechu10 commented Oct 26, 2020

I suspect we're getting into the weeds of what makes Rust Rust, compiler enforced memory management. F# and ReasonML/OCaml are both languages whose ecosystems fully support React hooks and require less minutia (at the cost of greater memory consumption and garbage collection). If Rust can do what you are hoping for as a low-cost abstraction, I would be interested in achieving that level of enlightenment.

(I did experiment with your code snippet without success.)

I managed to create a prototype. Here is the macro:

#[macro_export]
macro_rules! capture {
    ([$($arg:ident),*], $closure:expr) => {{
        // clone all the args
        $( let $arg = std::rc::Rc::clone(&$arg); )*
        
        // return the closure using the cloned Rc
        $closure
    }};
}

And here is how I use it:

use std::rc::Rc;
use yew::prelude::*;
use yew_functional::*;
mod capture;

#[derive(Properties, Clone, PartialEq)]
pub struct CounterProps {}
pub struct CounterFunc {}
impl FunctionProvider for CounterFunc {
    type TProps = CounterProps;

    fn run(_props: &Self::TProps) -> Html {
        let (counter, set_counter) = use_state(|| 0);
        let set_counter = Rc::new(set_counter);

        // the closure does not move counter or set_counter into its scope even though it is move.
        let increment = capture!([counter, set_counter], move |_| set_counter(*counter + 1));
        // counter and set_counter are still accessible here.
        let decrement = capture!([counter, set_counter], move |_| set_counter(*counter - 1));

        html! {
            <>
                <h1>{format!("Counter value: {}", counter)}</h1>
                <button onclick=Callback::from(increment)>{"+"}</button>
                <button onclick=Callback::from(decrement)>{"-"}</button>
            </>
        }
    }
}
pub type Counter = FunctionComponent<CounterFunc>;

I am a bit surprised that Rust allows you to do this. $capture in the macro uses the variables that are overridden inside the macro instead of those outside of the macro. This seems a bit unhygienic but it works.

I still haven't tried this out in an app yet but I will do soon.

Edit:
I just tested it and it works.

@jkelleyrtp
Copy link
Contributor Author

jkelleyrtp commented Oct 26, 2020

Why re-invent the wheel? https://docs.rs/closure/0.3.0/closure/

However, I would prefer to limit the amount of macros required to write ergonomic reactive code. Right now, my functional components all use a proc macro to implement the function component trait, use derive macros for props, use macros to draw the html, and use macros to clone the Rcs into the closure scope. It would be nice to not always have to reach for macros to solve these types of problems.

Are there any types within the language we can leverage to craft a better smart pointer, knowing that the closures are FnOnce and single-threaded?

EDIT:

This has been a debate for a long time, only API changes would actually make a difference here.

rust-lang/rfcs#2407

@jkelleyrtp
Copy link
Contributor Author

jkelleyrtp commented Oct 26, 2020

Why re-invent the wheel? https://docs.rs/closure/0.3.0/closure/

However, I would prefer to limit the amount of macros required to write ergonomic reactive code. Right now, my functional components all use a proc macro to implement the function component trait, use derive macros for props, use macros to draw the html, and use macros to clone the Rcs into the closure scope. It would be nice to not always have to reach for macros to solve these types of problems.

Are there any types within the language we can leverage to craft a better smart pointer, knowing that the closures are FnOnce and single-threaded?

EDIT:

This has been a debate for a long time, only API changes would actually make a difference here.

rust-lang/rfcs#2407

Okay, hacked something together.

I really want to port recoiljs to Rust. I think it would fit well in the Yew ecosystem (relies on subscriptions + message passing) and it hooks into the global state. This solution would expose a use_recoil which pulls into a global "any map." Below, it's listed as a "use_state2".

This concept exposes a recoil selector, but instead of a distinct "value" and "set_value" only stores a key to the atom, and then ::get and ::set find the corresponding value and set_value in the global recoil atom map.

I haven't yet hooked up the "hook" part, so it's not causing a re-render of the component. That might require channels to achieve, or me just getting better with hooks.

https://github.com/MinusKelvin/webutil/blob/fb24e2bce4e3811fd92238232981eaa2fcd4e6c4/src/channel.rs

thread_local! {
    static HASHMAP: Mutex<HashMap<&'static str, Rc<Box<dyn std::any::Any>>>> = {
        Mutex::new(HashMap::new())
    };
}

#[derive(Clone, Copy)]
pub struct MyThreadLocalPointer<T> {
    pub key: &'static str,
    pub _m: std::marker::PhantomData<T>,
}
impl<T> MyThreadLocalPointer<T> {
    pub fn get<'a>(&'a self) -> Rc<Box<T>> {
        HASHMAP.with(|map| {
            let map = map.lock().unwrap();
            let c = map.get(self.key).unwrap();
            let f = Rc::into_raw(c.clone());
            let b: Rc<Box<T>> = unsafe { Rc::from_raw(f as _) };
            // let r = b.as_ref().as_ref();
            b
        })
    }

    // A bit of a weird lifetime on T, but it should be an owned value anyways
    pub fn set(&self, newval: T)
    where
        T: 'static,
    {
        HASHMAP.with(|map| {
            let mut map = map.lock().unwrap();
            let b = Box::new(newval) as Box<dyn Any>;
            let new = Rc::new(b);
            map.insert(self.key, new);
        });
    }

    pub fn modify(&self, modify_fn: impl FnOnce(&T) -> T)
    where
        T: 'static,
    {
        HASHMAP.with(|map| {
            let mut map = map.lock().unwrap();
            let c = map.get(self.key).unwrap();
            let f = Rc::into_raw(c.clone());
            let b: Rc<Box<T>> = unsafe { Rc::from_raw(f as _) };
            let old_val = b.as_ref().as_ref();
            let newval = modify_fn(old_val);
            let b = Box::new(newval) as Box<dyn Any>;
            let new = Rc::new(b);
            map.insert(self.key, new);
        });
    }
}

fn use_state2<F, T>(state: F, key: &'static str) -> MyThreadLocalPointer<T>
where
    F: FnOnce() -> T,
    T: 'static,
{
    let init = state();
    let my_pointer = MyThreadLocalPointer {
        key,
        _m: std::marker::PhantomData {},
    };
    my_pointer.set(init);
    my_pointer
}

#[functional_component]
pub fn incrementer() -> yew::Html {
    let counter = use_state2(|| 100_i32, "counter");

    // the closure does not move counter or set_counter into its scope even though it is move.
    let increment = move |_| {
        let count = **counter.get();
        counter.set(count + 1_i32);
        log::debug!(
            "Value from thread-local storage via increment is {:#?}",
            count
        );
    };

    // counter and set_counter are still accessible here.
    let decrement = move |_| {
        let count = **counter.get();
        counter.set(count - 1_i32);
        log::debug!(
            "Value from thread-local storage via increment is {:#?}",
            count
        );
    };

    let doubler = move |_| {
        counter.modify(|old: &i32| old * 2);
        log::debug!(
            "Value from thread-local storage via double is {:#?}",
            counter.get()
        );
    };

    yew::prelude::html! {
        <div>
            {format!("counter: {:}", **counter.get())}
            <button onclick=Callback::from(increment)>{"+"}</button>
            <button onclick=Callback::from(decrement)>{"-"}</button>
            <button onclick=Callback::from(doubler)>{"x2"}</button>
        </div>
    }
}

@jkelleyrtp
Copy link
Contributor Author

jkelleyrtp commented Oct 26, 2020

With the ^ code, I've got a basic prototype of recoil's atom working across components using use_recoil.

trait Atom {
    const KEY: &'static str;
    type Output: Default;
}

struct MyAtom;
impl Atom for MyAtom {
    const KEY: &'static str = "blah";
    type Output = i32;
}

fn use_recoil<T: Atom>() -> MyThreadLocalPointer<T::Output> {
    MyThreadLocalPointer {
        key: T::KEY,
        _m: std::marker::PhantomData {},
    }
}

#[functional_component]
pub fn incrementer() -> yew::Html {
    let counter = use_recoil::<Counter>();

    let increment = move |_| counter.modify(|old| old + 1);
    let decrement = move |_| counter.modify(|old| old - 1);
    let reset = move |_| counter.set(0);

    html! {
        <div>
            {format!("counter: {:}", **counter.get())}
            <button onclick=Callback::from(increment)>{"+"}</button>
            <button onclick=Callback::from(decrement)>{"-"}</button>
            <button onclick=Callback::from(reset)>{"reset"}</button>
        </div>
    }
}

I will probably add a derive macro for making atoms easier to construct. Something like

#[Atom(key = "blah")]
struct Agents(Vec<f32>);

@mattferrin
Copy link
Contributor

I think porting a RecoilJS equivalent to Rust is really important work. It's state API is very well thought out.

@lukechu10
Copy link
Contributor

Maybe this should belong in #576

@intendednull
Copy link
Contributor

Dropping yew-state here because it can also fill this role:

use yew::prelude::*;
use yew_state::{component, SharedHandle, StateView};

fn view_counter() -> Html {
    type Handle = SharedHandle<u64>;

    let view = component::view(|handle: &Handle| {
        // Increment count by 1.
        let incr = handle.reduce_callback(|count| *count += 1);
        // Reset count to 0.
        let reset = handle.reduce_callback(|count| *count = 0);

        html! {
            <>
            <button onclick=incr>{ handle.state() }</button>
            <button onclick=reset>{"Reset"}</button>
            </>
        }
    });

    html! {
        <StateView<Handle> view=view />
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants