From 48751d2f98a609f75ec56e7d49e933abc7a93907 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Wed, 14 Feb 2024 09:33:22 -0600 Subject: [PATCH] only subscribe scopes to signals when rendering --- Cargo.lock | 1 + packages/core/src/scope_context.rs | 6 - packages/hooks/Cargo.toml | 1 + packages/hooks/src/lib.rs | 8 +- packages/hooks/src/use_memo.rs | 26 ++-- packages/hooks/src/use_signal.rs | 18 ++- packages/hooks/tests/effect.rs | 50 ++++++++ packages/signals/src/copy_value.rs | 8 +- packages/signals/src/global/memo.rs | 2 +- packages/signals/src/reactive_context.rs | 13 +- packages/signals/src/signal.rs | 13 +- packages/signals/tests/create.rs | 85 +++++++++++++ packages/signals/tests/memo.rs | 146 +++++++++++++++++++++++ packages/signals/tests/subscribe.rs | 95 +++++++++++++++ 14 files changed, 424 insertions(+), 48 deletions(-) create mode 100644 packages/hooks/tests/effect.rs create mode 100644 packages/signals/tests/create.rs create mode 100644 packages/signals/tests/memo.rs create mode 100644 packages/signals/tests/subscribe.rs diff --git a/Cargo.lock b/Cargo.lock index 427495b02..20f3620fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2623,6 +2623,7 @@ dependencies = [ "futures-util", "slab", "thiserror", + "tokio", "tracing", "web-sys", ] diff --git a/packages/core/src/scope_context.rs b/packages/core/src/scope_context.rs index 4514dcc3c..9fa54e71c 100644 --- a/packages/core/src/scope_context.rs +++ b/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(self) -> Option { Runtime::with_scope(self, |cx| cx.consume_context::()).flatten() diff --git a/packages/hooks/Cargo.toml b/packages/hooks/Cargo.toml index f826a5ba3..734b9b76e 100644 --- a/packages/hooks/Cargo.toml +++ b/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"] } diff --git a/packages/hooks/src/lib.rs b/packages/hooks/src/lib.rs index 5d842b5ff..a179e22ee 100644 --- a/packages/hooks/src/lib.rs +++ b/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) -> 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 /// }); /// }; diff --git a/packages/hooks/src/use_memo.rs b/packages/hooks/src/use_memo.rs index 7912259a0..b8eb06452 100644 --- a/packages/hooks/src/use_memo.rs +++ b/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(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>>( /// 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( 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 = 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>, diff --git a/packages/hooks/src/use_signal.rs b/packages/hooks/src/use_signal.rs index 5ae93897c..ac0f83d98 100644 --- a/packages/hooks/src/use_signal.rs +++ b/packages/hooks/src/use_signal.rs @@ -17,9 +17,7 @@ use dioxus_signals::{Signal, SignalData, Storage, SyncStorage, UnsyncStorage}; /// /// #[component] /// fn Child(state: Signal) -> 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(f: impl FnOnce() -> T) -> Signal /// 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) -> Element { -/// let state = *state; -/// -/// use_future!(cx, |()| async move { +/// fn Child(state: Signal) -> 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(f: impl FnOnce() -> T) -> Signal /// }).await; /// }); /// -/// render! { +/// rsx! { /// button { /// onclick: move |_| *state.write() += 1, /// "{state}" diff --git a/packages/hooks/tests/effect.rs b/packages/hooks/tests/effect.rs new file mode 100644 index 000000000..e43dc4bf0 --- /dev/null +++ b/packages/hooks/tests/effect.rs @@ -0,0 +1,50 @@ +#![allow(unused, non_upper_case_globals, non_snake_case)] +use std::collections::HashMap; +use std::rc::Rc; +use std::cell::RefCell; + +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>| { + 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); +} \ No newline at end of file diff --git a/packages/signals/src/copy_value.rs b/packages/signals/src/copy_value.rs index 4961fb2b8..697532b31 100644 --- a/packages/signals/src/copy_value.rs +++ b/packages/signals/src/copy_value.rs @@ -86,8 +86,12 @@ fn current_owner, T>() -> Owner { } // 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, T>(scope: ScopeId) -> Owner { diff --git a/packages/signals/src/global/memo.rs b/packages/signals/src/global/memo.rs index 9bea70da1..4a94d29f6 100644 --- a/packages/signals/src/global/memo.rs +++ b/packages/signals/src/global/memo.rs @@ -33,7 +33,7 @@ impl GlobalMemo { 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 } diff --git a/packages/signals/src/reactive_context.rs b/packages/signals/src/reactive_context.rs index 0e62f0dd9..b581a3425 100644 --- a/packages/signals/src/reactive_context.rs +++ b/packages/signals/src/reactive_context.rs @@ -1,5 +1,5 @@ use dioxus_core::prelude::{ - current_scope_id, has_context, provide_context, schedule_update_any, ScopeId, + current_scope_id, has_context, provide_context, schedule_update_any, ScopeId }; use generational_box::SyncStorage; use rustc_hash::FxHashSet; @@ -68,21 +68,24 @@ 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 { 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 diff --git a/packages/signals/src/signal.rs b/packages/signals/src/signal.rs index 0b11051b1..03b18d4f4 100644 --- a/packages/signals/src/signal.rs +++ b/packages/signals/src/signal.rs @@ -90,19 +90,19 @@ impl Signal { /// /// Selectors can be used to efficiently compute derived data from signals. #[track_caller] - pub fn selector(f: impl FnMut() -> T + 'static) -> ReadOnlySignal { - Self::maybe_sync_memo(f) + pub fn memo(f: impl FnMut() -> T + 'static) -> ReadOnlySignal { + 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>>( + pub fn use_maybe_sync_memo>>( mut f: impl FnMut() -> T + 'static, ) -> ReadOnlySignal { // 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 = rc.run_in(|| Signal::new_maybe_sync(f())); @@ -201,8 +201,9 @@ impl>> Readable for Signal { fn try_read(&self) -> Result, 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)) } diff --git a/packages/signals/tests/create.rs b/packages/signals/tests/create.rs new file mode 100644 index 000000000..5c4553080 --- /dev/null +++ b/packages/signals/tests/create.rs @@ -0,0 +1,85 @@ +#![allow(unused, non_upper_case_globals, non_snake_case)] + +use dioxus_core::NoOpMutations; +use dioxus::prelude::*; +use dioxus_core::ElementId; +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 { + 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 { + Signal::new("hello world".to_string()) + } +} \ No newline at end of file diff --git a/packages/signals/tests/memo.rs b/packages/signals/tests/memo.rs new file mode 100644 index 000000000..17f6ce433 --- /dev/null +++ b/packages/signals/tests/memo.rs @@ -0,0 +1,146 @@ +#![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>| { + counter.borrow_mut().component += 1; + + let mut signal = use_signal(|| 0); + let memo = use_hook(move || { + to_owned![counter]; + Signal::memo(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>| { + 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, + counter: Rc>, + } + + 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_hook(move || { + to_owned![counter]; + Signal::memo(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); + } +} \ No newline at end of file diff --git a/packages/signals/tests/subscribe.rs b/packages/signals/tests/subscribe.rs new file mode 100644 index 000000000..b4e6a94e7 --- /dev/null +++ b/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, + } + + let counter = Rc::new(RefCell::new(RunCounter::default())); + let mut dom = VirtualDom::new_with_props( + |props: Rc>| { + 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, + counter: Rc>, + } + + 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); + } + } +} \ No newline at end of file