Skip to content

Commit a36fb55

Browse files
ealmloffealmloff
authored
fix removing keyed items from a vec store (#5312)
1 parent ad4981e commit a36fb55

5 files changed

Lines changed: 196 additions & 28 deletions

File tree

‎packages/core-macro/src/props/mod.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -661,10 +661,13 @@ mod struct_info {
661661
// If they are equal, we don't need to rerun the component we can just update the existing signals
662662
#(
663663
// Try to memo the signal
664-
let field_eq = {
665-
let old_value: &_ = &*#signal_fields.peek();
666-
let new_value: &_ = &*new.#signal_fields.peek();
667-
(&old_value).compare(&&new_value)
664+
let field_eq = match (#signal_fields.try_peek(), new.#signal_fields.try_peek()) {
665+
(Ok(old_ref), Ok(new_ref)) => {
666+
let old_value: &_ = &*old_ref;
667+
let new_value: &_ = &*new_ref;
668+
(&old_value).compare(&&new_value)
669+
}
670+
_ => false,
668671
};
669672
// Make the old fields point to the new fields
670673
#signal_fields.point_to(new.#signal_fields).unwrap();

‎packages/signals/src/boxed.rs‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -132,21 +132,15 @@ impl<T: ?Sized, S: BoxedSignalStorage<T>> Readable for ReadSignal<T, S> {
132132
where
133133
T: 'static,
134134
{
135-
self.value
136-
.try_peek_unchecked()
137-
.unwrap()
138-
.try_read_unchecked()
135+
self.value.try_peek_unchecked()?.try_read_unchecked()
139136
}
140137

141138
#[track_caller]
142139
fn try_peek_unchecked(&self) -> BorrowResult<ReadableRef<'static, Self>>
143140
where
144141
T: 'static,
145142
{
146-
self.value
147-
.try_peek_unchecked()
148-
.unwrap()
149-
.try_peek_unchecked()
143+
self.value.try_peek_unchecked()?.try_peek_unchecked()
150144
}
151145

152146
fn subscribers(&self) -> Subscribers
@@ -367,21 +361,15 @@ impl<T: ?Sized, S: BoxedSignalStorage<T>> Readable for WriteSignal<T, S> {
367361
where
368362
T: 'static,
369363
{
370-
self.value
371-
.try_peek_unchecked()
372-
.unwrap()
373-
.try_read_unchecked()
364+
self.value.try_peek_unchecked()?.try_read_unchecked()
374365
}
375366

376367
#[track_caller]
377368
fn try_peek_unchecked(&self) -> BorrowResult<ReadableRef<'static, Self>>
378369
where
379370
T: 'static,
380371
{
381-
self.value
382-
.try_peek_unchecked()
383-
.unwrap()
384-
.try_peek_unchecked()
372+
self.value.try_peek_unchecked()?.try_peek_unchecked()
385373
}
386374

387375
fn subscribers(&self) -> Subscribers

‎packages/stores/src/impls/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ pub mod hashmap;
44
pub mod index;
55
mod option;
66
mod result;
7-
mod slice;
7+
pub mod slice;
88
mod vec;

‎packages/stores/src/impls/slice.rs‎

Lines changed: 131 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
1-
use std::iter::FusedIterator;
1+
//! Additional utilities for `Vec` stores.
22
3-
use crate::{impls::index::IndexWrite, store::Store};
4-
use dioxus_signals::{Readable, ReadableExt};
3+
use std::{iter::FusedIterator, panic::Location};
4+
5+
use crate::{impls::index::IndexSelector, store::Store, ReadStore};
6+
use dioxus_signals::{
7+
AnyStorage, BorrowError, BorrowMutError, ReadSignal, Readable, ReadableExt, UnsyncStorage,
8+
Writable, WriteLock, WriteSignal,
9+
};
10+
use generational_box::ValueDroppedError;
511

612
impl<Lens, I> Store<Vec<I>, Lens>
713
where
@@ -46,16 +52,18 @@ where
4652
/// println!("{}", item);
4753
/// }
4854
/// ```
55+
#[track_caller]
4956
pub fn iter(
5057
&self,
51-
) -> impl ExactSizeIterator<Item = Store<I, IndexWrite<usize, Lens>>>
58+
) -> impl ExactSizeIterator<Item = Store<I, VecGetWrite<Lens>>>
5259
+ DoubleEndedIterator
5360
+ FusedIterator
5461
+ '_
5562
where
5663
Lens: Clone,
5764
{
58-
(0..self.len()).map(move |i| self.clone().index(i))
65+
let location = Location::caller();
66+
(0..self.len()).map(move |i| self.clone().get_unchecked_at(i, location))
5967
}
6068

6169
/// Try to get an item from slice. This will only track the shallow state of the slice.
@@ -70,14 +78,130 @@ where
7078
/// // The indexed store can access the store methods of the indexed store.
7179
/// assert_eq!(indexed_store(), 2);
7280
/// ```
73-
pub fn get(&self, index: usize) -> Option<Store<I, IndexWrite<usize, Lens>>>
81+
pub fn get(&self, index: usize) -> Option<Store<I, VecGetWrite<Lens>>>
7482
where
7583
Lens: Clone,
7684
{
7785
if index >= self.len() {
7886
None
7987
} else {
80-
Some(self.clone().index(index))
88+
Some(self.clone().get_unchecked(index))
8189
}
8290
}
91+
92+
/// Get a store for the item at the given index without checking if it is in bounds.
93+
///
94+
/// This is not unsafe, but reads will return a [BorrowError::Dropped] error if the index is out of bounds.
95+
#[track_caller]
96+
pub fn get_unchecked(self, index: usize) -> Store<I, VecGetWrite<Lens>> {
97+
self.get_unchecked_at(index, Location::caller())
98+
}
99+
100+
fn get_unchecked_at(
101+
self,
102+
index: usize,
103+
location: &'static Location<'static>,
104+
) -> Store<I, VecGetWrite<Lens>> {
105+
<Vec<I>>::scope_selector(self.into_selector(), &index)
106+
.map_writer(move |write| VecGetWrite {
107+
index,
108+
write,
109+
created: location,
110+
})
111+
.into()
112+
}
113+
}
114+
115+
/// A specific index in a `Readable` / `Writable` Vec that uses safe `.get()` / `.get_mut()` access.
116+
#[derive(Clone, Copy)]
117+
pub struct VecGetWrite<Write> {
118+
index: usize,
119+
write: Write,
120+
created: &'static Location<'static>,
121+
}
122+
123+
impl<Write, T> Readable for VecGetWrite<Write>
124+
where
125+
Write: Readable<Target = Vec<T>>,
126+
T: 'static,
127+
{
128+
type Target = T;
129+
type Storage = Write::Storage;
130+
131+
fn try_read_unchecked(&self) -> Result<dioxus_signals::ReadableRef<'static, Self>, BorrowError>
132+
where
133+
Self::Target: 'static,
134+
{
135+
self.write.try_read_unchecked().and_then(|value| {
136+
let index = self.index;
137+
Self::Storage::try_map(value, move |value: &Vec<T>| value.get(index))
138+
.ok_or_else(|| BorrowError::Dropped(ValueDroppedError::new(self.created)))
139+
})
140+
}
141+
142+
fn try_peek_unchecked(&self) -> Result<dioxus_signals::ReadableRef<'static, Self>, BorrowError>
143+
where
144+
Self::Target: 'static,
145+
{
146+
self.write.try_peek_unchecked().and_then(|value| {
147+
let index = self.index;
148+
Self::Storage::try_map(value, move |value: &Vec<T>| value.get(index))
149+
.ok_or_else(|| BorrowError::Dropped(ValueDroppedError::new(self.created)))
150+
})
151+
}
152+
153+
fn subscribers(&self) -> dioxus_core::Subscribers
154+
where
155+
Self::Target: 'static,
156+
{
157+
self.write.subscribers()
158+
}
159+
}
160+
161+
impl<Write, T> Writable for VecGetWrite<Write>
162+
where
163+
Write: Writable<Target = Vec<T>>,
164+
T: 'static,
165+
{
166+
type WriteMetadata = Write::WriteMetadata;
167+
168+
fn try_write_unchecked(
169+
&self,
170+
) -> Result<dioxus_signals::WritableRef<'static, Self>, BorrowMutError>
171+
where
172+
Self::Target: 'static,
173+
{
174+
self.write.try_write_unchecked().and_then(|value| {
175+
let index = self.index;
176+
WriteLock::filter_map(value, move |value: &mut Vec<T>| value.get_mut(index))
177+
.ok_or_else(|| BorrowMutError::Dropped(ValueDroppedError::new(self.created)))
178+
})
179+
}
180+
}
181+
182+
impl<T, Write> ::std::convert::From<Store<T, VecGetWrite<Write>>> for Store<T, WriteSignal<T>>
183+
where
184+
Write: Writable<Target = Vec<T>, Storage = UnsyncStorage> + 'static,
185+
Write::WriteMetadata: 'static,
186+
T: 'static,
187+
{
188+
fn from(value: Store<T, VecGetWrite<Write>>) -> Self {
189+
value
190+
.into_selector()
191+
.map_writer(|writer| WriteSignal::new(writer))
192+
.into()
193+
}
194+
}
195+
196+
impl<T, Write> ::std::convert::From<Store<T, VecGetWrite<Write>>> for ReadStore<T>
197+
where
198+
Write: Readable<Target = Vec<T>, Storage = UnsyncStorage> + 'static,
199+
T: 'static,
200+
{
201+
fn from(value: Store<T, VecGetWrite<Write>>) -> Self {
202+
value
203+
.into_selector()
204+
.map_writer(|writer| ReadSignal::new(writer))
205+
.into()
206+
}
83207
}

‎packages/stores/tests/vec_remove.rs‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
use dioxus::prelude::*;
2+
use dioxus_core::generation;
3+
4+
#[derive(Debug, Clone, dioxus_stores::Store)]
5+
struct ListItem {
6+
id: usize,
7+
text: String,
8+
}
9+
10+
/// Removing a non-last item from a Vec store should not panic during memoization.
11+
#[test]
12+
fn vec_store_remove_non_last_item() {
13+
fn app() -> Element {
14+
let mut store = use_store(|| {
15+
vec![
16+
ListItem {
17+
id: 0,
18+
text: "Item 0".to_string(),
19+
},
20+
ListItem {
21+
id: 1,
22+
text: "Item 1".to_string(),
23+
},
24+
]
25+
});
26+
27+
// On the second render, remove the first item.
28+
if generation() > 0 {
29+
store.remove(0);
30+
}
31+
32+
rsx! {
33+
for item in store.iter() {
34+
li { key: "{item.id()}",
35+
ListItemElement { list_item: item }
36+
}
37+
}
38+
}
39+
}
40+
41+
#[component]
42+
fn ListItemElement(list_item: ReadSignal<ListItem>) -> Element {
43+
rsx! {
44+
div { "{list_item.read().text}" }
45+
}
46+
}
47+
48+
let mut dom = VirtualDom::new(app);
49+
dom.rebuild(&mut dioxus_core::NoOpMutations);
50+
// Second render triggers remove(0), which previously panicked during memoization
51+
dom.mark_dirty(ScopeId::APP);
52+
_ = dom.render_immediate_to_vec();
53+
}

0 commit comments

Comments
 (0)