Warnings for signal writes that may cause infinite loops (#2277)

* warnings for signal writes that may cause infinite loops

* improve debugging output for reactive contexts on scopes

* expand help message

packages/signals/src/reactive_context.rs

@@ -28,6 +28,9 @@ impl std::fmt::Display for ReactiveContext {
         {
             use crate::Readable;
             if let Ok(read) = self.inner.try_read() {
+                if let Some(scope) = read.scope {
+                    return write!(f, "ReactiveContext(for scope: {:?})", scope);
+                }
                 return write!(f, "ReactiveContext created at {}", read.origin);
             }
         }
@@ -66,6 +69,8 @@ impl ReactiveContext {
             update: Box::new(callback),
             #[cfg(debug_assertions)]
             origin,
+            #[cfg(debug_assertions)]
+            scope: None,
         };
 
         let mut self_ = Self {
@@ -105,11 +110,18 @@ impl ReactiveContext {
         };
 
         // Otherwise, create a new context at the current scope
-        Some(provide_context(ReactiveContext::new_with_callback(
+        #[allow(unused_mut)]
+        let mut reactive_context = ReactiveContext::new_with_callback(
             update_scope,
             scope_id,
             std::panic::Location::caller(),
-        )))
+        );
+        #[cfg(debug_assertions)]
+        {
+            // Associate the reactive context with the current scope for debugging
+            reactive_context.inner.write().scope = Some(scope_id);
+        }
+        Some(provide_context(reactive_context))
     }
 
     /// Run this function in the context of this reactive context
@@ -167,4 +179,8 @@ struct Inner {
     // Debug information for signal subscriptions
     #[cfg(debug_assertions)]
     origin: &'static std::panic::Location<'static>,
+
+    #[cfg(debug_assertions)]
+    // The scope that this reactive context is associated with
+    scope: Option<ScopeId>,
 }

packages/signals/src/signal.rs

@@ -346,6 +346,8 @@ impl<T: 'static, S: Storage<SignalData<T>>> Writable for Signal<T, S> {
     fn try_write_unchecked(
         &self,
     ) -> Result<WritableRef<'static, Self>, generational_box::BorrowMutError> {
+        #[cfg(debug_assertions)]
+        let origin = std::panic::Location::caller();
         self.inner.try_write_unchecked().map(|inner| {
             let borrow = S::map_mut(inner, |v| &mut v.value);
             Write {
@@ -353,7 +355,7 @@ impl<T: 'static, S: Storage<SignalData<T>>> Writable for Signal<T, S> {
                 drop_signal: Box::new(SignalSubscriberDrop {
                     signal: *self,
                     #[cfg(debug_assertions)]
-                    origin: std::panic::Location::caller(),
+                    origin,
                 }),
             }
         })
@@ -476,6 +478,68 @@ impl<T: ?Sized, S: AnyStorage> DerefMut for Write<'_, T, S> {
     }
 }
 
+#[allow(unused)]
+const SIGNAL_READ_WRITE_SAME_SCOPE_HELP: &str = r#"This issue is caused by reading and writing to the same signal in a reactive scope. Components, effects, memos, and resources each have their own a reactive scopes. Reactive scopes rerun when any signal you read inside of them are changed. If you read and write to the same signal in the same scope, the write will cause the scope to rerun and trigger the write again. This can cause an infinite loop.
+
+You can fix the issue by either:
+1) Splitting up your state and Writing, reading to different signals:
+
+For example, you could change this broken code:
+
+#[derive(Clone, Copy)]
+struct Counts {
+    count1: i32,
+    count2: i32,
+}
+
+fn app() -> Element {
+    let mut counts = use_signal(|| Counts { count1: 0, count2: 0 });
+    
+    use_effect(move || {
+        // This effect both reads and writes to counts
+        counts.write().count1 = counts().count2;
+    })
+}
+
+Into this working code:
+
+fn app() -> Element {
+    let mut count1 = use_signal(|| 0);
+    let mut count2 = use_signal(|| 0);
+
+    use_effect(move || {
+        count1.write(count2());
+    });
+}
+2) Reading and Writing to the same signal in different scopes:
+
+For example, you could change this broken code:
+
+fn app() -> Element {
+    let mut count = use_signal(|| 0);
+
+    use_effect(move || {
+        // This effect both reads and writes to count
+        println!("{}", count());
+        count.write(count());
+    });
+}
+
+
+To this working code:
+
+fn app() -> Element {
+    let mut count = use_signal(|| 0);
+
+    use_effect(move || {
+        count.write(count());
+    });
+    use_effect(move || {
+        println!("{}", count());
+    });
+}
+"#;
+
 struct SignalSubscriberDrop<T: 'static, S: Storage<SignalData<T>>> {
     signal: Signal<T, S>,
     #[cfg(debug_assertions)]
@@ -485,10 +549,36 @@ struct SignalSubscriberDrop<T: 'static, S: Storage<SignalData<T>>> {
 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
-        );
+        {
+            tracing::trace!(
+                "Write on signal at {} finished, updating subscribers",
+                self.origin
+            );
+
+            // Check if the write happened during a render. If it did, we should warn the user that this is generally a bad practice.
+            if dioxus_core::vdom_is_rendering() {
+                tracing::warn!(
+                    "Write on signal at {} happened while a component was running. Writing to signals during a render can cause infinite rerenders when you read the same signal in the component. Consider writing to the signal in an effect, future, or event handler if possible.",
+                    self.origin
+                );
+            }
+
+            // Check if the write happened during a scope that the signal is also subscribed to. If it did, this will probably cause an infinite loop.
+            if let Some(reactive_context) = ReactiveContext::current() {
+                if let Ok(inner) = self.signal.inner.try_read() {
+                    if let Ok(subscribers) = inner.subscribers.lock() {
+                        for subscriber in subscribers.iter() {
+                            if reactive_context == *subscriber {
+                                let origin = self.origin;
+                                tracing::warn!(
+                                    "Write on signal at {origin} finished in {reactive_context} which is also subscribed to the signal. This will likely cause an infinite loop. When the write finishes, {reactive_context} will rerun which may cause the write to be rerun again.\nHINT:\n{SIGNAL_READ_WRITE_SAME_SCOPE_HELP}",
+                                );
+                            }
+                        }
+                    }
+                }
+            }
+        }
         self.signal.update_subscribers();
     }
 }