intendednull / yewdux

Ergonomic state management for Yew applications
https://intendednull.github.io/yewdux/
Apache License 2.0
322 stars 31 forks source link

Confusion on make_mut tip on documentation #45

Closed norbertkeri closed 2 years ago

norbertkeri commented 2 years ago

The documentation on https://intendednull.github.io/yewdux/dispatch.html#mutate-state-predictably has a part that says:

Tip Rc::make_mut is handy if you prefer CoW:

impl Reducer<Counter> for Msg {
    fn apply(&self, mut counter: Rc<Counter>) -> Rc<Counter> {
        let state = Rc::make_mut(&mut counter);

        match self {
            Msg::AddOne => state.count += 1,
        };

        counter
    }
}

Shouldn't the return value be Rc::new(state)? Since make_mut will clone the value if there are other outstanding Rcs pointing to the object, mutating state inside the match block will mutate a fresh clone of it, which effectively gets discarded at the end of the function.

Edit:
Actually, Rc::new(state) doesn't work either, because it wants Counter, while state is only &mut Counter.

Edit2:
I managed to make it work by:

impl Reducer<Counter> for Msg {
    fn apply(&self, mut counter: Rc<Counter>) -> Rc<Counter> {
        let state = (*state).clone();

        match self {
            Msg::AddOne => state.count += 1,
        };

        Rc::new(state)
    }
}

Does this look ok, or am I using it incorrectly?

intendednull commented 2 years ago

What you have works too! The example is correct, but we can definitely improve docs to help with confusion.

To clarify the example, Rc::make_mut returns a &mut Counter taken from the Rc<Counter>. You're correct, it does clone the value if needed, but the new cloned value is automatically put into the same Rc<Counter> container we called make_mut on. So there's no need to create a new one.

I think Rc::make_mut is a little more efficient, because it (probably) doesn't need to allocate new memory

norbertkeri commented 2 years ago

You are right, sorry. It was actually the example in the example on make_mut in the official docs that confused me, because it ends up with 2 different values, so I thought it must be the same thing happening in yewdux.

When I read the official docs, I understood that when I do let other = Rc::make_mut(&mut target), it will clone target to a new location, and other will point to that new location, but now I see that it will also change target to point to that new location. This was really not obvious to me from the docs, but maybe I'm just reading it wrong.

Thanks for clearing this up.