diff --git a/packages/core/src/reactive_context.rs b/packages/core/src/reactive_context.rs index 2cbb0ce399..1135683f5b 100644 --- a/packages/core/src/reactive_context.rs +++ b/packages/core/src/reactive_context.rs @@ -1,6 +1,6 @@ use crate::{Runtime, ScopeId, current_scope_id, scope_context::Scope, tasks::SchedulerMsg}; use futures_channel::mpsc::UnboundedReceiver; -use generational_box::{BorrowMutError, GenerationalBox, SyncStorage}; +use generational_box::{BorrowMutError, GenerationalBox, Owner, SyncStorage}; use std::{ cell::RefCell, collections::HashSet, @@ -70,6 +70,20 @@ impl ReactiveContext { pub fn new_with_callback( callback: impl FnMut() + Send + Sync + 'static, scope: ScopeId, + origin: &'static std::panic::Location<'static>, + ) -> Self { + let owner = Runtime::current().scope_owner::(scope); + Self::new_with_callback_in_owner(callback, scope, owner, origin) + } + + /// Create a new reactive context owned by the given owner. + /// + /// This can be used when the reactive context is embedded in another owned value and should be + /// dropped with that value instead of with the component scope. + pub fn new_with_callback_in_owner( + callback: impl FnMut() + Send + Sync + 'static, + scope: ScopeId, + owner: Owner, #[allow(unused)] origin: &'static std::panic::Location<'static>, ) -> Self { let inner = Inner { @@ -82,8 +96,6 @@ impl ReactiveContext { scope: None, }; - let owner = Runtime::current().scope_owner(scope); - let self_ = Self { scope, inner: owner.insert(inner), @@ -134,7 +146,17 @@ impl ReactiveContext { pub fn clear_subscribers(&self) { // The key type is mutable, but the hash is stable through mutations because we hash by pointer #[allow(clippy::mutable_key_type)] - let old_subscribers = std::mem::take(&mut self.inner.write().subscribers); + let old_subscribers = match self.inner.try_write() { + Ok(mut inner) => std::mem::take(&mut inner.subscribers), + // If the context was dropped, it cannot actively unsubscribe itself; + // stale subscriber-list entries are cleaned lazily when they fail to mark dirty. + Err(BorrowMutError::Dropped(_)) => return, + Err(expect) => { + panic!( + "Expected to be able to write to reactive context to clear subscribers, but it failed with: {expect:?}" + ); + } + }; for subscriber in old_subscribers { subscriber.0.remove(self); } diff --git a/packages/signals/src/boxed.rs b/packages/signals/src/boxed.rs index 0b0a72c65f..c9a62560b5 100644 --- a/packages/signals/src/boxed.rs +++ b/packages/signals/src/boxed.rs @@ -1,7 +1,9 @@ use std::{any::Any, ops::Deref}; -use dioxus_core::{IntoAttributeValue, IntoDynNode, Subscribers}; -use generational_box::{BorrowResult, Storage, SyncStorage, UnsyncStorage}; +use dioxus_core::{ + IntoAttributeValue, IntoDynNode, ReactiveContext, Subscribers, current_scope_id, +}; +use generational_box::{BorrowResult, Owner, Storage, SyncStorage, UnsyncStorage}; use crate::{ CopyValue, Global, InitializeFromFunction, MappedMutSignal, MappedSignal, Memo, Readable, @@ -15,9 +17,118 @@ use crate::{ )] pub type ReadOnlySignal = ReadSignal; +/// Wrapper subscriber state plus a reactive context that forwards updates from the current +/// readable source. +/// +/// `ReadSignal` is a reactive proxy. A child component subscribes to this wrapper, and the +/// forwarding context subscribes to the readable source the wrapper currently points at. `point_to` +/// retargets only that source subscription; wrapper subscribers stay attached to this +/// `ForwardingContext`. +/// +/// Running source reads under this context preserves normal `Readable` subscription behavior for +/// signals, stores, and memos. When the source changes, this context marks wrapper subscribers +/// dirty without moving or clearing direct subscriptions made outside the wrapper. +/// +/// # Example +/// +/// ```rust,ignore +/// fn app() -> Element { +/// let mut use_b = use_signal(|| false); +/// let signal_a = use_signal(|| 0); +/// let signal_b = use_signal(|| 0); +/// +/// use_effect(move || { +/// signal_a(); +/// // This effect's context subscribes directly to signal_a. It is not a +/// // wrapper-level `ReadSignal` subscriber, so retargeting a child prop +/// // must not move this subscription to signal_b. +/// }); +/// +/// let child_signal = if use_b() { signal_b } else { signal_a }; +/// // When signal_a and signal_b currently hold equal values, props can be +/// // memoized in place: +/// // +/// // old ReadSignal(signal_a).point_to(new ReadSignal(signal_b)) +/// // +/// // That swap should keep wrapper subscribers attached to the existing +/// // wrapper and retarget only its source subscription. +/// rsx! { Child { sig: child_signal } } +/// } +/// +/// #[component] +/// fn Child(sig: ReadSignal) -> Element { +/// rsx! { +/// // This read subscribes the child to the ReadSignal wrapper. The +/// // wrapper's forwarding context subscribes to the current source. +/// "{sig}" +/// } +/// } +/// ``` +#[doc(hidden)] +pub struct ForwardingContext { + subscribers: Subscribers, + forwarding_context: ReactiveContext, + _owner: Owner, +} + +impl ForwardingContext { + fn new(wrapped_subscribers: Subscribers) -> Self { + let subscribers = Subscribers::new(); + let subscribers_to_notify = subscribers.clone(); + let owner = Owner::::default(); + let forwarding_context = ReactiveContext::new_with_callback_in_owner( + move || mark_subscribers_dirty(&subscribers_to_notify), + current_scope_id(), + owner.clone(), + std::panic::Location::caller(), + ); + forwarding_context.subscribe(wrapped_subscribers); + + Self { + subscribers, + forwarding_context, + _owner: owner, + } + } + + fn subscribers(&self) -> Subscribers { + self.subscribers.clone() + } + + fn run_in(&self, f: impl FnOnce() -> O) -> O { + self.forwarding_context.run_in(f) + } + + fn retarget_source(&self, wrapped_subscribers: Subscribers) { + self.forwarding_context.clear_subscribers(); + self.forwarding_context.subscribe(wrapped_subscribers); + } + + fn mark_dirty(&self) { + mark_subscribers_dirty(&self.subscribers); + } +} + +impl Drop for ForwardingContext { + fn drop(&mut self) { + self.forwarding_context.clear_subscribers(); + } +} + +fn mark_subscribers_dirty(subscribers: &Subscribers) { + let mut subscribers_to_notify = Vec::new(); + subscribers.visit(|subscriber| subscribers_to_notify.push(*subscriber)); + for subscriber in subscribers_to_notify { + if !subscriber.mark_dirty() { + subscribers.remove(&subscriber); + } + } +} + /// A boxed version of [Readable] that can be used to store any readable type. pub struct ReadSignal = UnsyncStorage> { value: CopyValue>, S>, + forwarding: CopyValue, } impl ReadSignal { @@ -34,22 +145,33 @@ impl> ReadSignal { S: CreateBoxedSignalStorage, R: Readable, { + let value = S::new_readable(value, sealed::SealedToken); + let subscribers = ForwardingContext::new(value.subscribers()); Self { - value: CopyValue::new_maybe_sync(S::new_readable(value, sealed::SealedToken)), + value: CopyValue::new_maybe_sync(value), + forwarding: CopyValue::new_maybe_sync(subscribers), } } - /// Point to another [ReadSignal]. This will subscribe the other [ReadSignal] to all subscribers of this [ReadSignal]. + /// Point to another [ReadSignal]. Wrapper-level subscribers stay attached to this wrapper; + /// subscribers attached directly to the underlying readable are left alone. pub fn point_to(&self, other: Self) -> BorrowResult { - let this_subscribers = self.subscribers(); - let mut this_subscribers_vec = Vec::new(); - // Note we don't subscribe directly in the visit closure to avoid a deadlock when pointing to self - this_subscribers.visit(|subscriber| this_subscribers_vec.push(*subscriber)); - let other_subscribers = other.subscribers(); - for subscriber in this_subscribers_vec { - subscriber.subscribe(other_subscribers.clone()); + if self.forwarding == other.forwarding { + return Ok(()); } - self.value.point_to(other.value)?; + + let forwarding = match self.forwarding.try_peek_unchecked() { + Ok(forwarding) => forwarding, + Err(_) => return Ok(()), + }; + + let new_value = other.value; + let new_source_subscribers = new_value.try_peek_unchecked()?.subscribers(); + + // Keep `other` usable; rsx clones can retarget multiple props from it. + self.value.point_to(new_value)?; + + forwarding.retarget_source(new_source_subscribers); Ok(()) } @@ -57,13 +179,8 @@ impl> ReadSignal { /// This is only used by the `props` macro. /// Mark any readers of the signal as dirty pub fn mark_dirty(&mut self) { - let subscribers = self.subscribers(); - let mut this_subscribers_vec = Vec::new(); - subscribers.visit(|subscriber| this_subscribers_vec.push(*subscriber)); - for subscriber in this_subscribers_vec { - subscribers.remove(&subscriber); - subscriber.mark_dirty(); - } + let forwarding = self.forwarding.try_peek_unchecked().unwrap(); + forwarding.mark_dirty(); } } @@ -77,7 +194,7 @@ impl> Copy for ReadSignal {} impl> PartialEq for ReadSignal { fn eq(&self, other: &Self) -> bool { - self.value == other.value + self.forwarding == other.forwarding } } @@ -131,7 +248,12 @@ impl> Readable for ReadSignal { where T: 'static, { - self.value.try_peek_unchecked()?.try_read_unchecked() + let forwarding = self.forwarding.try_peek_unchecked()?; + let wrapped = self.value.try_peek_unchecked()?; + if let Some(reactive_context) = ReactiveContext::current() { + reactive_context.subscribe(forwarding.subscribers()); + } + forwarding.run_in(|| wrapped.try_read_unchecked()) } #[track_caller] @@ -146,7 +268,8 @@ impl> Readable for ReadSignal { where T: 'static, { - self.value.try_peek_unchecked().unwrap().subscribers() + let forwarding = self.forwarding.try_peek_unchecked().unwrap(); + forwarding.subscribers() } } @@ -386,10 +509,7 @@ impl> Writable for WriteSignal { where T: 'static, { - self.value - .try_peek_unchecked() - .unwrap() - .try_write_unchecked() + self.value.try_peek_unchecked()?.try_write_unchecked() } } @@ -442,6 +562,7 @@ where pub trait BoxedSignalStorage: Storage>> + Storage>> + + Storage + sealed::Sealed + 'static { diff --git a/packages/signals/tests/memo.rs b/packages/signals/tests/memo.rs index ed0fa8eac2..df6bdd6c09 100644 --- a/packages/signals/tests/memo.rs +++ b/packages/signals/tests/memo.rs @@ -178,3 +178,32 @@ fn memos_sync_rerun_after_unrelated_write() { dom.render_immediate(&mut NoOpMutations); assert!(PASSED.load(Ordering::SeqCst)); } + +// Boxed ReadSignal wrappers should read through Memo::try_read_unchecked. +#[test] +fn boxed_memo_reads_recompute_dirty_memos() { + let boxed_value_after_write = Rc::new(RefCell::new(None)); + let mut dom = VirtualDom::new_with_props( + { + let boxed_value_after_write = boxed_value_after_write.clone(); + move |boxed_value_after_write: Rc>>| { + let mut signal = use_signal(|| 0); + #[allow(clippy::redundant_closure)] + let memo = use_memo(move || signal()); + let boxed = ReadSignal::from(memo); + + assert_eq!(boxed(), 0); + signal.set(1); + *boxed_value_after_write.borrow_mut() = Some(boxed()); + + rsx! { + div {} + } + } + }, + boxed_value_after_write.clone(), + ); + + dom.rebuild_in_place(); + assert_eq!(*boxed_value_after_write.borrow(), Some(1)); +} diff --git a/packages/signals/tests/subscribe.rs b/packages/signals/tests/subscribe.rs index ac315f7651..37c227784b 100644 --- a/packages/signals/tests/subscribe.rs +++ b/packages/signals/tests/subscribe.rs @@ -1,14 +1,38 @@ #![allow(unused, non_upper_case_globals, non_snake_case)] -use dioxus_core::{NoOpMutations, current_scope_id, generation}; +use dioxus_core::{NoOpMutations, ReactiveContext, RuntimeGuard, current_scope_id, generation}; use std::collections::HashMap; use std::rc::Rc; +use std::sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, +}; use dioxus::prelude::*; use dioxus_core::ElementId; use dioxus_signals::*; use std::cell::RefCell; +fn flush(dom: &mut VirtualDom) { + dom.render_immediate(&mut NoOpMutations); + dom.render_immediate(&mut NoOpMutations); +} + +fn rerender_app(dom: &mut VirtualDom) { + dom.mark_dirty(ScopeId::APP); + flush(dom); +} + +fn dirty_counter_context(dirty_count: Arc) -> ReactiveContext { + ReactiveContext::new_with_callback( + move || { + dirty_count.fetch_add(1, Ordering::SeqCst); + }, + current_scope_id(), + std::panic::Location::caller(), + ) +} + #[test] fn reading_subscribes() { tracing_subscriber::fmt::init(); @@ -80,9 +104,7 @@ fn reading_subscribes() { } } - dom.mark_dirty(ScopeId::APP); - dom.render_immediate(&mut NoOpMutations); - dom.render_immediate(&mut NoOpMutations); + rerender_app(&mut dom); { let current_counter = counter.borrow(); @@ -93,3 +115,828 @@ fn reading_subscribes() { } } } + +#[test] +fn read_signal_point_to_moves_only_read_subscribers() { + #[derive(Default)] + struct RunCounter { + parent_effect_runs: usize, + child_renders: usize, + } + + #[derive(Default)] + struct Handles { + use_b: Option>, + signal_b: Option>, + } + + let run_counter = Rc::new(RefCell::new(RunCounter::default())); + let handles = Rc::new(RefCell::new(Handles::default())); + + let mut dom = VirtualDom::new_with_props( + { + let handles = handles.clone(); + move |counter: Rc>| { + let counter = counter.clone(); + let effect_counter = counter.clone(); + let mut use_b = use_signal(|| false); + let signal_a = use_signal(|| 0); + let mut signal_b = use_signal(|| 0); + + { + let mut slots = handles.borrow_mut(); + slots.use_b = Some(use_b); + slots.signal_b = Some(signal_b); + } + + use_effect(move || { + signal_a(); + effect_counter.borrow_mut().parent_effect_runs += 1; + }); + + let child_signal = if use_b() { signal_b } else { signal_a }; + + rsx! { + Child { + sig: child_signal, + counts: counter + } + } + } + }, + run_counter.clone(), + ); + + #[derive(Props, Clone)] + struct ChildProps { + sig: ReadSignal, + counts: Rc>, + } + + impl PartialEq for ChildProps { + fn eq(&self, other: &Self) -> bool { + self.sig == other.sig + } + } + + fn Child(props: ChildProps) -> Element { + let mut counts = props.counts.borrow_mut(); + counts.child_renders += 1; + let _value = (props.sig)(); + rsx! { + "{props.sig}" + } + } + + dom.rebuild_in_place(); + dom.render_immediate(&mut NoOpMutations); + + { + let current_counter = run_counter.borrow(); + assert_eq!(current_counter.parent_effect_runs, 1); + assert_eq!(current_counter.child_renders, 1); + } + + let mut use_b = handles.borrow().use_b.unwrap(); + use_b.set(true); + rerender_app(&mut dom); + + { + let current_counter = run_counter.borrow(); + assert_eq!(current_counter.parent_effect_runs, 1); + assert_eq!(current_counter.child_renders, 1); + } + + let mut signal_b = handles.borrow().signal_b.unwrap(); + signal_b.set(1); + rerender_app(&mut dom); + + { + let current_counter = run_counter.borrow(); + assert_eq!(current_counter.parent_effect_runs, 1); + assert_eq!(current_counter.child_renders, 2); + } +} + +#[test] +fn read_signal_point_to_leaves_direct_underlying_subscribers() { + #[derive(Default)] + struct RunCounter { + direct_effect_runs: usize, + child_renders: usize, + } + + #[derive(Default)] + struct Handles { + use_b: Option>, + signal_a: Option>, + signal_b: Option>, + } + + let run_counter = Rc::new(RefCell::new(RunCounter::default())); + let handles = Rc::new(RefCell::new(Handles::default())); + + let mut dom = VirtualDom::new_with_props( + { + let handles = handles.clone(); + move |counter: Rc>| { + let counter = counter.clone(); + let effect_counter = counter.clone(); + let mut use_b = use_signal(|| false); + let mut signal_a = use_signal(|| 0); + let mut signal_b = use_signal(|| 0); + + { + let mut slots = handles.borrow_mut(); + slots.use_b = Some(use_b); + slots.signal_a = Some(signal_a); + slots.signal_b = Some(signal_b); + } + + use_effect(move || { + signal_a(); + effect_counter.borrow_mut().direct_effect_runs += 1; + }); + + let child_signal = if use_b() { signal_b } else { signal_a }; + + rsx! { + Child { + sig: ReadSignal::from(child_signal), + counts: counter + } + } + } + }, + run_counter.clone(), + ); + + #[derive(Props, Clone)] + struct ChildProps { + sig: ReadSignal, + counts: Rc>, + } + + impl PartialEq for ChildProps { + fn eq(&self, other: &Self) -> bool { + self.sig == other.sig + } + } + + fn Child(props: ChildProps) -> Element { + props.counts.borrow_mut().child_renders += 1; + let _ = (props.sig)(); + rsx! { "{props.sig}" } + } + + dom.rebuild_in_place(); + dom.render_immediate(&mut NoOpMutations); + + let mut use_b = handles.borrow().use_b.unwrap(); + use_b.set(true); + rerender_app(&mut dom); + + let mut signal_a = handles.borrow().signal_a.unwrap(); + signal_a.set(1); + rerender_app(&mut dom); + + { + let current_counter = run_counter.borrow(); + assert_eq!(current_counter.direct_effect_runs, 2); + assert_eq!(current_counter.child_renders, 1); + } + + let mut signal_b = handles.borrow().signal_b.unwrap(); + signal_b.set(1); + rerender_app(&mut dom); + + { + let current_counter = run_counter.borrow(); + assert_eq!(current_counter.direct_effect_runs, 2); + assert_eq!(current_counter.child_renders, 2); + } +} + +#[test] +fn read_signal_point_to_preserves_overlapping_direct_source_subscription() { + let dirty_count = Arc::new(AtomicUsize::new(0)); + + let mut dom = VirtualDom::new_with_props( + |dirty_count: Arc| { + let mut signal_a = use_signal(|| 0); + let signal_b = use_signal(|| 0); + let target = use_hook(|| ReadSignal::from(signal_a)); + let replacement = ReadSignal::from(signal_b); + let dirty_count = dirty_count.clone(); + let context = ReactiveContext::new_with_callback( + move || { + dirty_count.fetch_add(1, Ordering::SeqCst); + }, + current_scope_id(), + std::panic::Location::caller(), + ); + + context.run_in(|| { + assert_eq!(signal_a(), 0); + assert_eq!(target(), 0); + }); + target.point_to(replacement).unwrap(); + signal_a.set(1); + + rsx! { "" } + }, + dirty_count.clone(), + ); + + dom.rebuild_in_place(); + + assert_eq!(dirty_count.load(Ordering::SeqCst), 1); +} + +#[test] +fn read_signal_point_to_migrated_subscribers_can_unsubscribe() { + type Props = ( + Rc, ReactiveContext)>>>, + Arc, + ); + + let handles = Rc::new(RefCell::new(None)); + let dirty_count = Arc::new(AtomicUsize::new(0)); + + let mut dom = VirtualDom::new_with_props( + |(handles, dirty_count): Props| { + let signal_a = use_signal(|| 0); + let signal_b = use_signal(|| 0); + let target = use_hook(|| ReadSignal::from(signal_a)); + let replacement = ReadSignal::from(signal_b); + let context = use_hook({ + let dirty_count = dirty_count.clone(); + move || { + ReactiveContext::new_with_callback( + { + let dirty_count = dirty_count.clone(); + move || { + dirty_count.fetch_add(1, Ordering::SeqCst); + } + }, + current_scope_id(), + std::panic::Location::caller(), + ) + } + }); + + context.run_in(|| assert_eq!(target(), 0)); + target.point_to(replacement).unwrap(); + *handles.borrow_mut() = Some((signal_b, context)); + + rsx! { "" } + }, + (handles.clone(), dirty_count.clone()), + ); + + dom.rebuild_in_place(); + assert_eq!(dirty_count.load(Ordering::SeqCst), 0); + + let (mut signal_b, context) = handles.borrow().unwrap(); + context.reset_and_run_in(|| {}); + signal_b.set(1); + + assert_eq!(dirty_count.load(Ordering::SeqCst), 0); +} + +#[test] +fn boxed_read_signal_subscribes_to_underlying_updates() { + type Props = (Rc>, Rc>>>); + + let render_count = Rc::new(RefCell::new(0usize)); + let signal_handle = Rc::new(RefCell::new(None)); + + let mut dom = VirtualDom::new_with_props( + |(render_count, signal_handle): Props| { + let mut signal = use_signal(|| 0); + *signal_handle.borrow_mut() = Some(signal); + + let boxed = ReadSignal::from(signal); + let _ = boxed(); + *render_count.borrow_mut() += 1; + + rsx! { "{boxed}" } + }, + (render_count.clone(), signal_handle.clone()), + ); + + dom.rebuild_in_place(); + + { + let current_render_count = render_count.borrow(); + assert_eq!(*current_render_count, 1); + } + + let mut signal = signal_handle.borrow().unwrap(); + signal.set(1); + flush(&mut dom); + + { + let current_render_count = render_count.borrow(); + assert_eq!(*current_render_count, 2); + } +} + +#[test] +fn boxed_read_signal_subscribers_bridge_before_tracked_read() { + type Props = (Arc, Rc>>>); + + let dirty_count = Arc::new(AtomicUsize::new(0)); + let signal_handle = Rc::new(RefCell::new(None)); + + let mut dom = VirtualDom::new_with_props( + |(dirty_count, signal_handle): Props| { + let signal = use_signal(|| 0); + *signal_handle.borrow_mut() = Some(signal); + + let boxed = ReadSignal::from(signal); + let _context = use_hook({ + let dirty_count = dirty_count.clone(); + move || { + let context = ReactiveContext::new_with_callback( + { + let dirty_count = dirty_count.clone(); + move || { + dirty_count.fetch_add(1, Ordering::SeqCst); + } + }, + current_scope_id(), + std::panic::Location::caller(), + ); + context.subscribe(boxed.subscribers()); + context + } + }); + + rsx! { "" } + }, + (dirty_count.clone(), signal_handle.clone()), + ); + + dom.rebuild_in_place(); + assert_eq!(dirty_count.load(Ordering::SeqCst), 0); + + let mut signal = signal_handle.borrow().unwrap(); + signal.set(1); + + assert_eq!(dirty_count.load(Ordering::SeqCst), 1); +} + +#[test] +fn boxed_read_signal_read_in_context_without_current_scope_does_not_panic() { + type Props = ( + Rc, ReadSignal, ReactiveContext)>>>, + Arc, + ); + + let captured = Rc::new(RefCell::new(None)); + let dirty_count = Arc::new(AtomicUsize::new(0)); + + let mut dom = VirtualDom::new_with_props( + |(captured, dirty_count): Props| { + let signal = use_signal(|| 0); + let boxed = ReadSignal::from(signal); + let context = dirty_counter_context(dirty_count.clone()); + + *captured.borrow_mut() = Some((signal, boxed, context)); + + rsx! { "" } + }, + (captured.clone(), dirty_count.clone()), + ); + + dom.rebuild_in_place(); + + let (mut signal, boxed, context) = captured.borrow().unwrap(); + { + let _runtime = RuntimeGuard::new(dom.runtime()); + context.run_in(|| assert_eq!(boxed(), 0)); + } + + signal.set(1); + + assert_eq!(dirty_count.load(Ordering::SeqCst), 1); +} + +#[test] +fn boxed_read_signal_custom_context_stays_subscribed_after_forwarded_write() { + type Props = ( + Rc, ReadSignal, ReactiveContext)>>>, + Arc, + ); + + let captured = Rc::new(RefCell::new(None)); + let dirty_count = Arc::new(AtomicUsize::new(0)); + + let mut dom = VirtualDom::new_with_props( + |(captured, dirty_count): Props| { + let signal = use_signal(|| 0); + let boxed = ReadSignal::from(signal); + let context = ReactiveContext::new_with_callback( + { + let dirty_count = dirty_count.clone(); + move || { + dirty_count.fetch_add(1, Ordering::SeqCst); + } + }, + current_scope_id(), + std::panic::Location::caller(), + ); + + *captured.borrow_mut() = Some((signal, boxed, context)); + + rsx! { "" } + }, + (captured.clone(), dirty_count.clone()), + ); + + dom.rebuild_in_place(); + + let (mut signal, boxed, context) = captured.borrow().unwrap(); + { + let _runtime = RuntimeGuard::new(dom.runtime()); + context.run_in(|| assert_eq!(boxed(), 0)); + } + + signal.set(1); + assert_eq!(dirty_count.load(Ordering::SeqCst), 1); + + signal.set(2); + assert_eq!( + dirty_count.load(Ordering::SeqCst), + 2, + "boxed ReadSignal wrapper subscribers should survive forwarded writes" + ); +} + +#[test] +fn boxed_read_signal_try_read_in_context_returns_error_after_wrapped_signal_drops() { + type Props = Rc, ReadSignal, ReactiveContext)>>>; + + let captured = Rc::new(RefCell::new(None)); + + let mut dom = VirtualDom::new_with_props( + |captured: Props| { + let signal = use_signal(|| 0); + let boxed = ReadSignal::from(signal); + let context = ReactiveContext::new_with_callback( + || {}, + current_scope_id(), + std::panic::Location::caller(), + ); + + *captured.borrow_mut() = Some((signal, boxed, context)); + + rsx! { "" } + }, + captured.clone(), + ); + + dom.rebuild_in_place(); + + let (signal, boxed, context) = captured.borrow().unwrap(); + signal.manually_drop(); + + let _runtime = RuntimeGuard::new(dom.runtime()); + context.run_in(|| { + assert!(boxed.try_read().is_err()); + }); +} + +// `point_to` must not panic when called on a wrapper that has never been read. +#[test] +fn point_to_on_never_read_wrapper_does_not_panic() { + let captured = Rc::new(RefCell::new(None)); + + let mut dom = VirtualDom::new_with_props( + |captured: Rc>>| { + let signal_a = use_signal(|| 7); + let signal_b = use_signal(|| 42); + + // Build two wrappers without ever reading them. + let target = ReadSignal::from(signal_a); + let replacement = ReadSignal::from(signal_b); + + target.point_to(replacement).unwrap(); + + // Now read; should reflect signal_b's value. + *captured.borrow_mut() = Some(target()); + + rsx! { "{target}" } + }, + captured.clone(), + ); + + dom.rebuild_in_place(); + + assert_eq!(*captured.borrow(), Some(42)); +} + +// `point_to` with a copy of `self` must be a no-op. +#[test] +fn point_to_self_is_noop() { + let captured = Rc::new(RefCell::new(None)); + + let mut dom = VirtualDom::new_with_props( + |captured: Rc>>| { + let signal = use_signal(|| 42); + let target = ReadSignal::from(signal); + + // Self-point must be a no-op after first-read init. + let _ = target(); + + target.point_to(target).unwrap(); + + // Slot must still be valid. + *captured.borrow_mut() = Some(target()); + + rsx! { "{target}" } + }, + captured.clone(), + ); + + dom.rebuild_in_place(); + + assert_eq!(*captured.borrow(), Some(42)); +} + +// The forwarding context must outlive the first reader. +#[test] +fn forwarding_context_survives_first_reader_scope_drop() { + type Props = ( + Rc>, + Rc>>>, + Rc>>>, + ); + + let parent_renders = Rc::new(RefCell::new(0usize)); + let signal_slot: Rc>>> = Rc::new(RefCell::new(None)); + let show_child_slot: Rc>>> = Rc::new(RefCell::new(None)); + + let mut dom = VirtualDom::new_with_props( + |(parent_renders, signal_slot, show_child_slot): Props| { + let show_child = use_signal(|| true); + let signal = use_signal(|| 0); + *signal_slot.borrow_mut() = Some(signal); + *show_child_slot.borrow_mut() = Some(show_child); + + // The child does the wrapper's first read. + let wrapper: ReadSignal = use_hook(|| ReadSignal::from(signal)); + + *parent_renders.borrow_mut() += 1; + + rsx! { + if show_child() { + Child { sig: wrapper } + } else { + "after-child: {wrapper}" + } + } + }, + ( + parent_renders.clone(), + signal_slot.clone(), + show_child_slot.clone(), + ), + ); + + #[derive(Props, Clone, PartialEq)] + struct ChildProps { + sig: ReadSignal, + } + + fn Child(props: ChildProps) -> Element { + // First read of `wrapper` happens here, in the child scope. + let _ = (props.sig)(); + rsx! { "{props.sig}" } + } + + dom.rebuild_in_place(); + let after_initial = *parent_renders.borrow(); + + // Unmount the first reader before reading the wrapper from the parent. + let mut show_child = show_child_slot.borrow().unwrap(); + show_child.set(false); + rerender_app(&mut dom); + let after_unmount = *parent_renders.borrow(); + assert!(after_unmount > after_initial); + + // Parent should rerender if forwarding survived the child unmount. + let mut signal = signal_slot.borrow().unwrap(); + signal.set(7); + flush(&mut dom); + + assert!( + *parent_renders.borrow() > after_unmount, + "parent should re-render on inner write: forwarding context must outlive the first-reader scope" + ); +} + +#[test] +fn point_to_keeps_source_handle_readable() { + let observed = Rc::new(RefCell::new(None)); + + let mut dom = VirtualDom::new_with_props( + |observed: Rc>>| { + let signal_a = use_signal(|| 7); + let signal_b = use_signal(|| 42); + + let target = ReadSignal::from(signal_a); + let source = ReadSignal::from(signal_b); + let source_copy = source; + + target.point_to(source).unwrap(); + + *observed.borrow_mut() = Some((target(), source(), source_copy())); + rsx! { "{target}" } + }, + observed.clone(), + ); + + dom.rebuild_in_place(); + + assert_eq!(*observed.borrow(), Some((42, 42, 42))); +} + +// Sibling `point_to` calls may share the same replacement slot. +#[test] +fn point_to_tolerates_shared_other_slot() { + let observed = Rc::new(RefCell::new(None)); + + let mut dom = VirtualDom::new_with_props( + |observed: Rc>>| { + let signal_a = use_signal(|| 1); + let signal_b = use_signal(|| 99); + + // Two sibling stored props. + let target_a = ReadSignal::from(signal_a); + let target_a2 = ReadSignal::from(signal_a); + + // One shared replacement. + let replacement = ReadSignal::from(signal_b); + + target_a.point_to(replacement).unwrap(); + // The replacement must remain reusable. + target_a2.point_to(replacement).unwrap(); + + *observed.borrow_mut() = Some((target_a(), target_a2(), replacement())); + rsx! { "{target_a}-{target_a2}" } + }, + observed.clone(), + ); + + dom.rebuild_in_place(); + + assert_eq!(*observed.borrow(), Some((99, 99, 99))); +} + +#[test] +fn point_to_retargeting_one_shared_replacement_keeps_other_subscribed() { + type Props = (Rc>>>, Arc); + + let signal_y_handle = Rc::new(RefCell::new(None)); + let dirty_count = Arc::new(AtomicUsize::new(0)); + + let mut dom = VirtualDom::new_with_props( + |(signal_y_handle, dirty_count): Props| { + let signal_x = use_signal(|| 0); + let signal_y = use_signal(|| 10); + let signal_z = use_signal(|| 20); + + let target_a = use_hook(|| ReadSignal::from(signal_x)); + let target_b = use_hook(|| ReadSignal::from(signal_x)); + let replacement_y = ReadSignal::from(signal_y); + let replacement_z = ReadSignal::from(signal_z); + let context_b = use_hook({ + let dirty_count = dirty_count.clone(); + move || dirty_counter_context(dirty_count.clone()) + }); + + target_a.point_to(replacement_y).unwrap(); + target_b.point_to(replacement_y).unwrap(); + context_b.run_in(|| assert_eq!(target_b(), 10)); + + target_a.point_to(replacement_z).unwrap(); + *signal_y_handle.borrow_mut() = Some(signal_y); + + rsx! { "" } + }, + (signal_y_handle.clone(), dirty_count.clone()), + ); + + dom.rebuild_in_place(); + assert_eq!(dirty_count.load(Ordering::SeqCst), 0); + + let mut signal_y = signal_y_handle.borrow().unwrap(); + signal_y.set(11); + + assert_eq!( + dirty_count.load(Ordering::SeqCst), + 1, + "retargeting one wrapper must not detach other wrappers that share the old forwarding context" + ); +} + +#[test] +fn failed_point_to_preserves_existing_wrapper_subscribers() { + #[derive(Default)] + struct Captured { + signal_a: Option>, + show_child: Option>, + target: Option>, + stale_replacement: Option>, + context: Option, + } + + type Props = (Rc>, Arc); + + #[derive(Props, Clone)] + struct ReplacementOwnerProps { + captured: Rc>, + } + + impl PartialEq for ReplacementOwnerProps { + fn eq(&self, other: &Self) -> bool { + Rc::ptr_eq(&self.captured, &other.captured) + } + } + + let captured = Rc::new(RefCell::new(Captured::default())); + let dirty_count = Arc::new(AtomicUsize::new(0)); + + let mut dom = VirtualDom::new_with_props( + |(captured, dirty_count): Props| { + let signal_a = use_signal(|| 0); + let show_child = use_signal(|| true); + let target = use_hook(|| ReadSignal::from(signal_a)); + let context = use_hook({ + let dirty_count = dirty_count.clone(); + move || { + ReactiveContext::new_with_callback( + { + let dirty_count = dirty_count.clone(); + move || { + dirty_count.fetch_add(1, Ordering::SeqCst); + } + }, + current_scope_id(), + std::panic::Location::caller(), + ) + } + }); + + context.run_in(|| assert_eq!(target(), 0)); + + { + let mut captured = captured.borrow_mut(); + captured.signal_a = Some(signal_a); + captured.show_child = Some(show_child); + captured.target = Some(target); + captured.context = Some(context); + } + + rsx! { + if show_child() { + ReplacementOwner { captured: captured.clone() } + } + } + }, + (captured.clone(), dirty_count.clone()), + ); + + fn ReplacementOwner(props: ReplacementOwnerProps) -> Element { + let signal_b = use_signal(|| 1); + props.captured.borrow_mut().stale_replacement = Some(ReadSignal::from(signal_b)); + rsx! { "" } + } + + dom.rebuild_in_place(); + assert_eq!(dirty_count.load(Ordering::SeqCst), 0); + + let mut show_child = captured.borrow().show_child.unwrap(); + show_child.set(false); + rerender_app(&mut dom); + + let (mut signal_a, target, stale_replacement) = { + let captured = captured.borrow(); + ( + captured.signal_a.unwrap(), + captured.target.unwrap(), + captured.stale_replacement.unwrap(), + ) + }; + + assert!(target.point_to(stale_replacement).is_err()); + + signal_a.set(1); + assert_eq!( + dirty_count.load(Ordering::SeqCst), + 1, + "failed point_to should leave the old wrapper subscriptions attached" + ); +} diff --git a/packages/stores/tests/store.rs b/packages/stores/tests/store.rs index 75e390859b..b7fa8544db 100644 --- a/packages/stores/tests/store.rs +++ b/packages/stores/tests/store.rs @@ -174,8 +174,11 @@ fn deep_root_reader_sees_future_child_write() { assert_eq!(*runs.borrow(), 2); } +// `ReadSignal::subscribers` returns the wrapper's own subscriber set, so this +// deep-subscription test exercises the underlying child signal directly instead of going +// through a boxed wrapper. #[test] -fn boxed_child_subscribers_remove_visited_parent_deep_subscriber() { +fn child_store_subscribers_remove_visited_parent_deep_subscriber() { fn default_x() -> X { X::default() } @@ -193,8 +196,7 @@ fn boxed_child_subscribers_remove_visited_parent_deep_subscriber() { dom.rebuild_in_place(); dom.in_scope(ScopeId::APP, || { - let boxed_child = ReadSignal::new(STORE.resolve().inner1()); - let subscribers = boxed_child.subscribers(); + let subscribers = STORE.resolve().inner1().subscribers(); let mut visited = Vec::new(); subscribers.visit(|subscriber| visited.push(*subscriber)); @@ -211,7 +213,7 @@ fn boxed_child_subscribers_remove_visited_parent_deep_subscriber() { subscribers.visit(|subscriber| after_remove.push(*subscriber)); assert!( after_remove.is_empty(), - "removing through boxed child subscribers should remove visited subscribers, got {} left", + "removing through child subscribers should remove visited subscribers, got {} left", after_remove.len() ); });