Skip to content

Commit

Permalink
Fix #2612: adjust readable trait to allow try_peek
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jkelleyrtp committed Jul 26, 2024
1 parent 443b9a4 commit 477fc0a
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 50 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions packages/generational-box/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ use std::fmt::Display;

use crate::GenerationalLocation;

/// A result that can be returned from a borrow operation.
pub type BorrowResult<T> = std::result::Result<T, BorrowError>;

#[derive(Debug, Clone, PartialEq)]
/// An error that can occur when trying to borrow a value.
pub enum BorrowError {
Expand Down
6 changes: 4 additions & 2 deletions packages/hooks/src/use_future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ReadableRef<'static, Self>, generational_box::BorrowError> {
self.state.try_peek_unchecked()
}
}

Expand Down
6 changes: 4 additions & 2 deletions packages/hooks/src/use_resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,10 @@ impl<T> Readable for Resource<T> {
}

#[track_caller]
fn peek_unchecked(&self) -> ReadableRef<'static, Self> {
self.value.peek_unchecked()
fn try_peek_unchecked(
&self,
) -> Result<ReadableRef<'static, Self>, generational_box::BorrowError> {
self.value.try_peek_unchecked()
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/signals/src/copy_value.rs
Original file line number Diff line number Diff line change
@@ -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::*;
Expand Down Expand Up @@ -129,8 +129,8 @@ impl<T: 'static, S: Storage<T>> Readable for CopyValue<T, S> {
}

#[track_caller]
fn peek_unchecked(&self) -> ReadableRef<'static, Self> {
self.value.read()
fn try_peek_unchecked(&self) -> BorrowResult<ReadableRef<'static, Self>> {
self.value.try_read()
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/signals/src/global/memo.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -75,8 +75,8 @@ impl<T: PartialEq + 'static> Readable for GlobalMemo<T> {
}

#[track_caller]
fn peek_unchecked(&self) -> ReadableRef<'static, Self> {
self.memo().peek_unchecked()
fn try_peek_unchecked(&self) -> BorrowResult<ReadableRef<'static, Self>> {
self.memo().try_peek_unchecked()
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/signals/src/global/signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -110,8 +110,8 @@ impl<T: 'static> Readable for GlobalSignal<T> {
}

#[track_caller]
fn peek_unchecked(&self) -> ReadableRef<'static, Self> {
self.signal().peek_unchecked()
fn try_peek_unchecked(&self) -> BorrowResult<ReadableRef<'static, Self>> {
self.signal().try_peek_unchecked()
}
}

Expand Down
19 changes: 11 additions & 8 deletions packages/signals/src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<O: ?Sized + 'static, S: AnyStorage = UnsyncStorage> {
try_read: Rc<dyn Fn() -> Result<S::Ref<'static, O>, generational_box::BorrowError> + 'static>,
peek: Rc<dyn Fn() -> S::Ref<'static, O> + 'static>,
try_peek: Rc<dyn Fn() -> Result<S::Ref<'static, O>, generational_box::BorrowError> + 'static>,
}

impl<O: ?Sized, S: AnyStorage> Clone for MappedSignal<O, S> {
fn clone(&self) -> Self {
MappedSignal {
try_read: self.try_read.clone(),
peek: self.peek.clone(),
try_peek: self.try_peek.clone(),
}
}
}
Expand All @@ -29,9 +29,11 @@ where
try_read: Rc<
dyn Fn() -> Result<S::Ref<'static, O>, generational_box::BorrowError> + 'static,
>,
peek: Rc<dyn Fn() -> S::Ref<'static, O> + 'static>,
try_peek: Rc<
dyn Fn() -> Result<S::Ref<'static, O>, generational_box::BorrowError> + 'static,
>,
) -> Self {
MappedSignal { try_read, peek }
MappedSignal { try_read, try_peek }
}
}

Expand All @@ -49,8 +51,8 @@ where
(self.try_read)()
}

fn peek_unchecked(&self) -> ReadableRef<'static, Self> {
(self.peek)()
fn try_peek_unchecked(&self) -> BorrowResult<ReadableRef<'static, Self>> {
(self.try_peek)()
}
}

Expand All @@ -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)
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/signals/src/memo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
dirty: Arc<AtomicBool>,
Expand Down Expand Up @@ -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<ReadableRef<'static, Self>> {
self.inner.try_peek_unchecked()
}
}

Expand Down
57 changes: 42 additions & 15 deletions packages/signals/src/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,20 @@ pub trait Readable {
dyn Fn() -> Result<ReadableRef<'static, Self, O>, generational_box::BorrowError>
+ 'static,
>;
let peek = Rc::new(move || {
<Self::Storage as AnyStorage>::map(self.peek_unchecked(), |r| mapping(r))
}) as Rc<dyn Fn() -> 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_| <Self::Storage as AnyStorage>::map(ref_, |r| mapping(r)))
}
})
as Rc<
dyn Fn() -> Result<ReadableRef<'static, Self, O>, 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.
Expand All @@ -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<ReadableRef<'static, Self>, 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.
Expand All @@ -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<ReadableRef<'static, Self>, generational_box::BorrowError>;

/// Get the current value of the state without subscribing to updates. If the value has been dropped, this will panic.
///
Expand Down Expand Up @@ -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<ReadableRef<Self>, 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<ReadableRef<'static, Self>, 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
Expand Down
6 changes: 3 additions & 3 deletions packages/signals/src/read_only_signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: 'static, S: Storage<SignalData<T>> = UnsyncStorage> {
Expand Down Expand Up @@ -79,8 +79,8 @@ impl<T, S: Storage<SignalData<T>>> Readable for ReadOnlySignal<T, S> {
///
/// 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<S::Ref<'static, T>> {
self.inner.try_peek_unchecked()
}
}

Expand Down
13 changes: 6 additions & 7 deletions packages/signals/src/signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -349,9 +349,7 @@ impl<T, S: Storage<SignalData<T>>> Readable for Signal<T, S> {
type Storage = S;

#[track_caller]
fn try_read_unchecked(
&self,
) -> Result<ReadableRef<'static, Self>, generational_box::BorrowError> {
fn try_read_unchecked(&self) -> BorrowResult<ReadableRef<'static, Self>> {
let inner = self.inner.try_read_unchecked()?;

if let Some(reactive_context) = ReactiveContext::current() {
Expand All @@ -366,9 +364,10 @@ impl<T, S: Storage<SignalData<T>>> Readable for Signal<T, S> {
///
/// 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<ReadableRef<'static, Self>> {
self.inner
.try_read_unchecked()
.map(|inner| S::map(inner, |v| &v.value))
}
}

Expand Down

0 comments on commit 477fc0a

Please sign in to comment.