From 477fc0ae3b571e0f4d91fafe15570f6b25717e31 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 26 Jul 2024 14:20:44 -0700 Subject: [PATCH] Fix #2612: adjust readable trait to allow try_peek Our implementation for Readable was inconsistent. We had a try_unchecked variant for read but not for peek. This resolves that by making a breaking change to the Readable interface. --- Cargo.lock | 2 +- packages/generational-box/src/error.rs | 3 ++ packages/hooks/src/use_future.rs | 6 ++- packages/hooks/src/use_resource.rs | 6 ++- packages/signals/src/copy_value.rs | 6 +-- packages/signals/src/global/memo.rs | 6 +-- packages/signals/src/global/signal.rs | 6 +-- packages/signals/src/map.rs | 19 ++++---- packages/signals/src/memo.rs | 6 +-- packages/signals/src/read.rs | 57 +++++++++++++++++------- packages/signals/src/read_only_signal.rs | 6 +-- packages/signals/src/signal.rs | 13 +++--- 12 files changed, 86 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a4cccc8c13..b04abf757b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10382,7 +10382,7 @@ source = "git+https://github.com/DioxusLabs/warnings#9889b96cccb6ac91a8af924cfee dependencies = [ "proc-macro2", "quote", - "syn 2.0.71", + "syn 2.0.72", ] [[package]] diff --git a/packages/generational-box/src/error.rs b/packages/generational-box/src/error.rs index 4ffd673077..74bd8882ad 100644 --- a/packages/generational-box/src/error.rs +++ b/packages/generational-box/src/error.rs @@ -4,6 +4,9 @@ use std::fmt::Display; use crate::GenerationalLocation; +/// A result that can be returned from a borrow operation. +pub type BorrowResult = std::result::Result; + #[derive(Debug, Clone, PartialEq)] /// An error that can occur when trying to borrow a value. pub enum BorrowError { diff --git a/packages/hooks/src/use_future.rs b/packages/hooks/src/use_future.rs index 2f749b145e..430b215de3 100644 --- a/packages/hooks/src/use_future.rs +++ b/packages/hooks/src/use_future.rs @@ -175,8 +175,10 @@ impl Readable for UseFuture { } #[track_caller] - fn peek_unchecked(&self) -> ReadableRef<'static, Self> { - self.state.peek_unchecked() + fn try_peek_unchecked( + &self, + ) -> Result, generational_box::BorrowError> { + self.state.try_peek_unchecked() } } diff --git a/packages/hooks/src/use_resource.rs b/packages/hooks/src/use_resource.rs index 7ad1afaf40..ed52d70292 100644 --- a/packages/hooks/src/use_resource.rs +++ b/packages/hooks/src/use_resource.rs @@ -441,8 +441,10 @@ impl Readable for Resource { } #[track_caller] - fn peek_unchecked(&self) -> ReadableRef<'static, Self> { - self.value.peek_unchecked() + fn try_peek_unchecked( + &self, + ) -> Result, generational_box::BorrowError> { + self.value.try_peek_unchecked() } } diff --git a/packages/signals/src/copy_value.rs b/packages/signals/src/copy_value.rs index 771326afc8..581a171d39 100644 --- a/packages/signals/src/copy_value.rs +++ b/packages/signals/src/copy_value.rs @@ -1,5 +1,5 @@ -use generational_box::GenerationalBoxId; use generational_box::UnsyncStorage; +use generational_box::{BorrowResult, GenerationalBoxId}; use std::ops::Deref; use dioxus_core::prelude::*; @@ -129,8 +129,8 @@ impl> Readable for CopyValue { } #[track_caller] - fn peek_unchecked(&self) -> ReadableRef<'static, Self> { - self.value.read() + fn try_peek_unchecked(&self) -> BorrowResult> { + self.value.try_read() } } diff --git a/packages/signals/src/global/memo.rs b/packages/signals/src/global/memo.rs index eceef3cddc..4bf8753083 100644 --- a/packages/signals/src/global/memo.rs +++ b/packages/signals/src/global/memo.rs @@ -1,7 +1,7 @@ use crate::{read::Readable, Memo, ReadableRef}; use crate::{read_impls, GlobalKey}; use dioxus_core::prelude::ScopeId; -use generational_box::UnsyncStorage; +use generational_box::{BorrowResult, UnsyncStorage}; use std::ops::Deref; use crate::Signal; @@ -75,8 +75,8 @@ impl Readable for GlobalMemo { } #[track_caller] - fn peek_unchecked(&self) -> ReadableRef<'static, Self> { - self.memo().peek_unchecked() + fn try_peek_unchecked(&self) -> BorrowResult> { + self.memo().try_peek_unchecked() } } diff --git a/packages/signals/src/global/signal.rs b/packages/signals/src/global/signal.rs index 3c1b687005..096c958c96 100644 --- a/packages/signals/src/global/signal.rs +++ b/packages/signals/src/global/signal.rs @@ -2,7 +2,7 @@ use crate::{read::Readable, ReadableRef}; use crate::{write::Writable, GlobalKey}; use crate::{WritableRef, Write}; use dioxus_core::{prelude::ScopeId, Runtime}; -use generational_box::UnsyncStorage; +use generational_box::{BorrowResult, UnsyncStorage}; use std::ops::Deref; use super::get_global_context; @@ -110,8 +110,8 @@ impl Readable for GlobalSignal { } #[track_caller] - fn peek_unchecked(&self) -> ReadableRef<'static, Self> { - self.signal().peek_unchecked() + fn try_peek_unchecked(&self) -> BorrowResult> { + self.signal().try_peek_unchecked() } } diff --git a/packages/signals/src/map.rs b/packages/signals/src/map.rs index 4905941561..06e0feb4d5 100644 --- a/packages/signals/src/map.rs +++ b/packages/signals/src/map.rs @@ -2,19 +2,19 @@ use std::{ops::Deref, rc::Rc}; use crate::{read::Readable, read_impls, ReadableRef}; use dioxus_core::prelude::*; -use generational_box::{AnyStorage, UnsyncStorage}; +use generational_box::{AnyStorage, BorrowResult, UnsyncStorage}; /// A read only signal that has been mapped to a new type. pub struct MappedSignal { try_read: Rc Result, generational_box::BorrowError> + 'static>, - peek: Rc S::Ref<'static, O> + 'static>, + try_peek: Rc Result, generational_box::BorrowError> + 'static>, } impl Clone for MappedSignal { fn clone(&self) -> Self { MappedSignal { try_read: self.try_read.clone(), - peek: self.peek.clone(), + try_peek: self.try_peek.clone(), } } } @@ -29,9 +29,11 @@ where try_read: Rc< dyn Fn() -> Result, generational_box::BorrowError> + 'static, >, - peek: Rc S::Ref<'static, O> + 'static>, + try_peek: Rc< + dyn Fn() -> Result, generational_box::BorrowError> + 'static, + >, ) -> Self { - MappedSignal { try_read, peek } + MappedSignal { try_read, try_peek } } } @@ -49,8 +51,8 @@ where (self.try_read)() } - fn peek_unchecked(&self) -> ReadableRef<'static, Self> { - (self.peek)() + fn try_peek_unchecked(&self) -> BorrowResult> { + (self.try_peek)() } } @@ -70,7 +72,8 @@ where S: AnyStorage, { fn eq(&self, other: &Self) -> bool { - std::ptr::eq(&self.peek, &other.peek) && std::ptr::eq(&self.try_read, &other.try_read) + std::ptr::eq(&self.try_peek, &other.try_peek) + && std::ptr::eq(&self.try_read, &other.try_read) } } diff --git a/packages/signals/src/memo.rs b/packages/signals/src/memo.rs index 13a42af7b9..60b5ea61f4 100644 --- a/packages/signals/src/memo.rs +++ b/packages/signals/src/memo.rs @@ -10,7 +10,7 @@ use std::{ use dioxus_core::prelude::*; use futures_util::StreamExt; -use generational_box::{AnyStorage, UnsyncStorage}; +use generational_box::{AnyStorage, BorrowResult, UnsyncStorage}; struct UpdateInformation { dirty: Arc, @@ -164,8 +164,8 @@ where /// /// If the signal has been dropped, this will panic. #[track_caller] - fn peek_unchecked(&self) -> ReadableRef<'static, Self> { - self.inner.peek_unchecked() + fn try_peek_unchecked(&self) -> BorrowResult> { + self.inner.try_peek_unchecked() } } diff --git a/packages/signals/src/read.rs b/packages/signals/src/read.rs index cddc4ed318..4ac8482fbb 100644 --- a/packages/signals/src/read.rs +++ b/packages/signals/src/read.rs @@ -83,10 +83,20 @@ pub trait Readable { dyn Fn() -> Result, generational_box::BorrowError> + 'static, >; - let peek = Rc::new(move || { - ::map(self.peek_unchecked(), |r| mapping(r)) - }) as Rc ReadableRef<'static, Self, O> + 'static>; - MappedSignal::new(try_read, peek) + let try_peek = Rc::new({ + let self_ = self.clone(); + let mapping = mapping.clone(); + move || { + self_ + .try_peek_unchecked() + .map(|ref_| ::map(ref_, |r| mapping(r))) + } + }) + as Rc< + dyn Fn() -> Result, generational_box::BorrowError> + + 'static, + >; + MappedSignal::new(try_read, try_peek) } /// Get the current value of the state. If this is a signal, this will subscribe the current scope to the signal. @@ -104,13 +114,6 @@ pub trait Readable { .map(Self::Storage::downcast_lifetime_ref) } - /// Try to get a reference to the value without checking the lifetime. This will subscribe the current scope to the signal. - /// - /// NOTE: This method is completely safe because borrow checking is done at runtime. - fn try_read_unchecked( - &self, - ) -> Result, generational_box::BorrowError>; - /// Get a reference to the value without checking the lifetime. This will subscribe the current scope to the signal. /// /// NOTE: This method is completely safe because borrow checking is done at runtime. @@ -119,12 +122,12 @@ pub trait Readable { self.try_read_unchecked().unwrap() } - /// Get the current value of the signal without checking the lifetime. **Unlike read, this will not subscribe the current scope to the signal which can cause parts of your UI to not update.** - /// - /// If the signal has been dropped, this will panic. + /// Try to get a reference to the value without checking the lifetime. This will subscribe the current scope to the signal. /// /// NOTE: This method is completely safe because borrow checking is done at runtime. - fn peek_unchecked(&self) -> ReadableRef<'static, Self>; + fn try_read_unchecked( + &self, + ) -> Result, generational_box::BorrowError>; /// Get the current value of the state without subscribing to updates. If the value has been dropped, this will panic. /// @@ -164,6 +167,30 @@ pub trait Readable { Self::Storage::downcast_lifetime_ref(self.peek_unchecked()) } + /// Try to peek the current value of the signal without subscribing to updates. If the value has + /// been dropped, this will return an error. + #[track_caller] + fn try_peek(&self) -> Result, generational_box::BorrowError> { + self.try_peek_unchecked() + .map(Self::Storage::downcast_lifetime_ref) + } + + /// Get the current value of the signal without checking the lifetime. **Unlike read, this will not subscribe the current scope to the signal which can cause parts of your UI to not update.** + /// + /// If the signal has been dropped, this will panic. + #[track_caller] + fn peek_unchecked(&self) -> ReadableRef<'static, Self> { + self.try_peek_unchecked().unwrap() + } + + /// Try to peek the current value of the signal without subscribing to updates. If the value has + /// been dropped, this will return an error. + /// + /// NOTE: This method is completely safe because borrow checking is done at runtime. + fn try_peek_unchecked( + &self, + ) -> Result, generational_box::BorrowError>; + /// Clone the inner value and return it. If the value has been dropped, this will panic. #[track_caller] fn cloned(&self) -> Self::Target diff --git a/packages/signals/src/read_only_signal.rs b/packages/signals/src/read_only_signal.rs index 0794d3f212..5399d08582 100644 --- a/packages/signals/src/read_only_signal.rs +++ b/packages/signals/src/read_only_signal.rs @@ -4,7 +4,7 @@ use std::ops::Deref; use crate::{default_impl, read_impls}; use dioxus_core::{prelude::IntoAttributeValue, ScopeId}; -use generational_box::{Storage, UnsyncStorage}; +use generational_box::{BorrowResult, Storage, UnsyncStorage}; /// A signal that can only be read from. pub struct ReadOnlySignal> = UnsyncStorage> { @@ -79,8 +79,8 @@ impl>> Readable for ReadOnlySignal { /// /// If the signal has been dropped, this will panic. #[track_caller] - fn peek_unchecked(&self) -> S::Ref<'static, T> { - self.inner.peek_unchecked() + fn try_peek_unchecked(&self) -> BorrowResult> { + self.inner.try_peek_unchecked() } } diff --git a/packages/signals/src/signal.rs b/packages/signals/src/signal.rs index c0d5e67770..73806f803f 100644 --- a/packages/signals/src/signal.rs +++ b/packages/signals/src/signal.rs @@ -2,7 +2,7 @@ use crate::{default_impl, fmt_impls, write_impls}; use crate::{read::*, write::*, CopyValue, GlobalMemo, GlobalSignal, ReadableRef}; use crate::{Memo, WritableRef}; use dioxus_core::prelude::*; -use generational_box::{AnyStorage, Storage, SyncStorage, UnsyncStorage}; +use generational_box::{AnyStorage, BorrowResult, Storage, SyncStorage, UnsyncStorage}; use std::sync::Arc; use std::{ any::Any, @@ -349,9 +349,7 @@ impl>> Readable for Signal { type Storage = S; #[track_caller] - fn try_read_unchecked( - &self, - ) -> Result, generational_box::BorrowError> { + fn try_read_unchecked(&self) -> BorrowResult> { let inner = self.inner.try_read_unchecked()?; if let Some(reactive_context) = ReactiveContext::current() { @@ -366,9 +364,10 @@ impl>> Readable for Signal { /// /// If the signal has been dropped, this will panic. #[track_caller] - fn peek_unchecked(&self) -> ReadableRef<'static, Self> { - let inner = self.inner.try_read_unchecked().unwrap(); - S::map(inner, |v| &v.value) + fn try_peek_unchecked(&self) -> BorrowResult> { + self.inner + .try_read_unchecked() + .map(|inner| S::map(inner, |v| &v.value)) } }