diff --git a/packages/check/src/check.rs b/packages/check/src/check.rs index 5022fea77a..1b0c01cb95 100644 --- a/packages/check/src/check.rs +++ b/packages/check/src/check.rs @@ -37,6 +37,7 @@ pub fn check_file(path: PathBuf, file_content: &str) -> IssueReport { ) } +#[allow(unused)] #[derive(Debug, Clone)] enum Node { If(IfInfo), diff --git a/packages/core-macro/src/props/mod.rs b/packages/core-macro/src/props/mod.rs index 7f9aefa6e8..f0aee564b3 100644 --- a/packages/core-macro/src/props/mod.rs +++ b/packages/core-macro/src/props/mod.rs @@ -648,38 +648,45 @@ mod struct_info { )* }; + // If there are signals, we automatically try to memoize the signals if !signal_fields.is_empty() { Ok(quote! { - // First check if the fields are equal + // First check if the fields are equal. This will compare the signal fields by pointer let exactly_equal = self == new; if exactly_equal { + // If they are return early, they can be memoized without any changes return true; } - // If they are not, move over the signal fields and check if they are equal + // If they are not, move the signal fields into self and check if they are equal now that the signal fields are equal #( let mut #signal_fields = self.#signal_fields; self.#signal_fields = new.#signal_fields; )* - // Then check if the fields are equal again + // Then check if the fields are equal now that we know the signal fields are equal + // 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 still need to rerun the component + // If they are not equal, we need to move over all the fields to self if !non_signal_fields_equal { - return false; + *self = new.clone(); } - + // 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 #move_signal_fields #move_event_handlers - true + non_signal_fields_equal }) } else { Ok(quote! { let equal = self == new; - if equal { - #move_event_handlers + // 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 !equal { + *self = new.clone(); } equal }) diff --git a/packages/core-macro/tests/values_memoize_in_place.rs b/packages/core-macro/tests/values_memoize_in_place.rs new file mode 100644 index 0000000000..0aac110769 --- /dev/null +++ b/packages/core-macro/tests/values_memoize_in_place.rs @@ -0,0 +1,75 @@ +use dioxus::prelude::*; + +thread_local! { + static DROP_COUNT: std::cell::RefCell = const { std::cell::RefCell::new(0) }; +} + +#[test] +fn values_memoize_in_place() { + let mut dom = VirtualDom::new(app); + + dom.rebuild_in_place(); + dom.mark_dirty(ScopeId::ROOT); + for _ in 0..20 { + dom.render_immediate(&mut dioxus_core::NoOpMutations); + } + dom.render_immediate(&mut dioxus_core::NoOpMutations); + // As we rerun the app, the drop count should be 15 one for each render of the app component + let drop_count = DROP_COUNT.with(|c| *c.borrow()); + assert_eq!(drop_count, 15); +} + +struct CountsDrop; + +impl Drop for CountsDrop { + fn drop(&mut self) { + DROP_COUNT.with(|c| *c.borrow_mut() += 1); + } +} + +fn app() -> Element { + let mut count = use_signal(|| 0); + let x = CountsDrop; + + if generation() < 15 { + count += 1; + } + + rsx! { + TakesEventHandler { + click: move |num| { + // Force the closure to own the drop counter + let _ = &x; + println!("num is {num}"); + }, + children: count() / 2 + } + TakesSignal { sig: count(), children: count() / 2 } + } +} + +#[component] +fn TakesEventHandler(click: EventHandler, children: usize) -> Element { + let first_render_click = use_hook(move || click); + if generation() > 0 { + // Make sure the event handler is memoized in place and never gets dropped + first_render_click(children); + } + + rsx! { + button { "{children}" } + } +} + +#[component] +fn TakesSignal(sig: ReadOnlySignal, children: usize) -> Element { + let first_render_sig = use_hook(move || sig); + if generation() > 0 { + // Make sure the signal is memoized in place and never gets dropped + println!("{first_render_sig}"); + } + + rsx! { + button { "{children}" } + } +} diff --git a/packages/core/src/any_props.rs b/packages/core/src/any_props.rs index 760b821dcb..dadcec4797 100644 --- a/packages/core/src/any_props.rs +++ b/packages/core/src/any_props.rs @@ -11,7 +11,7 @@ pub(crate) type BoxedAnyProps = Box; pub(crate) trait AnyProps: 'static { /// Render the component with the internal props. fn render(&self) -> RenderReturn; - /// Check if the props are the same as the type erased props of another component. + /// Make the old props equal to the new type erased props. Return if the props were equal and should be memoized. fn memoize(&mut self, other: &dyn Any) -> bool; /// Get the props as a type erased `dyn Any`. fn props(&self) -> &dyn Any; diff --git a/packages/core/src/diff/component.rs b/packages/core/src/diff/component.rs index eae4146621..2b75fe40ba 100644 --- a/packages/core/src/diff/component.rs +++ b/packages/core/src/diff/component.rs @@ -83,9 +83,6 @@ impl VNode { return; } - // First, move over the props from the old to the new, dropping old props in the process - dom.scopes[scope_id.0].props = new.props.duplicate(); - // Now run the component and diff it let new = dom.run_scope(scope_id); dom.diff_scope(to, scope_id, new); diff --git a/packages/core/src/properties.rs b/packages/core/src/properties.rs index f660cafda2..073b248eb4 100644 --- a/packages/core/src/properties.rs +++ b/packages/core/src/properties.rs @@ -31,7 +31,7 @@ pub trait Properties: Clone + Sized + 'static { /// Create a builder for this component. fn builder() -> Self::Builder; - /// Compare two props to see if they are memoizable. + /// Make the old props equal to the new props. Return if the props were equal and should be memoized. fn memoize(&mut self, other: &Self) -> bool; /// Create a component from the props. diff --git a/packages/hooks/tests/memo.rs b/packages/hooks/tests/memo.rs index 9a33506c81..186370f798 100644 --- a/packages/hooks/tests/memo.rs +++ b/packages/hooks/tests/memo.rs @@ -5,7 +5,7 @@ async fn memo_updates() { use dioxus::prelude::*; thread_local! { - static VEC_SIGNAL: RefCell, SyncStorage>>> = RefCell::new(None); + static VEC_SIGNAL: RefCell, SyncStorage>>> = const { RefCell::new(None) }; } fn app() -> Element {