Merge pull request #1933 from ealmloff/restore-rendering-check

Only subscribe scopes to signals when rendering

Cargo.lock

@@ -2624,6 +2624,7 @@ dependencies = [
  "futures-util",
  "slab",
  "thiserror",
+ "tokio",
  "tracing",
  "web-sys",
 ]

packages/core/src/scope_context.rs

@@ -315,12 +315,6 @@ impl ScopeId {
         Runtime::with(|rt| rt.current_scope_id()).flatten()
     }
 
-    #[doc(hidden)]
-    /// Check if the virtual dom is currently inside of the body of a component
-    pub fn vdom_is_rendering(self) -> bool {
-        Runtime::with(|rt| rt.rendering.get()).unwrap_or_default()
-    }
-
     /// Consume context from the current scope
     pub fn consume_context<T: 'static + Clone>(self) -> Option<T> {
         Runtime::with_scope(self, |cx| cx.consume_context::<T>()).flatten()

packages/hooks/Cargo.toml

@@ -28,3 +28,4 @@ futures-util = { workspace = true, default-features = false }
 dioxus-core = { workspace = true }
 dioxus = { workspace = true }
 web-sys = { version = "0.3.64", features = ["Document", "Window", "Element"] }
+tokio = { version = "1.0", features = ["full"] }

packages/hooks/src/lib.rs

@@ -11,17 +11,17 @@
 /// ```
 /// # use dioxus::prelude::*;
 /// #
-/// # #[derive(Props, PartialEq)]
+/// # #[derive(Props, PartialEq, Clone)]
 /// # struct Props {
 /// #    prop: String,
 /// # }
-/// # fn Component(cx: Scope<Props>) -> Element {
+/// # fn Component(props: Props) -> Element {
 ///
 /// let (data) = use_signal(|| {});
 ///
 /// let handle_thing = move |_| {
-///     to_owned![data, cx.props.prop];
-///     cx.spawn(async move {
+///     to_owned![data, props.prop];
+///     spawn(async move {
 ///         // do stuff
 ///     });
 /// };

packages/hooks/src/use_memo.rs

@@ -16,7 +16,7 @@ use dioxus_signals::{Storage, Writable};
 ///     let mut count = use_signal(|| 0);
 ///     let double = use_memo(move || count * 2);
 ///     count += 1;
-///     assert_eq!(double.value(), count * 2);
+///     assert_eq!(double(), count * 2);
 ///
 ///     rsx! { "{double}" }
 /// }
@@ -35,12 +35,12 @@ pub fn use_memo<R: PartialEq>(f: impl FnMut() -> R + 'static) -> ReadOnlySignal<
 /// use dioxus_signals::*;
 ///
 /// fn App() -> Element {
-///     let mut count = use_signal(cx, || 0);
-///     let double = use_memo(cx, move || count * 2);
+///     let mut count = use_signal(|| 0);
+///     let double = use_memo(move || count * 2);
 ///     count += 1;
-///     assert_eq!(double.value(), count * 2);
+///     assert_eq!(double(), count * 2);
 ///
-///     render! { "{double}" }
+///     rsx! { "{double}" }
 /// }
 /// ```
 #[track_caller]
@@ -79,11 +79,11 @@ pub fn use_maybe_sync_memo<R: PartialEq, S: Storage<SignalData<R>>>(
 /// use dioxus::prelude::*;
 ///
 /// fn App() -> Element {
-///     let mut local_state = use_state(|| 0);
-///     let double = use_memo_with_dependencies(cx, (local_state.get(),), move |(local_state,)| local_state * 2);
+///     let mut local_state = use_signal(|| 0);
+///     let double = use_memo_with_dependencies((&local_state(),), move |(local_state,)| local_state * 2);
 ///     local_state.set(1);
 ///
-///     render! { "{double}" }
+///     rsx! { "{double}" }
 /// }
 /// ```
 #[track_caller]
@@ -94,7 +94,7 @@ pub fn use_memo_with_dependencies<R: PartialEq, D: Dependency>(
 where
     D::Out: 'static,
 {
-    use_maybe_sync_selector_with_dependencies(dependencies, f)
+    use_maybe_sync_memo_with_dependencies(dependencies, f)
 }
 
 /// Creates a new Selector that may be sync with some local dependencies. The selector will be run immediately and whenever any signal it reads or any dependencies it tracks changes
@@ -106,15 +106,15 @@ where
 /// use dioxus_signals::*;
 ///
 /// fn App() -> Element {
-///     let mut local_state = use_state(|| 0);
-///     let double = use_memo_with_dependencies(cx, (local_state.get(),), move |(local_state,)| local_state * 2);
+///     let mut local_state = use_signal(|| 0i32);
+///     let double: ReadOnlySignal<i32, SyncStorage> = use_maybe_sync_memo_with_dependencies((&local_state(),), move |(local_state,)| local_state * 2);
 ///     local_state.set(1);
 ///
-///     render! { "{double}" }
+///     rsx! { "{double}" }
 /// }
 /// ```
 #[track_caller]
-pub fn use_maybe_sync_selector_with_dependencies<
+pub fn use_maybe_sync_memo_with_dependencies<
     R: PartialEq,
     D: Dependency,
     S: Storage<SignalData<R>>,

packages/hooks/src/use_signal.rs

@@ -17,9 +17,7 @@ use dioxus_signals::{Signal, SignalData, Storage, SyncStorage, UnsyncStorage};
 ///
 /// #[component]
 /// fn Child(state: Signal<u32>) -> Element {
-///     let state = *state;
-///
-///     use_future( |()| async move {
+///     use_future(move || async move {
 ///         // Because the signal is a Copy type, we can use it in an async block without cloning it.
 ///         *state.write() += 1;
 ///     });
@@ -44,19 +42,17 @@ pub fn use_signal<T: 'static>(f: impl FnOnce() -> T) -> Signal<T, UnsyncStorage>
 /// use dioxus::prelude::*;
 /// use dioxus_signals::*;
 ///
-/// fn App(cx: Scope) -> Element {
-///     let mut count = use_signal_sync(cx, || 0);
+/// fn App() -> Element {
+///     let mut count = use_signal_sync(|| 0);
 ///
 ///     // Because signals have automatic dependency tracking, if you never read them in a component, that component will not be re-rended when the signal is updated.
 ///     // The app component will never be rerendered in this example.
-///     render! { Child { state: count } }
+///     rsx! { Child { state: count } }
 /// }
 ///
 /// #[component]
-/// fn Child(cx: Scope, state: Signal<u32, SyncStorage>) -> Element {
-///     let state = *state;
-///
-///     use_future!(cx,  |()| async move {
+/// fn Child(state: Signal<u32, SyncStorage>) -> Element {
+///     use_future(move || async move {
 ///         // This signal is Send + Sync, so we can use it in an another thread
 ///         tokio::spawn(async move {
 ///             // Because the signal is a Copy type, we can use it in an async block without cloning it.
@@ -64,7 +60,7 @@ pub fn use_signal<T: 'static>(f: impl FnOnce() -> T) -> Signal<T, UnsyncStorage>
 ///         }).await;
 ///     });
 ///
-///     render! {
+///     rsx! {
 ///         button {
 ///             onclick: move |_| *state.write() += 1,
 ///             "{state}"

packages/hooks/tests/effect.rs

@@ -0,0 +1,50 @@
+#![allow(unused, non_upper_case_globals, non_snake_case)]
+use std::cell::RefCell;
+use std::collections::HashMap;
+use std::rc::Rc;
+
+use dioxus::prelude::*;
+use dioxus_core::ElementId;
+use dioxus_signals::*;
+
+#[tokio::test]
+async fn effects_rerun() {
+    #[derive(Default)]
+    struct RunCounter {
+        component: usize,
+        effect: usize,
+    }
+
+    let counter = Rc::new(RefCell::new(RunCounter::default()));
+    let mut dom = VirtualDom::new_with_props(
+        |counter: Rc<RefCell<RunCounter>>| {
+            counter.borrow_mut().component += 1;
+
+            let mut signal = use_signal(|| 0);
+            use_effect({
+                to_owned![counter];
+                move || {
+                    counter.borrow_mut().effect += 1;
+                    // This will subscribe the effect to the signal
+                    println!("Signal: {:?}", signal);
+
+                    // Stop the wait for work manually
+                    needs_update();
+                }
+            });
+            signal += 1;
+
+            rsx! {
+                div {}
+            }
+        },
+        counter.clone(),
+    );
+
+    dom.rebuild_in_place();
+    dom.wait_for_work().await;
+
+    let current_counter = counter.borrow();
+    assert_eq!(current_counter.component, 1);
+    assert_eq!(current_counter.effect, 1);
+}

packages/signals/src/copy_value.rs

@@ -86,8 +86,10 @@ fn current_owner<S: Storage<T>, T>() -> Owner<S> {
     }
 
     // Otherwise get the owner from the current reactive context.
-    let current_reactive_context = ReactiveContext::current();
-    owner_in_scope(current_reactive_context.origin_scope())
+    match ReactiveContext::current() {
+        Some(current_reactive_context) => owner_in_scope(current_reactive_context.origin_scope()),
+        None => owner_in_scope(current_scope_id().expect("in a virtual dom")),
+    }
 }
 
 fn owner_in_scope<S: Storage<T>, T>(scope: ScopeId) -> Owner<S> {

packages/signals/src/global/memo.rs

@@ -33,7 +33,7 @@ impl<T: PartialEq + 'static> GlobalMemo<T> {
             None => {
                 drop(read);
                 // Constructors are always run in the root scope
-                let signal = ScopeId::ROOT.in_runtime(|| Signal::selector(self.selector));
+                let signal = ScopeId::ROOT.in_runtime(|| Signal::memo(self.selector));
                 context.signal.borrow_mut().insert(key, Box::new(signal));
                 signal
             }

packages/signals/src/reactive_context.rs

@@ -68,21 +68,26 @@ impl ReactiveContext {
     /// If this was set manually, then that value will be returned.
     ///
     /// If there's no current reactive context, then a new one will be created for the current scope and returned.
-    pub fn current() -> Self {
+    pub fn current() -> Option<Self> {
         let cur = CURRENT.with(|current| current.borrow().last().cloned());
 
         // If we're already inside a reactive context, then return that
         if let Some(cur) = cur {
-            return cur;
+            return Some(cur);
         }
 
         // If we're rendering, then try and use the reactive context attached to this component
+        if !dioxus_core::vdom_is_rendering() {
+            return None;
+        }
         if let Some(cx) = has_context() {
-            return cx;
+            return Some(cx);
         }
 
         // Otherwise, create a new context at the current scope
-        provide_context(ReactiveContext::new_for_scope(current_scope_id()))
+        Some(provide_context(ReactiveContext::new_for_scope(
+            current_scope_id(),
+        )))
     }
 
     /// Run this function in the context of this reactive context

packages/signals/src/signal.rs

@@ -90,19 +90,19 @@ impl<T: PartialEq + 'static> Signal<T> {
     ///
     /// Selectors can be used to efficiently compute derived data from signals.
     #[track_caller]
-    pub fn selector(f: impl FnMut() -> T + 'static) -> ReadOnlySignal<T> {
-        Self::maybe_sync_memo(f)
+    pub fn memo(f: impl FnMut() -> T + 'static) -> ReadOnlySignal<T> {
+        Self::use_maybe_sync_memo(f)
     }
 
     /// Creates a new Selector that may be Sync + Send. The selector will be run immediately and whenever any signal it reads changes.
     ///
     /// Selectors can be used to efficiently compute derived data from signals.
     #[track_caller]
-    pub fn maybe_sync_memo<S: Storage<SignalData<T>>>(
+    pub fn use_maybe_sync_memo<S: Storage<SignalData<T>>>(
         mut f: impl FnMut() -> T + 'static,
     ) -> ReadOnlySignal<T, S> {
         // Get the current reactive context
-        let rc = ReactiveContext::current();
+        let rc = ReactiveContext::new();
 
         // Create a new signal in that context, wiring up its dependencies and subscribers
         let mut state: Signal<T, S> = rc.run_in(|| Signal::new_maybe_sync(f()));
@@ -201,8 +201,9 @@ impl<T, S: Storage<SignalData<T>>> Readable for Signal<T, S> {
     fn try_read(&self) -> Result<ReadableRef<Self>, generational_box::BorrowError> {
         let inner = self.inner.try_read()?;
 
-        let reactive_context = ReactiveContext::current();
-        inner.subscribers.lock().unwrap().insert(reactive_context);
+        if let Some(reactive_context) = ReactiveContext::current() {
+            inner.subscribers.lock().unwrap().insert(reactive_context);
+        }
 
         Ok(S::map(inner, |v| &v.value))
     }

packages/signals/tests/create.rs

@@ -0,0 +1,85 @@
+#![allow(unused, non_upper_case_globals, non_snake_case)]
+
+use dioxus::prelude::*;
+use dioxus_core::ElementId;
+use dioxus_core::NoOpMutations;
+use dioxus_signals::*;
+
+#[test]
+fn create_signals_global() {
+    let mut dom = VirtualDom::new(|| {
+        rsx! {
+            for _ in 0..10 {
+                Child {}
+            }
+        }
+    });
+
+    fn Child() -> Element {
+        let signal = create_without_cx();
+
+        rsx! {
+            "{signal}"
+        }
+    }
+
+    dom.rebuild_in_place();
+
+    fn create_without_cx() -> Signal<String> {
+        Signal::new("hello world".to_string())
+    }
+}
+
+#[test]
+fn deref_signal() {
+    let mut dom = VirtualDom::new(|| {
+        rsx! {
+            for _ in 0..10 {
+                Child {}
+            }
+        }
+    });
+
+    fn Child() -> Element {
+        let signal = Signal::new("hello world".to_string());
+
+        // You can call signals like functions to get a Ref of their value.
+        assert_eq!(&*signal(), "hello world");
+
+        rsx! {
+            "hello world"
+        }
+    }
+
+    dom.rebuild_in_place();
+}
+
+#[test]
+fn drop_signals() {
+    let mut dom = VirtualDom::new(|| {
+        let generation = generation();
+
+        let count = if generation % 2 == 0 { 10 } else { 0 };
+        rsx! {
+            for _ in 0..count {
+                Child {}
+            }
+        }
+    });
+
+    fn Child() -> Element {
+        let signal = create_without_cx();
+
+        rsx! {
+            "{signal}"
+        }
+    }
+
+    dom.rebuild_in_place();
+    dom.mark_dirty(ScopeId::ROOT);
+    dom.render_immediate(&mut NoOpMutations);
+
+    fn create_without_cx() -> Signal<String> {
+        Signal::new("hello world".to_string())
+    }
+}

packages/signals/tests/memo.rs

@@ -0,0 +1,148 @@
+// TODO: fix #1935
+
+// #![allow(unused, non_upper_case_globals, non_snake_case)]
+// use dioxus_core::NoOpMutations;
+// use std::collections::HashMap;
+// use std::rc::Rc;
+
+// use dioxus::html::p;
+// use dioxus::prelude::*;
+// use dioxus_core::ElementId;
+// use dioxus_signals::*;
+// use std::cell::RefCell;
+
+// #[test]
+// fn memos_rerun() {
+//     let _ = simple_logger::SimpleLogger::new().init();
+
+//     #[derive(Default)]
+//     struct RunCounter {
+//         component: usize,
+//         effect: usize,
+//     }
+
+//     let counter = Rc::new(RefCell::new(RunCounter::default()));
+//     let mut dom = VirtualDom::new_with_props(
+//         |counter: Rc<RefCell<RunCounter>>| {
+//             counter.borrow_mut().component += 1;
+
+//             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);
+
+//             rsx! {
+//                 div {}
+//             }
+//         },
+//         counter.clone(),
+//     );
+
+//     dom.rebuild_in_place();
+
+//     let current_counter = counter.borrow();
+//     assert_eq!(current_counter.component, 1);
+//     assert_eq!(current_counter.effect, 2);
+// }
+
+// #[test]
+// fn memos_prevents_component_rerun() {
+//     let _ = simple_logger::SimpleLogger::new().init();
+
+//     #[derive(Default)]
+//     struct RunCounter {
+//         component: usize,
+//         memo: usize,
+//     }
+
+//     let counter = Rc::new(RefCell::new(RunCounter::default()));
+//     let mut dom = VirtualDom::new_with_props(
+//         |props: Rc<RefCell<RunCounter>>| {
+//             let mut signal = use_signal(|| 0);
+
+//             if generation() == 1 {
+//                 *signal.write() = 0;
+//             }
+//             if generation() == 2 {
+//                 println!("Writing to signal");
+//                 *signal.write() = 1;
+//             }
+
+//             rsx! {
+//                 Child {
+//                     signal: signal,
+//                     counter: props.clone(),
+//                 }
+//             }
+//         },
+//         counter.clone(),
+//     );
+
+//     #[derive(Default, Props, Clone)]
+//     struct ChildProps {
+//         signal: Signal<usize>,
+//         counter: Rc<RefCell<RunCounter>>,
+//     }
+
+//     impl PartialEq for ChildProps {
+//         fn eq(&self, other: &Self) -> bool {
+//             self.signal == other.signal
+//         }
+//     }
+
+//     fn Child(props: ChildProps) -> Element {
+//         let counter = &props.counter;
+//         let signal = props.signal;
+//         counter.borrow_mut().component += 1;
+
+//         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"),
+//         }
+
+//         rsx! {
+//             div {}
+//         }
+//     }
+
+//     dom.rebuild_in_place();
+//     dom.mark_dirty(ScopeId::ROOT);
+//     dom.render_immediate(&mut NoOpMutations);
+
+//     {
+//         let current_counter = counter.borrow();
+//         assert_eq!(current_counter.component, 1);
+//         assert_eq!(current_counter.memo, 2);
+//     }
+
+//     dom.mark_dirty(ScopeId::ROOT);
+//     dom.render_immediate(&mut NoOpMutations);
+//     dom.render_immediate(&mut NoOpMutations);
+
+//     {
+//         let current_counter = counter.borrow();
+//         assert_eq!(current_counter.component, 2);
+//         assert_eq!(current_counter.memo, 3);
+//     }
+// }

packages/signals/tests/subscribe.rs

@@ -0,0 +1,95 @@
+#![allow(unused, non_upper_case_globals, non_snake_case)]
+
+use dioxus_core::NoOpMutations;
+use std::collections::HashMap;
+use std::rc::Rc;
+
+use dioxus::prelude::*;
+use dioxus_core::ElementId;
+use dioxus_signals::*;
+use std::cell::RefCell;
+
+#[test]
+fn reading_subscribes() {
+    simple_logger::SimpleLogger::new().init().unwrap();
+
+    #[derive(Default)]
+    struct RunCounter {
+        parent: usize,
+        children: HashMap<ScopeId, usize>,
+    }
+
+    let counter = Rc::new(RefCell::new(RunCounter::default()));
+    let mut dom = VirtualDom::new_with_props(
+        |props: Rc<RefCell<RunCounter>>| {
+            let mut signal = use_signal(|| 0);
+
+            println!("Parent: {:?}", current_scope_id());
+            if generation() == 1 {
+                signal += 1;
+            }
+
+            props.borrow_mut().parent += 1;
+
+            rsx! {
+                for id in 0..10 {
+                    Child {
+                        signal: signal,
+                        counter: props.clone()
+                    }
+                }
+            }
+        },
+        counter.clone(),
+    );
+
+    #[derive(Props, Clone)]
+    struct ChildProps {
+        signal: Signal<usize>,
+        counter: Rc<RefCell<RunCounter>>,
+    }
+
+    impl PartialEq for ChildProps {
+        fn eq(&self, other: &Self) -> bool {
+            self.signal == other.signal
+        }
+    }
+
+    fn Child(props: ChildProps) -> Element {
+        println!("Child: {:?}", current_scope_id());
+        *props
+            .counter
+            .borrow_mut()
+            .children
+            .entry(current_scope_id().unwrap())
+            .or_default() += 1;
+
+        rsx! {
+            "{props.signal}"
+        }
+    }
+
+    dom.rebuild_in_place();
+
+    {
+        let current_counter = counter.borrow();
+        assert_eq!(current_counter.parent, 1);
+
+        for (scope_id, rerun_count) in current_counter.children.iter() {
+            assert_eq!(rerun_count, &1);
+        }
+    }
+
+    dom.mark_dirty(ScopeId::ROOT);
+    dom.render_immediate(&mut NoOpMutations);
+    dom.render_immediate(&mut NoOpMutations);
+
+    {
+        let current_counter = counter.borrow();
+        assert_eq!(current_counter.parent, 2);
+
+        for (scope_id, rerun_count) in current_counter.children.iter() {
+            assert_eq!(rerun_count, &2);
+        }
+    }
+}