From 16ff0a91c80abf8b40ce8f23e3748c73c09b37ed Mon Sep 17 00:00:00 2001 From: Luke Chu <37006668+lukechu10@users.noreply.github.com> Date: Mon, 29 Nov 2021 21:08:35 -0800 Subject: [PATCH] Better debugging for `ReactiveScope`s (#307) * Store ReactiveScope creation Location * DebugScopeHierarchy * Add track_caller attributes * Add more track_callers * Attach all attributes except doc comments to function instead of struct --- packages/sycamore-macro/src/component/mod.rs | 17 +++- packages/sycamore-macro/src/lib.rs | 2 +- packages/sycamore-reactive/src/context.rs | 27 +++++- packages/sycamore-reactive/src/effect.rs | 98 +++++++++++++++++++- packages/sycamore-reactive/src/lib.rs | 10 +- packages/sycamore/src/context.rs | 1 + 6 files changed, 143 insertions(+), 12 deletions(-) diff --git a/packages/sycamore-macro/src/component/mod.rs b/packages/sycamore-macro/src/component/mod.rs index a1c09c585..6347a100b 100644 --- a/packages/sycamore-macro/src/component/mod.rs +++ b/packages/sycamore-macro/src/component/mod.rs @@ -177,11 +177,23 @@ pub fn component_impl( arg, mut generics, vis, - attrs, + mut attrs, name, return_type, } = component; + let mut doc_attrs = Vec::new(); + let mut i = 0; + while i < attrs.len() { + if attrs[i].path.is_ident("doc") { + // Attribute is a doc attribute. Remove from attrs and add to doc_attrs. + let at = attrs.remove(i); + doc_attrs.push(at); + } else { + i += 1; + } + } + let prop_ty = match &arg { FnArg::Receiver(_) => unreachable!(), FnArg::Typed(pat_ty) => &pat_ty.ty, @@ -226,7 +238,7 @@ pub fn component_impl( } let quoted = quote! { - #(#attrs)* + #(#doc_attrs)* #vis struct #component_name#generics { #[doc(hidden)] _marker: ::std::marker::PhantomData<(#phantom_generics)>, @@ -239,6 +251,7 @@ pub fn component_impl( const NAME: &'static ::std::primitive::str = #component_name_str; type Props = #prop_ty; + #(#attrs)* fn create_component(#arg) -> #return_type{ #block } diff --git a/packages/sycamore-macro/src/lib.rs b/packages/sycamore-macro/src/lib.rs index 50d46f87f..872be7855 100644 --- a/packages/sycamore-macro/src/lib.rs +++ b/packages/sycamore-macro/src/lib.rs @@ -7,7 +7,7 @@ mod view; /// A macro for ergonomically creating complex UI structures. /// /// To learn more about the template syntax, see the chapter on -/// [the `view!` macro](https://sycamore-rs.netlify.app/docs/basics/template) in the Sycamore Book. +/// [the `view!` macro](https://sycamore-rs.netlify.app/docs/basics/view) in the Sycamore Book. #[proc_macro] pub fn view(component: TokenStream) -> TokenStream { let component = parse_macro_input!(component as view::HtmlRoot); diff --git a/packages/sycamore-reactive/src/context.rs b/packages/sycamore-reactive/src/context.rs index 51eaada14..d15ec59b9 100644 --- a/packages/sycamore-reactive/src/context.rs +++ b/packages/sycamore-reactive/src/context.rs @@ -16,11 +16,17 @@ pub(super) trait ContextAny { /// Get the value stored in the context. The concrete type of the returned value is guaranteed /// to match the type when calling [`get_type_id`](ContextAny::get_type_id). fn get_value(&self) -> &dyn Any; + + /// Get the name of type of context or `None` if not available. + fn get_type_name(&self) -> Option<&'static str>; } /// Inner representation of a context. struct Context { value: T, + /// The type name of the context. Only available in debug mode. + #[cfg(debug_assertions)] + type_name: &'static str, } impl ContextAny for Context { @@ -31,6 +37,13 @@ impl ContextAny for Context { fn get_value(&self) -> &dyn Any { &self.value } + + fn get_type_name(&self) -> Option<&'static str> { + #[cfg(debug_assertions)] + return Some(self.type_name); + #[cfg(not(debug_assertions))] + return None; + } } /// Get the value of a context in the current [`ReactiveScope`] or `None` if not found. @@ -68,11 +81,19 @@ pub fn use_context() -> T { } /// Creates a new [`ReactiveScope`] with a context and runs the supplied callback function. +#[cfg_attr(debug_assertions, track_caller)] pub fn create_context_scope(value: T, f: impl FnOnce() -> Out) -> Out { + // Create a new ReactiveScope. + // We make sure to create the ReactiveScope outside of the closure so that track_caller can do + // its thing. + let scope = ReactiveScope::new(); SCOPES.with(|scopes| { - // Create a new ReactiveScope with a context. - let scope = ReactiveScope::new(); - scope.0.borrow_mut().context = Some(Box::new(Context { value })); + // Attach the context to the scope. + scope.0.borrow_mut().context = Some(Box::new(Context { + value, + #[cfg(debug_assertions)] + type_name: std::any::type_name::(), + })); scopes.borrow_mut().push(scope); let out = f(); let scope = scopes.borrow_mut().pop().unwrap_throw(); diff --git a/packages/sycamore-reactive/src/effect.rs b/packages/sycamore-reactive/src/effect.rs index f6aaaef46..bff7a18f1 100644 --- a/packages/sycamore-reactive/src/effect.rs +++ b/packages/sycamore-reactive/src/effect.rs @@ -1,6 +1,8 @@ use std::cell::RefCell; +use std::fmt::{Debug, Formatter}; use std::future::Future; use std::hash::{Hash, Hasher}; +use std::panic::Location; use std::rc::{Rc, Weak}; use std::{mem, ptr}; @@ -57,7 +59,6 @@ impl Listener { } /// Internal representation for [`ReactiveScope`]. -#[derive(Default)] pub(crate) struct ReactiveScopeInner { /// Effects created in this scope. effects: SmallVec<[Rc>>; REACTIVE_SCOPE_EFFECTS_STACK_CAPACITY]>, @@ -66,6 +67,31 @@ pub(crate) struct ReactiveScopeInner { /// Contexts created in this scope. pub context: Option>, pub parent: ReactiveScopeWeak, + /// The source location where this scope was created. + /// Only available when in debug mode. + #[cfg(debug_assertions)] + pub loc: &'static Location<'static>, +} + +impl ReactiveScopeInner { + #[cfg_attr(debug_assertions, track_caller)] + pub fn new() -> Self { + Self { + effects: SmallVec::new(), + cleanup: Vec::new(), + context: None, + parent: ReactiveScopeWeak::default(), + #[cfg(debug_assertions)] + loc: Location::caller(), + } + } +} + +impl Default for ReactiveScopeInner { + #[cfg_attr(debug_assertions, track_caller)] + fn default() -> Self { + Self::new() + } } /// Owns the effects created in the current reactive scope. @@ -75,15 +101,17 @@ pub(crate) struct ReactiveScopeInner { /// A new [`ReactiveScope`] is usually created with [`create_root`]. A new [`ReactiveScope`] is also /// created when a new effect is created with [`create_effect`] and other reactive utilities that /// call it under the hood. -#[derive(Default)] pub struct ReactiveScope(pub(crate) Rc>); impl ReactiveScope { /// Create a new empty [`ReactiveScope`]. /// /// This should be rarely used and only serve as a placeholder. + #[cfg_attr(debug_assertions, track_caller)] pub fn new() -> Self { - Self::default() + // We call this first to make sure that track_caller can do its thing. + let inner = ReactiveScopeInner::new(); + Self(Rc::new(RefCell::new(inner))) } /// Add an effect that is owned by this [`ReactiveScope`]. @@ -126,6 +154,21 @@ impl ReactiveScope { }); u } + + /// Returns the source code [`Location`] where this [`ReactiveScope`] was created. + pub fn creation_loc(&self) -> Option<&'static Location<'static>> { + #[cfg(debug_assertions)] + return Some(self.0.borrow().loc); + #[cfg(not(debug_assertions))] + return None; + } +} + +impl Default for ReactiveScope { + #[cfg_attr(debug_assertions, track_caller)] + fn default() -> Self { + Self::new() + } } impl Drop for ReactiveScope { @@ -621,6 +664,55 @@ pub fn current_scope() -> Option { }) } +/// A struct that can be debug-printed to view the scope hierarchy at the location it was created. +pub struct DebugScopeHierarchy { + scope: Option>>, + loc: &'static Location<'static>, +} + +/// Returns a [`DebugScopeHierarchy`] which can be printed using [`std::fmt::Debug`] to debug the +/// scope hierarchy at the current level. +#[track_caller] +pub fn debug_scope_hierarchy() -> DebugScopeHierarchy { + let loc = Location::caller(); + SCOPES.with(|scope| DebugScopeHierarchy { + scope: scope.borrow().last().map(|x| x.0.clone()), + loc, + }) +} + +impl Debug for DebugScopeHierarchy { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + writeln!(f, "Reactive scope hierarchy at {}:", self.loc)?; + if let Some(scope) = &self.scope { + let mut s = Some(scope.clone()); + while let Some(x) = s { + // Print scope. + if let Some(loc) = ReactiveScope(x.clone()).creation_loc() { + write!(f, "\tScope created at {}", loc)?; + } else { + write!(f, "\tScope")?; + } + // Print context. + if let Some(context) = &x.borrow().context { + let type_name = context.get_type_name(); + if let Some(type_name) = type_name { + write!(f, " with context (type = {})", type_name)?; + } else { + write!(f, " with context")?; + } + } + writeln!(f)?; + // Set next iteration with scope parent. + s = x.borrow().parent.0.upgrade(); + } + } else { + writeln!(f, "Not inside a reactive scope")?; + } + Ok(()) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/packages/sycamore-reactive/src/lib.rs b/packages/sycamore-reactive/src/lib.rs index c5bfcc97f..667e2b4b8 100644 --- a/packages/sycamore-reactive/src/lib.rs +++ b/packages/sycamore-reactive/src/lib.rs @@ -81,19 +81,23 @@ pub fn create_child_scope_in<'a>( /// ``` /// TODO: deprecate this method in favor of [`create_scope`]. #[must_use = "create_root returns the reactive scope of the effects created inside this scope"] +#[cfg_attr(debug_assertions, track_caller)] pub fn create_root<'a>(callback: impl FnOnce() + 'a) -> ReactiveScope { _create_child_scope_in(None, Box::new(callback)) } /// Internal implementation: use dynamic dispatch to reduce code bloat. +#[cfg_attr(debug_assertions, track_caller)] fn _create_child_scope_in<'a>( parent: Option<&ReactiveScopeWeak>, callback: Box, ) -> ReactiveScope { - SCOPES.with(|scopes| { - // Push new empty scope on the stack. - let scope = ReactiveScope::new(); + // Push new empty scope on the stack. + // We make sure to create the ReactiveScope outside of the closure so that track_caller can do + // its thing. + let scope = ReactiveScope::new(); + SCOPES.with(|scopes| { // If `parent` was specified, use it as the parent of the new scope. Else use the parent of // the scope this function is called in. if let Some(parent) = parent { diff --git a/packages/sycamore/src/context.rs b/packages/sycamore/src/context.rs index 74c16abe5..3e4f0c2d8 100644 --- a/packages/sycamore/src/context.rs +++ b/packages/sycamore/src/context.rs @@ -53,6 +53,7 @@ where /// # } /// ``` #[component(ContextProvider)] +#[cfg_attr(debug_assertions, track_caller)] pub fn context_provider(props: ContextProviderProps) -> View where T: 'static,