From b19a546c0ac639a5cf9e942fff7729ad30e91a5e Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Fri, 22 Mar 2024 17:18:38 -0500 Subject: [PATCH] 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 --- packages/core-macro/src/props/mod.rs | 23 +++++++++++++++---- .../tests/values_memoize_in_place.rs | 17 ++++++++++++-- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/packages/core-macro/src/props/mod.rs b/packages/core-macro/src/props/mod.rs index f0aee564b..f0f8b7c8d 100644 --- a/packages/core-macro/src/props/mod.rs +++ b/packages/core-macro/src/props/mod.rs @@ -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 }) diff --git a/packages/core-macro/tests/values_memoize_in_place.rs b/packages/core-macro/tests/values_memoize_in_place.rs index 0aac11076..4e0e306ff 100644 --- a/packages/core-macro/tests/values_memoize_in_place.rs +++ b/packages/core-macro/tests/values_memoize_in_place.rs @@ -1,4 +1,6 @@ use dioxus::prelude::*; +use dioxus_core::ElementId; +use std::rc::Rc; thread_local! { static DROP_COUNT: std::cell::RefCell = 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::::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, children: usize) -> Element { } rsx! { - button { "{children}" } + button { + onclick: move |_| click(children), + "{children}" + } } }