From 052cd018b092a9ecb0c5b8c2f74a50b3dd8cb16c Mon Sep 17 00:00:00 2001 From: Luke Chu <37006668+lukechu10@users.noreply.github.com> Date: Thu, 14 Oct 2021 17:28:32 -0700 Subject: [PATCH 1/7] Add ReactiveScope::extend --- .../sycamore-macro/src/template/element.rs | 2 +- packages/sycamore-reactive/src/effect.rs | 102 +++++++++++++++--- 2 files changed, 86 insertions(+), 18 deletions(-) diff --git a/packages/sycamore-macro/src/template/element.rs b/packages/sycamore-macro/src/template/element.rs index ae71116ac..a7a0abb25 100644 --- a/packages/sycamore-macro/src/template/element.rs +++ b/packages/sycamore-macro/src/template/element.rs @@ -47,7 +47,7 @@ impl ToTokens for Element { attributes, children, } = self; - + let tag = tag_name.to_string(); let mut quoted = quote! { let __el = ::sycamore::generic_node::GenericNode::element(#tag); diff --git a/packages/sycamore-reactive/src/effect.rs b/packages/sycamore-reactive/src/effect.rs index 374daffe4..5ce148776 100644 --- a/packages/sycamore-reactive/src/effect.rs +++ b/packages/sycamore-reactive/src/effect.rs @@ -99,26 +99,35 @@ impl ReactiveScope { pub(crate) fn downgrade(&self) -> ReactiveScopeWeak { ReactiveScopeWeak(Rc::downgrade(&self.0)) } + + /// Runs the passed callback in the reactive scope pointed to by this handle. + pub fn extend(&self, f: impl FnOnce() -> U) -> U { + SCOPES.with(|scopes| { + scopes.borrow_mut().push(ReactiveScope(self.0.clone())); // We now have 2 references to the scope. + let u = f(); + scopes.borrow_mut().pop().unwrap(); // Rationale: pop the scope we pushed above. + // Since we have 2 references to the scope, this will not drop the scope. + u + }) + } } impl Drop for ReactiveScope { fn drop(&mut self) { - debug_assert_eq!( - Rc::strong_count(&self.0), - 1, - "should only have 1 strong link to ReactiveScopeInner" - ); - - for effect in &self.0.borrow().effects { - effect - .borrow_mut() - .as_mut() - .unwrap_throw() - .clear_dependencies(); - } + if Rc::strong_count(&self.0) == 1 { + // This is the last reference to the scope. Drop all effects and call the cleanup + // callbacks. + for effect in &self.0.borrow().effects { + effect + .borrow_mut() + .as_mut() + .unwrap_throw() + .clear_dependencies(); + } - for cleanup in mem::take(&mut self.0.borrow_mut().cleanup) { - untrack(cleanup); + for cleanup in mem::take(&mut self.0.borrow_mut().cleanup) { + untrack(cleanup); + } } } } @@ -127,9 +136,32 @@ impl Drop for ReactiveScope { /// [`ReactiveScope::downgrade`]. /// /// There can only ever be one strong reference (it is impossible to clone a [`ReactiveScope`]). -/// However, there can be multiple weak references to the same [`ReactiveScope`]. +/// However, there can be multiple weak references to the same [`ReactiveScope`]. As such, it is +/// impossible to obtain a [`ReactiveScope`] from a [`ReactiveScopeWeak`] because that would allow +/// creating multiple [`ReactiveScope`]s. #[derive(Default)] -pub(crate) struct ReactiveScopeWeak(pub Weak>); +pub struct ReactiveScopeWeak(pub(crate) Weak>); + +impl ReactiveScopeWeak { + /// Runs the passed callback in the reactive scope pointed to by this handle. + /// + /// If the scope has already been destroyed, the callback is not run and `None` is returned. + pub fn extend(&self, f: impl FnOnce() -> U) -> Option { + // We only upgrade this temporarily for the duration of this + // function call. + if let Some(this) = self.0.upgrade() { + SCOPES.with(|scopes| { + scopes.borrow_mut().push(ReactiveScope(this)); // We now have 2 references to the scope. + let u = f(); + scopes.borrow_mut().pop().unwrap(); // Rationale: pop the scope we pushed above. + // Since we have 2 references to the scope, this will not drop the scope. + Some(u) + }) + } else { + None + } + } +} pub(super) type CallbackPtr = *const RefCell; @@ -540,6 +572,17 @@ pub fn dependency_count() -> Option { }) } +/// Returns a [`ReactiveScopeWeak`] handle to the current reactive scope or `None` if outside of a +/// reactive scope. +pub fn current_scope() -> Option { + SCOPES.with(|scope| { + scope + .borrow() + .last() + .map(|last_context| last_context.downgrade()) + }) +} + #[cfg(test)] mod tests { use super::*; @@ -959,4 +1002,29 @@ mod tests { trigger.set(()); assert_eq!(*counter.get(), 1); } + + #[test] + fn cleanup_in_extended_scope() { + let counter = Signal::new(0); + + let root = create_root(cloned!((counter) => move || { + on_cleanup(cloned!((counter) => move || { + counter.set(*counter.get_untracked() + 1); + })); + })); + + assert_eq!(*counter.get(), 0); + + // Extend the root and add a new on_cleanup callback that increments counter. + root.extend(cloned!((counter) => move || { + on_cleanup(cloned!((counter) => move || { + counter.set(*counter.get_untracked() + 1); + })); + })); + + assert_eq!(*counter.get(), 0); + + drop(root); + assert_eq!(*counter.get(), 2); + } } From 933d5cf2a82649deffa00b243c86c2d1d8ad8f8a Mon Sep 17 00:00:00 2001 From: Luke Chu <37006668+lukechu10@users.noreply.github.com> Date: Thu, 14 Oct 2021 20:41:50 -0700 Subject: [PATCH 2/7] spawn_local_in_scope --- packages/sycamore-reactive/src/effect.rs | 35 ++++++++++++++++++++ packages/sycamore/Cargo.toml | 2 ++ packages/sycamore/src/futures.rs | 41 ++++++++++++++++++++++++ packages/sycamore/src/lib.rs | 5 +++ website/Cargo.toml | 7 ++-- website/src/main.rs | 4 +-- 6 files changed, 88 insertions(+), 6 deletions(-) create mode 100644 packages/sycamore/src/futures.rs diff --git a/packages/sycamore-reactive/src/effect.rs b/packages/sycamore-reactive/src/effect.rs index 5ce148776..17332f94a 100644 --- a/packages/sycamore-reactive/src/effect.rs +++ b/packages/sycamore-reactive/src/effect.rs @@ -1,4 +1,5 @@ use std::cell::RefCell; +use std::future::Future; use std::hash::{Hash, Hasher}; use std::rc::{Rc, Weak}; use std::{mem, ptr}; @@ -110,6 +111,19 @@ impl ReactiveScope { u }) } + + /// Runs the passed future in the reactive scope pointed to by this handle. + pub async fn extend_future(&self, f: impl Future) -> U { + SCOPES.with(|scopes| { + scopes.borrow_mut().push(ReactiveScope(self.0.clone())); // We now have 2 references to the scope. + }); + let u = f.await; + SCOPES.with(|scopes| { + scopes.borrow_mut().pop().unwrap(); // Rationale: pop the scope we pushed above. + // Since we have 2 references to the scope, this will not drop the scope. + }); + u + } } impl Drop for ReactiveScope { @@ -161,6 +175,27 @@ impl ReactiveScopeWeak { None } } + + /// Runs the passed future in the reactive scope pointed to by this handle. + /// + /// If the scope has already been destroyed, the callback is not run and `None` is returned. + pub async fn extend_future(&self, f: impl Future) -> Option { + // We only upgrade this temporarily for the duration of this + // function call. + if let Some(this) = self.0.upgrade() { + SCOPES.with(|scopes| { + scopes.borrow_mut().push(ReactiveScope(this)); // We now have 2 references to the scope. + }); + let u = f.await; + SCOPES.with(|scopes| { + scopes.borrow_mut().pop().unwrap(); // Rationale: pop the scope we pushed above. + // Since we have 2 references to the scope, this will not drop the scope. + Some(u) + }) + } else { + None + } + } } pub(super) type CallbackPtr = *const RefCell; diff --git a/packages/sycamore/Cargo.toml b/packages/sycamore/Cargo.toml index 62de1b3fa..a9afdb7b1 100644 --- a/packages/sycamore/Cargo.toml +++ b/packages/sycamore/Cargo.toml @@ -23,6 +23,7 @@ smallvec = "1.6.1" sycamore-macro = { path = "../sycamore-macro", version = "=0.6.3" } sycamore-reactive = { path = "../sycamore-reactive", version = "=0.6.3" } wasm-bindgen = { version = "0.2.78", features = ["enable-interning"] } +wasm-bindgen-futures = { version = "0.4.28", optional = true } [dependencies.lexical] version = "6.0.0" @@ -53,6 +54,7 @@ wasm-bindgen-test = "0.3.28" [features] default = ["dom"] dom = [] +futures = ["wasm-bindgen-futures"] ssr = ["html-escape", "once_cell"] serde = ["sycamore-reactive/serde"] diff --git a/packages/sycamore/src/futures.rs b/packages/sycamore/src/futures.rs new file mode 100644 index 000000000..89f6aec79 --- /dev/null +++ b/packages/sycamore/src/futures.rs @@ -0,0 +1,41 @@ +use std::future::Future; + +use sycamore_reactive::current_scope; +use wasm_bindgen_futures::spawn_local; + +/// A wrapper around [`wasm_bindgen_futures::spawn_local`] that extends the current reactive scope +/// that it is called in. +/// +/// If the scope is dropped by the time the future is spawned, the callback will not be called. +/// +/// If not on `wasm32` target arch, this function is a no-op. +/// +/// # Panics +/// This function panics if called outside of a reactive scope. +/// +/// # Example +/// ``` +/// use sycamore::futures::spawn_local_in_reactive_scope; +/// use sycamore::prelude::*; +/// +/// create_root(|| { +/// // Inside reactive scope. +/// spawn_local_in_reactive_scope(|| { +/// // Still inside reactive scope. +/// }); +/// }) +/// ``` +pub fn spawn_local_in_scope(future: F) +where + F: Future + 'static, +{ + if cfg!(target_arch = "wasm32") { + if let Some(scope) = current_scope() { + spawn_local(async move { + scope.extend_future(future).await; + }); + } else { + panic!("spawn_local_in_scope called outside of reactive scope"); + } + } +} diff --git a/packages/sycamore/src/lib.rs b/packages/sycamore/src/lib.rs index 51af82242..482009cbd 100644 --- a/packages/sycamore/src/lib.rs +++ b/packages/sycamore/src/lib.rs @@ -8,6 +8,8 @@ //! ## Features //! - `dom` (_default_) - Enables rendering templates to DOM nodes. Only useful on //! `wasm32-unknown-unknown` target. +//! - `futures` - Enables wrappers around `wasm-bindgen-futures` to make it easier to extend a +//! reactive scope into an `async` function. //! - `ssr` - Enables rendering templates to static strings (useful for Server Side Rendering / //! Pre-rendering). //! - `serde` - Enables serializing and deserializing `Signal`s and other wrapper types using @@ -34,6 +36,9 @@ pub mod portal; pub mod template; pub mod utils; +#[cfg(feature = "futures")] +pub mod futures; + /// Alias self to sycamore for proc-macros. extern crate self as sycamore; diff --git a/website/Cargo.toml b/website/Cargo.toml index a174811c6..ab7d595c9 100644 --- a/website/Cargo.toml +++ b/website/Cargo.toml @@ -15,13 +15,12 @@ pulldown-cmark = "0.8.0" reqwasm = "0.2.1" serde-lite = { version = "0.2.0", features = ["derive"] } serde_json = "1.0.68" -sycamore = {path = "../packages/sycamore"} -sycamore-router = {path = "../packages/sycamore-router"} +sycamore = { path = "../packages/sycamore", features = ["futures"] } +sycamore-router = { path = "../packages/sycamore-router" } wasm-bindgen = "0.2.78" -wasm-bindgen-futures = "0.4.28" [dev-dependencies] -docs = {path = "../docs"} +docs = { path = "../docs" } [dependencies.web-sys] features = ["MediaQueryList", "Storage", "Window"] diff --git a/website/src/main.rs b/website/src/main.rs index 8d91965c3..46873057f 100644 --- a/website/src/main.rs +++ b/website/src/main.rs @@ -10,9 +10,9 @@ use reqwasm::http::Request; use serde_lite::Deserialize; use sidebar::SidebarData; use sycamore::context::{use_context, ContextProvider, ContextProviderProps}; +use sycamore::futures::spawn_local_in_scope; use sycamore::prelude::*; use sycamore_router::{HistoryIntegration, Route, Router, RouterProps}; -use wasm_bindgen_futures::spawn_local; const LATEST_MAJOR_VERSION: &str = "v0.6"; const NEXT_VERSION: &str = "next"; @@ -68,7 +68,7 @@ fn switch(route: StateHandle) -> Template { let cached_sidebar_data: Signal, SidebarData)>> = Signal::new(None); create_effect(cloned!((template) => move || { let route = route.get(); - spawn_local(cloned!((template, cached_sidebar_data) => async move { + spawn_local_in_scope(cloned!((template, cached_sidebar_data) => async move { let t = match route.as_ref() { Routes::Index => template! { div(class="container mx-auto") { From a381549dae0399a302788413e5f75a90f77a71eb Mon Sep 17 00:00:00 2001 From: Luke Chu <37006668+lukechu10@users.noreply.github.com> Date: Thu, 14 Oct 2021 21:00:46 -0700 Subject: [PATCH 3/7] Update documentation --- docs/next/advanced/routing.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/next/advanced/routing.md b/docs/next/advanced/routing.md index 2ebbecc9f..1d6b918ce 100644 --- a/docs/next/advanced/routing.md +++ b/docs/next/advanced/routing.md @@ -242,15 +242,19 @@ When data fetching (e.g. from a REST API) is required to load a page, it is reco the data. This will cause the router to wait until the data is loaded before rendering the page, removing the need for some "Loading..." indicator. +`spawn_local_in_scope` is a simple wrapper around `wasm_bindgen_futures::spawn_local` that extends +the current scope into inside the `async` block. Make sure you enable the `"futures"` feature on +`sycamore`. + ```rust -use wasm_bindgen_futures::spawn_local; +use sycamore::futures::spawn_local_in_scope; template! { Router(RouterProps::new(HistoryIntegration::new(), |route: StateHandle| { let template = Signal::new(Template::empty()); create_effect(cloned!((template) => move || { let route = route.get(); - spawn_local(cloned!((template) => async move { + spawn_local_in_scope(cloned!((template) => async move { let t = match route.as_ref() { AppRoutes::Index => template! { "This is the index page" From 7d9218a49fb37b6bf1176c4ca5e015c8d43a8b4d Mon Sep 17 00:00:00 2001 From: Luke Chu <37006668+lukechu10@users.noreply.github.com> Date: Thu, 14 Oct 2021 21:12:32 -0700 Subject: [PATCH 4/7] Fix doc test --- packages/sycamore/src/futures.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/sycamore/src/futures.rs b/packages/sycamore/src/futures.rs index 89f6aec79..9a41d441c 100644 --- a/packages/sycamore/src/futures.rs +++ b/packages/sycamore/src/futures.rs @@ -15,15 +15,15 @@ use wasm_bindgen_futures::spawn_local; /// /// # Example /// ``` -/// use sycamore::futures::spawn_local_in_reactive_scope; +/// use sycamore::futures::spawn_local_in_scope; /// use sycamore::prelude::*; /// /// create_root(|| { /// // Inside reactive scope. -/// spawn_local_in_reactive_scope(|| { +/// spawn_local_in_scope(|| { /// // Still inside reactive scope. /// }); -/// }) +/// }); /// ``` pub fn spawn_local_in_scope(future: F) where From 3ae61f841d75c001afa86b1fc448d3f39ef8bd47 Mon Sep 17 00:00:00 2001 From: Luke Chu <37006668+lukechu10@users.noreply.github.com> Date: Thu, 14 Oct 2021 21:13:54 -0700 Subject: [PATCH 5/7] Run more tests in Miri --- .github/workflows/test.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 72968e79b..0aee57036 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -129,3 +129,7 @@ jobs: run: | cd packages/sycamore cargo miri test + cd ../sycamore-reactive + cargo miri test + cd ../sycamore-router + cargo miri test From 1e9db8ecb2182654f91312a5b266f7bdeb5c41ce Mon Sep 17 00:00:00 2001 From: Luke Chu <37006668+lukechu10@users.noreply.github.com> Date: Thu, 14 Oct 2021 21:17:19 -0700 Subject: [PATCH 6/7] Do not test router in miri --- .github/workflows/test.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0aee57036..62502605f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -131,5 +131,3 @@ jobs: cargo miri test cd ../sycamore-reactive cargo miri test - cd ../sycamore-router - cargo miri test From d4c5128560d79abf14cbb938de26f10406ea183d Mon Sep 17 00:00:00 2001 From: Luke Chu <37006668+lukechu10@users.noreply.github.com> Date: Thu, 14 Oct 2021 21:18:23 -0700 Subject: [PATCH 7/7] Fix doc test again --- packages/sycamore/src/futures.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sycamore/src/futures.rs b/packages/sycamore/src/futures.rs index 9a41d441c..df1e74b02 100644 --- a/packages/sycamore/src/futures.rs +++ b/packages/sycamore/src/futures.rs @@ -20,7 +20,7 @@ use wasm_bindgen_futures::spawn_local; /// /// create_root(|| { /// // Inside reactive scope. -/// spawn_local_in_scope(|| { +/// spawn_local_in_scope(async { /// // Still inside reactive scope. /// }); /// });