From 98ee336d0c89549fbe29579fb0f03c16840d05d5 Mon Sep 17 00:00:00 2001 From: Greg Johnston Date: Wed, 2 Apr 2025 13:03:21 -0400 Subject: [PATCH] fix: remove `SendOption` from public API of actions (#3812) --- .../action-form-error-handling/src/app.rs | 1 - examples/server_fns_axum/src/app.rs | 2 +- leptos/src/form.rs | 11 +- reactive_graph/src/actions/action.rs | 136 ++++--- reactive_graph/src/signal.rs | 2 + reactive_graph/src/signal/mapped.rs | 358 ++++++++++++++++++ reactive_graph/src/traits.rs | 6 + 7 files changed, 446 insertions(+), 70 deletions(-) create mode 100644 reactive_graph/src/signal/mapped.rs diff --git a/examples/action-form-error-handling/src/app.rs b/examples/action-form-error-handling/src/app.rs index 5e4c33c26..fd380f48e 100644 --- a/examples/action-form-error-handling/src/app.rs +++ b/examples/action-form-error-handling/src/app.rs @@ -39,7 +39,6 @@ fn HomePage() -> impl IntoView { do_something_action .value() .get() - .take() .unwrap_or_else(|| Ok(String::new())) }); diff --git a/examples/server_fns_axum/src/app.rs b/examples/server_fns_axum/src/app.rs index 27d2bd4a0..46315b9d9 100644 --- a/examples/server_fns_axum/src/app.rs +++ b/examples/server_fns_axum/src/app.rs @@ -389,7 +389,7 @@ pub fn FileUpload() -> impl IntoView { "Upload a file.".to_string() } else if upload_action.pending().get() { "Uploading...".to_string() - } else if let Some(Ok(value)) = *upload_action.value().get() { + } else if let Some(Ok(value)) = upload_action.value().get() { value.to_string() } else { format!("{:?}", upload_action.value().get()) diff --git a/leptos/src/form.rs b/leptos/src/form.rs index 9b8532a7b..553810883 100644 --- a/leptos/src/form.rs +++ b/leptos/src/form.rs @@ -125,13 +125,10 @@ where "Error converting form field into server function \ arguments: {err:?}" ); - value.set( - Some(Err(ServerFnErrorErr::Serialization( - err.to_string(), - ) - .into_app_error())) - .into(), - ); + value.set(Some(Err(ServerFnErrorErr::Serialization( + err.to_string(), + ) + .into_app_error()))); version.update(|n| *n += 1); } } diff --git a/reactive_graph/src/actions/action.rs b/reactive_graph/src/actions/action.rs index e4f90c9a0..3ac5e1b16 100644 --- a/reactive_graph/src/actions/action.rs +++ b/reactive_graph/src/actions/action.rs @@ -1,16 +1,22 @@ use crate::{ computed::{ArcMemo, Memo}, diagnostics::is_suppressing_resource_load, - owner::{ArcStoredValue, ArenaItem, FromLocal, LocalStorage}, + owner::{ArcStoredValue, ArenaItem}, send_wrapper_ext::SendOption, - signal::{ArcRwSignal, RwSignal}, + signal::{ArcMappedSignal, ArcRwSignal, MappedSignal, RwSignal}, traits::{DefinedAt, Dispose, Get, GetUntracked, GetValue, Update, Write}, unwrap_signal, }; use any_spawner::Executor; use futures::{channel::oneshot, select, FutureExt}; use send_wrapper::SendWrapper; -use std::{future::Future, panic::Location, pin::Pin, sync::Arc}; +use std::{ + future::Future, + ops::{Deref, DerefMut}, + panic::Location, + pin::Pin, + sync::Arc, +}; /// An action runs some asynchronous code when you dispatch a new value to it, and gives you /// reactive access to the result. @@ -48,25 +54,25 @@ use std::{future::Future, panic::Location, pin::Pin, sync::Arc}; /// let version = save_data.version(); /// /// // before we do anything -/// assert_eq!(*input.get(), None); // no argument yet +/// assert_eq!(input.get(), None); // no argument yet /// assert_eq!(pending.get(), false); // isn't pending a response -/// assert_eq!(*result_of_call.get(), None); // there's no "last value" +/// assert_eq!(result_of_call.get(), None); // there's no "last value" /// assert_eq!(version.get(), 0); /// /// // dispatch the action /// save_data.dispatch("My todo".to_string()); /// /// // when we're making the call -/// assert_eq!(*input.get(), Some("My todo".to_string())); +/// assert_eq!(input.get(), Some("My todo".to_string())); /// assert_eq!(pending.get(), true); // is pending -/// assert_eq!(*result_of_call.get(), None); // has not yet gotten a response +/// assert_eq!(result_of_call.get(), None); // has not yet gotten a response /// /// # any_spawner::Executor::tick().await; /// /// // after call has resolved -/// assert_eq!(*input.get(), None); // input clears out after resolved +/// assert_eq!(input.get(), None); // input clears out after resolved /// assert_eq!(pending.get(), false); // no longer pending -/// assert_eq!(*result_of_call.get(), Some(42)); +/// assert_eq!(result_of_call.get(), Some(42)); /// assert_eq!(version.get(), 1); /// # }); /// ``` @@ -146,7 +152,7 @@ where /// }); /// /// act.dispatch(3); - /// assert_eq!(*act.input().get(), Some(3)); + /// assert_eq!(act.input().get(), Some(3)); /// /// // Remember that async functions already return a future if they are /// // not `await`ed. You can save keystrokes by leaving out the `async move` @@ -156,7 +162,7 @@ where /// # tokio::time::sleep(std::time::Duration::from_millis(10)).await; /// /// // after it resolves - /// assert_eq!(*act2.value().get(), Some("I'M IN A DOCTEST".to_string())); + /// assert_eq!(act2.value().get(), Some("I'M IN A DOCTEST".to_string())); /// /// async fn yell(n: String) -> String { /// n.to_uppercase() @@ -380,7 +386,11 @@ where } } -impl ArcAction { +impl ArcAction +where + I: 'static, + O: 'static, +{ /// The number of times the action has successfully completed. /// /// ```rust @@ -423,18 +433,22 @@ impl ArcAction { /// }); /// /// let input = act.input(); - /// assert_eq!(*input.get(), None); + /// assert_eq!(input.get(), None); /// act.dispatch(3); - /// assert_eq!(*input.get(), Some(3)); + /// assert_eq!(input.get(), Some(3)); /// /// # tokio::time::sleep(std::time::Duration::from_millis(10)).await; /// // after it resolves - /// assert_eq!(*input.get(), None); + /// assert_eq!(input.get(), None); /// # }); /// ``` #[track_caller] - pub fn input(&self) -> ArcRwSignal> { - self.input.clone() + pub fn input(&self) -> ArcMappedSignal> { + ArcMappedSignal::new( + self.input.clone(), + |n| n.deref(), + |n| n.deref_mut(), + ) } /// The most recent return value of the `async` function. This will be `None` before @@ -453,21 +467,25 @@ impl ArcAction { /// }); /// /// let value = act.value(); - /// assert_eq!(*value.get(), None); + /// assert_eq!(value.get(), None); /// act.dispatch(3); - /// assert_eq!(*value.get(), None); + /// assert_eq!(value.get(), None); /// /// # tokio::time::sleep(std::time::Duration::from_millis(10)).await; /// // after it resolves - /// assert_eq!(*value.get(), Some(6)); + /// assert_eq!(value.get(), Some(6)); /// // dispatch another value, and it still holds the old value /// act.dispatch(3); - /// assert_eq!(*value.get(), Some(6)); + /// assert_eq!(value.get(), Some(6)); /// # }); /// ``` #[track_caller] - pub fn value(&self) -> ArcRwSignal> { - self.value.clone() + pub fn value(&self) -> ArcMappedSignal> { + ArcMappedSignal::new( + self.value.clone(), + |n| n.deref(), + |n| n.deref_mut(), + ) } /// Whether the action has been dispatched and is currently waiting to resolve. @@ -553,25 +571,25 @@ where /// let version = save_data.version(); /// /// // before we do anything -/// assert_eq!(*input.get(), None); // no argument yet +/// assert_eq!(input.get(), None); // no argument yet /// assert_eq!(pending.get(), false); // isn't pending a response -/// assert_eq!(*result_of_call.get(), None); // there's no "last value" +/// assert_eq!(result_of_call.get(), None); // there's no "last value" /// assert_eq!(version.get(), 0); /// /// // dispatch the action /// save_data.dispatch("My todo".to_string()); /// /// // when we're making the call -/// assert_eq!(*input.get(), Some("My todo".to_string())); +/// assert_eq!(input.get(), Some("My todo".to_string())); /// assert_eq!(pending.get(), true); // is pending -/// assert_eq!(*result_of_call.get(), None); // has not yet gotten a response +/// assert_eq!(result_of_call.get(), None); // has not yet gotten a response /// /// # any_spawner::Executor::tick().await; /// /// // after call has resolved -/// assert_eq!(*input.get(), None); // input clears out after resolved +/// assert_eq!(input.get(), None); // input clears out after resolved /// assert_eq!(pending.get(), false); // no longer pending -/// assert_eq!(*result_of_call.get(), Some(42)); +/// assert_eq!(result_of_call.get(), Some(42)); /// assert_eq!(version.get(), 1); /// # }); /// ``` @@ -635,7 +653,7 @@ where /// }); /// /// act.dispatch(3); - /// assert_eq!(*act.input().get(), Some(3)); + /// assert_eq!(act.input().get(), Some(3)); /// /// // Remember that async functions already return a future if they are /// // not `await`ed. You can save keystrokes by leaving out the `async move` @@ -645,7 +663,7 @@ where /// # tokio::time::sleep(std::time::Duration::from_millis(10)).await; /// /// // after it resolves - /// assert_eq!(*act2.value().get(), Some("I'M IN A DOCTEST".to_string())); + /// assert_eq!(act2.value().get(), Some("I'M IN A DOCTEST".to_string())); /// /// async fn yell(n: String) -> String { /// n.to_uppercase() @@ -829,28 +847,27 @@ where /// # tokio_test::block_on(async move { /// # any_spawner::Executor::init_tokio(); let owner = reactive_graph::owner::Owner::new(); owner.set(); /// # let _guard = reactive_graph::diagnostics::SpecialNonReactiveZone::enter(); - /// let act = ArcAction::new(|n: &u8| { + /// let act = Action::new(|n: &u8| { /// let n = n.to_owned(); /// async move { n * 2 } /// }); /// /// let input = act.input(); - /// assert_eq!(*input.get(), None); + /// assert_eq!(input.get(), None); /// act.dispatch(3); - /// assert_eq!(*input.get(), Some(3)); + /// assert_eq!(input.get(), Some(3)); /// /// # tokio::time::sleep(std::time::Duration::from_millis(10)).await; /// // after it resolves - /// assert_eq!(*input.get(), None); + /// assert_eq!(input.get(), None); /// # }); /// ``` #[track_caller] - pub fn input(&self) -> RwSignal> { - let inner = self - .inner + pub fn input(&self) -> MappedSignal> { + self.inner .try_with_value(|inner| inner.input()) - .unwrap_or_else(unwrap_signal!(self)); - inner.into() + .unwrap_or_else(unwrap_signal!(self)) + .into() } /// The current argument that was dispatched to the async function. This value will @@ -860,12 +877,11 @@ where #[track_caller] #[deprecated = "You can now use .input() for any value, whether it's \ thread-safe or not."] - pub fn input_local(&self) -> RwSignal, LocalStorage> { - let inner = self - .inner + pub fn input_local(&self) -> MappedSignal> { + self.inner .try_with_value(|inner| inner.input()) - .unwrap_or_else(unwrap_signal!(self)); - RwSignal::from_local(inner) + .unwrap_or_else(unwrap_signal!(self)) + .into() } } @@ -890,25 +906,24 @@ where /// }); /// /// let value = act.value(); - /// assert_eq!(*value.get(), None); + /// assert_eq!(value.get(), None); /// act.dispatch(3); - /// assert_eq!(*value.get(), None); + /// assert_eq!(value.get(), None); /// /// # tokio::time::sleep(std::time::Duration::from_millis(10)).await; /// // after it resolves - /// assert_eq!(*value.get(), Some(6)); + /// assert_eq!(value.get(), Some(6)); /// // dispatch another value, and it still holds the old value /// act.dispatch(3); - /// assert_eq!(*value.get(), Some(6)); + /// assert_eq!(value.get(), Some(6)); /// # }); /// ``` #[track_caller] - pub fn value(&self) -> RwSignal> { - let inner = self - .inner + pub fn value(&self) -> MappedSignal> { + self.inner .try_with_value(|inner| inner.value()) - .unwrap_or_else(unwrap_signal!(self)); - inner.into() + .unwrap_or_else(unwrap_signal!(self)) + .into() } /// The most recent return value of the `async` function. This will be `None` before @@ -919,15 +934,14 @@ where #[deprecated = "You can now use .value() for any value, whether it's \ thread-safe or not."] #[track_caller] - pub fn value_local(&self) -> RwSignal, LocalStorage> + pub fn value_local(&self) -> MappedSignal> where O: Send + Sync, { - let inner = self - .inner + self.inner .try_with_value(|inner| inner.value()) - .unwrap_or_else(unwrap_signal!(self)); - RwSignal::from_local(inner) + .unwrap_or_else(unwrap_signal!(self)) + .into() } } @@ -1100,7 +1114,7 @@ impl Copy for Action {} /// }); /// /// act.dispatch(3); -/// assert_eq!(*act.input().get(), Some(3)); +/// assert_eq!(act.input().get(), Some(3)); /// /// // Remember that async functions already return a future if they are /// // not `await`ed. You can save keystrokes by leaving out the `async move` @@ -1110,7 +1124,7 @@ impl Copy for Action {} /// # tokio::time::sleep(std::time::Duration::from_millis(10)).await; /// /// // after it resolves -/// assert_eq!(*act2.value().get(), Some("I'M IN A DOCTEST".to_string())); +/// assert_eq!(act2.value().get(), Some("I'M IN A DOCTEST".to_string())); /// /// async fn yell(n: String) -> String { /// n.to_uppercase() diff --git a/reactive_graph/src/signal.rs b/reactive_graph/src/signal.rs index 5136fdacb..c3230fffa 100644 --- a/reactive_graph/src/signal.rs +++ b/reactive_graph/src/signal.rs @@ -6,6 +6,7 @@ mod arc_rw; mod arc_trigger; mod arc_write; pub mod guards; +mod mapped; mod read; mod rw; mod subscriber_traits; @@ -17,6 +18,7 @@ pub use arc_read::*; pub use arc_rw::*; pub use arc_trigger::*; pub use arc_write::*; +pub use mapped::*; pub use read::*; pub use rw::*; pub use trigger::*; diff --git a/reactive_graph/src/signal/mapped.rs b/reactive_graph/src/signal/mapped.rs new file mode 100644 index 000000000..f3cbb12a5 --- /dev/null +++ b/reactive_graph/src/signal/mapped.rs @@ -0,0 +1,358 @@ +use super::{ + guards::{Mapped, MappedMutArc}, + ArcRwSignal, RwSignal, +}; +use crate::{ + owner::{StoredValue, SyncStorage}, + signal::guards::WriteGuard, + traits::{ + DefinedAt, GetValue, IsDisposed, Notify, ReadUntracked, Track, + UntrackableGuard, Write, + }, +}; +use guardian::ArcRwLockWriteGuardian; +use std::{ + fmt::Debug, + ops::{Deref, DerefMut}, + panic::Location, + sync::Arc, +}; + +/// A derived signal type that wraps an [`ArcRwSignal`] with a mapping function, +/// allowing you to read or write directly to one of its field. +/// +/// Tracking the mapped signal tracks changes to *any* part of the signal, and updating the signal notifies +/// and notifies *all* depenendencies of the signal. This is not a mechanism for fine-grained reactive updates +/// to more complex data structures. Instead, it allows you to provide a signal-like API for wrapped types +/// without exposing the original type directly to users. +pub struct ArcMappedSignal { + #[cfg(debug_assertions)] + defined_at: &'static Location<'static>, + #[allow(clippy::type_complexity)] + try_read_untracked: Arc< + dyn Fn() -> Option>>> + + Send + + Sync, + >, + try_write: Arc< + dyn Fn() -> Option>> + Send + Sync, + >, + notify: Arc, + track: Arc, +} + +impl Clone for ArcMappedSignal { + fn clone(&self) -> Self { + Self { + #[cfg(debug_assertions)] + defined_at: self.defined_at, + try_read_untracked: self.try_read_untracked.clone(), + try_write: self.try_write.clone(), + notify: self.notify.clone(), + track: self.track.clone(), + } + } +} + +impl ArcMappedSignal { + /// Wraps a signal with the given mapping functions for shared and exclusive references. + #[track_caller] + pub fn new( + inner: ArcRwSignal, + map: fn(&U) -> &T, + map_mut: fn(&mut U) -> &mut T, + ) -> Self + where + T: 'static, + U: Send + Sync + 'static, + { + Self { + #[cfg(debug_assertions)] + defined_at: Location::caller(), + try_read_untracked: { + let this = inner.clone(); + Arc::new(move || { + this.try_read_untracked().map(|guard| DoubleDeref { + inner: Box::new(Mapped::new_with_guard(guard, map)) + as Box>, + }) + }) + }, + try_write: { + let this = inner.clone(); + Arc::new(move || { + let guard = ArcRwLockWriteGuardian::try_take(Arc::clone( + &this.value, + ))? + .ok()?; + let mapped = WriteGuard::new( + this.clone(), + MappedMutArc::new(guard, map, map_mut), + ); + Some(Box::new(mapped)) + }) + }, + notify: { + let this = inner.clone(); + Arc::new(move || { + this.notify(); + }) + }, + track: { + Arc::new(move || { + inner.track(); + }) + }, + } + } +} + +impl Debug for ArcMappedSignal { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut partial = f.debug_struct("ArcMappedSignal"); + #[cfg(debug_assertions)] + partial.field("defined_at", &self.defined_at); + partial.finish() + } +} + +impl DefinedAt for ArcMappedSignal { + fn defined_at(&self) -> Option<&'static Location<'static>> { + #[cfg(debug_assertions)] + { + Some(self.defined_at) + } + #[cfg(not(debug_assertions))] + { + None + } + } +} + +impl Notify for ArcMappedSignal { + fn notify(&self) { + (self.notify)() + } +} + +impl Track for ArcMappedSignal { + fn track(&self) { + (self.track)() + } +} + +impl ReadUntracked for ArcMappedSignal { + type Value = DoubleDeref>>; + + fn try_read_untracked(&self) -> Option { + (self.try_read_untracked)() + } +} + +impl IsDisposed for ArcMappedSignal { + fn is_disposed(&self) -> bool { + false + } +} + +impl Write for ArcMappedSignal +where + T: 'static, +{ + type Value = T; + + fn try_write_untracked( + &self, + ) -> Option> { + let mut guard = self.try_write()?; + guard.untrack(); + Some(guard) + } + + fn try_write(&self) -> Option> { + let inner = (self.try_write)()?; + let inner = DoubleDeref { inner }; + Some(inner) + } +} + +/// A wrapper for a smart pointer that implements [`Deref`] and [`DerefMut`] +/// by dereferencing the type *inside* the smart pointer. +/// +/// This is quite obscure and mostly useful for situations in which we want +/// a wrapper for `Box>` that dereferences to `T` rather +/// than dereferencing to `dyn Deref`. +/// +/// This is used internally in [`MappedSignal`] and [`ArcMappedSignal`]. +pub struct DoubleDeref { + inner: T, +} + +impl Deref for DoubleDeref +where + T: Deref, + T::Target: Deref, +{ + type Target = ::Target; + + fn deref(&self) -> &Self::Target { + self.inner.deref().deref() + } +} + +impl DerefMut for DoubleDeref +where + T: DerefMut, + T::Target: DerefMut, +{ + fn deref_mut(&mut self) -> &mut Self::Target { + self.inner.deref_mut().deref_mut() + } +} + +impl UntrackableGuard for DoubleDeref +where + T: UntrackableGuard, + T::Target: DerefMut, +{ + fn untrack(&mut self) { + self.inner.untrack(); + } +} + +/// A derived signal type that wraps an [`RwSignal`] with a mapping function, +/// allowing you to read or write directly to one of its field. +/// +/// Tracking the mapped signal tracks changes to *any* part of the signal, and updating the signal notifies +/// and notifies *all* depenendencies of the signal. This is not a mechanism for fine-grained reactive updates +/// to more complex data structures. Instead, it allows you to provide a signal-like API for wrapped types +/// without exposing the original type directly to users. +pub struct MappedSignal { + #[cfg(debug_assertions)] + defined_at: &'static Location<'static>, + inner: StoredValue, S>, +} + +impl MappedSignal { + /// Wraps a signal with the given mapping functions for shared and exclusive references. + #[track_caller] + pub fn new( + inner: RwSignal, + map: fn(&U) -> &T, + map_mut: fn(&mut U) -> &mut T, + ) -> Self + where + T: Send + Sync + 'static, + U: Send + Sync + 'static, + { + Self { + #[cfg(debug_assertions)] + defined_at: Location::caller(), + inner: { + let this = ArcRwSignal::from(inner); + StoredValue::new_with_storage(ArcMappedSignal::new( + this, map, map_mut, + )) + }, + } + } +} + +impl Debug for MappedSignal { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut partial = f.debug_struct("MappedSignal"); + #[cfg(debug_assertions)] + partial.field("defined_at", &self.defined_at); + partial.finish() + } +} + +impl DefinedAt for MappedSignal { + fn defined_at(&self) -> Option<&'static Location<'static>> { + #[cfg(debug_assertions)] + { + Some(self.defined_at) + } + #[cfg(not(debug_assertions))] + { + None + } + } +} + +impl Notify for MappedSignal +where + T: 'static, +{ + fn notify(&self) { + if let Some(inner) = self.inner.try_get_value() { + inner.notify(); + } + } +} + +impl Track for MappedSignal +where + T: 'static, +{ + fn track(&self) { + if let Some(inner) = self.inner.try_get_value() { + inner.track(); + } + } +} + +impl ReadUntracked for MappedSignal +where + T: 'static, +{ + type Value = DoubleDeref>>; + + fn try_read_untracked(&self) -> Option { + self.inner + .try_get_value() + .and_then(|inner| inner.try_read_untracked()) + } +} + +impl Write for MappedSignal +where + T: 'static, +{ + type Value = T; + + fn try_write_untracked( + &self, + ) -> Option> { + let mut guard = self.try_write()?; + guard.untrack(); + Some(guard) + } + + fn try_write(&self) -> Option> { + let inner = self.inner.try_get_value()?; + let inner = (inner.try_write)()?; + let inner = DoubleDeref { inner }; + Some(inner) + } +} + +impl From> for MappedSignal +where + T: 'static, +{ + #[track_caller] + fn from(value: ArcMappedSignal) -> Self { + MappedSignal { + #[cfg(debug_assertions)] + defined_at: Location::caller(), + inner: StoredValue::new(value), + } + } +} + +impl IsDisposed for MappedSignal { + fn is_disposed(&self) -> bool { + self.inner.is_disposed() + } +} diff --git a/reactive_graph/src/traits.rs b/reactive_graph/src/traits.rs index ebe34ef64..4bb0c5287 100644 --- a/reactive_graph/src/traits.rs +++ b/reactive_graph/src/traits.rs @@ -230,6 +230,12 @@ pub trait UntrackableGuard: DerefMut { fn untrack(&mut self); } +impl UntrackableGuard for Box> { + fn untrack(&mut self) { + (**self).untrack(); + } +} + /// Gives mutable access to a signal's value through a guard type. When the guard is dropped, the /// signal's subscribers will be notified. pub trait Write: Sized + DefinedAt + Notify {