Don't move over event handler fields when diffing props (#2129)

* don't move over event handler fields when diffing props

* Expand values_memoize_in_place test to cover the new bug case
This commit is contained in:
Evan Almloff 2024-03-22 17:18:38 -05:00 committed by GitHub
parent 0662033c84
commit b19a546c0a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 34 additions and 6 deletions

View file

@ -641,6 +641,15 @@ mod struct_info {
})
.collect();
let regular_fields: Vec<_> = self
.included_fields()
.filter(|f| !looks_like_signal_type(f.ty) && !looks_like_event_handler_type(f.ty))
.map(|f| {
let name = f.name;
quote!(#name)
})
.collect();
let move_event_handlers = quote! {
#(
// Update the event handlers
@ -668,9 +677,12 @@ mod struct_info {
// NOTE: we don't compare other fields individually because we want to let users opt-out of memoization for certain fields by implementing PartialEq themselves
let non_signal_fields_equal = self == new;
// If they are not equal, we need to move over all the fields to self
// If they are not equal, we need to move over all the fields that are not event handlers or signals to self
if !non_signal_fields_equal {
*self = new.clone();
let new_clone = new.clone();
#(
self.#regular_fields = new_clone.#regular_fields;
)*
}
// Move any signal and event fields into their old container.
// We update signals and event handlers in place so that they are always up to date even if they were moved into a future in a previous render
@ -684,9 +696,12 @@ mod struct_info {
let equal = self == new;
// Move any signal and event fields into their old container.
#move_event_handlers
// If they are not equal, we need to move over all the fields to self
// If they are not equal, we need to move over all the fields that are not event handlers to self
if !equal {
*self = new.clone();
let new_clone = new.clone();
#(
self.#regular_fields = new_clone.#regular_fields;
)*
}
equal
})

View file

@ -1,4 +1,6 @@
use dioxus::prelude::*;
use dioxus_core::ElementId;
use std::rc::Rc;
thread_local! {
static DROP_COUNT: std::cell::RefCell<usize> = const { std::cell::RefCell::new(0) };
@ -6,11 +8,19 @@ thread_local! {
#[test]
fn values_memoize_in_place() {
set_event_converter(Box::new(dioxus::html::SerializedHtmlEventConverter));
let mut dom = VirtualDom::new(app);
dom.rebuild_in_place();
let mutations = dom.rebuild_to_vec();
println!("{:#?}", mutations);
dom.mark_dirty(ScopeId::ROOT);
for _ in 0..20 {
dom.handle_event(
"click",
Rc::new(PlatformEventData::new(Box::<SerializedMouseData>::default())),
ElementId(1),
true,
);
dom.render_immediate(&mut dioxus_core::NoOpMutations);
}
dom.render_immediate(&mut dioxus_core::NoOpMutations);
@ -57,7 +67,10 @@ fn TakesEventHandler(click: EventHandler<usize>, children: usize) -> Element {
}
rsx! {
button { "{children}" }
button {
onclick: move |_| click(children),
"{children}"
}
}
}