Merge pull request #1982 from ealmloff/debug-subscriptions

Add debug information for signal subscriptions

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;
         }
 

packages/core/src/scopes.rs

@@ -1,7 +1,7 @@
 use crate::{
     any_props::BoxedAnyProps, nodes::RenderReturn, runtime::Runtime, scope_context::Scope,
 };
-use std::{cell::Ref, fmt::Debug, rc::Rc};
+use std::{cell::Ref, rc::Rc};
 
 /// A component's unique identifier.
 ///
@@ -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.
     ///

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,

packages/hooks/src/use_effect.rs

@@ -17,17 +17,19 @@ use dioxus_signals::ReactiveContext;
 ///     }
 /// }
 /// ```
+#[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);

packages/signals/src/reactive_context.rs

@@ -22,20 +22,47 @@ thread_local! {
     static CURRENT: RefCell<Vec<ReactiveContext>> = 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<ScopeId>) -> Self {
+    #[allow(unused)]
+    pub(crate) fn new_for_scope(
+        scope: Option<ScopeId>,
+        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>,
 }

packages/signals/src/signal.rs

@@ -202,6 +202,7 @@ impl<T, S: Storage<SignalData<T>>> Readable for Signal<T, S> {
         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<T: 'static, S: Storage<SignalData<T>>> Writable for Signal<T, S> {
             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<T: ?Sized, S: AnyStorage> DerefMut for Write<T, S> {
 
 struct SignalSubscriberDrop<T: 'static, S: Storage<SignalData<T>>> {
     signal: Signal<T, S>,
+    #[cfg(debug_assertions)]
+    origin: &'static std::panic::Location<'static>,
 }
 
 impl<T: 'static, S: Storage<SignalData<T>>> Drop for SignalSubscriberDrop<T, S> {
     fn drop(&mut self) {
+        #[cfg(debug_assertions)]
+        tracing::trace!(
+            "Write on signal at {:?} finished, updating subscribers",
+            self.origin
+        );
         self.signal.update_subscribers();
     }
 }