From f53969896317ed96aec8224e5e197e3592e45951 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 26 Feb 2024 11:46:01 -0600 Subject: [PATCH] add debug information for signal subscriptions --- Cargo.lock | 11 ++++++ packages/core/src/diff/component.rs | 6 +-- packages/core/src/scopes.rs | 19 ++++++++- packages/core/src/virtual_dom.rs | 2 +- packages/hooks/src/use_effect.rs | 8 ++-- packages/signals/src/reactive_context.rs | 49 +++++++++++++++++++++++- packages/signals/src/signal.rs | 14 ++++++- 7 files changed, 96 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 52f5a173a..28babd636 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2429,6 +2429,7 @@ name = "dioxus-examples" version = "0.4.3" dependencies = [ "dioxus", + "dioxus-logger", "dioxus-ssr", "form_urlencoded", "futures-util", @@ -2614,6 +2615,16 @@ dependencies = [ "tracing", ] +[[package]] +name = "dioxus-logger" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3d7cbab0b5519060fe9e14b3c21e3f2329b8386cd905618f78c7b929cd00cf54" +dependencies = [ + "log", + "web-sys", +] + [[package]] name = "dioxus-mobile" version = "0.4.3" diff --git a/packages/core/src/diff/component.rs b/packages/core/src/diff/component.rs index 6d4ab005e..651823c5c 100644 --- a/packages/core/src/diff/component.rs +++ b/packages/core/src/diff/component.rs @@ -79,11 +79,7 @@ impl VNode { // The target ScopeState still has the reference to the old props, so there's no need to update anything // This also implicitly drops the new props since they're not used if old_props.memoize(new_props.props()) { - tracing::trace!( - "Memoized props for component {:#?} ({})", - scope_id, - old_scope.state().name - ); + tracing::trace!("Memoized props for component {:#?}", scope_id,); return; } diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 632b42fa3..b34187472 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -9,9 +9,26 @@ use std::{cell::Ref, fmt::Debug, rc::Rc}; /// time. We do try and guarantee that between calls to `wait_for_work`, no ScopeIds will be recycled in order to give /// time for any logic that relies on these IDs to properly update. #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, PartialOrd, Ord)] +#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct ScopeId(pub usize); +impl std::fmt::Debug for ScopeId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut builder = f.debug_tuple("ScopeId"); + let mut builder = builder.field(&self.0); + #[cfg(debug_assertions)] + { + if let Some(name) = Runtime::current() + .as_ref() + .and_then(|rt| rt.get_state(*self)) + { + builder = builder.field(&name.name); + } + } + builder.finish() + } +} + impl ScopeId { /// The root ScopeId. /// diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 2de7f0514..9e78f53db 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -373,7 +373,7 @@ impl VirtualDom { return; }; - tracing::trace!("Marking scope {:?} ({}) as dirty", id, scope.name); + tracing::trace!("Marking scope {:?} as dirty", id); self.dirty_scopes.insert(DirtyScope { height: scope.height(), id, diff --git a/packages/hooks/src/use_effect.rs b/packages/hooks/src/use_effect.rs index a3d16f420..7811d5171 100644 --- a/packages/hooks/src/use_effect.rs +++ b/packages/hooks/src/use_effect.rs @@ -5,17 +5,19 @@ use dioxus_signals::ReactiveContext; /// The signal will be owned by the current component and will be dropped when the component is dropped. /// /// If the use_effect call was skipped due to an early return, the effect will no longer activate. +#[track_caller] pub fn use_effect(mut callback: impl FnMut() + 'static) { // let mut run_effect = use_hook(|| CopyValue::new(true)); // use_hook_did_run(move |did_run| run_effect.set(did_run)); + let location = std::panic::Location::caller(); + use_hook(|| { spawn(async move { - let rc = ReactiveContext::new(); - + let rc = ReactiveContext::new_with_origin(location); loop { // Wait for the dom the be finished with sync work - flush_sync().await; + // flush_sync().await; // Run the effect rc.run_in(&mut callback); diff --git a/packages/signals/src/reactive_context.rs b/packages/signals/src/reactive_context.rs index fdbaf6429..98b611bb1 100644 --- a/packages/signals/src/reactive_context.rs +++ b/packages/signals/src/reactive_context.rs @@ -22,20 +22,47 @@ thread_local! { static CURRENT: RefCell> = const { RefCell::new(vec![]) }; } +impl std::fmt::Display for ReactiveContext { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let read = self.inner.read(); + match read.scope_subscriber { + Some(scope) => write!(f, "ReactiveContext for scope {:?}", scope), + None => { + #[cfg(debug_assertions)] + return write!(f, "ReactiveContext created at {}", read.origin); + #[cfg(not(debug_assertions))] + write!(f, "ReactiveContext") + } + } + } +} + impl Default for ReactiveContext { + #[track_caller] fn default() -> Self { - Self::new_for_scope(None) + Self::new_for_scope(None, std::panic::Location::caller()) } } impl ReactiveContext { /// Create a new reactive context + #[track_caller] pub fn new() -> Self { Self::default() } + /// Create a new reactive context with a location for debugging purposes + /// This is useful for reactive contexts created within closures + pub fn new_with_origin(origin: &'static std::panic::Location<'static>) -> Self { + Self::new_for_scope(None, origin) + } + /// Create a new reactive context that may update a scope - pub(crate) fn new_for_scope(scope: Option) -> Self { + #[allow(unused)] + pub(crate) fn new_for_scope( + scope: Option, + origin: &'static std::panic::Location<'static>, + ) -> Self { let (tx, rx) = flume::unbounded(); let mut scope_subscribers = FxHashSet::default(); @@ -49,6 +76,8 @@ impl ReactiveContext { self_: None, update_any: schedule_update_any(), receiver: rx, + #[cfg(debug_assertions)] + origin, }; let mut self_ = Self { @@ -87,6 +116,7 @@ impl ReactiveContext { // Otherwise, create a new context at the current scope Some(provide_context(ReactiveContext::new_for_scope( current_scope_id(), + std::panic::Location::caller(), ))) } @@ -108,6 +138,17 @@ impl ReactiveContext { /// Returns true if the context was marked as dirty, or false if the context has been dropped pub fn mark_dirty(&self) -> bool { if let Ok(self_read) = self.inner.try_read() { + #[cfg(debug_assertions)] + { + if let Some(scope) = self_read.scope_subscriber { + tracing::trace!("Marking reactive context for scope {:?} as dirty", scope); + } else { + tracing::trace!( + "Marking reactive context created at {} as dirty", + self_read.origin + ); + } + } if let Some(scope) = self_read.scope_subscriber { (self_read.update_any)(scope); } @@ -148,4 +189,8 @@ struct Inner { // Futures will call .changed().await sender: flume::Sender<()>, receiver: flume::Receiver<()>, + + // Debug information for signal subscriptions + #[cfg(debug_assertions)] + origin: &'static std::panic::Location<'static>, } diff --git a/packages/signals/src/signal.rs b/packages/signals/src/signal.rs index 44de51082..faa39009b 100644 --- a/packages/signals/src/signal.rs +++ b/packages/signals/src/signal.rs @@ -202,6 +202,7 @@ impl>> Readable for Signal { let inner = self.inner.try_read()?; if let Some(reactive_context) = ReactiveContext::current() { + tracing::trace!("Subscribing to the reactive context {}", reactive_context); inner.subscribers.lock().unwrap().insert(reactive_context); } @@ -244,7 +245,11 @@ impl>> Writable for Signal { let borrow = S::map_mut(inner, |v| &mut v.value); Write { write: borrow, - drop_signal: Box::new(SignalSubscriberDrop { signal: *self }), + drop_signal: Box::new(SignalSubscriberDrop { + signal: *self, + #[cfg(debug_assertions)] + origin: std::panic::Location::caller(), + }), } }) } @@ -344,10 +349,17 @@ impl DerefMut for Write { struct SignalSubscriberDrop>> { signal: Signal, + #[cfg(debug_assertions)] + origin: &'static std::panic::Location<'static>, } impl>> Drop for SignalSubscriberDrop { fn drop(&mut self) { + #[cfg(debug_assertions)] + tracing::trace!( + "Write on signal at {:?} finished, updating subscribers", + self.origin + ); self.signal.update_subscribers(); } }