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

Context with Rc<Refcell<T>> reducer type has unexpected behavior #3601

Open
1 of 3 tasks
ghost opened this issue Feb 12, 2024 · 1 comment
Open
1 of 3 tasks

Context with Rc<Refcell<T>> reducer type has unexpected behavior #3601

ghost opened this issue Feb 12, 2024 · 1 comment

Comments

@ghost
Copy link

ghost commented Feb 12, 2024

Problem

Thank you for taking the time to read my issue.

I'm not sure if this is my implementation error or a yew bug but I'll try to explain the situation as clear as possible.

I am developing an application where I need a context that requires some fields that are of type Vec<T> so for sake of not copying data around I wrap each struct member in Rc<RefCell<>>.

there are two implementations of Reducible trait that each behave differently.

First implementation:
Create a new Self.
Causes the context subscribers and the context itself to reload when I click either button which is expected behavior.
image

Second implementation:
Change underlying data and return self.
Causes only the context itself to reload on click and not the subscribers even if underlying data is changed, there for the UI does not update which is not what I want.
image

How should I approach this problem since I cannot create a new Self on each state update ?

Steps To Reproduce
Code:

use yew::prelude::*;
use gloo::console::log;
use std::rc::Rc;
use std::cell::RefCell;


#[derive(PartialEq)]
pub struct StateData {
    pub preview_open: Rc<RefCell<bool>>,
    // some other vector data that I dont want to copy on each state change
}

impl Default for StateData {
    fn default() -> Self {
        Self {
            preview_open: Rc::new(RefCell::new(false)),
        }
    }
}

pub enum StateAction {
    SetPreviewOpen(bool),
}

// First implementation
impl Reducible for StateData {
    type Action = StateAction;

    fn reduce(self: Rc<Self>, action: Self::Action) -> Rc<Self> {
        match action {
            StateAction::SetPreviewOpen(x) => {
                return Self {
                    preview_open: Rc::new(RefCell::new(x)),
                }.into();
            }
        }
    }
}

// Second implementation
// impl Reducible for StateData {
//     type Action = StateAction;
// 
//     fn reduce(self: Rc<Self>, action: Self::Action) -> Rc<Self> {
//         match action {
//             StateAction::SetPreviewOpen(x) => {
//                 *self.preview_open.borrow_mut() = x.into();
//             }
//         }
// 
//         self
//     }
// }

type ChatroomContext = UseReducerHandle<StateData>;

#[derive(Properties, PartialEq)]
pub struct ChildrenProps {
    pub children: Children
}

#[function_component]
pub fn ChatContext(props: &ChildrenProps) -> Html {

    let state = use_reducer(|| StateData::default());

    log!("reload context");
    log!(format!("{}", *state.preview_open.borrow()));

    html! {
        <ContextProvider<ChatroomContext> context={state} >
            {props.children.clone()}
        </ContextProvider<ChatroomContext>>
    }
}


#[function_component]
fn SubOne() -> Html {

    let ctx = use_context::<ChatroomContext>().unwrap();

    let onclick = {
        let ctx = ctx.clone();
        Callback::from(
            move |_| {
                ctx.dispatch(StateAction::SetPreviewOpen(true));
            }
        )
    };

    log!("reload sub one");

    html! {
        <>
            <div>{"SUB One"}</div>
            <button {onclick} >{"One button"}</button>
            <div>{format!("State from One: {}", ctx.preview_open.borrow().clone().to_string())}</div>
        </>
    }
}


#[function_component]
fn SubTwo() -> Html {

    let ctx = use_context::<ChatroomContext>().unwrap();

    let onclick = {
        let ctx = ctx.clone();
        Callback::from(
            move |_| {
                ctx.dispatch(StateAction::SetPreviewOpen(false));
            }
        )
    };

    log!("reload sub two");

    html! {
        <>
            <div>{"SUB Two"}</div>
            <button {onclick} >{"Two button"}</button>
            <div>{format!("State from Two: {}", ctx.preview_open.borrow().clone().to_string())}</div>
        </>
    }
}


#[function_component]
fn App() -> Html {

    html! {
        <div>
            <ChatContext>
                <SubOne />
                <SubTwo />
            </ChatContext>
        </div>
    }
}

fn main() {
    yew::Renderer::<App>::new().render();
}

Environment:

  • Yew version: [v0.21]
  • Rust version: [1.76.0, stable]
  • Target, if relevant: [wasm32-unknown-unknown]
  • Build tool, if relevant: [trunk]
  • OS, if relevant: [Windows 11]
  • Browser and version, if relevant: [Brave 1.62.162 Chromium: 121.0.6167.164 (Official Build) (64-bit)]

Questionnaire

  • I'm interested in fixing this myself but don't know where to start
  • I would like to fix and I have a solution
  • I don't have time to fix this right now, but maybe later
@ghost ghost added the bug label Feb 12, 2024
@WorldSEnder
Copy link
Member

I think the question boils down to "how can I use iternally mutable data with Reducible". I'll try to answer this, but first I want to clarify why the behaviour of Implementation 2 is expected.

First of all, use_reducer always leads to a re-render when you dispatch an action to it, no matter what the implementation of Reducible does. This is what you experience by the context providing component always re-rendering. The difference between the two examples is how the ContextProvider handles the value in its context; to decide whether to re-render any subscribed children, it compares the old to the new value when it itself re-renders. This means we should look at the PartialEq for state: UseReducerHandle<_>, which simply forwards to <StateData as PartialEq>::eq. But here lies the problem: if you use only interior mutability in the reducer, then the old value will have been also updated, and the comparison is a trivially true by reflexivity! So the context provider concludes that it doesn't need to re-render its children.

So how to fix it? The simplest, but perhaps too lazy of an approach, would be to wrap the context value in a struct that simply always returns false from its PartialEq. This way, the ContextProvider will always re-render children consumers, whenever it itself re-renders.

struct ChatroomContext(UseReducerHandle<StateData>);
impl PartialEq for ChatroomContext {
    fn eq(&self, other: &Self) { false }
}
...
        <ContextProvider<ChatroomContext> context={ChatroomContext(state)} >
...

In case this is too coarse and leads to too many unnecessary re-renders, you should introduce some sort of generational counter into the state that lives outside the shared mutable state that counts which "version" the state represents:

pub struct SharedState {
    pub open: bool, // etc..
}
#[derive(Clone)]
pub struct StateData {
    generation: u32,
    pub state: Rc<RefCell<SharedState>>,
}
impl PartialEq for StateData {
    fn eq(&self, other: &Self) -> bool {
        // this assumes that there is only one history for each state, which is true in use_reducer
        Rc::ptr_eq(&self.state, &other.state) && self.generation == other.generation
    }
}
impl StateData {
    // small helper method to facilitate incrementing the generation counter
    fn mutate<'a>(self: &'a mut Rc<Self>) -> impl 'a + DerefMut<Target = SharedState> {
        // We can cheaply clone ourself. With some further work you can write this without
        // leaking the `Clone` impl for StateData
        let this = Rc::make_mut(self);
        // Since we are borrowing mutably, we assume that the data "changed" and we increase the generation
        this.generation += 1;
        this.state.borrow_mut()
    }
}
impl Reducible for StateData {
    type Action = StateAction;

    fn reduce(mut self: Rc<Self>, action: Self::Action) -> Rc<Self> {
        match action {
            StateAction::SetPreviewOpen(x) => {
                let mut state = StateData::mutate(&mut self);
                state.open = x;
            }
        }
        self
    }
}
....
        // This context provider will see the same shared state, but different generation numbers
        // On subsequent re-renders, it will trigger re-renders of downstream when the generation changed
        <ContextProvider<ChatroomContext> context={state} >
....

@WorldSEnder WorldSEnder added question ergonomics documentation A-yew Area: The main yew crate and removed bug labels Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant