Button input onclick not updating as expected in view

[gesterhuizen]

Disclaimer: I am very new to Rust, so it is quite likely that the behaviour that I'm describing is totally expected and I'm just getting stumped by my lack of clear understanding of the language.

I'm seeing unexpected behaviour in how a button input onclick handler functions. The closure is supposed to change during each rerender (i.e. view invocation), based on application state. However, the observed behaviour seems to indicate that the onclick handler that is first set on the input is always used. This is despite the onclick being update multiple times in view. In the same update, the input value is also updated, and this is correctly reflected.

I made a small app, based off of the minimal Sauron example, which illustrates this: https://github.com/gesterhuizen/sauron-onclick

The README from that project is duplicated below:

Steps to reproduce

1. Clone this repo: https://github.com/gesterhuizen/sauron-onclick
2. Build it:

. ./bootstrap.sh
make build server

3. Open the app in the browser: http://localhost:4001/
4. Click the button twice.

Expected

1. After two button clicks, the VIEW_COUNT on the button and in the text field should be 3. This is expected because the button value and text field reflects the value that VIEW_COUNT held at the time that the view was rendered.
2. After two button clicks, the number field should be 3. This is expected because, when the button is clicked, the number field is updated with the value that VIEW_COUNT held at the time that the view was rendered. We assign a new onclick closure each time the view is rendered (https://github.com/gesterhuizen/sauron-onclick/blob/master/src/lib.rs#L40):

    fn view(&self) -> Node<Msg> {
        VIEW_COUNT.fetch_add(1, Ordering::SeqCst);
        let vc = VIEW_COUNT.load(Ordering::SeqCst);

...
                    [input(
                        [
                            r#type("button"),
                            value(format!("VIEW_COUNT: {}", vc)),
                            onclick(move |_| {
                                sauron::log(format!("Button is clicked (VIEW_COUNT = {})", vc));
                                Msg::Click(vc)
                            }),
                        ],
                        [],
                    )],

Actual

1. Expected: After two button clicks, VIEW_COUNT on the button and in the text field is 3 (as expected).
2. Unexpected: After two button clicks, the number field displays 1 (and stays in this state, no matter how many times the button is clicked). This is unexpected. It seems like first onclick handler that was set (i.e. when VIEW_COUNT held the value 1) is used even after multiple view rerenders.

[ivanceras]

Thanks for reporting,

In your code, the closure on the onclick function is capturing a value usize vc at the time the onclick is assigned to the actual event listener in js code and then this fix value is always passed to the Msg::Click enum variant. What you may need to do is to load again the value from VIEW_COUNT each time the button is clicked. So it will be reading from the actual source, rather than a snapshot at which the closure is constructed and assigned to the event listener.

Here is the modified view of the code, which works as they way you wanted.

    fn view(&self) -> Node<Msg> {
        div(
            [],
            [
                div(
                    [],
                    [input(
                        [
                            r#type("button"),
                            value(format!("VIEW_COUNT: {}",self.number)),
                            onclick(move |_| {
                                VIEW_COUNT.fetch_add(1, Ordering::SeqCst);
                                let vc = VIEW_COUNT.load(Ordering::SeqCst);
                                sauron::log(format!("Button is clicked (VIEW_COUNT = {})", vc));
                                Msg::Click(vc)
                            }),
                        ],
                        [],
                    )],
                ),
                div(
                    [],
                    [text(format!(
                        "VIEW_COUNT: {}",
                        VIEW_COUNT.load(Ordering::SeqCst)
                    ))],
                ),
                div(
                    [],
                    [text(format!(
                        "number: {}",
                        self.number
                    ))],
                ),
            ],
        )
    }

[gesterhuizen]

Thanks for the response.

Although the proposed solution works in this particular case, I don't think that it generalises to the actual problem I'm trying to solve. In my particular case, I need the onclick behaviour to depend on the application state at the time that view is called (i.e. when the closure is created). That's the situation I tried to recreate in this example. In your proposal, the onclick behaviour depends on the application state at the time that the closure is called (as opposed to when it is created).

The originally observed behaviour, therefore, still surprises me.

Inside view, the vc variable appears in two places in the button's state: it controls the value and it controls the onclick behaviour. The value updates with each view invocation. However, the onclick behaviour always stays the same (for a particular button) between view invocations (each with a different value of vc).

Relevant snippet as follows:

                    [input(
                        [
                            r#type("button"),
                            value(format!("VIEW_COUNT: {}", vc)),
                            onclick(move |_| {
                                sauron::log(format!("Button is clicked (VIEW_COUNT = {})", vc));
                                Msg::Click(vc)
                            }),
                        ],
                        [],
                    )],

[ivanceras]

That's the behavior of the framework for optimization reason. The event listener is attached only one time at the time it is created and is never replaced even with a different closure/function, as there is no easy way to compare closure if they have changed or not. The attributes(ie: value) however changed since there is no performance penalty to keep changing these values at every re-render of the DOM.

Maybe you could also passed the VIEW_COUNT to the Msg::Click directly and read the value in the update call.

[gesterhuizen]

Thanks. The behaviour now makes sense.

What now does not make sense is why your example todomvc app works. Specifically, in view_entry, you register a number of onclick handlers which depend on the item index, idx. The value of idx for a particular item can differ between different view invocations (e.g. when the item at the top of the list is removed). However, for some reason these closures are recreated with the correct idx value after each view invocation. For example, https://github.com/ivanceras/sauron/blob/master/examples/todomvc/src/app.rs#L267:

   input(
                        [
                            class("toggle"),
                            r#type("checkbox"),
                            checked(entry.completed),
                            onclick(move |_| Msg::Toggle(idx)),
                        ],
                        [],
                    ),

[ivanceras]

It works beacuase idx is a usize, which is a Copy type, everytime it is passed around, it will be copy of a new usize. However, you can not apply the same for AtomicUsize, since these are a bit complex types in rust. Simple types such as the primitives: u8,f32,u64,usize, etc, is easy to work on and works as expected while using clean code.

[gesterhuizen]

The vc value used in the closure is not an AtomicUsize, but a usize (it's loaded as a usize at the top of view): https://github.com/gesterhuizen/sauron-onclick/blob/master/src/lib.rs#L31

        let vc : usize = VIEW_COUNT.load(Ordering::SeqCst);

[ivanceras]

I'm closing this issue now, since this not going anywhere, and is not related to sauron in particular but with how you are seeing rust types. If you need more answers about closures and atomics please use the rust-users or /r/rust forums.

[gesterhuizen]

Not trying to be annoying, but a port of the same code to Yew seems to work just fine: https://github.com/gesterhuizen/yew-onclick

I'm really just trying to understand what I'm doing wrong here and what I'm missing. Unfortunately, the explanations given above does not make sense to me (and the issue does not seem fundamentally tied to the way I'm using an atomic).

[ivanceras]

I realized that the root cause of this issue is caused from using Fn inside the Callback instead of FnMut as it is easier to deal with Fn not having to passed arround a lot of mutable reference to Callback. The purpose of event callbacks in the nodes should only be limited to passing what kind of Msg to process, anything complicated should be done in the update method which has access to a mutable reference of the App.