diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 80dd279d..f592c076 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -25,6 +25,7 @@ jobs: - name: Checkout repository head uses: actions/checkout@v2 with: + repository: ${{ github.repository }} ref: ${{ github.head_ref }} clean: false # Do not remove benchmark results from base branch diff --git a/examples/motion/src/main.rs b/examples/motion/src/main.rs index c4f148ba..236595cd 100644 --- a/examples/motion/src/main.rs +++ b/examples/motion/src/main.rs @@ -13,8 +13,8 @@ fn CreateRAF(cx: Scope) -> View { view! { cx, div { p { (state.get()) " frames" } - button(on:click=|_| start()) { "Start" } - button(on:click=|_| stop()) { "Stop" } + button(on:click=move |_| start()) { "Start" } + button(on:click=move |_| stop()) { "Stop" } } } } diff --git a/packages/sycamore-reactive/examples/signal-of-signal.rs b/packages/sycamore-reactive/examples/signal-of-signal.rs deleted file mode 100644 index 73641580..00000000 --- a/packages/sycamore-reactive/examples/signal-of-signal.rs +++ /dev/null @@ -1,10 +0,0 @@ -use sycamore_reactive::*; - -fn main() { - create_scope_immediate(|cx| { - let data = create_signal(cx, 123); - dbg!(data.get()); - let signal_ref = create_signal(cx, data); - dbg!(signal_ref.get().get()); - }); -} diff --git a/packages/sycamore-reactive/src/arena.rs b/packages/sycamore-reactive/src/arena.rs index e5c77680..fc7a11d9 100644 --- a/packages/sycamore-reactive/src/arena.rs +++ b/packages/sycamore-reactive/src/arena.rs @@ -23,16 +23,26 @@ impl<'a> ScopeArena<'a> { /// Allocate a value onto the arena. Returns a mutable reference that lasts as long as the arena /// itself. #[allow(clippy::mut_from_ref)] // We return a new reference each time so this is a false-positive. - pub fn alloc(&'a self, value: T) -> &'a mut T { + pub fn alloc(&'a self, value: T) -> &'a mut T { + // SAFETY: We know that the lifetime is `'static` so this is completely safe. + unsafe { self.alloc_non_static(value) } + } + + /// Allocate a value onto the arena. Returns a mutable reference that lasts as long as the arena + /// itself. + /// + /// # Safety + /// + /// See docs for [`create_ref`](crate::create_ref). + #[allow(clippy::mut_from_ref)] // We return a new reference each time so this is a false-positive. + pub unsafe fn alloc_non_static(&'a self, value: T) -> &'a mut T { let boxed = bumpalo::boxed::Box::new_in(value, &self.bump); let ptr = bumpalo::boxed::Box::into_raw(boxed); - unsafe { - // SAFETY: The only place where self.inner.get() is mutably borrowed is right here. - // It is impossible to have two alloc() calls on the same ScopeArena at the same time so - // the mutable reference here is effectively unique. - let inner_exclusive = &mut *self.inner.get(); - inner_exclusive.push(ptr); - }; + // SAFETY: The only place where self.inner.get() is mutably borrowed is right here. + // It is impossible to have two alloc() calls on the same ScopeArena at the same time so + // the mutable reference here is effectively unique. + let inner_exclusive = &mut *self.inner.get(); + inner_exclusive.push(ptr); // SAFETY: the address of the ptr lives as long as 'a because: // - It is allocated on the heap and therefore has a stable address. @@ -40,7 +50,7 @@ impl<'a> ScopeArena<'a> { // dropped. // - The drop code for Scope never reads the allocated value and therefore does not create a // stacked-borrows violation. - unsafe { &mut *ptr } + &mut *ptr } /// Cleanup the resources owned by the [`ScopeArena`]. This is automatically called in [`Drop`]. diff --git a/packages/sycamore-reactive/src/effect.rs b/packages/sycamore-reactive/src/effect.rs index 6b3e6eb3..b02817a2 100644 --- a/packages/sycamore-reactive/src/effect.rs +++ b/packages/sycamore-reactive/src/effect.rs @@ -69,13 +69,13 @@ impl<'a> EffectState<'a> { /// # }); /// ``` pub fn create_effect<'a>(cx: Scope<'a>, f: impl FnMut() + 'a) { - let f = cx.alloc(f); - _create_effect(cx, f) + _create_effect(cx, Box::new(f)) } /// Internal implementation for `create_effect`. Use dynamic dispatch to reduce code-bloat. -fn _create_effect<'a>(cx: Scope<'a>, f: &'a mut (dyn FnMut() + 'a)) { - let effect = &*cx.alloc(RefCell::new(None::>)); +fn _create_effect<'a>(cx: Scope<'a>, mut f: Box<(dyn FnMut() + 'a)>) { + // SAFETY: We do not access the scope in the Drop implementation for EffectState. + let effect = unsafe { create_ref_unsafe(cx, RefCell::new(None::>)) }; let cb = Rc::new(RefCell::new({ move || { EFFECTS.with(|effects| { diff --git a/packages/sycamore-reactive/src/iter.rs b/packages/sycamore-reactive/src/iter.rs index b4255568..3e4ae147 100644 --- a/packages/sycamore-reactive/src/iter.rs +++ b/packages/sycamore-reactive/src/iter.rs @@ -21,7 +21,7 @@ use crate::*; /// * `key_fn` - A closure that returns an _unique_ key to each entry. /// /// _Credits: Based on TypeScript implementation in _ -pub fn map_keyed<'a, T, K, U>( +pub fn map_keyed<'a, T, K, U: 'static>( cx: Scope<'a>, list: &'a ReadSignal>, map_fn: impl for<'child_lifetime> Fn(BoundedScope<'child_lifetime, 'a>, T) -> U + 'a, @@ -194,7 +194,7 @@ where /// * `list` - The list to be mapped. The list must be a [`ReadSignal`] (obtained from a [`Signal`]) /// and therefore reactive. /// * `map_fn` - A closure that maps from the input type to the output type. -pub fn map_indexed<'a, T, U>( +pub fn map_indexed<'a, T, U: 'static>( cx: Scope<'a>, list: &'a ReadSignal>, map_fn: impl for<'child_lifetime> Fn(BoundedScope<'child_lifetime, 'a>, T) -> U + 'a, diff --git a/packages/sycamore-reactive/src/lib.rs b/packages/sycamore-reactive/src/lib.rs index cf114cf1..9c55b294 100644 --- a/packages/sycamore-reactive/src/lib.rs +++ b/packages/sycamore-reactive/src/lib.rs @@ -99,11 +99,6 @@ impl<'a, 'b: 'a> BoundedScope<'a, 'b> { _phantom: PhantomData, } } - - /// Alias for `self.raw.arena.alloc`. - fn alloc(&self, value: T) -> &'a mut T { - self.raw.arena.alloc(value) - } } /// A type-alias for [`BoundedScope`] where both lifetimes are the same. @@ -342,10 +337,43 @@ pub fn create_scope_immediate(f: impl for<'a> FnOnce(Scope<'a>)) { /// let _ = outer.unwrap(); /// # }); /// ``` -pub fn create_ref(cx: Scope, value: T) -> &T { +pub fn create_ref(cx: Scope, value: T) -> &T { cx.raw.arena.alloc(value) } +/// Allocate a new arbitrary value under the current [`Scope`]. +/// The allocated value lasts as long as the scope and cannot be used outside of the scope. +/// +/// # Ref lifetime +/// +/// The lifetime of the returned ref is the same as the [`Scope`]. +/// As such, the reference cannot escape the [`Scope`]. +/// ```compile_fail +/// # use sycamore_reactive::*; +/// # create_scope_immediate(|cx| { +/// let mut outer = None; +/// let disposer = create_child_scope(cx, |cx| { +/// let data = create_ref(cx, 0); +/// let raw: &i32 = &data; +/// outer = Some(raw); +/// // ^^^ +/// }); +/// disposer(); +/// let _ = outer.unwrap(); +/// # }); +/// ``` +/// +/// # Safety +/// +/// The allocated value must not access any value allocated on the scope in its `Drop` +/// implementation. +/// +/// This should almost never happen in the wild, but beware that accessing allocated data in `Drop` +/// might cause an use-after-free because the accessed value might have been dropped already. +pub unsafe fn create_ref_unsafe<'a, T: 'a>(cx: Scope<'a>, value: T) -> &'a T { + cx.raw.arena.alloc_non_static(value) +} + /// Adds a callback that is called when the scope is destroyed. pub fn on_cleanup<'a>(cx: Scope<'a>, f: impl FnOnce() + 'a) { cx.raw.inner.borrow_mut().cleanups.push(Box::new(f)); @@ -511,15 +539,6 @@ mod tests { }); } - #[test] - fn can_store_disposer_in_own_signal() { - create_scope_immediate(|cx| { - let signal = create_signal(cx, None); - let disposer = create_child_scope(cx, |_cx| {}); - signal.set(Some(disposer)); - }); - } - #[test] fn refs_are_dropped_on_dispose() { thread_local! { @@ -561,23 +580,4 @@ mod tests { unsafe { disposer.dispose() }; } - - #[test] - fn access_previous_ref_in_drop() { - struct ReadRefOnDrop<'a> { - r: &'a i32, - expect: i32, - } - impl<'a> Drop for ReadRefOnDrop<'a> { - fn drop(&mut self) { - assert_eq!(*self.r, self.expect); - } - } - - let disposer = create_scope(|cx| { - let r = create_ref(cx, 123); - create_ref(cx, ReadRefOnDrop { r, expect: 123 }); - }); - unsafe { disposer.dispose() }; - } } diff --git a/packages/sycamore-reactive/src/memo.rs b/packages/sycamore-reactive/src/memo.rs index 7edd5817..f4c4017f 100644 --- a/packages/sycamore-reactive/src/memo.rs +++ b/packages/sycamore-reactive/src/memo.rs @@ -45,7 +45,7 @@ use crate::*; /// assert_eq!(*double.get(), 2); /// # }); /// ``` -pub fn create_memo<'a, U: 'a>(cx: Scope<'a>, f: impl FnMut() -> U + 'a) -> &'a ReadSignal { +pub fn create_memo<'a, U: 'static>(cx: Scope<'a>, f: impl FnMut() -> U + 'a) -> &'a ReadSignal { create_selector_with(cx, f, |_, _| false) } @@ -69,7 +69,7 @@ pub fn create_memo<'a, U: 'a>(cx: Scope<'a>, f: impl FnMut() -> U + 'a) -> &'a R /// assert_eq!(*double.get(), 2); /// # }); /// ``` -pub fn create_selector<'a, U: PartialEq + 'a>( +pub fn create_selector<'a, U: PartialEq + 'static>( cx: Scope<'a>, f: impl FnMut() -> U + 'a, ) -> &'a ReadSignal { @@ -85,22 +85,25 @@ pub fn create_selector<'a, U: PartialEq + 'a>( /// /// To use the type's [`PartialEq`] implementation instead of a custom function, use /// [`create_selector`]. -pub fn create_selector_with<'a, U: 'a>( +pub fn create_selector_with<'a, U: 'static>( cx: Scope<'a>, mut f: impl FnMut() -> U + 'a, eq_f: impl Fn(&U, &U) -> bool + 'a, ) -> &'a ReadSignal { - let signal: &Cell>> = create_ref(cx, Cell::new(None)); + let signal: Rc>>> = Rc::new(Cell::new(None)); - create_effect(cx, move || { - let new = f(); - if let Some(signal) = signal.get() { - // Check if new value is different from old value. - if !eq_f(&new, &*signal.get_untracked()) { - signal.set(new) + create_effect(cx, { + let signal = Rc::clone(&signal); + move || { + let new = f(); + if let Some(signal) = signal.get() { + // Check if new value is different from old value. + if !eq_f(&new, &*signal.get_untracked()) { + signal.set(new) + } + } else { + signal.set(Some(create_signal(cx, new))) } - } else { - signal.set(Some(create_signal(cx, new))) } }); @@ -140,7 +143,7 @@ pub fn create_selector_with<'a, U: 'a>( /// assert_eq!(*state.get(), 0); /// # }); /// ``` -pub fn create_reducer<'a, U, Msg>( +pub fn create_reducer<'a, U: 'static, Msg>( cx: Scope<'a>, initial: U, reduce: impl Fn(&U, Msg) -> U + 'a, diff --git a/packages/sycamore-reactive/src/signal.rs b/packages/sycamore-reactive/src/signal.rs index be254b23..90e542dd 100644 --- a/packages/sycamore-reactive/src/signal.rs +++ b/packages/sycamore-reactive/src/signal.rs @@ -164,7 +164,7 @@ impl ReadSignal { /// # }); /// ``` #[must_use] - pub fn map<'a, U>( + pub fn map<'a, U: 'static>( &'a self, cx: Scope<'a>, mut f: impl FnMut(&T) -> U + 'a, @@ -461,7 +461,7 @@ impl<'a, T> AnyReadSignal<'a> for ReadSignal { /// outer = Some(signal); /// }); /// ``` -pub fn create_signal(cx: Scope, value: T) -> &Signal { +pub fn create_signal(cx: Scope, value: T) -> &Signal { let signal = Signal::new(value); create_ref(cx, signal) } @@ -469,7 +469,7 @@ pub fn create_signal(cx: Scope, value: T) -> &Signal { /// Create a new [`Signal`] under the current [`Scope`] but with an initial value wrapped in a /// [`Rc`]. This is useful to avoid having to clone a value that is already wrapped in a [`Rc`] when /// creating a new signal. Otherwise, this is identical to [`create_signal`]. -pub fn create_signal_from_rc(cx: Scope, value: Rc) -> &Signal { +pub fn create_signal_from_rc(cx: Scope, value: Rc) -> &Signal { let signal = Signal(ReadSignal { value: RefCell::new(value), emitter: Default::default(), diff --git a/packages/sycamore-router/src/router.rs b/packages/sycamore-router/src/router.rs index 1c772a4e..dc93318d 100644 --- a/packages/sycamore-router/src/router.rs +++ b/packages/sycamore-router/src/router.rs @@ -211,7 +211,7 @@ where #[component] pub fn Router<'a, G: Html, R, F, I>(cx: Scope<'a>, props: RouterProps<'a, R, F, I, G>) -> View where - R: Route + 'a, + R: Route + 'static, F: FnOnce(Scope<'a>, &'a ReadSignal) -> View + 'a, I: Integration + 'static, { @@ -235,7 +235,7 @@ pub fn RouterBase<'a, G: Html, R, F, I>( props: RouterBaseProps<'a, R, F, I, G>, ) -> View where - R: Route + 'a, + R: Route + 'static, F: FnOnce(Scope<'a>, &'a ReadSignal) -> View + 'a, I: Integration + 'static, { diff --git a/packages/sycamore/src/builder.rs b/packages/sycamore/src/builder.rs index 1e244736..b77bcd59 100644 --- a/packages/sycamore/src/builder.rs +++ b/packages/sycamore/src/builder.rs @@ -397,7 +397,7 @@ impl<'a, G: GenericNode, F: FnOnce(Scope<'a>) -> G + 'a> ElementBuilder<'a, G, F /// .dyn_t(|| name.get().to_string()) /// # .view(cx) } /// ``` - pub fn dyn_t + 'a>( + pub fn dyn_t + 'static>( self, f: impl FnMut() -> S + 'a, ) -> ElementBuilder<'a, G, impl FnOnce(Scope<'a>) -> G + 'a> { diff --git a/packages/sycamore/src/motion.rs b/packages/sycamore/src/motion.rs index 6f492420..b85114c4 100644 --- a/packages/sycamore/src/motion.rs +++ b/packages/sycamore/src/motion.rs @@ -1,12 +1,12 @@ //! Utilities for smooth transitions and animations. -use std::cell::{Cell, RefCell}; +use std::cell::RefCell; use std::rc::Rc; use crate::reactive::*; /// Type returned by [`create_raf`] and [`create_raf_loop`]. -type RafState<'a> = (RcSignal, &'a dyn Fn(), &'a dyn Fn()); +type RafState<'a> = (RcSignal, Rc, Rc); /// Schedule a callback to be called on each animation frame. /// Does nothing if not on `wasm32` target. @@ -18,8 +18,8 @@ type RafState<'a> = (RcSignal, &'a dyn Fn(), &'a dyn Fn()); /// The raf is not started by default. Call the `start` function to initiate the raf. pub fn create_raf<'a>(cx: Scope<'a>, _f: impl FnMut() + 'a) -> RafState<'a> { let running = create_ref(cx, create_rc_signal(false)); - let start: &dyn Fn(); - let stop: &dyn Fn(); + let start: Rc; + let stop: Rc; // Only run on wasm32 architecture. #[cfg(all(target_arch = "wasm32", feature = "web"))] @@ -53,7 +53,7 @@ pub fn create_raf<'a>(cx: Scope<'a>, _f: impl FnMut() + 'a) -> RafState<'a> { } } }))); - start = create_ref(cx, move || { + start = Rc::new(move || { if !*running.get() { running.set(true); web_sys::window() @@ -64,12 +64,12 @@ pub fn create_raf<'a>(cx: Scope<'a>, _f: impl FnMut() + 'a) -> RafState<'a> { .unwrap_throw(); } }); - stop = create_ref(cx, || running.set(false)); + stop = Rc::new(|| running.set(false)); } #[cfg(not(all(target_arch = "wasm32", feature = "web")))] { - start = create_ref(cx, || running.set(true)); - stop = create_ref(cx, || running.set(false)); + start = Rc::new(|| running.set(true)); + stop = Rc::new(|| running.set(false)); } (running.clone(), start, stop) @@ -84,13 +84,16 @@ pub fn create_raf<'a>(cx: Scope<'a>, _f: impl FnMut() + 'a) -> RafState<'a> { /// /// The raf is not started by default. Call the `start` function to initiate the raf. pub fn create_raf_loop<'a>(cx: Scope<'a>, mut f: impl FnMut() -> bool + 'a) -> RafState<'a> { - let stop_shared = create_ref(cx, Cell::new(None::<&dyn Fn()>)); - let (running, start, stop) = create_raf(cx, move || { - if !f() { - stop_shared.get().unwrap()(); + let stop_shared = Rc::new(RefCell::new(None::>)); + let (running, start, stop) = create_raf(cx, { + let stop_shared = Rc::clone(&stop_shared); + move || { + if !f() { + stop_shared.borrow().as_ref().unwrap()(); + } } }); - stop_shared.set(Some(stop)); + *stop_shared.borrow_mut() = Some(Rc::clone(&stop)); (running, start, stop) } @@ -101,10 +104,13 @@ pub fn create_tweened_signal<'a, T: Lerp + Clone + 'a>( transition_duration: std::time::Duration, easing_fn: impl Fn(f32) -> f32 + 'static, ) -> &'a Tweened<'a, T> { - create_ref( - cx, - Tweened::new(cx, initial, transition_duration, easing_fn), - ) + // SAFETY: We do not access any referenced data in the Drop implementation of Tweened. + unsafe { + create_ref_unsafe( + cx, + Tweened::new(cx, initial, transition_duration, easing_fn), + ) + } } /// Describes a trait that can be linearly interpolate between two points. diff --git a/packages/sycamore/src/suspense.rs b/packages/sycamore/src/suspense.rs index 5475c435..3e6e4d98 100644 --- a/packages/sycamore/src/suspense.rs +++ b/packages/sycamore/src/suspense.rs @@ -157,7 +157,8 @@ impl<'a> TransitionHandle<'a> { pub fn use_transition(cx: Scope<'_>) -> &TransitionHandle<'_> { let is_pending = create_signal(cx, false); - create_ref(cx, TransitionHandle { cx, is_pending }) + // SAFETY: We do not access any referenced data in the Drop implementation for TransitionHandle. + unsafe { create_ref_unsafe(cx, TransitionHandle { cx, is_pending }) } } #[cfg(all(test, feature = "ssr", not(miri)))] diff --git a/packages/sycamore/tests/web/indexed.rs b/packages/sycamore/tests/web/indexed.rs index 8c8bed18..735dd6a2 100644 --- a/packages/sycamore/tests/web/indexed.rs +++ b/packages/sycamore/tests/web/indexed.rs @@ -270,7 +270,7 @@ fn nested_reactivity() { cx, vec![1, 2, 3] .into_iter() - .map(|x| create_signal(cx, x)) + .map(|x| create_rc_signal(x)) .collect(), ); @@ -293,11 +293,7 @@ fn nested_reactivity() { count.get()[0].set(4); assert_text_content!(p, "423"); - count.set({ - let mut tmp = (*count.get()).clone(); - tmp.push(create_signal(cx, 5)); - tmp - }); + count.modify().push(create_rc_signal(5)); assert_text_content!(p, "4235"); }); } diff --git a/packages/sycamore/tests/web/keyed.rs b/packages/sycamore/tests/web/keyed.rs index c3f3a941..51839a0e 100644 --- a/packages/sycamore/tests/web/keyed.rs +++ b/packages/sycamore/tests/web/keyed.rs @@ -279,7 +279,7 @@ fn nested_reactivity() { cx, vec![1, 2, 3] .into_iter() - .map(|x| create_signal(cx, x)) + .map(|x| create_rc_signal(x)) .collect(), ); @@ -303,11 +303,7 @@ fn nested_reactivity() { count.get()[0].set(4); assert_text_content!(p, "423"); - count.set({ - let mut tmp = (*count.get()).clone(); - tmp.push(create_signal(cx, 5)); - tmp - }); + count.modify().push(create_rc_signal(5)); assert_text_content!(p, "4235"); }); }