From 38274da3ee10ea04948440517b6eef92f90e4b71 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 21 Aug 2023 14:42:56 -0500 Subject: [PATCH 1/3] Fix creating signals in effects --- packages/signals/src/effect.rs | 2 ++ packages/signals/src/rt.rs | 25 ++++++++++++++++--------- packages/signals/src/selector.rs | 1 + 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/signals/src/effect.rs b/packages/signals/src/effect.rs index 0685938a0a..1a42c227f9 100644 --- a/packages/signals/src/effect.rs +++ b/packages/signals/src/effect.rs @@ -54,6 +54,7 @@ pub fn use_effect_with_dependencies( /// Effects allow you to run code when a signal changes. Effects are run immediately and whenever any signal it reads changes. #[derive(Copy, Clone, PartialEq)] pub struct Effect { + pub(crate) source: ScopeId, pub(crate) callback: CopyValue>, } @@ -73,6 +74,7 @@ impl Effect { /// The signal will be owned by the current component and will be dropped when the component is dropped. pub fn new(callback: impl FnMut() + 'static) -> Self { let myself = Self { + source: current_scope_id().expect("in a virtual dom"), callback: CopyValue::new(Box::new(callback)), }; diff --git a/packages/signals/src/rt.rs b/packages/signals/src/rt.rs index 7322d43b4c..113fe9b305 100644 --- a/packages/signals/src/rt.rs +++ b/packages/signals/src/rt.rs @@ -2,14 +2,13 @@ use std::cell::{Ref, RefMut}; use std::rc::Rc; -use dioxus_core::prelude::{ - consume_context, consume_context_from_scope, current_scope_id, provide_context, - provide_context_to_scope, provide_root_context, -}; +use dioxus_core::prelude::*; use dioxus_core::ScopeId; use generational_box::{GenerationalBox, Owner, Store}; +use crate::Effect; + fn current_store() -> Store { match consume_context() { Some(rt) => rt, @@ -21,12 +20,20 @@ fn current_store() -> Store { } fn current_owner() -> Rc { - match consume_context() { - Some(rt) => rt, - None => { - let owner = Rc::new(current_store().owner()); - provide_context(owner).expect("in a virtual dom") + match Effect::current() { + // If we are inside of an effect, we should use the owner of the effect as the owner of the value. + Some(effect) => { + let scope_id = effect.source; + owner_in_scope(scope_id) } + // Otherwise either get an owner from the current scope or create a new one. + None => match has_context() { + Some(rt) => rt, + None => { + let owner = Rc::new(current_store().owner()); + provide_context(owner).expect("in a virtual dom") + } + }, } } diff --git a/packages/signals/src/selector.rs b/packages/signals/src/selector.rs index 5eb3fa3c91..346506ff7a 100644 --- a/packages/signals/src/selector.rs +++ b/packages/signals/src/selector.rs @@ -74,6 +74,7 @@ pub fn selector(mut f: impl FnMut() -> R + 'static) -> ReadOnlySig inner: CopyValue::invalid(), }; let effect = Effect { + source: current_scope_id().expect("in a virtual dom"), callback: CopyValue::invalid(), }; From 3ac2346e46afa5fa790c50c2415c3f85e7fa6229 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Fri, 25 Aug 2023 07:54:04 -0500 Subject: [PATCH 2/3] fix a few new clippy lints --- packages/router/src/contexts/router.rs | 34 ++++++++++++-------------- packages/web/src/dom.rs | 1 + 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/router/src/contexts/router.rs b/packages/router/src/contexts/router.rs index 5a9a3f0c41..b159569918 100644 --- a/packages/router/src/contexts/router.rs +++ b/packages/router/src/contexts/router.rs @@ -2,7 +2,7 @@ use std::{ any::Any, collections::HashSet, rc::Rc, - sync::{Arc, RwLock, RwLockWriteGuard}, + sync::{Arc, RwLock}, }; use dioxus::prelude::*; @@ -36,7 +36,7 @@ struct MutableRouterState { /// A collection of router data that manages all routing functionality. #[derive(Clone)] pub struct RouterContext { - state: Arc>, + state: Rc>, subscribers: Arc>>, subscriber_update: Arc, @@ -56,7 +56,7 @@ impl RouterContext { R: Clone, ::Err: std::fmt::Display, { - let state = Arc::new(RwLock::new(MutableRouterState { + let state = Rc::new(RefCell::new(MutableRouterState { prefix: Default::default(), history: cfg.take_history(), unresolved_error: None, @@ -105,7 +105,7 @@ impl RouterContext { // set the updater { - let mut state = myself.state.write().unwrap(); + let mut state = myself.state.borrow_mut(); state.history.updater(Arc::new(move || { for &id in subscribers.read().unwrap().iter() { (mark_dirty)(id); @@ -117,20 +117,20 @@ impl RouterContext { } pub(crate) fn route_from_str(&self, route: &str) -> Result, String> { - let state = self.state.read().unwrap(); + let state = self.state.borrow(); state.history.parse_route(route) } /// Check whether there is a previous page to navigate back to. #[must_use] pub fn can_go_back(&self) -> bool { - self.state.read().unwrap().history.can_go_back() + self.state.borrow().history.can_go_back() } /// Check whether there is a future page to navigate forward to. #[must_use] pub fn can_go_forward(&self) -> bool { - self.state.read().unwrap().history.can_go_forward() + self.state.borrow().history.can_go_forward() } /// Go back to the previous location. @@ -138,7 +138,7 @@ impl RouterContext { /// Will fail silently if there is no previous location to go to. pub fn go_back(&self) { { - self.state.write().unwrap().history.go_back(); + self.state.borrow_mut().history.go_back(); } self.change_route(); @@ -149,7 +149,7 @@ impl RouterContext { /// Will fail silently if there is no next location to go to. pub fn go_forward(&self) { { - self.state.write().unwrap().history.go_forward(); + self.state.borrow_mut().history.go_forward(); } self.change_route(); @@ -206,8 +206,7 @@ impl RouterContext { /// The route that is currently active. pub fn current(&self) -> R { self.state - .read() - .unwrap() + .borrow() .history .current_route() .downcast::() @@ -218,7 +217,7 @@ impl RouterContext { /// The route that is currently active. pub fn current_route_string(&self) -> String { - self.any_route_to_string(&*self.state.read().unwrap().history.current_route()) + self.any_route_to_string(&*self.state.borrow().history.current_route()) } pub(crate) fn any_route_to_string(&self, route: &dyn Any) -> String { @@ -243,7 +242,7 @@ impl RouterContext { /// The prefix that is currently active. pub fn prefix(&self) -> Option { - self.state.read().unwrap().prefix.clone() + self.state.borrow().prefix.clone() } fn external(&self, external: String) -> Option { @@ -261,8 +260,8 @@ impl RouterContext { } } - fn state_mut(&self) -> RwLockWriteGuard { - self.state.write().unwrap() + fn state_mut(&self) -> RefMut { + self.state.borrow_mut() } /// Manually subscribe to the current route @@ -283,15 +282,14 @@ impl RouterContext { /// Clear any unresolved errors pub fn clear_error(&self) { - self.state.write().unwrap().unresolved_error = None; + self.state.borrow_mut().unresolved_error = None; self.update_subscribers(); } pub(crate) fn render_error<'a>(&self, cx: Scope<'a>) -> Element<'a> { self.state - .read() - .unwrap() + .borrow() .unresolved_error .as_ref() .and_then(|_| (self.failure_external_navigation)(cx)) diff --git a/packages/web/src/dom.rs b/packages/web/src/dom.rs index 9d0d837c54..52f2ab1a94 100644 --- a/packages/web/src/dom.rs +++ b/packages/web/src/dom.rs @@ -392,6 +392,7 @@ fn read_input_to_data(target: Element) -> Rc { .dyn_ref() .and_then(|input: &web_sys::HtmlInputElement| { input.files().and_then(|files| { + #[allow(clippy::arc_with_non_send_sync)] crate::file_engine::WebFileEngine::new(files) .map(|f| std::sync::Arc::new(f) as std::sync::Arc) }) From f3e7f042b42c461d88fe7bfeae880529931ef1c9 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Sat, 26 Aug 2023 14:53:39 -0500 Subject: [PATCH 3/3] fix reading signals outside of the vdom --- packages/signals/src/effect.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/signals/src/effect.rs b/packages/signals/src/effect.rs index 1a42c227f9..9a54188d9e 100644 --- a/packages/signals/src/effect.rs +++ b/packages/signals/src/effect.rs @@ -18,7 +18,8 @@ pub(crate) fn get_effect_stack() -> EffectStack { Some(rt) => rt, None => { let store = EffectStack::default(); - provide_root_context(store).expect("in a virtual dom") + provide_root_context(store.clone()); + store } } }