diff --git a/packages/core/src/any_props.rs b/packages/core/src/any_props.rs index 232c3748a..801708fd5 100644 --- a/packages/core/src/any_props.rs +++ b/packages/core/src/any_props.rs @@ -1,4 +1,4 @@ -use crate::{innerlude::CapturedPanic, nodes::RenderReturn, ComponentFunction}; +use crate::{innerlude::CapturedPanic, ComponentFunction, Element}; use std::{any::Any, panic::AssertUnwindSafe}; pub(crate) type BoxedAnyProps = Box; @@ -6,7 +6,7 @@ pub(crate) type BoxedAnyProps = Box; /// A trait for a component that can be rendered. pub(crate) trait AnyProps: 'static { /// Render the component with the internal props. - fn render(&self) -> RenderReturn; + fn render(&self) -> Element; /// Make the old props equal to the new type erased props. Return if the props were equal and should be memoized. fn memoize(&mut self, other: &dyn Any) -> bool; /// Get the props as a type erased `dyn Any`. @@ -74,20 +74,18 @@ impl + Clone, P: Clone + 'static, M: 'static> AnyProp &mut self.props } - fn render(&self) -> RenderReturn { + fn render(&self) -> Element { let res = std::panic::catch_unwind(AssertUnwindSafe(move || { self.render_fn.rebuild(self.props.clone()) })); match res { - Ok(node) => RenderReturn { node }, + Ok(node) => node, Err(err) => { let component_name = self.name; tracing::error!("Panic while rendering component `{component_name}`: {err:?}"); let panic = CapturedPanic { error: err }; - RenderReturn { - node: Err(panic.into()), - } + Element::Err(panic.into()) } } } diff --git a/packages/core/src/diff/component.rs b/packages/core/src/diff/component.rs index 92c995c77..f66872ebc 100644 --- a/packages/core/src/diff/component.rs +++ b/packages/core/src/diff/component.rs @@ -9,11 +9,11 @@ use crate::{ ElementRef, MountId, ScopeOrder, SuspenseBoundaryProps, SuspenseBoundaryPropsWithOwner, VComponent, WriteMutations, }, - nodes::VNode, + nodes::{AsVNode, VNode}, prelude::SuspenseContext, scopes::ScopeId, virtual_dom::VirtualDom, - RenderReturn, + Element, }; impl VirtualDom { @@ -36,11 +36,11 @@ impl VirtualDom { &mut self, to: Option<&mut M>, scope: ScopeId, - new_nodes: RenderReturn, + new_nodes: Element, ) { 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 { + let Ok(new_real_nodes) = &new_nodes else { return; }; let scope_state = &mut self.scopes[scope.0]; @@ -51,7 +51,8 @@ impl VirtualDom { // 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()); + old.as_vnode() + .diff_node(new_real_nodes, self, render_to.as_deref_mut()); self.scopes[scope.0].last_rendered_node = Some(new_nodes); @@ -69,7 +70,7 @@ impl VirtualDom { &mut self, to: Option<&mut M>, scope: ScopeId, - new_nodes: RenderReturn, + new_nodes: Element, parent: Option, ) -> usize { self.runtime.clone().with_scope_on_stack(scope, || { @@ -79,7 +80,9 @@ impl VirtualDom { 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()); + let nodes = new_nodes + .as_vnode() + .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); @@ -104,8 +107,12 @@ impl VirtualDom { // Remove the component from the dom if let Some(node) = self.scopes[scope_id.0].last_rendered_node.as_ref() { - node.clone_mounted() - .remove_node_inner(self, to, destroy_component_state, replace_with) + node.clone().as_vnode().remove_node_inner( + self, + to, + destroy_component_state, + replace_with, + ) }; if destroy_component_state { diff --git a/packages/core/src/diff/iterator.rs b/packages/core/src/diff/iterator.rs index 2ec81518b..f8268fa43 100644 --- a/packages/core/src/diff/iterator.rs +++ b/packages/core/src/diff/iterator.rs @@ -133,29 +133,17 @@ impl VirtualDom { let new_middle = &new[left_offset..(new.len() - right_offset)]; debug_assert!( - !((old_middle.len() == new_middle.len()) && old_middle.is_empty()), - "keyed children must have the same number of children" + !old_middle.is_empty(), + "Old middle returned from `diff_keyed_ends` should not be empty" + ); + debug_assert!( + !new_middle.is_empty(), + "New middle returned from `diff_keyed_ends` should not be empty" ); + // A few nodes in the middle were removed, just remove the old nodes if new_middle.is_empty() { - // remove the old elements self.remove_nodes(to, old_middle, None); - } else if old_middle.is_empty() { - // there were no old elements, so just create the new elements - // we need to find the right "foothold" though - we shouldn't use the "append" at all - if left_offset == 0 { - // insert at the beginning of the old list - let foothold = &old[old.len() - right_offset]; - self.create_and_insert_before(to, new_middle, foothold, parent); - } else if right_offset == 0 { - // insert at the end the old list - let foothold = old.last().unwrap(); - self.create_and_insert_after(to, new_middle, foothold, parent); - } else { - // inserting in the middle - let foothold = &old[left_offset - 1]; - self.create_and_insert_after(to, new_middle, foothold, parent); - } } else { self.diff_keyed_middle(to, old_middle, new_middle, parent); } @@ -187,14 +175,7 @@ impl VirtualDom { // If that was all of the old children, then create and append the remaining // new children and we're finished. if left_offset == old.len() { - self.create_and_insert_after(to, &new[left_offset..], old.last().unwrap(), parent); - return None; - } - - // And if that was all of the new children, then remove all of the remaining - // old children and we're finished. - if left_offset == new.len() { - self.remove_nodes(to, &old[left_offset..], None); + self.create_and_insert_after(to, &new[left_offset..], &new[left_offset - 1], parent); return None; } @@ -209,6 +190,35 @@ impl VirtualDom { right_offset += 1; } + // If that was all of the old children, then create and prepend the remaining + // new children and we're finished. + if right_offset == old.len() { + self.create_and_insert_before( + to, + &new[..new.len() - right_offset], + &new[new.len() - right_offset], + parent, + ); + return None; + } + + // If the right offset + the left offset is the same as the new length, then we just need to remove the old nodes + if right_offset + left_offset == new.len() { + self.remove_nodes(to, &old[left_offset..old.len() - right_offset], None); + return None; + } + + // If the right offset + the left offset is the same as the old length, then we just need to add the new nodes + if right_offset + left_offset == old.len() { + self.create_and_insert_before( + to, + &new[left_offset..new.len() - right_offset], + &new[new.len() - right_offset], + parent, + ); + return None; + } + Some((left_offset, right_offset)) } diff --git a/packages/core/src/diff/node.rs b/packages/core/src/diff/node.rs index 55e94bfdc..70f3b3028 100644 --- a/packages/core/src/diff/node.rs +++ b/packages/core/src/diff/node.rs @@ -35,8 +35,6 @@ impl VNode { return self.replace(std::slice::from_ref(new), parent, dom, to); } - let mount_id = self.mount.get(); - self.move_mount_to(new, dom); // If the templates are the same, we don't need to do anything, except copy over the mount information @@ -52,6 +50,7 @@ impl VNode { } // Now diff the dynamic nodes + let mount_id = new.mount.get(); for (dyn_node_idx, (old, new)) in self .dynamic_nodes .iter() @@ -64,7 +63,7 @@ impl VNode { fn move_mount_to(&self, new: &VNode, dom: &mut VirtualDom) { // Copy over the mount information - let mount_id = self.mount.get(); + let mount_id = self.mount.take(); new.mount.set(mount_id); if mount_id.mounted() { @@ -72,7 +71,7 @@ impl VNode { let mount = &mut mounts[mount_id.0]; // Update the reference to the node for bubbling events - mount.node = new.clone_mounted(); + mount.node = new.clone(); } } @@ -293,6 +292,7 @@ impl VNode { self.reclaim_roots(mount, dom, to, destroy_component_state, replace_with); if destroy_component_state { + let mount = self.mount.take(); // Remove the mount information dom.runtime.mounts.borrow_mut().remove(mount.0); } @@ -416,7 +416,7 @@ impl VNode { dom: &mut VirtualDom, to: &mut impl WriteMutations, ) { - let mount_id = self.mount.get(); + let mount_id = new.mount.get(); for (idx, (old_attrs, new_attrs)) in self .dynamic_attrs .iter() @@ -535,7 +535,7 @@ impl VNode { self.mount.set(mount); tracing::trace!(?self, ?mount, "creating template"); entry.insert(VNodeMount { - node: self.clone_mounted(), + node: self.clone(), parent, root_ids: vec![ElementId(0); template.roots.len()].into_boxed_slice(), mounted_attributes: vec![ElementId(0); template.attr_paths.len()] @@ -559,7 +559,8 @@ impl VNode { .as_usize() .expect("node should already be mounted"), ), - "Node mount should be valid" + "Tried to find mount {:?} in dom.mounts, but it wasn't there", + self.mount.get() ); let mount = self.mount.get(); diff --git a/packages/core/src/error_boundary.rs b/packages/core/src/error_boundary.rs index 2de89ab49..332c7f406 100644 --- a/packages/core/src/error_boundary.rs +++ b/packages/core/src/error_boundary.rs @@ -451,17 +451,6 @@ impl CapturedError { self } - /// Clone the error while retaining the mounted information of the error - pub(crate) fn clone_mounted(&self) -> Self { - Self { - error: self.error.clone(), - backtrace: self.backtrace.clone(), - scope: self.scope, - render: self.render.clone_mounted(), - context: self.context.clone(), - } - } - /// Get a VNode representation of the error if the error provides one pub fn show(&self) -> Option { if self.render == VNode::placeholder() { diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index b58efa564..4f2c747c1 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -76,9 +76,9 @@ pub use crate::innerlude::{ fc_to_builder, generation, schedule_update, schedule_update_any, use_hook, vdom_is_rendering, AnyValue, Attribute, AttributeValue, CapturedError, Component, ComponentFunction, DynamicNode, Element, ElementId, Event, Fragment, HasAttributes, IntoDynNode, MarkerWrapper, Mutation, - Mutations, NoOpMutations, Ok, Properties, RenderReturn, Result, Runtime, ScopeId, ScopeState, - SpawnIfAsync, Task, Template, TemplateAttribute, TemplateNode, VComponent, VNode, VNodeInner, - VPlaceholder, VText, VirtualDom, WriteMutations, + Mutations, NoOpMutations, Ok, Properties, Result, Runtime, ScopeId, ScopeState, SpawnIfAsync, + Task, Template, TemplateAttribute, TemplateNode, VComponent, VNode, VNodeInner, VPlaceholder, + VText, VirtualDom, WriteMutations, }; /// The purpose of this module is to alleviate imports of many common types @@ -94,8 +94,8 @@ pub mod prelude { use_hook_with_cleanup, with_owner, AnyValue, Attribute, Callback, Component, ComponentFunction, Context, Element, ErrorBoundary, ErrorContext, Event, EventHandler, Fragment, HasAttributes, IntoAttributeValue, IntoDynNode, OptionStringFromMarker, - Properties, ReactiveContext, RenderError, RenderReturn, Runtime, RuntimeGuard, ScopeId, - ScopeState, SuperFrom, SuperInto, SuspendedFuture, SuspenseBoundary, SuspenseBoundaryProps, + Properties, ReactiveContext, RenderError, Runtime, RuntimeGuard, ScopeId, ScopeState, + SuperFrom, SuperInto, SuspendedFuture, SuspenseBoundary, SuspenseBoundaryProps, SuspenseContext, SuspenseExtension, Task, Template, TemplateAttribute, TemplateNode, VNode, VNodeInner, VirtualDom, }; diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 8c4458642..33c9d754d 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -1,4 +1,5 @@ -use crate::innerlude::{RenderError, VProps}; +use crate::innerlude::VProps; +use crate::prelude::RenderError; use crate::{any_props::BoxedAnyProps, innerlude::ScopeState}; use crate::{arena::ElementId, Element, Event}; use crate::{ @@ -6,7 +7,7 @@ use crate::{ properties::ComponentFunction, }; use crate::{Properties, ScopeId, VirtualDom}; -use std::ops::{Deref, DerefMut}; +use std::ops::Deref; use std::rc::Rc; use std::vec; use std::{ @@ -15,77 +16,6 @@ use std::{ fmt::{Arguments, Debug}, }; -/// The actual state of the component's most recent computation -/// -/// If the component returned early (e.g. `return None`), this will be Aborted(None) -#[derive(Debug)] -pub struct RenderReturn { - /// The node that was rendered - pub(crate) node: Element, -} - -impl From for VNode { - fn from(val: RenderReturn) -> Self { - match val.node { - Ok(node) => node, - Err(RenderError::Aborted(e)) => e.render, - Err(RenderError::Suspended(fut)) => fut.placeholder, - } - } -} - -impl From for RenderReturn { - fn from(node: Element) -> Self { - RenderReturn { node } - } -} - -impl Clone for RenderReturn { - fn clone(&self) -> Self { - match &self.node { - Ok(node) => RenderReturn { - node: Ok(node.clone_mounted()), - }, - Err(RenderError::Aborted(err)) => RenderReturn { - node: Err(RenderError::Aborted(err.clone_mounted())), - }, - Err(RenderError::Suspended(fut)) => RenderReturn { - node: Err(RenderError::Suspended(fut.clone_mounted())), - }, - } - } -} - -impl Default for RenderReturn { - fn default() -> Self { - RenderReturn { - node: Ok(VNode::placeholder()), - } - } -} - -impl Deref for RenderReturn { - type Target = VNode; - - fn deref(&self) -> &Self::Target { - match &self.node { - Ok(node) => node, - Err(RenderError::Aborted(err)) => &err.render, - Err(RenderError::Suspended(fut)) => &fut.placeholder, - } - } -} - -impl DerefMut for RenderReturn { - fn deref_mut(&mut self) -> &mut Self::Target { - match &mut self.node { - Ok(node) => node, - Err(RenderError::Aborted(err)) => &mut err.render, - Err(RenderError::Suspended(fut)) => &mut fut.placeholder, - } - } -} - /// The information about the #[derive(Debug)] pub(crate) struct VNodeMount { @@ -162,7 +92,7 @@ pub struct VNodeInner { /// /// The dynamic parts of the template are stored separately from the static parts. This allows faster diffing by skipping /// static parts of the template. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct VNode { vnode: Rc, @@ -170,15 +100,44 @@ pub struct VNode { pub(crate) mount: Cell, } -impl Clone for VNode { - fn clone(&self) -> Self { - Self { - vnode: self.vnode.clone(), - mount: Default::default(), +impl AsRef for Element { + fn as_ref(&self) -> &VNode { + match self { + Element::Ok(node) => node, + Element::Err(RenderError::Aborted(err)) => &err.render, + Element::Err(RenderError::Suspended(fut)) => &fut.placeholder, } } } +impl From<&Element> for VNode { + fn from(val: &Element) -> Self { + AsRef::as_ref(val).clone() + } +} + +impl From for VNode { + fn from(val: Element) -> Self { + match val { + Element::Ok(node) => node, + Element::Err(RenderError::Aborted(err)) => err.render, + Element::Err(RenderError::Suspended(fut)) => fut.placeholder, + } + } +} + +/// A tiny helper trait to get the vnode for a Element +pub(crate) trait AsVNode { + /// Get the vnode for this element + fn as_vnode(&self) -> &VNode; +} + +impl AsVNode for Element { + fn as_vnode(&self) -> &VNode { + AsRef::as_ref(self) + } +} + impl Default for VNode { fn default() -> Self { Self::placeholder() @@ -223,14 +182,6 @@ impl Deref for VNode { } impl VNode { - /// Clone the element while retaining the mount information of the node - pub(crate) fn clone_mounted(&self) -> Self { - Self { - vnode: self.vnode.clone(), - mount: self.mount.clone(), - } - } - /// Create a template with no nodes that will be skipped over during diffing pub fn empty() -> Element { Ok(Self::default()) diff --git a/packages/core/src/scope_arena.rs b/packages/core/src/scope_arena.rs index a9f696321..a84d80f56 100644 --- a/packages/core/src/scope_arena.rs +++ b/packages/core/src/scope_arena.rs @@ -1,4 +1,4 @@ -use crate::innerlude::{throw_error, RenderError, RenderReturn, ScopeOrder}; +use crate::innerlude::{throw_error, RenderError, ScopeOrder}; use crate::prelude::ReactiveContext; use crate::scope_context::SuspenseLocation; use crate::{ @@ -48,7 +48,7 @@ impl VirtualDom { /// Run a scope and return the rendered nodes. This will not modify the DOM or update the last rendered node of the scope. #[tracing::instrument(skip(self), level = "trace", name = "VirtualDom::run_scope")] #[track_caller] - pub(crate) fn run_scope(&mut self, scope_id: ScopeId) -> RenderReturn { + pub(crate) fn run_scope(&mut self, scope_id: ScopeId) -> Element { // Ensure we are currently inside a `Runtime`. crate::Runtime::current().unwrap_or_else(|e| panic!("{}", e)); @@ -70,11 +70,7 @@ impl VirtualDom { 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(), - ); + self.handle_element_return(&mut render_return, scope_id, &scope.state()); render_return }) }) @@ -102,7 +98,7 @@ impl VirtualDom { "Error while rendering component `{}`:\n{e}", scope_state.name ); - throw_error(e.clone_mounted()); + throw_error(e.clone()); e.render = VNode::placeholder(); } Err(RenderError::Suspended(e)) => { diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index fca163fc7..b3bb12535 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -1,6 +1,6 @@ use crate::{ - any_props::BoxedAnyProps, nodes::RenderReturn, reactive_context::ReactiveContext, - scope_context::Scope, Runtime, VNode, + any_props::BoxedAnyProps, nodes::AsVNode, reactive_context::ReactiveContext, + scope_context::Scope, Element, Runtime, VNode, }; use std::{cell::Ref, rc::Rc}; @@ -70,7 +70,7 @@ pub struct ScopeState { pub(crate) context_id: ScopeId, /// The last node that has been rendered for this component. This node may not ben mounted /// During suspense, this component can be rendered in the background multiple times - pub(crate) last_rendered_node: Option, + pub(crate) last_rendered_node: Option, pub(crate) props: BoxedAnyProps, pub(crate) reactive_context: ReactiveContext, } @@ -98,7 +98,7 @@ impl ScopeState { /// /// Returns [`None`] if the tree has not been built yet. pub fn try_root_node(&self) -> Option<&VNode> { - self.last_rendered_node.as_deref() + self.last_rendered_node.as_ref().map(AsVNode::as_vnode) } /// Returns the scope id of this [`ScopeState`]. diff --git a/packages/core/src/suspense/component.rs b/packages/core/src/suspense/component.rs index 8d55e6671..1b6ea90c5 100644 --- a/packages/core/src/suspense/component.rs +++ b/packages/core/src/suspense/component.rs @@ -306,23 +306,17 @@ impl SuspenseBoundaryProps { SuspenseContext::downcast_suspense_boundary_from_scope(&dom.runtime, scope_id) .unwrap(); - let children = RenderReturn { - node: props - .children - .as_ref() - .map(|node| node.clone_mounted()) - .map_err(Clone::clone), - }; + let children = props.children.clone(); // First always render the children in the background. Rendering the children may cause this boundary to suspend suspense_context.under_suspense_boundary(&dom.runtime(), || { - children.create(dom, parent, None::<&mut M>); + children.as_vnode().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_from_props(&mut *scope_state.props).unwrap(); - props.children = children.clone().node; + props.children = children.clone(); let scope_state = &mut dom.scopes[scope_id.0]; let suspense_context = scope_state @@ -345,11 +339,8 @@ impl SuspenseBoundaryProps { .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 nodes_created = suspense_placeholder.as_vnode().create(dom, parent, to); + (suspense_placeholder, nodes_created) }); let scope_state = &mut dom.scopes[scope_id.0]; @@ -358,9 +349,11 @@ impl SuspenseBoundaryProps { nodes_created } else { // Otherwise just render the children in the real dom - debug_assert!(children.mount.get().mounted()); + debug_assert!(children.as_vnode().mount.get().mounted()); let nodes_created = suspense_context - .under_suspense_boundary(&dom.runtime(), || children.create(dom, parent, to)); + .under_suspense_boundary(&dom.runtime(), || { + children.as_vnode().create(dom, parent, to) + }); let scope_state = &mut dom.scopes[scope_id.0]; scope_state.last_rendered_node = Some(children); let suspense_context = @@ -403,7 +396,7 @@ impl SuspenseBoundaryProps { // Get the parent of the suspense boundary to later create children with the right parent let currently_rendered = scope_state.last_rendered_node.as_ref().unwrap().clone(); - let mount = currently_rendered.mount.get(); + let mount = currently_rendered.as_vnode().mount.get(); let parent = { let mounts = dom.runtime.mounts.borrow(); mounts @@ -415,11 +408,7 @@ impl SuspenseBoundaryProps { 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 - .children - .as_ref() - .map(|node| node.clone_mounted()) - .map_err(Clone::clone); + let children = props.children.clone(); let suspense_context = SuspenseContext::downcast_suspense_boundary_from_scope(&dom.runtime, scope_id) .unwrap(); @@ -429,23 +418,24 @@ impl SuspenseBoundaryProps { node.remove_node(&mut *dom, None::<&mut M>, None); } // Replace the rendered nodes with resolved nodes - currently_rendered.remove_node(&mut *dom, Some(to), Some(replace_with)); + currently_rendered + .as_vnode() + .remove_node(&mut *dom, Some(to), Some(replace_with)); // Switch to only writing templates only_write_templates(to); - let children = RenderReturn { node: children }; - children.mount.take(); + children.as_vnode().mount.take(); // First always render the children in the background. Rendering the children may cause this boundary to suspend suspense_context.under_suspense_boundary(&dom.runtime(), || { - children.create(dom, parent, Some(to)); + children.as_vnode().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_from_props(&mut *scope_state.props).unwrap(); - props.children = children.clone().node; + props.children = children.clone(); scope_state.last_rendered_node = Some(children); }) } @@ -461,7 +451,7 @@ impl SuspenseBoundaryProps { .unwrap() .clone(); - let last_rendered_node = scope.last_rendered_node.as_ref().unwrap().clone_mounted(); + let last_rendered_node = scope.last_rendered_node.as_ref().unwrap().clone(); let Self { fallback, children, .. @@ -474,17 +464,19 @@ impl SuspenseBoundaryProps { // We already have suspended nodes that still need to be suspended // Just diff the normal and suspended nodes (Some(suspended_nodes), true) => { - let new_suspended_nodes: VNode = RenderReturn { node: children }.into(); + let new_suspended_nodes: VNode = children.into(); // Diff the placeholder nodes in the dom 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()), - }; + let new_placeholder = fallback.call(suspense_context.clone()); - old_placeholder.diff_node(&new_placeholder, dom, to); + old_placeholder.as_vnode().diff_node( + new_placeholder.as_vnode(), + dom, + to, + ); new_placeholder }); @@ -506,10 +498,12 @@ impl SuspenseBoundaryProps { // 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 }; + let new_children = children; suspense_context.under_suspense_boundary(&dom.runtime(), || { - old_children.diff_node(&new_children, dom, to); + old_children + .as_vnode() + .diff_node(new_children.as_vnode(), dom, to); }); // Set the last rendered node to the new children @@ -517,12 +511,10 @@ impl SuspenseBoundaryProps { } // We have no suspended nodes, but we just became suspended. Move the children to the background (None, true) => { - let old_children = last_rendered_node; - let new_children: VNode = RenderReturn { node: children }.into(); + let old_children = last_rendered_node.as_vnode(); + let new_children: VNode = children.into(); - let new_placeholder = RenderReturn { - node: fallback.call(suspense_context.clone()), - }; + let new_placeholder = fallback.call(suspense_context.clone()); // Move the children to the background let mount = old_children.mount.get(); @@ -530,7 +522,7 @@ impl SuspenseBoundaryProps { suspense_context.in_suspense_placeholder(&dom.runtime(), || { old_children.move_node_to_background( - std::slice::from_ref(&*new_placeholder), + std::slice::from_ref(new_placeholder.as_vnode()), parent, dom, to, @@ -559,17 +551,17 @@ impl SuspenseBoundaryProps { // Take the suspended nodes out of the suspense boundary so the children know that the boundary is not suspended while diffing let old_suspended_nodes = suspense_context.take_suspended_nodes().unwrap(); let old_placeholder = last_rendered_node; - let new_children = RenderReturn { node: children }; + let new_children = children; // First diff the two children nodes in the background suspense_context.under_suspense_boundary(&dom.runtime(), || { - old_suspended_nodes.diff_node(&new_children, dom, None::<&mut M>); + old_suspended_nodes.diff_node(new_children.as_vnode(), dom, None::<&mut M>); // Then replace the placeholder with the new children - let mount = old_placeholder.mount.get(); + let mount = old_placeholder.as_vnode().mount.get(); let parent = dom.get_mounted_parent(mount); - old_placeholder.replace( - std::slice::from_ref(&*new_children), + old_placeholder.as_vnode().replace( + std::slice::from_ref(new_children.as_vnode()), parent, dom, to, diff --git a/packages/core/src/suspense/mod.rs b/packages/core/src/suspense/mod.rs index 0fe5e6786..19e3477ca 100644 --- a/packages/core/src/suspense/mod.rs +++ b/packages/core/src/suspense/mod.rs @@ -70,15 +70,6 @@ impl SuspendedFuture { pub fn task(&self) -> Task { self.task } - - /// Clone the future while retaining the mounted information of the future - pub(crate) fn clone_mounted(&self) -> Self { - Self { - task: self.task, - origin: self.origin, - placeholder: self.placeholder.clone_mounted(), - } - } } /// A context with information about suspended components @@ -117,7 +108,7 @@ impl SuspenseContext { .suspended_nodes .borrow() .as_ref() - .map(|node| node.clone_mounted()) + .map(|node| node.clone()) } /// Set the suspense boundary's suspended nodes diff --git a/packages/core/tests/diff_dynamic_node.rs b/packages/core/tests/diff_dynamic_node.rs index a9ac68c66..0453b5b34 100644 --- a/packages/core/tests/diff_dynamic_node.rs +++ b/packages/core/tests/diff_dynamic_node.rs @@ -47,3 +47,68 @@ fn toggle_option_text() { ] ); } + +// Regression test for https://github.com/DioxusLabs/dioxus/issues/2815 +#[test] +fn toggle_template() { + fn app() -> Element { + rsx!( + Comp { + if true { + "{true}" + } + } + ) + } + + #[component] + fn Comp(children: Element) -> Element { + let show = generation() % 2 == 0; + + rsx! { + if show { + {children} + } + } + } + + let mut dom = VirtualDom::new(app); + dom.rebuild(&mut dioxus_core::NoOpMutations); + + // Rendering again should replace the placeholder with an text node + dom.mark_dirty(ScopeId::APP); + assert_eq!( + dom.render_immediate_to_vec().edits, + [ + CreatePlaceholder { id: ElementId(2) }, + ReplaceWith { id: ElementId(1), m: 1 }, + ] + ); + + dom.mark_dirty(ScopeId(ScopeId::APP.0 + 1)); + assert_eq!( + dom.render_immediate_to_vec().edits, + [ + CreateTextNode { value: "true".to_string(), id: ElementId(1) }, + ReplaceWith { id: ElementId(2), m: 1 }, + ] + ); + + dom.mark_dirty(ScopeId(ScopeId::APP.0 + 1)); + assert_eq!( + dom.render_immediate_to_vec().edits, + [ + CreatePlaceholder { id: ElementId(2) }, + ReplaceWith { id: ElementId(1), m: 1 }, + ] + ); + + dom.mark_dirty(ScopeId(ScopeId::APP.0 + 1)); + assert_eq!( + dom.render_immediate_to_vec().edits, + [ + CreateTextNode { value: "true".to_string(), id: ElementId(1) }, + ReplaceWith { id: ElementId(2), m: 1 }, + ] + ); +} diff --git a/packages/core/tests/diff_keyed_list.rs b/packages/core/tests/diff_keyed_list.rs index 3402d5412..81e30ecb6 100644 --- a/packages/core/tests/diff_keyed_list.rs +++ b/packages/core/tests/diff_keyed_list.rs @@ -356,3 +356,92 @@ fn no_common_keys() { ] ); } + +#[test] +fn perfect_reverse() { + let mut dom = VirtualDom::new(|| { + let order: &[_] = match generation() % 2 { + 0 => &[1, 2, 3, 4, 5, 6, 7, 8], + 1 => &[9, 8, 7, 6, 5, 4, 3, 2, 1, 0], + _ => unreachable!(), + }; + + rsx!({ order.iter().map(|i| rsx!(div { key: "{i}" })) }) + }); + + dom.rebuild(&mut dioxus_core::NoOpMutations); + + dom.mark_dirty(ScopeId::APP); + let edits = dom.render_immediate_to_vec().edits; + assert_eq!( + edits, + [ + LoadTemplate { index: 0, id: ElementId(9,) }, + InsertAfter { id: ElementId(1,), m: 1 }, + LoadTemplate { index: 0, id: ElementId(10,) }, + PushRoot { id: ElementId(8,) }, + PushRoot { id: ElementId(7,) }, + PushRoot { id: ElementId(6,) }, + PushRoot { id: ElementId(5,) }, + PushRoot { id: ElementId(4,) }, + PushRoot { id: ElementId(3,) }, + PushRoot { id: ElementId(2,) }, + InsertBefore { id: ElementId(1,), m: 8 }, + ] + ) +} + +#[test] +fn old_middle_empty_left_pivot() { + let mut dom = VirtualDom::new(|| { + let order: &[_] = match generation() % 2 { + 0 => &[/* */ /* */ 6, 7, 8, 9, 10], + 1 => &[/* */ 4, 5, /* */ 6, 7, 8, 9, 10], + _ => unreachable!(), + }; + + rsx!({ order.iter().map(|i| rsx!(div { key: "{i}" })) }) + }); + + dom.rebuild(&mut dioxus_core::NoOpMutations); + + dom.mark_dirty(ScopeId::APP); + let edits = dom.render_immediate_to_vec().edits; + assert_eq!( + edits, + [ + LoadTemplate { index: 0, id: ElementId(6,) }, + LoadTemplate { index: 0, id: ElementId(7,) }, + InsertBefore { id: ElementId(1,), m: 2 }, + ] + ) +} + +#[test] +fn old_middle_empty_right_pivot() { + let mut dom = VirtualDom::new(|| { + let order: &[_] = match generation() % 2 { + 0 => &[1, 2, 3, /* */ 6, 7, 8, 9, 10], + 1 => &[1, 2, 3, /* */ 4, 5, 6, 7, 8, 9, 10 /* */], + + // 0 => &[/* */ 6, 7, 8, 9, 10], + // 1 => &[/* */ 6, 7, 8, 9, 10, /* */ 4, 5], + _ => unreachable!(), + }; + + rsx!({ order.iter().map(|i| rsx!(div { key: "{i}" })) }) + }); + + dom.rebuild(&mut dioxus_core::NoOpMutations); + + dom.mark_dirty(ScopeId::APP); + let edits = dom.render_immediate_to_vec().edits; + assert_eq!( + edits, + [ + LoadTemplate { index: 0, id: ElementId(9) }, + LoadTemplate { index: 0, id: ElementId(10) }, + InsertBefore { id: ElementId(4), m: 2 }, + ] + ); +}