From e794be5d784a9f237a9745aa3f31fd407f2d6356 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Thu, 18 Jul 2024 04:32:33 +0200 Subject: [PATCH] Improve warnings when trying to update outside of pending suspense boundaries on the server (#2575) * Improve warnings when trying to update outside of pending suspense boundaries on the server * remove suspense resolve log --- packages/core/src/any_props.rs | 2 +- packages/core/src/arena.rs | 3 + packages/core/src/diff/component.rs | 77 +++-- packages/core/src/events.rs | 17 +- packages/core/src/runtime.rs | 39 ++- packages/core/src/scope_arena.rs | 78 +++-- packages/core/src/scope_context.rs | 25 +- packages/core/src/suspense/component.rs | 294 +++++++++--------- packages/core/src/suspense/mod.rs | 40 ++- packages/core/src/tasks.rs | 32 +- packages/core/src/virtual_dom.rs | 40 +-- .../fullstack/src/html_storage/serialize.rs | 11 +- packages/fullstack/src/render.rs | 58 ++-- packages/web/src/hydration/hydrate.rs | 6 +- 14 files changed, 409 insertions(+), 313 deletions(-) diff --git a/packages/core/src/any_props.rs b/packages/core/src/any_props.rs index bcb8631db..232c3748a 100644 --- a/packages/core/src/any_props.rs +++ b/packages/core/src/any_props.rs @@ -83,7 +83,7 @@ impl + Clone, P: Clone + 'static, M: 'static> AnyProp Ok(node) => RenderReturn { node }, Err(err) => { let component_name = self.name; - tracing::error!("Error while rendering component `{component_name}`: {err:?}"); + tracing::error!("Panic while rendering component `{component_name}`: {err:?}"); let panic = CapturedPanic { error: err }; RenderReturn { node: Err(panic.into()), diff --git a/packages/core/src/arena.rs b/packages/core/src/arena.rs index 2a4e955ea..b45048589 100644 --- a/packages/core/src/arena.rs +++ b/packages/core/src/arena.rs @@ -85,6 +85,9 @@ impl VirtualDom { }; self.dirty_scopes.remove(&ScopeOrder::new(height, id)); + + // If this scope was a suspense boundary, remove it from the resolved scopes + self.resolved_scopes.retain(|s| s != &id); } } diff --git a/packages/core/src/diff/component.rs b/packages/core/src/diff/component.rs index 6d6e168c7..6a8e77049 100644 --- a/packages/core/src/diff/component.rs +++ b/packages/core/src/diff/component.rs @@ -10,6 +10,7 @@ use crate::{ VComponent, WriteMutations, }, nodes::VNode, + prelude::SuspenseContext, scopes::ScopeId, virtual_dom::VirtualDom, RenderReturn, @@ -22,7 +23,7 @@ impl VirtualDom { scope_id: ScopeId, ) { let scope = &mut self.scopes[scope_id.0]; - if SuspenseBoundaryProps::downcast_mut_from_props(&mut *scope.props).is_some() { + if SuspenseBoundaryProps::downcast_from_props(&mut *scope.props).is_some() { SuspenseBoundaryProps::diff(scope_id, self, to) } else { let new_nodes = self.run_scope(scope_id); @@ -37,29 +38,27 @@ impl VirtualDom { scope: ScopeId, new_nodes: RenderReturn, ) { - // We don't diff the nodes if the scope is suspended or has an error - let Ok(new_real_nodes) = &new_nodes.node else { - return; - }; + self.runtime.clone().with_scope_on_stack(scope, || { + // We don't diff the nodes if the scope is suspended or has an error + let Ok(new_real_nodes) = &new_nodes.node else { + return; + }; + let scope_state = &mut self.scopes[scope.0]; + // Load the old and new rendered nodes + let old = scope_state.last_rendered_node.take().unwrap(); - self.runtime.push_scope(scope); - let scope_state = &mut self.scopes[scope.0]; - // Load the old and new rendered nodes - let old = scope_state.last_rendered_node.take().unwrap(); + // If there are suspended scopes, we need to check if the scope is suspended before we diff it + // If it is suspended, we need to diff it but write the mutations nothing + // Note: It is important that we still diff the scope even if it is suspended, because the scope may render other child components which may change between renders + let mut render_to = to.filter(|_| self.runtime.scope_should_render(scope)); + old.diff_node(new_real_nodes, self, render_to.as_deref_mut()); - // If there are suspended scopes, we need to check if the scope is suspended before we diff it - // If it is suspended, we need to diff it but write the mutations nothing - // Note: It is important that we still diff the scope even if it is suspended, because the scope may render other child components which may change between renders - let mut render_to = to.filter(|_| self.runtime.scope_should_render(scope)); - old.diff_node(new_real_nodes, self, render_to.as_deref_mut()); + self.scopes[scope.0].last_rendered_node = Some(new_nodes); - self.scopes[scope.0].last_rendered_node = Some(new_nodes); - - if render_to.is_some() { - self.runtime.get_state(scope).unwrap().mount(&self.runtime); - } - - self.runtime.pop_scope(); + if render_to.is_some() { + self.runtime.get_state(scope).unwrap().mount(&self.runtime); + } + }) } /// Create a new [`ScopeState`] for a component that has been created with [`VirtualDom::create_scope`] @@ -73,25 +72,24 @@ impl VirtualDom { new_nodes: RenderReturn, parent: Option, ) -> usize { - self.runtime.push_scope(scope); + self.runtime.clone().with_scope_on_stack(scope, || { + // If there are suspended scopes, we need to check if the scope is suspended before we diff it + // If it is suspended, we need to diff it but write the mutations nothing + // Note: It is important that we still diff the scope even if it is suspended, because the scope may render other child components which may change between renders + let mut render_to = to.filter(|_| self.runtime.scope_should_render(scope)); - // If there are suspended scopes, we need to check if the scope is suspended before we diff it - // If it is suspended, we need to diff it but write the mutations nothing - // Note: It is important that we still diff the scope even if it is suspended, because the scope may render other child components which may change between renders - let mut render_to = to.filter(|_| self.runtime.scope_should_render(scope)); + // Create the node + let nodes = new_nodes.create(self, parent, render_to.as_deref_mut()); - // Create the node - let nodes = new_nodes.create(self, parent, render_to.as_deref_mut()); + // Then set the new node as the last rendered node + self.scopes[scope.0].last_rendered_node = Some(new_nodes); - // Then set the new node as the last rendered node - self.scopes[scope.0].last_rendered_node = Some(new_nodes); + if render_to.is_some() { + self.runtime.get_state(scope).unwrap().mount(&self.runtime); + } - if render_to.is_some() { - self.runtime.get_state(scope).unwrap().mount(&self.runtime); - } - - self.runtime.pop_scope(); - nodes + nodes + }) } pub(crate) fn remove_component_node( @@ -102,12 +100,7 @@ impl VirtualDom { replace_with: Option, ) { // If this is a suspense boundary, remove the suspended nodes as well - if let Some(mut suspense) = - SuspenseBoundaryProps::downcast_mut_from_props(&mut *self.scopes[scope_id.0].props) - .cloned() - { - suspense.remove_suspended_nodes::(self, destroy_component_state); - } + SuspenseContext::remove_suspended_nodes::(self, scope_id, destroy_component_state); // Remove the component from the dom if let Some(node) = self.scopes[scope_id.0].last_rendered_node.as_ref() { diff --git a/packages/core/src/events.rs b/packages/core/src/events.rs index 534df70c8..0803ce41b 100644 --- a/packages/core/src/events.rs +++ b/packages/core/src/events.rs @@ -418,13 +418,16 @@ impl Callback { /// This borrows the callback using a RefCell. Recursively calling a callback will cause a panic. pub fn call(&self, arguments: Args) -> Ret { if let Some(callback) = self.callback.read().as_ref() { - Runtime::with(|rt| rt.push_scope(self.origin)); - let value = { - let mut callback = callback.borrow_mut(); - callback(arguments) - }; - Runtime::with(|rt| rt.pop_scope()); - value + Runtime::with(|rt| { + rt.with_scope_on_stack(self.origin, || { + let value = { + let mut callback = callback.borrow_mut(); + callback(arguments) + }; + value + }) + }) + .expect("Callback must be called from within the dioxus runtime") } else { panic!("Callback was manually dropped") } diff --git a/packages/core/src/runtime.rs b/packages/core/src/runtime.rs index d396eabd4..dfe8638ca 100644 --- a/packages/core/src/runtime.rs +++ b/packages/core/src/runtime.rs @@ -22,10 +22,12 @@ pub struct Runtime { pub(crate) scope_states: RefCell>>, // We use this to track the current scope - pub(crate) scope_stack: RefCell>, + // This stack should only be modified through [`Runtime::with_scope_on_stack`] to ensure that the stack is correctly restored + scope_stack: RefCell>, // We use this to track the current suspense location. Generally this lines up with the scope stack, but it may be different for children of a suspense boundary - pub(crate) suspense_stack: RefCell>, + // This stack should only be modified through [`Runtime::with_suspense_location`] to ensure that the stack is correctly restored + suspense_stack: RefCell>, // We use this to track the current task pub(crate) current_task: Cell>, @@ -123,21 +125,46 @@ impl Runtime { o } + /// Get the current suspense location + pub(crate) fn current_suspense_location(&self) -> Option { + self.suspense_stack.borrow().last().cloned() + } + + /// Run a callback a [`SuspenseLocation`] at the top of the stack + pub(crate) fn with_suspense_location( + &self, + suspense_location: SuspenseLocation, + f: impl FnOnce() -> O, + ) -> O { + self.suspense_stack.borrow_mut().push(suspense_location); + let o = f(); + self.suspense_stack.borrow_mut().pop(); + o + } + + /// Run a callback with the current scope at the top of the stack + pub(crate) fn with_scope_on_stack(&self, scope: ScopeId, f: impl FnOnce() -> O) -> O { + self.push_scope(scope); + let o = f(); + self.pop_scope(); + o + } + /// Push a scope onto the stack - pub(crate) fn push_scope(&self, scope: ScopeId) { + fn push_scope(&self, scope: ScopeId) { let suspense_location = self .scope_states .borrow() .get(scope.0) .and_then(|s| s.as_ref()) - .map(|s| s.suspense_boundary()) + .map(|s| s.suspense_location()) .unwrap_or_default(); self.suspense_stack.borrow_mut().push(suspense_location); self.scope_stack.borrow_mut().push(scope); } /// Pop a scope off the stack - pub(crate) fn pop_scope(&self) { + fn pop_scope(&self) { self.scope_stack.borrow_mut().pop(); self.suspense_stack.borrow_mut().pop(); } @@ -200,7 +227,7 @@ impl Runtime { // If this is not a suspended scope, and we are under a frozen context, then we should let scopes = self.scope_states.borrow(); let scope = &scopes[scope_id.0].as_ref().unwrap(); - !matches!(scope.suspense_boundary(), SuspenseLocation::UnderSuspense(suspense) if suspense.suspended()) + !matches!(scope.suspense_location(), SuspenseLocation::UnderSuspense(suspense) if suspense.has_suspended_tasks()) } } diff --git a/packages/core/src/scope_arena.rs b/packages/core/src/scope_arena.rs index 16b544877..80f2c723a 100644 --- a/packages/core/src/scope_arena.rs +++ b/packages/core/src/scope_arena.rs @@ -23,10 +23,7 @@ impl VirtualDom { }; let suspense_boundary = self .runtime - .suspense_stack - .borrow() - .last() - .cloned() + .current_suspense_location() .unwrap_or(SuspenseLocation::NotSuspended); let entry = self.scopes.vacant_entry(); let id = ScopeId(entry.key()); @@ -55,45 +52,46 @@ impl VirtualDom { crate::Runtime::current().is_some(), "Must be in a dioxus runtime" ); - self.runtime.push_scope(scope_id); + self.runtime.clone().with_scope_on_stack(scope_id, || { + let scope = &self.scopes[scope_id.0]; + let output = { + let scope_state = scope.state(); + + scope_state.hook_index.set(0); + + // Run all pre-render hooks + for pre_run in scope_state.before_render.borrow_mut().iter_mut() { + pre_run(); + } + + let props: &dyn AnyProps = &*scope.props; + + let span = tracing::trace_span!("render", scope = %scope.state().name); + span.in_scope(|| { + scope.reactive_context.reset_and_run_in(|| { + let mut render_return = props.render(); + self.handle_element_return( + &mut render_return.node, + scope_id, + &scope.state(), + ); + render_return + }) + }) + }; - let scope = &self.scopes[scope_id.0]; - let output = { let scope_state = scope.state(); - scope_state.hook_index.set(0); - - // Run all pre-render hooks - for pre_run in scope_state.before_render.borrow_mut().iter_mut() { - pre_run(); + // Run all post-render hooks + for post_run in scope_state.after_render.borrow_mut().iter_mut() { + post_run(); } - let props: &dyn AnyProps = &*scope.props; - - let span = tracing::trace_span!("render", scope = %scope.state().name); - span.in_scope(|| { - scope.reactive_context.reset_and_run_in(|| { - let mut render_return = props.render(); - self.handle_element_return(&mut render_return.node, scope_id, &scope.state()); - render_return - }) - }) - }; - - let scope_state = scope.state(); - - // Run all post-render hooks - for post_run in scope_state.after_render.borrow_mut().iter_mut() { - post_run(); - } - - // remove this scope from dirty scopes - self.dirty_scopes - .remove(&ScopeOrder::new(scope_state.height, scope_id)); - - self.runtime.pop_scope(); - - output + // remove this scope from dirty scopes + self.dirty_scopes + .remove(&ScopeOrder::new(scope_state.height, scope_id)); + output + }) } /// Insert any errors, or suspended tasks from an element return into the runtime @@ -101,7 +99,7 @@ impl VirtualDom { match node { Err(RenderError::Aborted(e)) => { tracing::error!( - "Error while rendering component `{}`: {e:?}", + "Error while rendering component `{}`:\n{e}", scope_state.name ); throw_error(e.clone_mounted()); @@ -114,7 +112,7 @@ impl VirtualDom { .runtime .get_state(scope_id) .unwrap() - .suspense_boundary(); + .suspense_location(); let already_suspended = self .runtime .tasks diff --git a/packages/core/src/scope_context.rs b/packages/core/src/scope_context.rs index af0215b28..7589be31c 100644 --- a/packages/core/src/scope_context.rs +++ b/packages/core/src/scope_context.rs @@ -24,8 +24,9 @@ pub(crate) enum ScopeStatus { pub(crate) enum SuspenseLocation { #[default] NotSuspended, - InSuspensePlaceholder(SuspenseContext), + SuspenseBoundary(SuspenseContext), UnderSuspense(SuspenseContext), + InSuspensePlaceholder(SuspenseContext), } impl SuspenseLocation { @@ -33,6 +34,7 @@ impl SuspenseLocation { match self { SuspenseLocation::InSuspensePlaceholder(context) => Some(context), SuspenseLocation::UnderSuspense(context) => Some(context), + SuspenseLocation::SuspenseBoundary(context) => Some(context), _ => None, } } @@ -108,17 +110,26 @@ impl Scope { } } - /// Get the suspense boundary this scope is currently in (if any) - pub(crate) fn suspense_boundary(&self) -> SuspenseLocation { + /// Get the suspense location of this scope + pub(crate) fn suspense_location(&self) -> SuspenseLocation { self.suspense_boundary.clone() } + /// If this scope is a suspense boundary, return the suspense context + pub(crate) fn suspense_boundary(&self) -> Option { + match self.suspense_location() { + SuspenseLocation::SuspenseBoundary(context) => Some(context), + _ => None, + } + } + /// Check if a node should run during suspense pub(crate) fn should_run_during_suspense(&self) -> bool { - matches!( - self.suspense_boundary, - SuspenseLocation::UnderSuspense(_) | SuspenseLocation::InSuspensePlaceholder(_) - ) + let Some(context) = self.suspense_boundary.suspense_context() else { + return false; + }; + + !context.frozen() } /// Mark this scope as dirty, and schedule a render for it. diff --git a/packages/core/src/suspense/component.rs b/packages/core/src/suspense/component.rs index acd352f25..93e0b5335 100644 --- a/packages/core/src/suspense/component.rs +++ b/packages/core/src/suspense/component.rs @@ -1,4 +1,4 @@ -use crate::innerlude::*; +use crate::{innerlude::*, scope_context::SuspenseLocation}; /// Properties for the [`SuspenseBoundary()`] component. #[allow(non_camel_case_types)] @@ -6,8 +6,6 @@ pub struct SuspenseBoundaryProps { fallback: Callback, /// The children of the suspense boundary children: Element, - /// THe nodes that are suspended under this boundary - pub suspended_nodes: Option, } impl Clone for SuspenseBoundaryProps { @@ -15,10 +13,6 @@ impl Clone for SuspenseBoundaryProps { Self { fallback: self.fallback, children: self.children.clone(), - suspended_nodes: self - .suspended_nodes - .as_ref() - .map(|node| node.clone_mounted()), } } } @@ -211,11 +205,7 @@ impl<__children: SuspenseBoundaryPropsBuilder_Optional> let fallback = fallback.0; let children = SuspenseBoundaryPropsBuilder_Optional::into_value(children, VNode::empty); SuspenseBoundaryPropsWithOwner { - inner: SuspenseBoundaryProps { - fallback, - children, - suspended_nodes: None, - }, + inner: SuspenseBoundaryProps { fallback, children }, owner: self.owner, } } @@ -272,29 +262,11 @@ pub use SuspenseBoundary_completions::Component::SuspenseBoundary; /// Suspense has a custom diffing algorithm that diffs the suspended nodes in the background without rendering them impl SuspenseBoundaryProps { /// Try to downcast [`AnyProps`] to [`SuspenseBoundaryProps`] - pub(crate) fn downcast_mut_from_props(props: &mut dyn AnyProps) -> Option<&mut Self> { + pub(crate) fn downcast_from_props(props: &mut dyn AnyProps) -> Option<&mut Self> { let inner: Option<&mut SuspenseBoundaryPropsWithOwner> = props.props_mut().downcast_mut(); inner.map(|inner| &mut inner.inner) } - /// Try to downcast [`AnyProps`] to [`SuspenseBoundaryProps`] - pub(crate) fn downcast_ref_from_props(props: &dyn AnyProps) -> Option<&Self> { - let inner: Option<&SuspenseBoundaryPropsWithOwner> = props.props().downcast_ref(); - inner.map(|inner| &inner.inner) - } - - /// Try to extract [`SuspenseBoundaryProps`] from [`ScopeState`] - pub fn downcast_from_scope(scope_state: &ScopeState) -> Option<&Self> { - let inner: Option<&SuspenseBoundaryPropsWithOwner> = - scope_state.props.props().downcast_ref(); - inner.map(|inner| &inner.inner) - } - - /// Check if the suspense boundary is currently holding its children in suspense - pub fn suspended(&self) -> bool { - self.suspended_nodes.is_some() - } - pub(crate) fn create( mount: MountId, idx: usize, @@ -310,17 +282,19 @@ impl SuspenseBoundaryProps { { let suspense_context = SuspenseContext::new(); - dom.runtime.suspense_stack.borrow_mut().push( - crate::scope_context::SuspenseLocation::UnderSuspense(suspense_context.clone()), - ); - { - let scope_state = dom - .new_scope(component.props.duplicate(), component.name) - .state(); - suspense_context.mount(scope_state.id); - scope_id = scope_state.id; - } - dom.runtime.suspense_stack.borrow_mut().pop(); + let suspense_boundary_location = + crate::scope_context::SuspenseLocation::SuspenseBoundary( + suspense_context.clone(), + ); + dom.runtime + .clone() + .with_suspense_location(suspense_boundary_location, || { + let scope_state = dom + .new_scope(component.props.duplicate(), component.name) + .state(); + suspense_context.mount(scope_state.id); + scope_id = scope_state.id; + }); } // Store the scope id for the next render @@ -328,7 +302,9 @@ impl SuspenseBoundaryProps { } let scope_state = &mut dom.scopes[scope_id.0]; - let props = Self::downcast_mut_from_props(&mut *scope_state.props).unwrap(); + let props = Self::downcast_from_props(&mut *scope_state.props).unwrap(); + let suspense_context = + SuspenseContext::downcast_suspense_boundary_from_scope(&dom.runtime, scope_id).unwrap(); let children = RenderReturn { node: props @@ -339,38 +315,41 @@ impl SuspenseBoundaryProps { }; // First always render the children in the background. Rendering the children may cause this boundary to suspend - dom.runtime.push_scope(scope_id); - children.create(dom, parent, None::<&mut M>); - dom.runtime.pop_scope(); + suspense_context.under_suspense_boundary(&dom.runtime(), || { + children.create(dom, parent, None::<&mut M>); + }); // Store the (now mounted) children back into the scope state let scope_state = &mut dom.scopes[scope_id.0]; - let props = Self::downcast_mut_from_props(&mut *scope_state.props).unwrap(); + let props = Self::downcast_from_props(&mut *scope_state.props).unwrap(); props.children = children.clone().node; let scope_state = &mut dom.scopes[scope_id.0]; let suspense_context = scope_state .state() - .suspense_boundary() + .suspense_location() .suspense_context() .unwrap() .clone(); // If there are suspended futures, render the fallback let nodes_created = if !suspense_context.suspended_futures().is_empty() { - let props = Self::downcast_mut_from_props(&mut *scope_state.props).unwrap(); - props.suspended_nodes = Some(children.into()); - - dom.runtime.suspense_stack.borrow_mut().push( - crate::scope_context::SuspenseLocation::InSuspensePlaceholder( - suspense_context.clone(), - ), - ); - let suspense_placeholder = props.fallback.call(suspense_context); - let node = RenderReturn { - node: suspense_placeholder, - }; - let nodes_created = node.create(dom, parent, to); - dom.runtime.suspense_stack.borrow_mut().pop(); + let (node, nodes_created) = + suspense_context.in_suspense_placeholder(&dom.runtime(), || { + let scope_state = &mut dom.scopes[scope_id.0]; + let props = Self::downcast_from_props(&mut *scope_state.props).unwrap(); + let suspense_context = SuspenseContext::downcast_suspense_boundary_from_scope( + &dom.runtime, + scope_id, + ) + .unwrap(); + suspense_context.set_suspended_nodes(children.into()); + let suspense_placeholder = props.fallback.call(suspense_context); + let node = RenderReturn { + node: suspense_placeholder, + }; + let nodes_created = node.create(dom, parent, to); + (node, nodes_created) + }); let scope_state = &mut dom.scopes[scope_id.0]; scope_state.last_rendered_node = Some(node); @@ -378,14 +357,17 @@ impl SuspenseBoundaryProps { nodes_created } else { // Otherwise just render the children in the real dom - dom.runtime.push_scope(scope_id); debug_assert!(children.mount.get().mounted()); - let nodes_created = children.create(dom, parent, to); - dom.runtime.pop_scope(); + let nodes_created = suspense_context + .under_suspense_boundary(&dom.runtime(), || children.create(dom, parent, to)); let scope_state = &mut dom.scopes[scope_id.0]; scope_state.last_rendered_node = Some(children); - let props = Self::downcast_mut_from_props(&mut *dom.scopes[scope_id.0].props).unwrap(); - props.suspended_nodes = None; + let suspense_context = + SuspenseContext::downcast_suspense_boundary_from_scope(&dom.runtime, scope_id) + .unwrap(); + suspense_context.take_suspended_nodes(); + mark_suspense_resolved(dom, scope_id); + nodes_created }; @@ -403,7 +385,7 @@ impl SuspenseBoundaryProps { only_write_templates: impl FnOnce(&mut M), replace_with: usize, ) { - let _runtime = RuntimeGuard::new(dom.runtime.clone()); + let _runtime = RuntimeGuard::new(dom.runtime()); let Some(scope_state) = dom.scopes.get_mut(scope_id.0) else { return; }; @@ -411,7 +393,7 @@ impl SuspenseBoundaryProps { // Reset the suspense context let suspense_context = scope_state .state() - .suspense_boundary() + .suspense_location() .suspense_context() .unwrap() .clone(); @@ -426,7 +408,7 @@ impl SuspenseBoundaryProps { .expect("suspense placeholder is not mounted") .parent; - let props = Self::downcast_mut_from_props(&mut *scope_state.props).unwrap(); + let props = Self::downcast_from_props(&mut *scope_state.props).unwrap(); // Unmount any children to reset any scopes under this suspense boundary let children = props @@ -434,10 +416,9 @@ impl SuspenseBoundaryProps { .as_ref() .map(|node| node.clone_mounted()) .map_err(Clone::clone); - let suspended = props - .suspended_nodes - .as_ref() - .map(|node| node.clone_mounted()); + let suspense_context = + SuspenseContext::downcast_suspense_boundary_from_scope(&dom.runtime, scope_id).unwrap(); + let suspended = suspense_context.suspended_nodes(); if let Some(node) = suspended { node.remove_node(&mut *dom, None::<&mut M>, None); } @@ -451,16 +432,18 @@ impl SuspenseBoundaryProps { children.mount.take(); // First always render the children in the background. Rendering the children may cause this boundary to suspend - dom.runtime.push_scope(scope_id); - children.create(dom, parent, Some(to)); - dom.runtime.pop_scope(); + suspense_context.under_suspense_boundary(&dom.runtime(), || { + children.create(dom, parent, Some(to)); + }); // Store the (now mounted) children back into the scope state let scope_state = &mut dom.scopes[scope_id.0]; - let props = Self::downcast_mut_from_props(&mut *scope_state.props).unwrap(); + let props = Self::downcast_from_props(&mut *scope_state.props).unwrap(); props.children = children.clone().node; scope_state.last_rendered_node = Some(children); - props.suspended_nodes = None; + let suspense_context = + SuspenseContext::downcast_suspense_boundary_from_scope(&dom.runtime, scope_id).unwrap(); + suspense_context.take_suspended_nodes(); } pub(crate) fn diff( @@ -469,25 +452,18 @@ impl SuspenseBoundaryProps { to: Option<&mut M>, ) { let scope = &mut dom.scopes[scope_id.0]; - let myself = Self::downcast_mut_from_props(&mut *scope.props) + let myself = Self::downcast_from_props(&mut *scope.props) .unwrap() .clone(); let last_rendered_node = scope.last_rendered_node.as_ref().unwrap().clone_mounted(); let Self { - fallback, - children, - suspended_nodes, - .. + fallback, children, .. } = myself; - let suspense_context = scope - .state() - .suspense_boundary() - .suspense_context() - .unwrap() - .clone(); + let suspense_context = scope.state().suspense_boundary().unwrap().clone(); + let suspended_nodes = suspense_context.suspended_nodes(); let suspended = !suspense_context.suspended_futures().is_empty(); match (suspended_nodes, suspended) { // We already have suspended nodes that still need to be suspended @@ -496,39 +472,38 @@ impl SuspenseBoundaryProps { let new_suspended_nodes: VNode = RenderReturn { node: children }.into(); // Diff the placeholder nodes in the dom - dom.runtime.suspense_stack.borrow_mut().push( - crate::scope_context::SuspenseLocation::InSuspensePlaceholder( - suspense_context.clone(), - ), - ); - let old_placeholder = last_rendered_node; - let new_placeholder = RenderReturn { - node: fallback.call(suspense_context), - }; + let new_placeholder = + suspense_context.in_suspense_placeholder(&dom.runtime(), || { + let old_placeholder = last_rendered_node; + let new_placeholder = RenderReturn { + node: fallback.call(suspense_context.clone()), + }; - old_placeholder.diff_node(&new_placeholder, dom, to); - dom.runtime.suspense_stack.borrow_mut().pop(); + old_placeholder.diff_node(&new_placeholder, dom, to); + new_placeholder + }); // Set the last rendered node to the placeholder dom.scopes[scope_id.0].last_rendered_node = Some(new_placeholder); // Diff the suspended nodes in the background - dom.runtime.push_scope(scope_id); - suspended_nodes.diff_node(&new_suspended_nodes, dom, None::<&mut M>); - dom.runtime.pop_scope(); + suspense_context.under_suspense_boundary(&dom.runtime(), || { + suspended_nodes.diff_node(&new_suspended_nodes, dom, None::<&mut M>); + }); - let props = - Self::downcast_mut_from_props(&mut *dom.scopes[scope_id.0].props).unwrap(); - props.suspended_nodes = Some(new_suspended_nodes); + let suspense_context = + SuspenseContext::downcast_suspense_boundary_from_scope(&dom.runtime, scope_id) + .unwrap(); + suspense_context.set_suspended_nodes(new_suspended_nodes); } // We have no suspended nodes, and we are not suspended. Just diff the children like normal (None, false) => { let old_children = last_rendered_node; let new_children = RenderReturn { node: children }; - dom.runtime.push_scope(scope_id); - old_children.diff_node(&new_children, dom, to); - dom.runtime.pop_scope(); + suspense_context.under_suspense_boundary(&dom.runtime(), || { + old_children.diff_node(&new_children, dom, to); + }); // Set the last rendered node to the new children dom.scopes[scope_id.0].last_rendered_node = Some(new_children); @@ -546,61 +521,102 @@ impl SuspenseBoundaryProps { let mount = old_children.mount.get(); let mount = dom.mounts.get(mount.0).expect("mount should exist"); let parent = mount.parent; - dom.runtime.push_scope(scope_id); - dom.runtime.suspense_stack.borrow_mut().push( - crate::scope_context::SuspenseLocation::InSuspensePlaceholder(suspense_context), - ); - old_children.move_node_to_background( - std::slice::from_ref(&*new_placeholder), - parent, - dom, - to, - ); - dom.runtime.suspense_stack.borrow_mut().pop(); + + suspense_context.in_suspense_placeholder(&dom.runtime(), || { + old_children.move_node_to_background( + std::slice::from_ref(&*new_placeholder), + parent, + dom, + to, + ); + }); // Then diff the new children in the background - old_children.diff_node(&new_children, dom, None::<&mut M>); - dom.runtime.pop_scope(); + suspense_context.under_suspense_boundary(&dom.runtime(), || { + old_children.diff_node(&new_children, dom, None::<&mut M>); + }); // Set the last rendered node to the new suspense placeholder dom.scopes[scope_id.0].last_rendered_node = Some(new_placeholder); - let props = - Self::downcast_mut_from_props(&mut *dom.scopes[scope_id.0].props).unwrap(); - props.suspended_nodes = Some(new_children); + let suspense_context = + SuspenseContext::downcast_suspense_boundary_from_scope(&dom.runtime, scope_id) + .unwrap(); + suspense_context.set_suspended_nodes(new_children); + + un_resolve_suspense(dom, scope_id); } // We have suspended nodes, but we just got out of suspense. Move the suspended nodes to the foreground (Some(old_suspended_nodes), false) => { let old_placeholder = last_rendered_node; let new_children = RenderReturn { node: children }; // First diff the two children nodes in the background - dom.runtime.push_scope(scope_id); - old_suspended_nodes.diff_node(&new_children, dom, None::<&mut M>); + suspense_context.under_suspense_boundary(&dom.runtime(), || { + old_suspended_nodes.diff_node(&new_children, dom, None::<&mut M>); - // Then replace the placeholder with the new children - let mount = old_placeholder.mount.get(); - let mount = dom.mounts.get(mount.0).expect("mount should exist"); - let parent = mount.parent; - old_placeholder.replace(std::slice::from_ref(&*new_children), parent, dom, to); - dom.runtime.pop_scope(); + // Then replace the placeholder with the new children + let mount = old_placeholder.mount.get(); + let mount = dom.mounts.get(mount.0).expect("mount should exist"); + let parent = mount.parent; + old_placeholder.replace(std::slice::from_ref(&*new_children), parent, dom, to); + }); // Set the last rendered node to the new children dom.scopes[scope_id.0].last_rendered_node = Some(new_children); - let props = - Self::downcast_mut_from_props(&mut *dom.scopes[scope_id.0].props).unwrap(); - props.suspended_nodes = None; + let suspense_context = + SuspenseContext::downcast_suspense_boundary_from_scope(&dom.runtime, scope_id) + .unwrap(); + suspense_context.take_suspended_nodes(); + + mark_suspense_resolved(dom, scope_id); } } } +} + +/// Move to a resolved suspense state +fn mark_suspense_resolved(dom: &mut VirtualDom, scope_id: ScopeId) { + dom.resolved_scopes.push(scope_id); +} + +/// Move from a resolved suspense state to an suspended state +fn un_resolve_suspense(dom: &mut VirtualDom, scope_id: ScopeId) { + dom.resolved_scopes.retain(|&id| id != scope_id); +} + +impl SuspenseContext { + /// Run a closure under a suspense boundary + pub fn under_suspense_boundary(&self, runtime: &Runtime, f: impl FnOnce() -> O) -> O { + runtime.with_suspense_location(SuspenseLocation::UnderSuspense(self.clone()), f) + } + + /// Run a closure under a suspense placeholder + pub fn in_suspense_placeholder(&self, runtime: &Runtime, f: impl FnOnce() -> O) -> O { + runtime.with_suspense_location(SuspenseLocation::InSuspensePlaceholder(self.clone()), f) + } + + /// Try to get a suspense boundary from a scope id + pub fn downcast_suspense_boundary_from_scope( + runtime: &Runtime, + scope_id: ScopeId, + ) -> Option { + runtime + .get_state(scope_id) + .and_then(|scope| scope.suspense_boundary()) + } pub(crate) fn remove_suspended_nodes( - &mut self, dom: &mut VirtualDom, + scope_id: ScopeId, destroy_component_state: bool, ) { + let Some(scope) = Self::downcast_suspense_boundary_from_scope(&dom.runtime, scope_id) + else { + return; + }; // Remove the suspended nodes - if let Some(node) = self.suspended_nodes.take() { + if let Some(node) = scope.take_suspended_nodes() { node.remove_node_inner(dom, None::<&mut M>, destroy_component_state, None) } } diff --git a/packages/core/src/suspense/mod.rs b/packages/core/src/suspense/mod.rs index c013de075..319366b55 100644 --- a/packages/core/src/suspense/mod.rs +++ b/packages/core/src/suspense/mod.rs @@ -100,6 +100,8 @@ impl SuspenseContext { inner: Rc::new(SuspenseBoundaryInner { suspended_tasks: RefCell::new(vec![]), id: Cell::new(ScopeId::ROOT), + suspended_nodes: Default::default(), + frozen: Default::default(), }), } } @@ -109,8 +111,40 @@ impl SuspenseContext { self.inner.id.set(scope); } + /// Get the suspense boundary's suspended nodes + pub fn suspended_nodes(&self) -> Option { + self.inner + .suspended_nodes + .borrow() + .as_ref() + .map(|node| node.clone_mounted()) + } + + /// Set the suspense boundary's suspended nodes + pub(crate) fn set_suspended_nodes(&self, suspended_nodes: VNode) { + self.inner + .suspended_nodes + .borrow_mut() + .replace(suspended_nodes); + } + + /// Take the suspense boundary's suspended nodes + pub(crate) fn take_suspended_nodes(&self) -> Option { + self.inner.suspended_nodes.borrow_mut().take() + } + + /// Check if the suspense boundary is resolved and frozen + pub fn frozen(&self) -> bool { + self.inner.frozen.get() + } + + /// Resolve the suspense boundary on the server and freeze it to prevent future reruns of any child nodes of the suspense boundary + pub fn freeze(&self) { + self.inner.frozen.set(true); + } + /// Check if there are any suspended tasks - pub fn suspended(&self) -> bool { + pub fn has_suspended_tasks(&self) -> bool { !self.inner.suspended_tasks.borrow().is_empty() } @@ -152,6 +186,10 @@ impl SuspenseContext { pub struct SuspenseBoundaryInner { suspended_tasks: RefCell>, id: Cell, + /// The nodes that are suspended under this boundary + suspended_nodes: RefCell>, + /// On the server, you can only resolve a suspense boundary once. This is used to track if the suspense boundary has been resolved and if it should be frozen + frozen: Cell, } /// Provides context methods to [`Result`] to show loading indicators for suspended results diff --git a/packages/core/src/tasks.rs b/packages/core/src/tasks.rs index e6d0ffba6..3723476f9 100644 --- a/packages/core/src/tasks.rs +++ b/packages/core/src/tasks.rs @@ -262,26 +262,26 @@ impl Runtime { let mut cx = std::task::Context::from_waker(&task.waker); - // update the scope stack - self.push_scope(task.scope); - self.rendering.set(false); - self.current_task.set(Some(id)); + // poll the future with the scope on the stack + let poll_result = self.with_scope_on_stack(task.scope, || { + self.rendering.set(false); + self.current_task.set(Some(id)); - let poll_result = task.task.borrow_mut().as_mut().poll(&mut cx); + let poll_result = task.task.borrow_mut().as_mut().poll(&mut cx); - if poll_result.is_ready() { - // Remove it from the scope so we dont try to double drop it when the scope dropes - self.get_state(task.scope) - .unwrap() - .spawned_tasks - .borrow_mut() - .remove(&id); + if poll_result.is_ready() { + // Remove it from the scope so we dont try to double drop it when the scope dropes + self.get_state(task.scope) + .unwrap() + .spawned_tasks + .borrow_mut() + .remove(&id); - self.remove_task(id); - } + self.remove_task(id); + } - // Remove the scope from the stack - self.pop_scope(); + poll_result + }); self.rendering.set(true); self.current_task.set(None); diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 167f6eef4..ea4920cde 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -2,7 +2,7 @@ //! //! This module provides the primary mechanics to create a hook-based, concurrent VDOM for Rust. -use crate::innerlude::{SuspenseBoundaryProps, Work}; +use crate::innerlude::Work; use crate::properties::RootProps; use crate::root_wrapper::RootScopeWrapper; use crate::{ @@ -225,6 +225,9 @@ pub struct VirtualDom { pub(crate) runtime: Rc, + // The scopes that have been resolved since the last render + pub(crate) resolved_scopes: Vec, + rx: futures_channel::mpsc::UnboundedReceiver, } @@ -330,6 +333,7 @@ impl VirtualDom { queued_templates: Default::default(), elements: Default::default(), mounts: Default::default(), + resolved_scopes: Default::default(), }; let root = VProps::new( @@ -776,8 +780,6 @@ impl VirtualDom { // Queue any new events before we start working self.queue_events(); - let mut resolved_scopes = Vec::new(); - // Render whatever work needs to be rendered, unlocking new futures and suspense leaves let _runtime = RuntimeGuard::new(self.runtime.clone()); @@ -791,34 +793,21 @@ impl VirtualDom { } } Work::RerunScope(scope) => { - if self + let scope_id: ScopeId = scope.id; + let run_scope = self .runtime .get_state(scope.id) .filter(|scope| scope.should_run_during_suspense()) - .is_some() - { - let scope_state = self.get_scope(scope.id).unwrap(); - let was_suspended = - SuspenseBoundaryProps::downcast_ref_from_props(&*scope_state.props) - .filter(|props| props.suspended()) - .is_some(); + .is_some(); + if run_scope { // If the scope is dirty, run the scope and get the mutations - self.run_and_diff_scope(None::<&mut NoOpMutations>, scope.id); - let scope_state = self.get_scope(scope.id).unwrap(); - let is_now_suspended = - SuspenseBoundaryProps::downcast_ref_from_props(&*scope_state.props) - .filter(|props| props.suspended()) - .is_some(); + self.run_and_diff_scope(None::<&mut NoOpMutations>, scope_id); - if is_now_suspended { - resolved_scopes.retain(|&id| id != scope.id); - } else if was_suspended { - resolved_scopes.push(scope.id); - } + tracing::trace!("Ran scope {:?} during suspense", scope_id); } else { tracing::warn!( "Scope {:?} was marked as dirty, but will not rerun during suspense. Only nodes that are under a suspense boundary rerun during suspense", - scope.id + scope_id ); } } @@ -833,8 +822,9 @@ impl VirtualDom { } } - resolved_scopes.sort_by_key(|&id| self.runtime.get_state(id).unwrap().height); - resolved_scopes + self.resolved_scopes + .sort_by_key(|&id| self.runtime.get_state(id).unwrap().height); + std::mem::take(&mut self.resolved_scopes) } /// Get the current runtime diff --git a/packages/fullstack/src/html_storage/serialize.rs b/packages/fullstack/src/html_storage/serialize.rs index 1f467a0a6..c6de74d86 100644 --- a/packages/fullstack/src/html_storage/serialize.rs +++ b/packages/fullstack/src/html_storage/serialize.rs @@ -1,6 +1,7 @@ use dioxus_lib::prelude::dioxus_core::DynamicNode; use dioxus_lib::prelude::{ - has_context, try_consume_context, ScopeId, SuspenseBoundaryProps, VNode, VirtualDom, + has_context, try_consume_context, ScopeId, SuspenseBoundaryProps, SuspenseContext, VNode, + VirtualDom, }; use serde::Serialize; @@ -49,9 +50,11 @@ impl super::HTMLData { // then continue to any children if let Some(scope) = vdom.get_scope(scope) { // If this is a suspense boundary, move into the children first (even if they are suspended) because that will be run first on the client - if let Some(suspense_boundary) = SuspenseBoundaryProps::downcast_from_scope(scope) { - if let Some(node) = suspense_boundary.suspended_nodes.as_ref() { - self.take_from_vnode(vdom, node); + if let Some(suspense_boundary) = + SuspenseContext::downcast_suspense_boundary_from_scope(&vdom.runtime(), scope.id()) + { + if let Some(node) = suspense_boundary.suspended_nodes() { + self.take_from_vnode(vdom, &node); } } if let Some(node) = scope.try_root_node() { diff --git a/packages/fullstack/src/render.rs b/packages/fullstack/src/render.rs index 085025212..3f7935906 100644 --- a/packages/fullstack/src/render.rs +++ b/packages/fullstack/src/render.rs @@ -207,10 +207,12 @@ impl SsrRendererPool { let scope_to_mount_mapping = scope_to_mount_mapping.clone(); let stream = stream.clone(); renderer.set_render_components(move |renderer, to, vdom, scope| { - let is_suspense_boundary = vdom - .get_scope(scope) - .and_then(|s| SuspenseBoundaryProps::downcast_from_scope(s)) - .filter(|s| s.suspended()) + let is_suspense_boundary = + SuspenseContext::downcast_suspense_boundary_from_scope( + &vdom.runtime(), + scope, + ) + .filter(|s| s.has_suspended_tasks()) .is_some(); if is_suspense_boundary { let mount = stream.render_placeholder( @@ -265,27 +267,37 @@ impl SsrRendererPool { for scope in resolved_suspense_nodes { let mount = { let mut lock = scope_to_mount_mapping.write().unwrap(); - lock.remove(&scope).unwrap() + lock.remove(&scope) }; - let mut resolved_chunk = String::new(); - // After we replace the placeholder in the dom with javascript, we need to send down the resolved data so that the client can hydrate the node - let render_suspense = |into: &mut String| { - renderer.reset_hydration(); - renderer.render_scope(into, &virtual_dom, scope) - }; - let resolved_data = serialize_server_data(&virtual_dom, scope); - if let Err(err) = stream.replace_placeholder( - mount, - render_suspense, - resolved_data, - &mut resolved_chunk, - ) { - throw_error!( - dioxus_ssr::incremental::IncrementalRendererError::RenderError(err) - ); - } + // If the suspense boundary was immediately removed, it may not have a mount. We can just skip resolving it + if let Some(mount) = mount { + let mut resolved_chunk = String::new(); + // After we replace the placeholder in the dom with javascript, we need to send down the resolved data so that the client can hydrate the node + let render_suspense = |into: &mut String| { + renderer.reset_hydration(); + renderer.render_scope(into, &virtual_dom, scope) + }; + let resolved_data = serialize_server_data(&virtual_dom, scope); + if let Err(err) = stream.replace_placeholder( + mount, + render_suspense, + resolved_data, + &mut resolved_chunk, + ) { + throw_error!( + dioxus_ssr::incremental::IncrementalRendererError::RenderError(err) + ); + } - stream.render(resolved_chunk); + stream.render(resolved_chunk); + } + // Freeze the suspense boundary to prevent future reruns of any child nodes of the suspense boundary + if let Some(suspense) = SuspenseContext::downcast_suspense_boundary_from_scope( + &virtual_dom.runtime(), + scope, + ) { + suspense.freeze(); + } } } diff --git a/packages/web/src/hydration/hydrate.rs b/packages/web/src/hydration/hydrate.rs index 19e41627a..a537a3283 100644 --- a/packages/web/src/hydration/hydrate.rs +++ b/packages/web/src/hydration/hydrate.rs @@ -226,8 +226,10 @@ impl WebsysDom { to_mount: &mut Vec, ) -> Result<(), RehydrationError> { // If this scope is a suspense boundary that is pending, add it to the list of pending suspense boundaries - if let Some(suspense) = SuspenseBoundaryProps::downcast_from_scope(scope) { - if suspense.suspended() { + if let Some(suspense) = + SuspenseContext::downcast_suspense_boundary_from_scope(&dom.runtime(), scope.id()) + { + if suspense.has_suspended_tasks() { self.suspense_hydration_ids .add_suspense_boundary(scope.id()); }