diff --git a/packages/hooks/src/use_memo.rs b/packages/hooks/src/use_memo.rs index 467795a95..8a5b71d38 100644 --- a/packages/hooks/src/use_memo.rs +++ b/packages/hooks/src/use_memo.rs @@ -1,6 +1,6 @@ use crate::use_callback; use dioxus_core::prelude::*; -use dioxus_signals::{Memo, Signal}; +use dioxus_signals::Memo; #[doc = include_str!("../docs/derived_state.md")] #[doc = include_str!("../docs/rules_of_hooks.md")] @@ -8,6 +8,7 @@ use dioxus_signals::{Memo, Signal}; #[track_caller] pub fn use_memo(mut f: impl FnMut() -> R + 'static) -> Memo { let callback = use_callback(move |_| f()); + let caller = std::panic::Location::caller(); #[allow(clippy::redundant_closure)] - use_hook(|| Signal::memo(move || callback(()))) + use_hook(|| Memo::new_with_location(move || callback(()), caller)) } diff --git a/packages/signals/src/memo.rs b/packages/signals/src/memo.rs index 2e9bbe793..3b1bdab70 100644 --- a/packages/signals/src/memo.rs +++ b/packages/signals/src/memo.rs @@ -139,10 +139,11 @@ impl Memo { drop(peak); let mut copy = self.inner; copy.set(new_value); - update_write - .dirty - .store(false, std::sync::atomic::Ordering::Relaxed); } + // Always mark the memo as no longer dirty even if the value didn't change + update_write + .dirty + .store(false, std::sync::atomic::Ordering::Relaxed); } /// Get the scope that the signal was created in. diff --git a/packages/signals/src/signal.rs b/packages/signals/src/signal.rs index 65f6cd85b..64fa34ac7 100644 --- a/packages/signals/src/signal.rs +++ b/packages/signals/src/signal.rs @@ -557,6 +557,7 @@ struct SignalSubscriberDrop>> { #[allow(clippy::no_effect)] impl>> Drop for SignalSubscriberDrop { fn drop(&mut self) { + println!("dropping signal subscriber"); #[cfg(debug_assertions)] { tracing::trace!( diff --git a/packages/signals/tests/memo.rs b/packages/signals/tests/memo.rs index 2b4ea2cab..0aa065d22 100644 --- a/packages/signals/tests/memo.rs +++ b/packages/signals/tests/memo.rs @@ -1,148 +1,178 @@ -// TODO: fix #1935 +#![allow(unused, non_upper_case_globals, non_snake_case)] +use dioxus::html::p; +use dioxus::prelude::*; +use dioxus_core::ElementId; +use dioxus_core::NoOpMutations; +use dioxus_signals::*; +use std::cell::RefCell; +use std::collections::HashMap; +use std::rc::Rc; +use std::sync::atomic::{AtomicBool, Ordering}; -// #![allow(unused, non_upper_case_globals, non_snake_case)] -// use dioxus_core::NoOpMutations; -// use std::collections::HashMap; -// use std::rc::Rc; +#[test] +fn memos_rerun() { + let _ = simple_logger::SimpleLogger::new().init(); -// use dioxus::html::p; -// use dioxus::prelude::*; -// use dioxus_core::ElementId; -// use dioxus_signals::*; -// use std::cell::RefCell; + #[derive(Default)] + struct RunCounter { + component: usize, + effect: usize, + } -// #[test] -// fn memos_rerun() { -// let _ = simple_logger::SimpleLogger::new().init(); + let counter = Rc::new(RefCell::new(RunCounter::default())); + let mut dom = VirtualDom::new_with_props( + |counter: Rc>| { + counter.borrow_mut().component += 1; -// #[derive(Default)] -// struct RunCounter { -// component: usize, -// effect: usize, -// } + let mut signal = use_signal(|| 0); + let memo = use_memo({ + to_owned![counter]; + move || { + counter.borrow_mut().effect += 1; + println!("Signal: {:?}", signal); + signal() + } + }); + assert_eq!(memo(), 0); + signal += 1; + assert_eq!(memo(), 1); -// let counter = Rc::new(RefCell::new(RunCounter::default())); -// let mut dom = VirtualDom::new_with_props( -// |counter: Rc>| { -// counter.borrow_mut().component += 1; + rsx! { + div {} + } + }, + counter.clone(), + ); -// let mut signal = use_signal(|| 0); -// let memo = use_memo({ -// to_owned![counter]; -// move || { -// counter.borrow_mut().effect += 1; -// println!("Signal: {:?}", signal); -// signal() -// } -// }); -// assert_eq!(memo(), 0); -// signal += 1; -// assert_eq!(memo(), 1); + dom.rebuild_in_place(); -// rsx! { -// div {} -// } -// }, -// counter.clone(), -// ); + let current_counter = counter.borrow(); + assert_eq!(current_counter.component, 1); + assert_eq!(current_counter.effect, 2); +} -// dom.rebuild_in_place(); +#[test] +fn memos_prevents_component_rerun() { + let _ = simple_logger::SimpleLogger::new().init(); -// let current_counter = counter.borrow(); -// assert_eq!(current_counter.component, 1); -// assert_eq!(current_counter.effect, 2); -// } + #[derive(Default)] + struct RunCounter { + component: usize, + memo: usize, + } -// #[test] -// fn memos_prevents_component_rerun() { -// let _ = simple_logger::SimpleLogger::new().init(); + let counter = Rc::new(RefCell::new(RunCounter::default())); + let mut dom = VirtualDom::new_with_props( + |props: Rc>| { + let mut signal = use_signal(|| 0); -// #[derive(Default)] -// struct RunCounter { -// component: usize, -// memo: usize, -// } + if generation() == 1 { + *signal.write() = 0; + } + if generation() == 2 { + println!("Writing to signal"); + *signal.write() = 1; + } -// let counter = Rc::new(RefCell::new(RunCounter::default())); -// let mut dom = VirtualDom::new_with_props( -// |props: Rc>| { -// let mut signal = use_signal(|| 0); + rsx! { + Child { + signal: signal, + counter: props.clone(), + } + } + }, + counter.clone(), + ); -// if generation() == 1 { -// *signal.write() = 0; -// } -// if generation() == 2 { -// println!("Writing to signal"); -// *signal.write() = 1; -// } + #[derive(Default, Props, Clone)] + struct ChildProps { + signal: Signal, + counter: Rc>, + } -// rsx! { -// Child { -// signal: signal, -// counter: props.clone(), -// } -// } -// }, -// counter.clone(), -// ); + impl PartialEq for ChildProps { + fn eq(&self, other: &Self) -> bool { + self.signal == other.signal + } + } -// #[derive(Default, Props, Clone)] -// struct ChildProps { -// signal: Signal, -// counter: Rc>, -// } + fn Child(props: ChildProps) -> Element { + let counter = &props.counter; + let signal = props.signal; + counter.borrow_mut().component += 1; -// impl PartialEq for ChildProps { -// fn eq(&self, other: &Self) -> bool { -// self.signal == other.signal -// } -// } + let memo = use_memo({ + to_owned![counter]; + move || { + counter.borrow_mut().memo += 1; + println!("Signal: {:?}", signal); + signal() + } + }); + match generation() { + 0 => { + assert_eq!(memo(), 0); + } + 1 => { + assert_eq!(memo(), 1); + } + _ => panic!("Unexpected generation"), + } -// fn Child(props: ChildProps) -> Element { -// let counter = &props.counter; -// let signal = props.signal; -// counter.borrow_mut().component += 1; + rsx! { + div {} + } + } -// let memo = use_memo({ -// to_owned![counter]; -// move || { -// counter.borrow_mut().memo += 1; -// println!("Signal: {:?}", signal); -// signal() -// } -// }); -// match generation() { -// 0 => { -// assert_eq!(memo(), 0); -// } -// 1 => { -// assert_eq!(memo(), 1); -// } -// _ => panic!("Unexpected generation"), -// } + dom.rebuild_in_place(); + dom.mark_dirty(ScopeId::APP); + dom.render_immediate(&mut NoOpMutations); -// rsx! { -// div {} -// } -// } + { + let current_counter = counter.borrow(); + assert_eq!(current_counter.component, 1); + assert_eq!(current_counter.memo, 2); + } -// dom.rebuild_in_place(); -// dom.mark_dirty(ScopeId::ROOT); -// dom.render_immediate(&mut NoOpMutations); + dom.mark_dirty(ScopeId::APP); + dom.render_immediate(&mut NoOpMutations); + dom.render_immediate(&mut NoOpMutations); -// { -// let current_counter = counter.borrow(); -// assert_eq!(current_counter.component, 1); -// assert_eq!(current_counter.memo, 2); -// } + { + let current_counter = counter.borrow(); + assert_eq!(current_counter.component, 2); + assert_eq!(current_counter.memo, 3); + } +} -// dom.mark_dirty(ScopeId::ROOT); -// dom.render_immediate(&mut NoOpMutations); -// dom.render_immediate(&mut NoOpMutations); +// Regression test for https://github.com/DioxusLabs/dioxus/issues/2990 +#[test] +fn memos_sync_rerun_after_unrelated_write() { + static PASSED: AtomicBool = AtomicBool::new(false); + let mut dom = VirtualDom::new(|| { + let mut signal = use_signal(|| 0); + let memo = use_memo(move || dbg!(signal() < 2)); + if generation() == 0 { + assert!(memo()); + signal += 1; + } else { + // It should be fine to hold the write and read the memo at the same time + let mut write = signal.write(); + println!("Memo: {:?}", memo()); + assert!(memo()); + *write = 2; + drop(write); + assert!(!memo()); + PASSED.store(true, std::sync::atomic::Ordering::SeqCst); + } -// { -// let current_counter = counter.borrow(); -// assert_eq!(current_counter.component, 2); -// assert_eq!(current_counter.memo, 3); -// } -// } + rsx! { + div {} + } + }); + + dom.rebuild_in_place(); + dom.mark_dirty(ScopeId::APP); + dom.render_immediate(&mut NoOpMutations); + assert!(PASSED.load(Ordering::SeqCst)); +}