window_events only appears to emit first event

[jgrund]

Consider:

        fn window_events(model: &Model) -> Vec<seed::dom_types::Listener<Msg>> {
            vec![
                mouse_ev(Ev::Click, |_| Msg::Increment),
                mouse_ev(Ev::Click, |_| Msg::Increment),
                mouse_ev(Ev::Click, |_| Msg::Increment),
                mouse_ev(Ev::Click, |_| Msg::Increment),
                mouse_ev(Ev::Click, |_| Msg::Increment),
            ]
        }

Where Msg::Increment just increments a counter on the model that starts at 0.

When a click event occurs, I'd expect the counter to increment to 5. Instead it only increments to 1.

Interestingly, all 5 listeners can be seen on the window:

I think there may be some sort of listener detach / re-attach that occurs after the first event and somehow causes the rest to be skipped.

[jgrund]

After the first event triggers an update, setup_window_listeners is called, which in turn detaches / reattaches the events.

Once the detach occurs, the remaining events are no longer registered as listeners and are skipped.

Commenting out self.setup_window_listeners here:

https://github.com/David-OConnor/seed/blob/48b814541242aab8166d0801fef21b7dbd1b5a32/src/vdom.rs#L441-L449

Allows the counter to increment to 5 as expected.

[David-OConnor]

I can reproduce. Removing that line isn't the solution, because it's required for the listeners to update when the model changes. I wonder if there's a way to flag the message as having come from a window listener, and not interrupt the process part-way through.

[jgrund]

Could extra info be added regarding currentTarget being window, and skip detach / reattach if that's the case?

I don't have enough context to know if that will have follow-on effects if skipping under certain situations...

[David-OConnor]

Could try that. Need to verify this doesn't also affect normal elements that have multiple listeners. If it does, may need to internally lump them into one

[jgrund]

Temporary workaround is to delegate the handling to the update fn by registering a single handler per event type needed. E.g.:

fn window_events(_model: &Model) -> Vec<seed::dom_types::Listener<Msg>> {
    vec![simple_ev(Ev::Click, Msg::WindowClick)]
}

[David-OConnor]

Concur on your workaround until fixed. As you observed, the process is being interrupted between update calls. Appears to only affect window events, where there are multiple listeners for the same trigger. I'd like to keep the API the same and fix this, but putting the code in the update function's fine as well.

[MartinKavik]

If I understand correctly, the main problem is that we can't diff event listeners?

I was looking for some inspiration in dodrio and on the first look:

• diff algorithm - it compares event ids and if both listeners handle the same event, then it only replaces old callback with the new one
• update_event_listener
• struct Listener

I don't know current Seed implementation and how much abstraction is hiding between those VirtualDOM listeners and native DOM listeners, but I think it should help with this issue and probably improve performance.

[David-OConnor]

You may be onto something. Currently, all listeners are replaced every update, due to being unable to diff the closures. This is probably associated with a performance penalty, as you mentioned. I'll take a look at dodrio's implementation. Of note, this bug doesn't appear to affect multiple same-trigger listeners on normal elements.

[MartinKavik]

Of note, this bug doesn't appear to affect multiple same-trigger listeners on normal elements.

My first guess: Maybe because we reattach window listeners in every update but diff of normal elements can be skipped (whole render is skipped or some of parents?).
Good luck, I'm working on the realworld example now, if you want some help / code review, etc. just ping me.

[David-OConnor]

I'm leaning towards a breaking change here: Msg must implement PartialEq, and the event constructors other than raw_ev will have signature (trigger, message), ie what simple_ev has now. This will allow us to diff most listeners completely by comparing trigger and message directly, without involving closures. In the event more complex behavior is desired in the view, you can still pass a closure with raw_ev, and accept the slower diffing. (And possible errors like this until resolved).

What do you think? I have most of it implemented already.

[MartinKavik]

I think it would be annoying - you would have to write custom implementation of PartialEq for your Msgs because you can't derive it automatically in many cases.
Could you push your branch?

[MartinKavik]

I want to know how do you pass data from event (e.g. changed input text) to message / update function / model when the new signature is (trigger, message)?

Look at my rust playground example or at the documentation - it should resolve "PartialEq problem" if we assume that Msg is always enum.

[David-OConnor]

Nice workaround with mem; (Related: the dodrio implementation you linked earlier appeared to use mem to diff the closures, but I didn't understand it) I hadn't considered cases where other data types are included in the Msg variants, which would indeed be annoying.

I'll post the branch once I've got it working, or if unable to fix the obstacle I'm stuck on.

[MartinKavik]

Related: the dodrio implementation you linked earlier appeared to use mem to diff the closures, but I didn't understand it

Could you send link to the code please?