Add a warning when a copy value is used in a higher scope than it was created in

packages/core/src/scope_context.rs

@@ -511,6 +511,18 @@ impl ScopeId {
             .flatten()
     }
 
+    /// Check if the current scope is a descendant of the given scope
+    pub fn is_descendant_of(self, other: ScopeId) -> bool {
+        let mut current = self;
+        while let Some(parent) = current.parent_scope() {
+            if parent == other {
+                return true;
+            }
+            current = parent;
+        }
+        false
+    }
+
     /// Mark the current scope as dirty, causing it to re-render
     pub fn needs_update(self) {
         Runtime::with_scope(self, |cx| cx.needs_update()).unwrap();

packages/generational-box/src/lib.rs

@@ -115,6 +115,18 @@ impl<T, S: Storage<T>> GenerationalBox<T, S> {
     {
         self.raw.take()
     }
+
+    /// Try to get the location the generational box was created at. In release mode this will always return None.
+    pub fn created_at(&self) -> Option<&'static std::panic::Location<'static>> {
+        #[cfg(debug_assertions)]
+        {
+            Some(self.raw.location.created_at)
+        }
+        #[cfg(not(debug_assertions))]
+        {
+            None
+        }
+    }
 }
 
 impl<T, S> Copy for GenerationalBox<T, S> {}

packages/signals/docs/hoist/error.rs

@@ -0,0 +1,31 @@
+#[component]
+fn Counters() -> Element {
+    let counts = use_signal(Vec::new);
+    let mut children = use_signal(|| 0);
+
+    rsx! {
+        button { onclick: move |_| children += 1, "Add child" }
+        button { onclick: move |_| children -= 1, "Remove child" }
+        // A signal from a child is read or written to in a parent scope
+        "{counts:?}"
+        for _ in 0..children() {
+            Counter {
+                counts
+            }
+        }
+    }
+}
+
+#[component]
+fn Counter(mut counts: Signal<Vec<Signal<i32>>>) -> Element {
+    let mut signal_owned_by_child = use_signal(|| 0);
+    // Moving the signal up to the parent may cause issues if you read the signal after the child scope is dropped
+    use_hook(|| counts.push(signal_owned_by_child));
+
+    rsx! {
+        button {
+            onclick: move |_| signal_owned_by_child += 1,
+            "{signal_owned_by_child}"
+        }
+    }
+}

packages/signals/docs/hoist/fixed_list.rs

@@ -0,0 +1,32 @@
+#[component]
+fn Counters() -> Element {
+    let mut counts = use_signal(Vec::new);
+
+    rsx! {
+        button { onclick: move |_| counts.write().push(0), "Add child" }
+        button {
+            onclick: move |_| {
+                counts.write().pop();
+            },
+            "Remove child"
+        }
+        "{counts:?}"
+        // Instead of passing up a signal, we can just write to the signal that lives in the parent
+        for index in 0..counts.len() {
+            Counter {
+                index,
+                counts
+            }
+        }
+    }
+}
+
+#[component]
+fn Counter(index: usize, mut counts: Signal<Vec<i32>>) -> Element {
+    rsx! {
+        button {
+            onclick: move |_| counts.write()[index] += 1,
+            "{counts.read()[index]}"
+        }
+    }
+}

packages/signals/src/copy_value.rs

@@ -1,3 +1,6 @@
+#![allow(clippy::unnecessary_operation)]
+#![allow(clippy::no_effect)]
+
 use generational_box::UnsyncStorage;
 use generational_box::{BorrowResult, GenerationalBoxId};
 use std::ops::Deref;
@@ -18,7 +21,7 @@ use crate::{default_impl, write_impls};
 /// It is internally backed by [`generational_box::GenerationalBox`].
 pub struct CopyValue<T: 'static, S: Storage<T> = UnsyncStorage> {
     pub(crate) value: GenerationalBox<T, S>,
-    origin_scope: ScopeId,
+    pub(crate) origin_scope: ScopeId,
 }
 
 #[cfg(feature = "serialize")]
@@ -65,12 +68,7 @@ impl<T: 'static, S: Storage<T>> CopyValue<T, S> {
     /// Once the component this value is created in is dropped, the value will be dropped.
     #[track_caller]
     pub fn new_maybe_sync(value: T) -> Self {
-        let owner = current_owner();
-
-        Self {
-            value: owner.insert(value),
-            origin_scope: current_scope_id().expect("in a virtual dom"),
-        }
+        Self::new_with_caller(value, std::panic::Location::caller())
     }
 
     pub(crate) fn new_with_caller(
@@ -88,10 +86,20 @@ impl<T: 'static, S: Storage<T>> CopyValue<T, S> {
     /// Create a new CopyValue. The value will be stored in the given scope. When the specified scope is dropped, the value will be dropped.
     #[track_caller]
     pub fn new_maybe_sync_in_scope(value: T, scope: ScopeId) -> Self {
+        Self::new_maybe_sync_in_scope_with_caller(value, scope, std::panic::Location::caller())
+    }
+
+    /// Create a new CopyValue with a custom caller. The value will be stored in the given scope. When the specified scope is dropped, the value will be dropped.
+    #[track_caller]
+    pub fn new_maybe_sync_in_scope_with_caller(
+        value: T,
+        scope: ScopeId,
+        caller: &'static std::panic::Location<'static>,
+    ) -> Self {
         let owner = scope.owner();
 
         Self {
-            value: owner.insert(value),
+            value: owner.insert_with_caller(value, caller),
             origin_scope: scope,
         }
     }
@@ -125,11 +133,13 @@ impl<T: 'static, S: Storage<T>> Readable for CopyValue<T, S> {
     fn try_read_unchecked(
         &self,
     ) -> Result<ReadableRef<'static, Self>, generational_box::BorrowError> {
+        crate::warnings::copy_value_hoisted(self, std::panic::Location::caller());
         self.value.try_read()
     }
 
     #[track_caller]
     fn try_peek_unchecked(&self) -> BorrowResult<ReadableRef<'static, Self>> {
+        crate::warnings::copy_value_hoisted(self, std::panic::Location::caller());
         self.value.try_read()
     }
 }
@@ -161,11 +171,13 @@ impl<T: 'static, S: Storage<T>> Writable for CopyValue<T, S> {
     fn try_write_unchecked(
         &self,
     ) -> Result<WritableRef<'static, Self>, generational_box::BorrowMutError> {
+        crate::warnings::copy_value_hoisted(self, std::panic::Location::caller());
         self.value.try_write()
     }
 
     #[track_caller]
     fn set(&mut self, value: T) {
+        crate::warnings::copy_value_hoisted(self, std::panic::Location::caller());
         self.value.set(value);
     }
 }

packages/signals/src/global/signal.rs

@@ -13,6 +13,7 @@ use crate::Signal;
 pub struct GlobalSignal<T> {
     initializer: fn() -> T,
     key: GlobalKey<'static>,
+    created_at: &'static std::panic::Location<'static>,
 }
 
 impl<T: 'static> GlobalSignal<T> {
@@ -23,6 +24,7 @@ impl<T: 'static> GlobalSignal<T> {
         GlobalSignal {
             initializer,
             key: GlobalKey::new(key),
+            created_at: key,
         }
     }
 
@@ -34,10 +36,12 @@ impl<T: 'static> GlobalSignal<T> {
     /// Create this global signal with a specific key.
     /// This is useful for ensuring that the signal is unique across the application and accessible from
     /// outside the application too.
+    #[track_caller]
     pub const fn with_key(initializer: fn() -> T, key: &'static str) -> GlobalSignal<T> {
         GlobalSignal {
             initializer,
             key: GlobalKey::new_from_str(key),
+            created_at: std::panic::Location::caller(),
         }
     }
 
@@ -56,7 +60,11 @@ impl<T: 'static> GlobalSignal<T> {
                 // Constructors are always run in the root scope
                 // The signal also exists in the root scope
                 let value = ScopeId::ROOT.in_runtime(self.initializer);
-                let signal = Signal::new_in_scope(value, ScopeId::ROOT);
+                let signal = Signal::new_maybe_sync_in_scope_with_caller(
+                    value,
+                    ScopeId::ROOT,
+                    self.created_at,
+                );
 
                 let entry = context.signal.borrow_mut().insert(key, Box::new(signal));
                 debug_assert!(entry.is_none(), "Global signal already exists");

packages/signals/src/lib.rs

@@ -39,3 +39,5 @@ pub use write::*;
 
 mod props;
 pub use props::*;
+
+pub mod warnings;

packages/signals/src/signal.rs

@@ -183,13 +183,24 @@ impl<T: 'static, S: Storage<SignalData<T>>> Signal<T, S> {
     #[track_caller]
     #[tracing::instrument(skip(value))]
     pub fn new_maybe_sync_in_scope(value: T, owner: ScopeId) -> Self {
+        Self::new_maybe_sync_in_scope_with_caller(value, owner, std::panic::Location::caller())
+    }
+
+    /// Create a new signal with a custom owner scope and a custom caller. The signal will be dropped when the owner scope is dropped instead of the current scope.
+    #[tracing::instrument(skip(value))]
+    pub fn new_maybe_sync_in_scope_with_caller(
+        value: T,
+        owner: ScopeId,
+        caller: &'static std::panic::Location<'static>,
+    ) -> Self {
         Self {
-            inner: CopyValue::<SignalData<T>, S>::new_maybe_sync_in_scope(
+            inner: CopyValue::<SignalData<T>, S>::new_maybe_sync_in_scope_with_caller(
                 SignalData {
                     subscribers: Default::default(),
                     value,
                 },
                 owner,
+                caller,
             ),
         }
     }
@@ -534,115 +545,12 @@ 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)]
     origin: &'static std::panic::Location<'static>,
 }
 
-pub mod warnings {
-    //! Warnings that can be triggered by suspicious usage of signals
-
-    use super::*;
-    use ::warnings::warning;
-
-    /// Check if the write happened during a render. If it did, warn the user that this is generally a bad practice.
-    #[warning]
-    pub fn signal_write_in_component_body(origin: &'static std::panic::Location<'static>) {
-        // 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.",
-            origin
-        );
-        }
-    }
-
-    /// Check if the write happened during a scope that the signal is also subscribed to. If it did, trigger a warning because it will likely cause an infinite loop.
-    #[warning]
-    pub fn signal_read_and_write_in_reactive_scope<T: 'static, S: Storage<SignalData<T>>>(
-        origin: &'static std::panic::Location<'static>,
-        signal: Signal<T, S>,
-    ) {
-        // 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) = signal.inner.try_read() {
-                if let Ok(subscribers) = inner.subscribers.lock() {
-                    for subscriber in subscribers.iter() {
-                        if reactive_context == *subscriber {
-                            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}",
-                        );
-                        }
-                    }
-                }
-            }
-        }
-    }
-}
-
 #[allow(clippy::no_effect)]
 impl<T: 'static, S: Storage<SignalData<T>>> Drop for SignalSubscriberDrop<T, S> {
     fn drop(&mut self) {
@@ -652,8 +560,11 @@ impl<T: 'static, S: Storage<SignalData<T>>> Drop for SignalSubscriberDrop<T, S>
                 "Write on signal at {} finished, updating subscribers",
                 self.origin
             );
-            warnings::signal_write_in_component_body(self.origin);
-            warnings::signal_read_and_write_in_reactive_scope::<T, S>(self.origin, self.signal);
+            crate::warnings::signal_write_in_component_body(self.origin);
+            crate::warnings::signal_read_and_write_in_reactive_scope::<T, S>(
+                self.origin,
+                self.signal,
+            );
         }
         self.signal.update_subscribers();
     }