diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index 499d9543ea..3f3629c106 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -1,33 +1,36 @@ -//! This module contains the stateful DiffState and all methods to diff VNodes, their properties, and their children. +#![warn(clippy::pedantic)] +#![allow(clippy::cast_possible_truncation)] + +//! This module contains the stateful [`DiffState`] and all methods to diff [`VNodes`], their properties, and their children. //! //! The [`DiffState`] calculates the diffs between the old and new frames, updates the new nodes, and generates a set -//! of mutations for the RealDom to apply. +//! of mutations for the [`RealDom`] to apply. //! //! ## Notice: //! //! The inspiration and code for this module was originally taken from Dodrio (@fitzgen) and then modified to support -//! Components, Fragments, Suspense, SubTree memoization, incremental diffing, cancellation, NodeRefs, pausing, priority +//! Components, Fragments, Suspense, [`SubTree`] memoization, incremental diffing, cancellation, [`NodeRefs`], pausing, priority //! scheduling, and additional batching operations. //! //! ## Implementation Details: //! //! ### IDs for elements //! -------------------- -//! All nodes are addressed by their IDs. The RealDom provides an imperative interface for making changes to these nodes. +//! All nodes are addressed by their IDs. The [`RealDom`] provides an imperative interface for making changes to these nodes. //! We don't necessarily require that DOM changes happen instantly during the diffing process, so the implementor may choose //! to batch nodes if it is more performant for their application. The element IDs are indices into the internal element //! array. The expectation is that implementors will use the ID as an index into a Vec of real nodes, allowing for passive -//! garbage collection as the VirtualDOM replaces old nodes. +//! garbage collection as the [`VirtualDOM`] replaces old nodes. //! //! When new vnodes are created through `cx.render`, they won't know which real node they correspond to. During diffing, -//! we always make sure to copy over the ID. If we don't do this properly, the ElementId will be populated incorrectly +//! we always make sure to copy over the ID. If we don't do this properly, the [`ElementId`] will be populated incorrectly //! and brick the user's page. //! //! ### Fragment Support //! -------------------- //! Fragments (nodes without a parent) are supported through a combination of "replace with" and anchor vnodes. Fragments //! can be particularly challenging when they are empty, so the anchor node lets us "reserve" a spot for the empty -//! fragment to be replaced with when it is no longer empty. This is guaranteed by logic in the NodeFactory - it is +//! fragment to be replaced with when it is no longer empty. This is guaranteed by logic in the [`NodeFactory`] - it is //! impossible to craft a fragment with 0 elements - they must always have at least a single placeholder element. Adding //! "dummy" nodes _is_ inefficient, but it makes our diffing algorithm faster and the implementation is completely up to //! the platform. @@ -41,13 +44,13 @@ //! into a promise-like value. React will then work on the next "ready" fiber, checking back on the previous fiber once //! it has finished its new work. In Dioxus, we use a similar approach, but try to completely render the tree before //! switching sub-fibers. Instead, each future is submitted into a futures-queue and the node is manually loaded later on. -//! Due to the frequent calls to "yield_now" we can get the pure "fetch-as-you-render" behavior of React Fiber. +//! Due to the frequent calls to [`yield_now`] we can get the pure "fetch-as-you-render" behavior of React Fiber. //! //! We're able to use this approach because we use placeholder nodes - futures that aren't ready still get submitted to //! DOM, but as a placeholder. //! //! Right now, the "suspense" queue is intertwined with hooks. In the future, we should allow any future to drive attributes -//! and contents, without the need for the "use_suspense" hook. In the interim, this is the quickest way to get Suspense working. +//! and contents, without the need for the [`use_suspense`] hook. In the interim, this is the quickest way to get Suspense working. //! //! ## Subtree Memoization //! ----------------------- @@ -88,254 +91,111 @@ //! More info on how to improve this diffing algorithm: //! - -use crate::innerlude::*; +use crate::innerlude::{ + AnyProps, ElementId, Mutations, ScopeArena, ScopeId, VComponent, VElement, VFragment, VNode, + VPlaceholder, VText, +}; use fxhash::{FxHashMap, FxHashSet}; use smallvec::{smallvec, SmallVec}; -use DomEdit::*; - -/// Our DiffState is an iterative tree differ. -/// -/// It uses techniques of a stack machine to allow pausing and restarting of the diff algorithm. This -/// was originally implemented using recursive techniques, but Rust lacks the ability to call async functions recursively, -/// meaning we could not "pause" the original diffing algorithm. -/// -/// Instead, we use a traditional stack machine approach to diff and create new nodes. The diff algorithm periodically -/// calls "yield_now" which allows the machine to pause and return control to the caller. The caller can then wait for -/// the next period of idle time, preventing our diff algorithm from blocking the main thread. -/// -/// Funnily enough, this stack machine's entire job is to create instructions for another stack machine to execute. It's -/// stack machines all the way down! + pub(crate) struct DiffState<'bump> { pub(crate) scopes: &'bump ScopeArena, pub(crate) mutations: Mutations<'bump>, - pub(crate) stack: DiffStack<'bump>, pub(crate) force_diff: bool, + pub(crate) element_stack: SmallVec<[ElementId; 10]>, + pub(crate) scope_stack: SmallVec<[ScopeId; 5]>, } -impl<'bump> DiffState<'bump> { - pub(crate) fn new(scopes: &'bump ScopeArena) -> Self { +impl<'b> DiffState<'b> { + pub fn new(scopes: &'b ScopeArena) -> Self { Self { scopes, mutations: Mutations::new(), - stack: DiffStack::new(), force_diff: false, - } - } -} - -/// The stack instructions we use to diff and create new nodes. -#[derive(Debug)] -pub(crate) enum DiffInstruction<'a> { - Diff { - old: &'a VNode<'a>, - new: &'a VNode<'a>, - }, - - Create { - node: &'a VNode<'a>, - }, - - /// pushes the node elements onto the stack for use in mount - PrepareMove { - node: &'a VNode<'a>, - }, - - Mount { - and: MountType<'a>, - }, - - PopScope, - PopElement, -} - -#[derive(Debug, Clone, Copy)] -pub(crate) enum MountType<'a> { - Absorb, - Append, - Replace { old: &'a VNode<'a> }, - InsertAfter { other_node: &'a VNode<'a> }, - InsertBefore { other_node: &'a VNode<'a> }, -} - -pub(crate) struct DiffStack<'bump> { - pub(crate) instructions: Vec>, - pub(crate) nodes_created_stack: SmallVec<[usize; 10]>, - pub(crate) scope_stack: SmallVec<[ScopeId; 5]>, - pub(crate) element_stack: SmallVec<[ElementId; 10]>, -} - -impl<'bump> DiffStack<'bump> { - fn new() -> Self { - Self { - instructions: Vec::with_capacity(1000), - nodes_created_stack: smallvec![], - scope_stack: smallvec![], element_stack: smallvec![], + scope_stack: smallvec![], } } - fn pop(&mut self) -> Option> { - self.instructions.pop() - } - - fn pop_off_scope(&mut self) { - self.scope_stack.pop(); - } - - pub(crate) fn push(&mut self, instruction: DiffInstruction<'bump>) { - self.instructions.push(instruction) - } - - fn create_children(&mut self, children: &'bump [VNode<'bump>], and: MountType<'bump>) { - self.nodes_created_stack.push(0); - self.instructions.push(DiffInstruction::Mount { and }); - - for child in children.iter().rev() { - self.instructions - .push(DiffInstruction::Create { node: child }); - } - } - - // todo: subtrees - // fn push_subtree(&mut self) { - // self.nodes_created_stack.push(0); - // self.instructions.push(DiffInstruction::Mount { - // and: MountType::Append, - // }); - // } + pub fn diff_scope(&mut self, scopeid: ScopeId) { + let (old, new) = (self.scopes.wip_head(scopeid), self.scopes.fin_head(scopeid)); + self.scope_stack.push(scopeid); + let scope = self.scopes.get_scope(scopeid).unwrap(); + self.element_stack.push(scope.container); - fn push_nodes_created(&mut self, count: usize) { - self.nodes_created_stack.push(count); - } + self.diff_node(old, new); - pub(crate) fn create_node(&mut self, node: &'bump VNode<'bump>, and: MountType<'bump>) { - self.nodes_created_stack.push(0); - self.instructions.push(DiffInstruction::Mount { and }); - self.instructions.push(DiffInstruction::Create { node }); + self.mutations.mark_dirty_scope(scopeid); } - fn add_child_count(&mut self, count: usize) { - *self.nodes_created_stack.last_mut().unwrap() += count; - } - - fn pop_nodes_created(&mut self) -> usize { - self.nodes_created_stack.pop().unwrap() - } - - fn current_scope(&self) -> Option { - self.scope_stack.last().copied() - } - - fn create_component(&mut self, idx: ScopeId, node: &'bump VNode<'bump>) { - // Push the new scope onto the stack - self.scope_stack.push(idx); - - self.instructions.push(DiffInstruction::PopScope); - - // Run the creation algorithm with this scope on the stack - // ?? I think we treat components as fragments?? - self.instructions.push(DiffInstruction::Create { node }); - } -} - -impl<'bump> DiffState<'bump> { - /// Progress the diffing for this "fiber" - /// - /// This method implements a depth-first iterative tree traversal. - /// - /// We do depth-first to maintain high cache locality (nodes were originally generated recursively). - /// - /// Returns a `bool` indicating that the work completed properly. - pub fn work(&mut self, mut deadline_expired: impl FnMut() -> bool) -> bool { - while let Some(instruction) = self.stack.pop() { - match instruction { - DiffInstruction::Diff { old, new } => self.diff_node(old, new), - DiffInstruction::Create { node } => self.create_node(node), - DiffInstruction::Mount { and } => self.mount(and), - DiffInstruction::PrepareMove { node } => { - let num_on_stack = self.push_all_nodes(node); - self.stack.add_child_count(num_on_stack); - } - DiffInstruction::PopScope => self.stack.pop_off_scope(), - DiffInstruction::PopElement => { - self.stack.element_stack.pop(); + pub fn diff_node(&mut self, old_node: &'b VNode<'b>, new_node: &'b VNode<'b>) { + use VNode::{Component, Element, Fragment, Placeholder, Text}; + match (old_node, new_node) { + // Check the most common cases first + // these are *actual* elements, not wrappers around lists + (Text(old), Text(new)) => { + if std::ptr::eq(old, new) { + return; } - }; - if deadline_expired() { - log::trace!("Deadline expired before we could finish!"); - return false; - } - } + let root = old + .id + .get() + .expect("existing text nodes should have an ElementId"); - true - } - - // recursively push all the nodes of a tree onto the stack and return how many are there - fn push_all_nodes(&mut self, node: &'bump VNode<'bump>) -> usize { - match node { - VNode::Text(_) | VNode::Placeholder(_) => { - self.mutations.push_root(node.mounted_id()); - 1 - } - - VNode::Fragment(_) | VNode::Component(_) => { - // - let mut added = 0; - for child in node.children() { - added += self.push_all_nodes(child); + if old.text != new.text { + self.mutations.set_text(new.text, root.as_u64()); } - added + self.scopes.update_node(new_node, root); + + new.id.set(Some(root)); } - VNode::Element(el) => { - let mut num_on_stack = 0; - for child in el.children.iter() { - num_on_stack += self.push_all_nodes(child); + (Placeholder(old), Placeholder(new)) => { + if std::ptr::eq(old, new) { + return; } - self.mutations.push_root(el.id.get().unwrap()); - num_on_stack + 1 - } - } - } + let root = old + .id + .get() + .expect("existing placeholder nodes should have an ElementId"); - fn mount(&mut self, and: MountType<'bump>) { - let nodes_created = self.stack.pop_nodes_created(); - match and { - // add the nodes from this virtual list to the parent - // used by fragments and components - MountType::Absorb => { - self.stack.add_child_count(nodes_created); + self.scopes.update_node(new_node, root); + new.id.set(Some(root)); } - MountType::Replace { old } => { - self.replace_node(old, nodes_created); + (Element(old), Element(new)) => { + if std::ptr::eq(old, new) { + return; + } + self.diff_element_nodes(old, new, old_node, new_node); } - MountType::Append => { - self.mutations.edits.push(AppendChildren { - many: nodes_created as u32, - }); + // These two sets are pointers to nodes but are not actually nodes themselves + (Component(old), Component(new)) => { + if std::ptr::eq(old, new) { + return; + } + self.diff_component_nodes(old_node, new_node, *old, *new); } - MountType::InsertAfter { other_node } => { - let root = self.find_last_element(other_node).unwrap(); - self.mutations.insert_after(root, nodes_created as u32); + (Fragment(old), Fragment(new)) => { + if std::ptr::eq(old, new) { + return; + } + self.diff_fragment_nodes(old, new); } - MountType::InsertBefore { other_node } => { - let root = self.find_first_element_id(other_node).unwrap(); - self.mutations.insert_before(root, nodes_created as u32); - } + // Anything else is just a basic replace and create + ( + Component(_) | Fragment(_) | Text(_) | Element(_) | Placeholder(_), + Component(_) | Fragment(_) | Text(_) | Element(_) | Placeholder(_), + ) => self.replace_node(old_node, new_node), } } - // ================================= - // Tools for creating new nodes - // ================================= - - fn create_node(&mut self, node: &'bump VNode<'bump>) { + pub fn create_node(&mut self, node: &'b VNode<'b>) -> usize { match node { VNode::Text(vtext) => self.create_text_node(vtext, node), VNode::Placeholder(anchor) => self.create_anchor_node(anchor, node), @@ -345,24 +205,21 @@ impl<'bump> DiffState<'bump> { } } - fn create_text_node(&mut self, vtext: &'bump VText<'bump>, node: &'bump VNode<'bump>) { + fn create_text_node(&mut self, text: &'b VText<'b>, node: &'b VNode<'b>) -> usize { let real_id = self.scopes.reserve_node(node); - - self.mutations.create_text_node(vtext.text, real_id); - vtext.id.set(Some(real_id)); - self.stack.add_child_count(1); + text.id.set(Some(real_id)); + self.mutations.create_text_node(text.text, real_id); + 1 } - fn create_anchor_node(&mut self, anchor: &'bump VPlaceholder, node: &'bump VNode<'bump>) { + fn create_anchor_node(&mut self, anchor: &'b VPlaceholder, node: &'b VNode<'b>) -> usize { let real_id = self.scopes.reserve_node(node); - - self.mutations.create_placeholder(real_id); anchor.id.set(Some(real_id)); - - self.stack.add_child_count(1); + self.mutations.create_placeholder(real_id); + 1 } - fn create_element_node(&mut self, element: &'bump VElement<'bump>, node: &'bump VNode<'bump>) { + fn create_element_node(&mut self, element: &'b VElement<'b>, node: &'b VNode<'b>) -> usize { let VElement { tag: tag_name, listeners, @@ -372,174 +229,120 @@ impl<'bump> DiffState<'bump> { id: dom_id, parent: parent_id, .. - } = element; + } = &element; - // set the parent ID for event bubbling - self.stack.instructions.push(DiffInstruction::PopElement); + parent_id.set(self.element_stack.last().copied()); - let parent = self.stack.element_stack.last().unwrap(); - parent_id.set(Some(*parent)); - - // set the id of the element let real_id = self.scopes.reserve_node(node); - self.stack.element_stack.push(real_id); + dom_id.set(Some(real_id)); - self.mutations.create_element(tag_name, *namespace, real_id); + self.element_stack.push(real_id); + { + self.mutations.create_element(tag_name, *namespace, real_id); - self.stack.add_child_count(1); + let cur_scope_id = self + .current_scope() + .expect("diffing should always have a scope"); - if let Some(cur_scope_id) = self.stack.current_scope() { - for listener in *listeners { + for listener in listeners.iter() { listener.mounted_node.set(Some(real_id)); self.mutations.new_event_listener(listener, cur_scope_id); } - } else { - log::warn!("create element called with no scope on the stack - this is an error for a live dom"); - } - - for attr in *attributes { - self.mutations.set_attribute(attr, real_id.as_u64()); - } - // todo: the settext optimization - // - // if children.len() == 1 { - // if let VNode::Text(vtext) = children[0] { - // self.mutations.set_text(vtext.text, real_id.as_u64()); - // return; - // } - // } + for attr in attributes.iter() { + self.mutations.set_attribute(attr, real_id.as_u64()); + } - if !children.is_empty() { - self.stack.create_children(children, MountType::Append); + if !children.is_empty() { + self.create_and_append_children(children); + } } + self.element_stack.pop(); + + 1 } - fn create_fragment_node(&mut self, frag: &'bump VFragment<'bump>) { - self.stack.create_children(frag.children, MountType::Absorb); + fn create_fragment_node(&mut self, frag: &'b VFragment<'b>) -> usize { + self.create_children(frag.children) } - fn create_component_node(&mut self, vcomponent: &'bump VComponent<'bump>) { - let parent_idx = self.stack.current_scope().unwrap(); + fn create_component_node(&mut self, vcomponent: &'b VComponent<'b>) -> usize { + let parent_idx = self.current_scope().unwrap(); - // the component might already exist - if it does, we need to reuse it - // this makes figure out when to drop the component more complicated - let new_idx = if let Some(idx) = vcomponent.scope.get() { - assert!(self.scopes.get_scope(idx).is_some()); - idx - } else { - // Insert a new scope into our component list - let props: Box = vcomponent.props.borrow_mut().take().unwrap(); - let props: Box = unsafe { std::mem::transmute(props) }; - let new_idx = self.scopes.new_with_key( - vcomponent.user_fc, - props, - Some(parent_idx), - self.stack.element_stack.last().copied().unwrap(), - 0, - ); + // ensure this scope doesn't already exist if we're trying to create it + debug_assert!( + vcomponent + .scope + .get() + .and_then(|f| self.scopes.get_scope(f)) + .is_none(), + "component scope already exists" + ); - new_idx - }; + // Insert a new scope into our component list + let props: Box = vcomponent.props.borrow_mut().take().unwrap(); + let props: Box = unsafe { std::mem::transmute(props) }; + let new_idx = self.scopes.new_with_key( + vcomponent.user_fc, + props, + Some(parent_idx), + self.element_stack.last().copied().unwrap(), + 0, + ); // Actually initialize the caller's slot with the right address vcomponent.scope.set(Some(new_idx)); - match vcomponent.can_memoize { - true => { - // todo: implement promotion logic. save us from boxing props that we don't need - } - false => { - // track this component internally so we know the right drop order - } - } - - // Run the scope for one iteration to initialize it - self.scopes.run_scope(new_idx); - - // Take the node that was just generated from running the component - let nextnode = self.scopes.fin_head(new_idx); - self.stack.create_component(new_idx, nextnode); - - // Finally, insert this scope as a seen node. - self.mutations.dirty_scopes.insert(new_idx); - } - - // ================================= - // Tools for diffing nodes - // ================================= - - pub fn diff_node(&mut self, old_node: &'bump VNode<'bump>, new_node: &'bump VNode<'bump>) { - use VNode::*; - match (old_node, new_node) { - // Check the most common cases first - // these are *actual* elements, not wrappers around lists - (Text(old), Text(new)) => { - if let Some(root) = old.id.get() { - if old.text != new.text { - self.mutations.set_text(new.text, root.as_u64()); - } - self.scopes.update_node(new_node, root); - - new.id.set(Some(root)); - } - } + log::trace!( + "created component \"{}\", id: {:?} parent {:?} orig: {:?}", + vcomponent.fn_name, + new_idx, + parent_idx, + vcomponent.originator + ); - (Placeholder(old), Placeholder(new)) => { - if let Some(root) = old.id.get() { - self.scopes.update_node(new_node, root); - new.id.set(Some(root)) - } - } + // if vcomponent.can_memoize { + // // todo: implement promotion logic. save us from boxing props that we don't need + // } else { + // // track this component internally so we know the right drop order + // } - (Element(old), Element(new)) => self.diff_element_nodes(old, new, old_node, new_node), + self.enter_scope(new_idx); - // These two sets are pointers to nodes but are not actually nodes themselves - (Component(old), Component(new)) => { - self.diff_component_nodes(old_node, new_node, *old, *new) - } + let created = { + // Run the scope for one iteration to initialize it + self.scopes.run_scope(new_idx); + self.mutations.mark_dirty_scope(new_idx); - (Fragment(old), Fragment(new)) => self.diff_fragment_nodes(old, new), + // Take the node that was just generated from running the component + let nextnode = self.scopes.fin_head(new_idx); + self.create_node(nextnode) + }; - // The normal pathway still works, but generates slightly weird instructions - // This pathway ensures uses the ReplaceAll, not the InsertAfter and remove - (Placeholder(_), Fragment(new)) => { - self.stack - .create_children(new.children, MountType::Replace { old: old_node }); - } + self.leave_scope(); - // Anything else is just a basic replace and create - ( - Component(_) | Fragment(_) | Text(_) | Element(_) | Placeholder(_), - Component(_) | Fragment(_) | Text(_) | Element(_) | Placeholder(_), - ) => self - .stack - .create_node(new_node, MountType::Replace { old: old_node }), - } + created } fn diff_element_nodes( &mut self, - old: &'bump VElement<'bump>, - new: &'bump VElement<'bump>, - old_node: &'bump VNode<'bump>, - new_node: &'bump VNode<'bump>, + old: &'b VElement<'b>, + new: &'b VElement<'b>, + old_node: &'b VNode<'b>, + new_node: &'b VNode<'b>, ) { - let root = old.id.get().unwrap(); + let root = old + .id + .get() + .expect("existing element nodes should have an ElementId"); // If the element type is completely different, the element needs to be re-rendered completely // This is an optimization React makes due to how users structure their code // // This case is rather rare (typically only in non-keyed lists) if new.tag != old.tag || new.namespace != old.namespace { - // maybe make this an instruction? - // issue is that we need the "vnode" but this method only has the velement - self.stack.push_nodes_created(0); - self.stack.push(DiffInstruction::Mount { - and: MountType::Replace { old: old_node }, - }); - self.create_element_node(new, new_node); + self.replace_node(old_node, new_node); return; } @@ -570,7 +373,7 @@ impl<'bump> DiffState<'bump> { self.mutations.remove_attribute(attribute, root.as_u64()); } for attribute in new.attributes { - self.mutations.set_attribute(attribute, root.as_u64()) + self.mutations.set_attribute(attribute, root.as_u64()); } } @@ -582,7 +385,7 @@ impl<'bump> DiffState<'bump> { // We also need to make sure that all listeners are properly attached to the parent scope (fix_listener) // // TODO: take a more efficient path than this - if let Some(cur_scope_id) = self.stack.current_scope() { + if let Some(cur_scope_id) = self.current_scope() { if old.listeners.len() == new.listeners.len() { for (old_l, new_l) in old.listeners.iter().zip(new.listeners.iter()) { if old_l.event != new_l.event { @@ -604,159 +407,97 @@ impl<'bump> DiffState<'bump> { } } - if old.children.is_empty() && !new.children.is_empty() { - self.mutations.edits.push(PushRoot { - root: root.as_u64(), - }); - self.stack.element_stack.push(root); - self.stack.instructions.push(DiffInstruction::PopElement); - self.stack.create_children(new.children, MountType::Append); - } else { - self.stack.element_stack.push(root); - self.stack.instructions.push(DiffInstruction::PopElement); - self.diff_children(old.children, new.children); - } - - // todo: this is for the "settext" optimization - // it works, but i'm not sure if it's the direction we want to take right away - // I haven't benchmarked the performance imporvemenet yet. Perhaps - // we can make it a config? - - // match (old.children.len(), new.children.len()) { - // (0, 0) => {} - // (1, 1) => { - // let old1 = &old.children[0]; - // let new1 = &new.children[0]; - - // match (old1, new1) { - // (VNode::Text(old_text), VNode::Text(new_text)) => { - // if old_text.text != new_text.text { - // self.mutations.set_text(new_text.text, root.as_u64()); - // } - // } - // (VNode::Text(_old_text), _) => { - // self.stack.element_stack.push(root); - // self.stack.instructions.push(DiffInstruction::PopElement); - // self.stack.create_node(new1, MountType::Append); - // } - // (_, VNode::Text(new_text)) => { - // self.remove_nodes([old1], false); - // self.mutations.set_text(new_text.text, root.as_u64()); - // } - // _ => { - // self.stack.element_stack.push(root); - // self.stack.instructions.push(DiffInstruction::PopElement); - // self.diff_children(old.children, new.children); - // } - // } - // } - // (0, 1) => { - // if let VNode::Text(text) = &new.children[0] { - // self.mutations.set_text(text.text, root.as_u64()); - // } else { - // self.stack.element_stack.push(root); - // self.stack.instructions.push(DiffInstruction::PopElement); - // } - // } - // (0, _) => { - // self.mutations.edits.push(PushRoot { - // root: root.as_u64(), - // }); - // self.stack.element_stack.push(root); - // self.stack.instructions.push(DiffInstruction::PopElement); - // self.stack.create_children(new.children, MountType::Append); - // } - // (_, 0) => { - // self.remove_nodes(old.children, false); - // self.mutations.set_text("", root.as_u64()); - // } - // (_, _) => { - // self.stack.element_stack.push(root); - // self.stack.instructions.push(DiffInstruction::PopElement); - // self.diff_children(old.children, new.children); - // } - // } + match (old.children.len(), new.children.len()) { + (0, 0) => {} + (0, _) => { + let created = self.create_children(new.children); + self.mutations.append_children(created as u32); + } + (_, _) => self.diff_children(old.children, new.children), + }; } fn diff_component_nodes( &mut self, - old_node: &'bump VNode<'bump>, - new_node: &'bump VNode<'bump>, - old: &'bump VComponent<'bump>, - new: &'bump VComponent<'bump>, + old_node: &'b VNode<'b>, + new_node: &'b VNode<'b>, + old: &'b VComponent<'b>, + new: &'b VComponent<'b>, ) { - let scope_addr = old.scope.get().unwrap(); - log::trace!("diff_component_nodes: {:?}", scope_addr); + let scope_addr = old + .scope + .get() + .expect("existing component nodes should have a scope"); if std::ptr::eq(old, new) { - log::trace!("skipping component diff - component is the sames"); return; } // Make sure we're dealing with the same component (by function pointer) if old.user_fc == new.user_fc { - self.stack.scope_stack.push(scope_addr); - - // Make sure the new component vnode is referencing the right scope id - new.scope.set(Some(scope_addr)); - - // make sure the component's caller function is up to date - let scope = self - .scopes - .get_scope(scope_addr) - .unwrap_or_else(|| panic!("could not find {:?}", scope_addr)); - - // take the new props out regardless - // when memoizing, push to the existing scope if memoization happens - let new_props = new.props.borrow_mut().take().unwrap(); - - let should_run = { - if old.can_memoize { - let props_are_the_same = unsafe { - scope - .props - .borrow() - .as_ref() - .unwrap() - .memoize(new_props.as_ref()) - }; - !props_are_the_same || self.force_diff - } else { - true - } - }; - - if should_run { - let _old_props = scope + self.enter_scope(scope_addr); + { + // Make sure the new component vnode is referencing the right scope id + new.scope.set(Some(scope_addr)); + + // make sure the component's caller function is up to date + let scope = self + .scopes + .get_scope(scope_addr) + .unwrap_or_else(|| panic!("could not find {:?}", scope_addr)); + + // take the new props out regardless + // when memoizing, push to the existing scope if memoization happens + let new_props = new .props - .replace(unsafe { std::mem::transmute(Some(new_props)) }); + .borrow_mut() + .take() + .expect("new component props should exist"); + + let should_diff = { + if old.can_memoize { + // safety: we trust the implementation of "memoize" + let props_are_the_same = unsafe { + let new_ref = new_props.as_ref(); + scope.props.borrow().as_ref().unwrap().memoize(new_ref) + }; + !props_are_the_same || self.force_diff + } else { + true + } + }; - // this should auto drop the previous props - self.scopes.run_scope(scope_addr); - self.mutations.dirty_scopes.insert(scope_addr); + if should_diff { + let _old_props = scope + .props + .replace(unsafe { std::mem::transmute(Some(new_props)) }); - self.diff_node( - self.scopes.wip_head(scope_addr), - self.scopes.fin_head(scope_addr), - ); - } else { - log::trace!("memoized"); - // memoization has taken place - drop(new_props); - }; + // this should auto drop the previous props + self.scopes.run_scope(scope_addr); + self.mutations.mark_dirty_scope(scope_addr); - self.stack.scope_stack.pop(); + self.diff_node( + self.scopes.wip_head(scope_addr), + self.scopes.fin_head(scope_addr), + ); + } else { + // memoization has taken place + drop(new_props); + }; + } + self.leave_scope(); } else { - self.stack - .create_node(new_node, MountType::Replace { old: old_node }); + self.replace_node(old_node, new_node); } } - fn diff_fragment_nodes(&mut self, old: &'bump VFragment<'bump>, new: &'bump VFragment<'bump>) { + fn diff_fragment_nodes(&mut self, old: &'b VFragment<'b>, new: &'b VFragment<'b>) { // This is the case where options or direct vnodes might be used. // In this case, it's faster to just skip ahead to their diff if old.children.len() == 1 && new.children.len() == 1 { - self.diff_node(&old.children[0], &new.children[0]); + if !std::ptr::eq(old, new) { + self.diff_node(&old.children[0], &new.children[0]); + } return; } @@ -766,10 +507,6 @@ impl<'bump> DiffState<'bump> { self.diff_children(old.children, new.children); } - // ============================================= - // Utilities for creating new diff instructions - // ============================================= - // Diff the given set of old and new children. // // The parent must be on top of the change list stack when this function is @@ -785,11 +522,15 @@ impl<'bump> DiffState<'bump> { // // Fragment nodes cannot generate empty children lists, so we can assume that when a list is empty, it belongs only // to an element, and appending makes sense. - fn diff_children(&mut self, old: &'bump [VNode<'bump>], new: &'bump [VNode<'bump>]) { + fn diff_children(&mut self, old: &'b [VNode<'b>], new: &'b [VNode<'b>]) { + if std::ptr::eq(old, new) { + return; + } + // Remember, fragments can never be empty (they always have a single child) match (old, new) { ([], []) => {} - ([], _) => self.stack.create_children(new, MountType::Append), + ([], _) => self.create_and_append_children(new), (_, []) => self.remove_nodes(old, true), _ => { let new_is_keyed = new[0].key().is_some(); @@ -821,29 +562,21 @@ impl<'bump> DiffState<'bump> { // [... parent] // // the change list stack is in the same state when this function returns. - fn diff_non_keyed_children(&mut self, old: &'bump [VNode<'bump>], new: &'bump [VNode<'bump>]) { + fn diff_non_keyed_children(&mut self, old: &'b [VNode<'b>], new: &'b [VNode<'b>]) { + use std::cmp::Ordering; + // Handled these cases in `diff_children` before calling this function. debug_assert!(!new.is_empty()); debug_assert!(!old.is_empty()); - for (new, old) in new.iter().zip(old.iter()).rev() { - self.stack.push(DiffInstruction::Diff { new, old }); - } - - use std::cmp::Ordering; match old.len().cmp(&new.len()) { Ordering::Greater => self.remove_nodes(&old[new.len()..], true), - Ordering::Less => { - self.stack.create_children( - &new[old.len()..], - MountType::InsertAfter { - other_node: old.last().unwrap(), - }, - ); - } - Ordering::Equal => { - // nothing - they're the same size - } + Ordering::Less => self.create_and_insert_after(&new[old.len()..], old.last().unwrap()), + Ordering::Equal => {} + } + + for (new, old) in new.iter().zip(old.iter()) { + self.diff_node(old, new); } } @@ -863,10 +596,10 @@ impl<'bump> DiffState<'bump> { // https://github.com/infernojs/inferno/blob/36fd96/packages/inferno/src/DOM/patching.ts#L530-L739 // // The stack is empty upon entry. - fn diff_keyed_children(&mut self, old: &'bump [VNode<'bump>], new: &'bump [VNode<'bump>]) { + fn diff_keyed_children(&mut self, old: &'b [VNode<'b>], new: &'b [VNode<'b>]) { if cfg!(debug_assertions) { let mut keys = fxhash::FxHashSet::default(); - let mut assert_unique_keys = |children: &'bump [VNode<'bump>]| { + let mut assert_unique_keys = |children: &'b [VNode<'b>]| { keys.clear(); for child in children { let key = child.key(); @@ -907,6 +640,7 @@ impl<'bump> DiffState<'bump> { !((old_middle.len() == new_middle.len()) && old_middle.is_empty()), "keyed children must have the same number of children" ); + if new_middle.is_empty() { // remove the old elements self.remove_nodes(old_middle, true); @@ -916,30 +650,15 @@ impl<'bump> DiffState<'bump> { if left_offset == 0 { // insert at the beginning of the old list let foothold = &old[old.len() - right_offset]; - self.stack.create_children( - new_middle, - MountType::InsertBefore { - other_node: foothold, - }, - ); + self.create_and_insert_before(new_middle, foothold); } else if right_offset == 0 { // insert at the end the old list let foothold = old.last().unwrap(); - self.stack.create_children( - new_middle, - MountType::InsertAfter { - other_node: foothold, - }, - ); + self.create_and_insert_after(new_middle, foothold); } else { // inserting in the middle let foothold = &old[left_offset - 1]; - self.stack.create_children( - new_middle, - MountType::InsertAfter { - other_node: foothold, - }, - ); + self.create_and_insert_after(new_middle, foothold); } } else { self.diff_keyed_middle(old_middle, new_middle); @@ -953,9 +672,8 @@ impl<'bump> DiffState<'bump> { /// If there is no offset, then this function returns None and the diffing is complete. fn diff_keyed_ends( &mut self, - - old: &'bump [VNode<'bump>], - new: &'bump [VNode<'bump>], + old: &'b [VNode<'b>], + new: &'b [VNode<'b>], ) -> Option<(usize, usize)> { let mut left_offset = 0; @@ -964,19 +682,14 @@ impl<'bump> DiffState<'bump> { if old.key() != new.key() { break; } - self.stack.push(DiffInstruction::Diff { old, new }); + self.diff_node(old, new); left_offset += 1; } // 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.stack.create_children( - &new[left_offset..], - MountType::InsertAfter { - other_node: old.last().unwrap(), - }, - ); + self.create_and_insert_after(&new[left_offset..], old.last().unwrap()); return None; } @@ -1014,7 +727,8 @@ impl<'bump> DiffState<'bump> { // This function will load the appropriate nodes onto the stack and do diffing in place. // // Upon exit from this function, it will be restored to that same self. - fn diff_keyed_middle(&mut self, old: &'bump [VNode<'bump>], new: &'bump [VNode<'bump>]) { + #[allow(clippy::too_many_lines)] + fn diff_keyed_middle(&mut self, old: &'b [VNode<'b>], new: &'b [VNode<'b>]) { /* 1. Map the old keys into a numerical ordering based on indices. 2. Create a map of old key to its index @@ -1039,8 +753,8 @@ impl<'bump> DiffState<'bump> { // 0. Debug sanity checks // Should have already diffed the shared-key prefixes and suffixes. - debug_assert_ne!(new.first().map(|n| n.key()), old.first().map(|o| o.key())); - debug_assert_ne!(new.last().map(|n| n.key()), old.last().map(|o| o.key())); + debug_assert_ne!(new.first().map(VNode::key), old.first().map(VNode::key)); + debug_assert_ne!(new.last().map(VNode::key), old.last().map(VNode::key)); // 1. Map the old keys into a numerical ordering based on indices. // 2. Create a map of old key to its index @@ -1072,10 +786,12 @@ impl<'bump> DiffState<'bump> { if shared_keys.is_empty() { if let Some(first_old) = old.get(0) { self.remove_nodes(&old[1..], true); - self.stack - .create_children(new, MountType::Replace { old: first_old }) + let nodes_created = self.create_children(new); + self.replace_inner(first_old, nodes_created); } else { - self.stack.create_children(new, MountType::Append {}); + // I think this is wrong - why are we appending? + // only valid of the if there are no trailing elements + self.create_and_append_children(new); } return; } @@ -1103,33 +819,31 @@ impl<'bump> DiffState<'bump> { lis_sequence.pop(); } - let apply = |new_idx, new_node: &'bump VNode<'bump>, stack: &mut DiffStack<'bump>| { - let old_index = new_index_to_old_index[new_idx]; - if old_index == u32::MAX as usize { - stack.create_node(new_node, MountType::Absorb); - } else { - // this function should never take LIS indices - stack.push(DiffInstruction::PrepareMove { node: new_node }); - stack.push(DiffInstruction::Diff { - new: new_node, - old: &old[old_index], - }); - } - }; + for idx in &lis_sequence { + self.diff_node(&old[new_index_to_old_index[*idx]], &new[*idx]); + } - // add mount instruction for the last items not covered by the lis - let first_lis = *lis_sequence.first().unwrap(); - if first_lis > 0 { - self.stack.push_nodes_created(0); - self.stack.push(DiffInstruction::Mount { - and: MountType::InsertBefore { - other_node: &new[first_lis], - }, - }); - - for (idx, new_node) in new[..first_lis].iter().enumerate().rev() { - apply(idx, new_node, &mut self.stack); + let mut nodes_created = 0; + + // add mount instruction for the first items not covered by the lis + let last = *lis_sequence.last().unwrap(); + if last < (new.len() - 1) { + for (idx, new_node) in new[(last + 1)..].iter().enumerate() { + let new_idx = idx + last + 1; + let old_index = new_index_to_old_index[new_idx]; + if old_index == u32::MAX as usize { + nodes_created += self.create_node(new_node); + } else { + self.diff_node(&old[old_index], new_node); + nodes_created += self.push_all_nodes(new_node); + } } + + self.mutations.insert_after( + self.find_last_element(&new[last]).unwrap(), + nodes_created as u32, + ); + nodes_created = 0; } // for each spacing, generate a mount instruction @@ -1137,85 +851,53 @@ impl<'bump> DiffState<'bump> { let mut last = *lis_iter.next().unwrap(); for next in lis_iter { if last - next > 1 { - self.stack.push_nodes_created(0); - self.stack.push(DiffInstruction::Mount { - and: MountType::InsertBefore { - other_node: &new[last], - }, - }); - for (idx, new_node) in new[(next + 1)..last].iter().enumerate().rev() { - apply(idx + next + 1, new_node, &mut self.stack); + for (idx, new_node) in new[(next + 1)..last].iter().enumerate() { + let new_idx = idx + next + 1; + let old_index = new_index_to_old_index[new_idx]; + if old_index == u32::MAX as usize { + nodes_created += self.create_node(new_node); + } else { + self.diff_node(&old[old_index], new_node); + nodes_created += self.push_all_nodes(new_node); + } } - } - last = *next; - } - // add mount instruction for the first items not covered by the lis - let last = *lis_sequence.last().unwrap(); - if last < (new.len() - 1) { - self.stack.push_nodes_created(0); - self.stack.push(DiffInstruction::Mount { - and: MountType::InsertAfter { - other_node: &new[last], - }, - }); - for (idx, new_node) in new[(last + 1)..].iter().enumerate().rev() { - apply(idx + last + 1, new_node, &mut self.stack); - } - } + self.mutations.insert_before( + self.find_first_element(&new[last]).unwrap(), + nodes_created as u32, + ); - for idx in lis_sequence.iter().rev() { - self.stack.push(DiffInstruction::Diff { - new: &new[*idx], - old: &old[new_index_to_old_index[*idx]], - }); + nodes_created = 0; + } + last = *next; } - } - - // ===================== - // Utilities - // ===================== - fn find_last_element(&mut self, vnode: &'bump VNode<'bump>) -> Option { - let mut search_node = Some(vnode); - - loop { - match &search_node.take().unwrap() { - VNode::Text(t) => break t.id.get(), - VNode::Element(t) => break t.id.get(), - VNode::Placeholder(t) => break t.id.get(), - VNode::Fragment(frag) => { - search_node = frag.children.last(); - } - VNode::Component(el) => { - let scope_id = el.scope.get().unwrap(); - search_node = Some(self.scopes.root_node(scope_id)); + // add mount instruction for the last items not covered by the lis + let first_lis = *lis_sequence.first().unwrap(); + if first_lis > 0 { + for (idx, new_node) in new[..first_lis].iter().enumerate() { + let old_index = new_index_to_old_index[idx]; + if old_index == u32::MAX as usize { + nodes_created += self.create_node(new_node); + } else { + self.diff_node(&old[old_index], new_node); + nodes_created += self.push_all_nodes(new_node); } } + + self.mutations.insert_before( + self.find_first_element(&new[first_lis]).unwrap(), + nodes_created as u32, + ); } } - fn find_first_element_id(&mut self, vnode: &'bump VNode<'bump>) -> Option { - let mut search_node = Some(vnode); - - loop { - match &search_node.take().unwrap() { - // the ones that have a direct id - VNode::Fragment(frag) => { - search_node = Some(&frag.children[0]); - } - VNode::Component(el) => { - let scope_id = el.scope.get().unwrap(); - search_node = Some(self.scopes.root_node(scope_id)); - } - VNode::Text(t) => break t.id.get(), - VNode::Element(t) => break t.id.get(), - VNode::Placeholder(t) => break t.id.get(), - } - } + fn replace_node(&mut self, old: &'b VNode<'b>, new: &'b VNode<'b>) { + let nodes_created = self.create_node(new); + self.replace_inner(old, nodes_created); } - fn replace_node(&mut self, old: &'bump VNode<'bump>, nodes_created: usize) { + fn replace_inner(&mut self, old: &'b VNode<'b>, nodes_created: usize) { match old { VNode::Element(el) => { let id = old @@ -1224,6 +906,7 @@ impl<'bump> DiffState<'bump> { self.mutations.replace_with(id, nodes_created as u32); self.remove_nodes(el.children, false); + self.scopes.collect_garbage(id); } VNode::Text(_) | VNode::Placeholder(_) => { @@ -1232,35 +915,38 @@ impl<'bump> DiffState<'bump> { .unwrap_or_else(|| panic!("broke on {:?}", old)); self.mutations.replace_with(id, nodes_created as u32); + self.scopes.collect_garbage(id); } VNode::Fragment(f) => { - self.replace_node(&f.children[0], nodes_created); + self.replace_inner(&f.children[0], nodes_created); self.remove_nodes(f.children.iter().skip(1), true); } VNode::Component(c) => { - let node = self.scopes.fin_head(c.scope.get().unwrap()); - self.replace_node(node, nodes_created); - + log::trace!("Replacing component {:?}", old); let scope_id = c.scope.get().unwrap(); + let node = self.scopes.fin_head(scope_id); + + self.enter_scope(scope_id); + { + self.replace_inner(node, nodes_created); + + log::trace!("Replacing component x2 {:?}", old); - // we can only remove components if they are actively being diffed - if self.stack.scope_stack.contains(&c.originator) { - self.scopes.try_remove(scope_id).unwrap(); + // we can only remove components if they are actively being diffed + if self.scope_stack.contains(&c.originator) { + log::trace!("Removing component {:?}", old); + + self.scopes.try_remove(scope_id).unwrap(); + } } + self.leave_scope(); } } } - /// schedules nodes for garbage collection and pushes "remove" to the mutation stack - /// remove can happen whenever - pub(crate) fn remove_nodes( - &mut self, - nodes: impl IntoIterator>, - gen_muts: bool, - ) { - // or cache the vec on the diff machine + pub fn remove_nodes(&mut self, nodes: impl IntoIterator>, gen_muts: bool) { for node in nodes { match node { VNode::Text(t) => { @@ -1288,6 +974,8 @@ impl<'bump> DiffState<'bump> { self.mutations.remove(id.as_u64()); } + self.scopes.collect_garbage(id); + self.remove_nodes(e.children, false); } @@ -1296,16 +984,124 @@ impl<'bump> DiffState<'bump> { } VNode::Component(c) => { - let scope_id = c.scope.get().unwrap(); - let root = self.scopes.root_node(scope_id); - self.remove_nodes(Some(root), gen_muts); - - // we can only remove this node if the originator is actively - if self.stack.scope_stack.contains(&c.originator) { - self.scopes.try_remove(scope_id).unwrap(); + self.enter_scope(c.scope.get().unwrap()); + { + let scope_id = c.scope.get().unwrap(); + let root = self.scopes.root_node(scope_id); + self.remove_nodes([root], gen_muts); + + // we can only remove this node if the originator is actively in our stackß + if self.scope_stack.contains(&c.originator) { + self.scopes.try_remove(scope_id).unwrap(); + } } + self.leave_scope(); + } + } + } + } + + fn create_children(&mut self, nodes: &'b [VNode<'b>]) -> usize { + let mut created = 0; + for node in nodes { + created += self.create_node(node); + } + created + } + + fn create_and_append_children(&mut self, nodes: &'b [VNode<'b>]) { + let created = self.create_children(nodes); + self.mutations.append_children(created as u32); + } + + fn create_and_insert_after(&mut self, nodes: &'b [VNode<'b>], after: &'b VNode<'b>) { + let created = self.create_children(nodes); + let last = self.find_last_element(after).unwrap(); + self.mutations.insert_after(last, created as u32); + } + + fn create_and_insert_before(&mut self, nodes: &'b [VNode<'b>], before: &'b VNode<'b>) { + let created = self.create_children(nodes); + let first = self.find_first_element(before).unwrap(); + self.mutations.insert_before(first, created as u32); + } + + fn current_scope(&self) -> Option { + self.scope_stack.last().copied() + } + + fn enter_scope(&mut self, scope: ScopeId) { + self.scope_stack.push(scope); + } + + fn leave_scope(&mut self) { + self.scope_stack.pop(); + } + + fn find_last_element(&self, vnode: &'b VNode<'b>) -> Option { + let mut search_node = Some(vnode); + loop { + match &search_node.take().unwrap() { + VNode::Text(t) => break t.id.get(), + VNode::Element(t) => break t.id.get(), + VNode::Placeholder(t) => break t.id.get(), + VNode::Fragment(frag) => search_node = frag.children.last(), + VNode::Component(el) => { + let scope_id = el.scope.get().unwrap(); + search_node = Some(self.scopes.root_node(scope_id)); } } } } + + fn find_first_element(&self, vnode: &'b VNode<'b>) -> Option { + let mut search_node = Some(vnode); + loop { + match &search_node.take().expect("search node to have an ID") { + VNode::Text(t) => break t.id.get(), + VNode::Element(t) => break t.id.get(), + VNode::Placeholder(t) => break t.id.get(), + VNode::Fragment(frag) => search_node = Some(&frag.children[0]), + VNode::Component(el) => { + let scope = el.scope.get().expect("element to have a scope assigned"); + search_node = Some(self.scopes.root_node(scope)); + } + } + } + } + + // recursively push all the nodes of a tree onto the stack and return how many are there + fn push_all_nodes(&mut self, node: &'b VNode<'b>) -> usize { + match node { + VNode::Text(_) | VNode::Placeholder(_) => { + self.mutations.push_root(node.mounted_id()); + 1 + } + + VNode::Fragment(frag) => { + let mut added = 0; + for child in frag.children { + added += self.push_all_nodes(child); + } + added + } + + VNode::Component(c) => { + let scope_id = c.scope.get().unwrap(); + let root = self.scopes.root_node(scope_id); + self.push_all_nodes(root) + } + + VNode::Element(el) => { + let mut num_on_stack = 0; + for child in el.children.iter() { + num_on_stack += self.push_all_nodes(child); + } + + self.mutations.push_root(el.id.get().unwrap()); + + num_on_stack + 1 + } + } + } } diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index dba4effbbc..e81ddca870 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -12,7 +12,6 @@ pub(crate) mod util; pub(crate) mod virtual_dom; pub(crate) mod innerlude { - pub(crate) use crate::diff::*; pub use crate::events::*; pub use crate::lazynodes::*; pub use crate::mutations::*; diff --git a/packages/core/src/mutations.rs b/packages/core/src/mutations.rs index fa96edd1b7..bd94d0b71c 100644 --- a/packages/core/src/mutations.rs +++ b/packages/core/src/mutations.rs @@ -143,6 +143,10 @@ impl<'a> Mutations<'a> { self.edits.push(InsertBefore { n, root }); } + pub(crate) fn append_children(&mut self, n: u32) { + self.edits.push(AppendChildren { many: n }); + } + // Remove Nodes from the dom pub(crate) fn remove(&mut self, id: u64) { self.edits.push(Remove { root: id }); @@ -217,6 +221,10 @@ impl<'a> Mutations<'a> { let name = attribute.name; self.edits.push(RemoveAttribute { name, root }); } + + pub(crate) fn mark_dirty_scope(&mut self, scope: ScopeId) { + self.dirty_scopes.insert(scope); + } } // refs are only assigned once diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index bc76e6bb9c..7a36f58cef 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -147,13 +147,6 @@ impl<'src> VNode<'src> { } } - pub(crate) fn children(&self) -> &[VNode<'src>] { - match &self { - VNode::Fragment(f) => f.children, - _ => &[], - } - } - // Create an "owned" version of the vnode. pub fn decouple(&self) -> VNode<'src> { match *self { @@ -175,6 +168,7 @@ impl Debug for VNode<'_> { .field("key", &el.key) .field("attrs", &el.attributes) .field("children", &el.children) + .field("id", &el.id) .finish(), VNode::Text(t) => write!(s, "VNode::VText {{ text: {} }}", t.text), VNode::Placeholder(t) => write!(s, "VNode::VPlaceholder {{ id: {:?} }}", t.id), diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 2faa47dabf..00c3024bf0 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -13,6 +13,8 @@ use std::{ rc::Rc, }; +/// for traceability, we use the raw fn pointer to identify the function +/// we also get the component name, but that's not necessarily unique in the app pub(crate) type ComponentPtr = *mut std::os::raw::c_void; pub(crate) struct Heuristic { @@ -94,7 +96,7 @@ impl ScopeArena { // Get the height of the scope let height = parent_scope - .map(|id| self.get_scope(id).map(|scope| scope.height)) + .map(|id| self.get_scope(id).map(|scope| scope.height + 1)) .flatten() .unwrap_or_default(); @@ -110,31 +112,62 @@ impl ScopeArena { if let Some(old_scope) = self.free_scopes.borrow_mut().pop() { // reuse the old scope let scope = unsafe { &mut *old_scope }; - scope.props.get_mut().replace(vcomp); + + scope.container = container; + scope.our_arena_idx = new_scope_id; scope.parent_scope = parent_scope; scope.height = height; + scope.fnptr = fc_ptr; + scope.props.get_mut().replace(vcomp); scope.subtree.set(subtree); - scope.our_arena_idx = new_scope_id; - scope.container = container; + scope.frames[0].reset(); + scope.frames[1].reset(); + scope.shared_contexts.get_mut().clear(); + scope.items.get_mut().listeners.clear(); + scope.items.get_mut().borrowed_props.clear(); + scope.hook_idx.set(0); + scope.hook_vals.get_mut().clear(); + let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); debug_assert!(any_item.is_none()); } else { // else create a new scope + let (node_capacity, hook_capacity) = self + .heuristics + .borrow() + .get(&fc_ptr) + .map(|h| (h.node_arena_size, h.hook_arena_size)) + .unwrap_or_default(); + self.scopes.borrow_mut().insert( new_scope_id, - self.bump.alloc(ScopeState::new( - height, + self.bump.alloc(ScopeState { container, - new_scope_id, + our_arena_idx: new_scope_id, parent_scope, - vcomp, - self.tasks.clone(), - self.heuristics - .borrow() - .get(&fc_ptr) - .map(|h| (h.node_arena_size, h.hook_arena_size)) - .unwrap_or_default(), - )), + height, + fnptr: fc_ptr, + props: RefCell::new(Some(vcomp)), + frames: [BumpFrame::new(node_capacity), BumpFrame::new(node_capacity)], + + // todo: subtrees + subtree: Cell::new(0), + is_subtree_root: Cell::new(false), + + generation: 0.into(), + + tasks: self.tasks.clone(), + shared_contexts: Default::default(), + + items: RefCell::new(SelfReferentialItems { + listeners: Default::default(), + borrowed_props: Default::default(), + }), + + hook_arena: Bump::new(), + hook_vals: RefCell::new(Vec::with_capacity(hook_capacity)), + hook_idx: Default::default(), + }), ); } @@ -189,8 +222,6 @@ impl ScopeArena { /// This also makes sure that drop order is consistent and predictable. All resources that rely on being dropped will /// be dropped. pub(crate) fn ensure_drop_safety(&self, scope_id: ScopeId) { - log::trace!("Ensuring drop safety for scope {:?}", scope_id); - if let Some(scope) = self.get_scope(scope_id) { let mut items = scope.items.borrow_mut(); @@ -201,7 +232,6 @@ impl ScopeArena { if let Some(scope_id) = comp.scope.get() { self.ensure_drop_safety(scope_id); } - drop(comp.props.take()); }); @@ -217,18 +247,18 @@ impl ScopeArena { // Cycle to the next frame and then reset it // This breaks any latent references, invalidating every pointer referencing into it. // Remove all the outdated listeners - log::trace!("Running scope {:?}", id); self.ensure_drop_safety(id); // todo: we *know* that this is aliased by the contents of the scope itself let scope = unsafe { &mut *self.get_scope_raw(id).expect("could not find scope") }; + log::trace!("running scope {:?} symbol: {:?}", id, scope.fnptr); + // Safety: // - We dropped the listeners, so no more &mut T can be used while these are held // - All children nodes that rely on &mut T are replaced with a new reference scope.hook_idx.set(0); - // book keeping to ensure safety around the borrowed data { // Safety: // - We've dropped all references to the wip bump frame with "ensure_drop_safety" @@ -241,8 +271,6 @@ impl ScopeArena { debug_assert!(items.borrowed_props.is_empty()); } - // safety: this is definitely not dropped - /* If the component returns None, then we fill in a placeholder node. This will wipe what was there. An alternate approach is to leave the Real Dom the same, but that can lead to safety issues and a lot more checks. @@ -284,14 +312,13 @@ impl ScopeArena { while let Some(id) = cur_el.take() { if let Some(el) = nodes.get(id.0) { - log::trace!("Found valid receiver element"); - let real_el = unsafe { &**el }; + log::debug!("looking for listener on {:?}", real_el); + if let VNode::Element(real_el) = real_el { for listener in real_el.listeners.borrow().iter() { if listener.event == event.name { - log::trace!("Found valid receiver event"); - + log::debug!("calling listener {:?}", listener.event); if state.canceled.get() { // stop bubbling if canceled break; @@ -421,6 +448,7 @@ pub struct ScopeState { pub(crate) container: ElementId, pub(crate) our_arena_idx: ScopeId, pub(crate) height: u32, + pub(crate) fnptr: ComponentPtr, // todo: subtrees pub(crate) is_subtree_root: Cell, @@ -449,43 +477,6 @@ pub struct SelfReferentialItems<'a> { // Public methods exposed to libraries and components impl ScopeState { - fn new( - height: u32, - container: ElementId, - our_arena_idx: ScopeId, - parent_scope: Option<*mut ScopeState>, - vcomp: Box, - tasks: Rc, - (node_capacity, hook_capacity): (usize, usize), - ) -> Self { - ScopeState { - container, - our_arena_idx, - parent_scope, - height, - props: RefCell::new(Some(vcomp)), - frames: [BumpFrame::new(node_capacity), BumpFrame::new(node_capacity)], - - // todo: subtrees - subtree: Cell::new(0), - is_subtree_root: Cell::new(false), - - generation: 0.into(), - - tasks, - shared_contexts: Default::default(), - - items: RefCell::new(SelfReferentialItems { - listeners: Default::default(), - borrowed_props: Default::default(), - }), - - hook_arena: Bump::new(), - hook_vals: RefCell::new(Vec::with_capacity(hook_capacity)), - hook_idx: Default::default(), - } - } - /// Get the subtree ID that this scope belongs to. /// /// Each component has its own subtree ID - the root subtree has an ID of 0. This ID is used by the renderer to route diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 0bd93335f9..a7092104c0 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -2,6 +2,7 @@ //! //! This module provides the primary mechanics to create a hook-based, concurrent VDOM for Rust. +use crate::diff::DiffState; use crate::innerlude::*; use futures_channel::mpsc::{UnboundedReceiver, UnboundedSender}; use futures_util::{future::poll_fn, StreamExt}; @@ -448,6 +449,7 @@ impl VirtualDom { /// apply_mutations(mutations); /// } /// ``` + #[allow(unused)] pub fn work_with_deadline(&mut self, mut deadline: impl FnMut() -> bool) -> Vec { let mut committed_mutations = vec![]; @@ -467,35 +469,40 @@ impl VirtualDom { h1.cmp(&h2).reverse() }); + log::debug!("dirty_scopes: {:?}", self.dirty_scopes); + if let Some(scopeid) = self.dirty_scopes.pop() { if !ran_scopes.contains(&scopeid) { ran_scopes.insert(scopeid); self.scopes.run_scope(scopeid); - let (old, new) = (self.scopes.wip_head(scopeid), self.scopes.fin_head(scopeid)); - diff_state.stack.push(DiffInstruction::Diff { new, old }); - diff_state.stack.scope_stack.push(scopeid); + diff_state.diff_scope(scopeid); - let scope = scopes.get_scope(scopeid).unwrap(); - diff_state.stack.element_stack.push(scope.container); - } - } + let DiffState { mutations, .. } = diff_state; - if diff_state.work(&mut deadline) { - let DiffState { mutations, .. } = diff_state; + log::debug!("succesffuly resolved scopes {:?}", mutations.dirty_scopes); + for scope in &mutations.dirty_scopes { + self.dirty_scopes.remove(scope); + } - for scope in &mutations.dirty_scopes { - self.dirty_scopes.remove(scope); + committed_mutations.push(mutations); + + // todo: pause the diff machine + // if diff_state.work(&mut deadline) { + // let DiffState { mutations, .. } = diff_state; + // for scope in &mutations.dirty_scopes { + // self.dirty_scopes.remove(scope); + // } + // committed_mutations.push(mutations); + // } else { + // // leave the work in an incomplete state + // // + // // todo: we should store the edits and re-apply them later + // // for now, we just dump the work completely (threadsafe) + // return committed_mutations; + // } } - - committed_mutations.push(mutations); - } else { - // leave the work in an incomplete state - // - // todo: we should store the edits and re-apply them later - // for now, we just dump the work completely (threadsafe) - return committed_mutations; } } @@ -524,13 +531,15 @@ impl VirtualDom { let mut diff_state = DiffState::new(&self.scopes); self.scopes.run_scope(scope_id); - diff_state - .stack - .create_node(self.scopes.fin_head(scope_id), MountType::Append); - diff_state.stack.element_stack.push(ElementId(0)); - diff_state.stack.scope_stack.push(scope_id); - diff_state.work(|| false); + diff_state.element_stack.push(ElementId(0)); + diff_state.scope_stack.push(scope_id); + + let node = self.scopes.fin_head(scope_id); + let created = diff_state.create_node(node); + + diff_state.mutations.append_children(created as u32); + self.dirty_scopes.clear(); assert!(self.dirty_scopes.is_empty()); @@ -577,12 +586,11 @@ impl VirtualDom { ); diff_machine.force_diff = true; - diff_machine.stack.push(DiffInstruction::Diff { old, new }); - diff_machine.stack.scope_stack.push(scope_id); - + diff_machine.scope_stack.push(scope_id); let scope = diff_machine.scopes.get_scope(scope_id).unwrap(); - diff_machine.stack.element_stack.push(scope.container); - diff_machine.work(|| false); + diff_machine.element_stack.push(scope.container); + + diff_machine.diff_node(old, new); diff_machine.mutations } @@ -621,10 +629,10 @@ impl VirtualDom { /// ``` pub fn diff_vnodes<'a>(&'a self, old: &'a VNode<'a>, new: &'a VNode<'a>) -> Mutations<'a> { let mut machine = DiffState::new(&self.scopes); - machine.stack.push(DiffInstruction::Diff { new, old }); - machine.stack.element_stack.push(ElementId(0)); - machine.stack.scope_stack.push(ScopeId(0)); - machine.work(|| false); + machine.element_stack.push(ElementId(0)); + machine.scope_stack.push(ScopeId(0)); + machine.diff_node(old, new); + machine.mutations } @@ -643,11 +651,11 @@ impl VirtualDom { /// ``` pub fn create_vnodes<'a>(&'a self, nodes: LazyNodes<'a, '_>) -> Mutations<'a> { let mut machine = DiffState::new(&self.scopes); - machine.stack.element_stack.push(ElementId(0)); - machine - .stack - .create_node(self.render_vnodes(nodes), MountType::Append); - machine.work(|| false); + machine.scope_stack.push(ScopeId(0)); + machine.element_stack.push(ElementId(0)); + let node = self.render_vnodes(nodes); + let created = machine.create_node(node); + machine.mutations.append_children(created as u32); machine.mutations } @@ -672,16 +680,15 @@ impl VirtualDom { let (old, new) = (self.render_vnodes(left), self.render_vnodes(right)); let mut create = DiffState::new(&self.scopes); - create.stack.scope_stack.push(ScopeId(0)); - create.stack.element_stack.push(ElementId(0)); - create.stack.create_node(old, MountType::Append); - create.work(|| false); + create.scope_stack.push(ScopeId(0)); + create.element_stack.push(ElementId(0)); + let created = create.create_node(old); + create.mutations.append_children(created as u32); let mut edit = DiffState::new(&self.scopes); - edit.stack.scope_stack.push(ScopeId(0)); - edit.stack.element_stack.push(ElementId(0)); - edit.stack.push(DiffInstruction::Diff { old, new }); - edit.work(|| false); + edit.scope_stack.push(ScopeId(0)); + edit.element_stack.push(ElementId(0)); + edit.diff_node(old, new); (create.mutations, edit.mutations) } diff --git a/packages/core/tests/diffing.rs b/packages/core/tests/diffing.rs index 66012cf99c..6bc9b5596b 100644 --- a/packages/core/tests/diffing.rs +++ b/packages/core/tests/diffing.rs @@ -1,4 +1,5 @@ #![allow(unused, non_upper_case_globals)] + //! Diffing Tests //! //! These tests only verify that the diffing algorithm works properly for single components. @@ -105,6 +106,7 @@ fn empty_fragments_create_many_anchors() { create.edits, [CreatePlaceholder { root: 1 }, AppendChildren { many: 1 }] ); + assert_eq!( change.edits, [ @@ -237,7 +239,7 @@ fn two_fragments_with_differrent_elements_are_differet() { // replace the divs with new h1s CreateElement { tag: "h1", root: 7 }, ReplaceWith { root: 1, m: 1 }, - CreateElement { tag: "h1", root: 8 }, + CreateElement { tag: "h1", root: 1 }, // notice how 1 gets re-used ReplaceWith { root: 2, m: 1 }, ] ); @@ -270,15 +272,18 @@ fn two_fragments_with_differrent_elements_are_differet_shorter() { AppendChildren { many: 6 }, ] ); + + // note: key reuse is always the last node that got used + // slab maintains a linked list, essentially assert_eq!( change.edits, [ Remove { root: 3 }, Remove { root: 4 }, Remove { root: 5 }, - CreateElement { root: 7, tag: "h1" }, - ReplaceWith { root: 1, m: 1 }, - CreateElement { root: 8, tag: "h1" }, + CreateElement { root: 5, tag: "h1" }, // 3 gets reused + ReplaceWith { root: 1, m: 1 }, // 1 gets deleted + CreateElement { root: 1, tag: "h1" }, // 1 gets reused ReplaceWith { root: 2, m: 1 }, ] ); @@ -564,13 +569,13 @@ fn keyed_diffing_additions_and_moves_in_middle() { let dom = new_dom(); let left = rsx!({ - [/**/ 4, 5, 6, 7 /**/].iter().map(|f| { + [/**/ 1, 2, 3, 4 /**/].iter().map(|f| { rsx! { div { key: "{f}" }} }) }); let right = rsx!({ - [/**/ 7, 4, 13, 17, 5, 11, 12, 6 /**/].iter().map(|f| { + [/**/ 4, 1, 7, 8, 2, 5, 6, 3 /**/].iter().map(|f| { rsx! { div { key: "{f}" }} }) }); @@ -581,14 +586,14 @@ fn keyed_diffing_additions_and_moves_in_middle() { assert_eq!( change.edits, [ - // create 13, 17 + // create 5, 6 CreateElement { tag: "div", root: 5 }, CreateElement { tag: "div", root: 6 }, - InsertBefore { root: 2, n: 2 }, - // create 11, 12 + InsertBefore { root: 3, n: 2 }, + // create 7, 8 CreateElement { tag: "div", root: 7 }, CreateElement { tag: "div", root: 8 }, - InsertBefore { root: 3, n: 2 }, + InsertBefore { root: 2, n: 2 }, // move 7 PushRoot { root: 4 }, InsertBefore { root: 1, n: 1 } diff --git a/packages/core/tests/earlyabort.rs b/packages/core/tests/earlyabort.rs index ae26c97b69..3999a74029 100644 --- a/packages/core/tests/earlyabort.rs +++ b/packages/core/tests/earlyabort.rs @@ -59,8 +59,8 @@ fn test_early_abort() { assert_eq!( edits.edits, [ - CreateElement { tag: "div", root: 2 }, - CreateTextNode { text: "Hello, world!", root: 4 }, + CreateElement { tag: "div", root: 1 }, // keys get reused + CreateTextNode { text: "Hello, world!", root: 2 }, // keys get reused AppendChildren { many: 1 }, ReplaceWith { root: 3, m: 1 }, ] diff --git a/packages/core/tests/lifecycle.rs b/packages/core/tests/lifecycle.rs index 8b7d7bb4a8..f977ba96ec 100644 --- a/packages/core/tests/lifecycle.rs +++ b/packages/core/tests/lifecycle.rs @@ -40,7 +40,7 @@ fn manual_diffing() { #[test] fn events_generate() { - static App: Component = |cx| { + fn app(cx: Scope) -> Element { let count = cx.use_hook(|_| 0); let inner = match *count { @@ -61,7 +61,7 @@ fn events_generate() { cx.render(inner) }; - let mut dom = VirtualDom::new(App); + let mut dom = VirtualDom::new(app); let mut channel = dom.get_scheduler_channel(); assert!(dom.has_work()); @@ -83,7 +83,7 @@ fn events_generate() { #[test] fn components_generate() { - static App: Component = |cx| { + fn app(cx: Scope) -> Element { let render_phase = cx.use_hook(|_| 0); *render_phase += 1; @@ -100,13 +100,14 @@ fn components_generate() { }) }; - static Child: Component = |cx| { + fn Child(cx: Scope) -> Element { + log::debug!("Running child"); cx.render(rsx! { h1 {} }) - }; + } - let mut dom = VirtualDom::new(App); + let mut dom = VirtualDom::new(app); let edits = dom.rebuild(); assert_eq!( edits.edits, @@ -116,72 +117,67 @@ fn components_generate() { ] ); - let edits = dom.hard_diff(ScopeId(0)); assert_eq!( - edits.edits, + dom.hard_diff(ScopeId(0)).edits, [ CreateElement { tag: "div", root: 2 }, ReplaceWith { root: 1, m: 1 }, ] ); - let edits = dom.hard_diff(ScopeId(0)); assert_eq!( - edits.edits, + dom.hard_diff(ScopeId(0)).edits, [ - CreateTextNode { text: "Text2", root: 3 }, + CreateTextNode { text: "Text2", root: 1 }, ReplaceWith { root: 2, m: 1 }, ] ); - let edits = dom.hard_diff(ScopeId(0)); + // child {} assert_eq!( - edits.edits, + dom.hard_diff(ScopeId(0)).edits, [ - CreateElement { tag: "h1", root: 4 }, - ReplaceWith { root: 3, m: 1 }, + CreateElement { tag: "h1", root: 2 }, + ReplaceWith { root: 1, m: 1 }, ] ); - let edits = dom.hard_diff(ScopeId(0)); + // placeholder assert_eq!( - edits.edits, - [CreatePlaceholder { root: 5 }, ReplaceWith { root: 4, m: 1 },] + dom.hard_diff(ScopeId(0)).edits, + [CreatePlaceholder { root: 1 }, ReplaceWith { root: 2, m: 1 },] ); - let edits = dom.hard_diff(ScopeId(0)); assert_eq!( - edits.edits, + dom.hard_diff(ScopeId(0)).edits, [ - CreateTextNode { text: "text 3", root: 6 }, - ReplaceWith { root: 5, m: 1 }, + CreateTextNode { text: "text 3", root: 2 }, + ReplaceWith { root: 1, m: 1 }, ] ); - let edits = dom.hard_diff(ScopeId(0)); assert_eq!( - edits.edits, + dom.hard_diff(ScopeId(0)).edits, [ - CreateTextNode { text: "text 0", root: 7 }, - CreateTextNode { text: "text 1", root: 8 }, - ReplaceWith { root: 6, m: 2 }, + CreateTextNode { text: "text 0", root: 1 }, + CreateTextNode { text: "text 1", root: 3 }, + ReplaceWith { root: 2, m: 2 }, ] ); - let edits = dom.hard_diff(ScopeId(0)); assert_eq!( - edits.edits, + dom.hard_diff(ScopeId(0)).edits, [ - CreateElement { tag: "h1", root: 9 }, - ReplaceWith { root: 7, m: 1 }, - Remove { root: 8 }, + CreateElement { tag: "h1", root: 2 }, + ReplaceWith { root: 1, m: 1 }, + Remove { root: 3 }, ] ); } #[test] fn component_swap() { - static App: Component = |cx| { + fn app(cx: Scope) -> Element { let render_phase = cx.use_hook(|_| 0); *render_phase += 1; @@ -257,7 +253,7 @@ fn component_swap() { }) }; - let mut dom = VirtualDom::new(App); + let mut dom = VirtualDom::new(app); let edits = dom.rebuild(); dbg!(&edits); diff --git a/packages/core/tests/miri_stress.rs b/packages/core/tests/miri_stress.rs index 87d1080942..8ce0301478 100644 --- a/packages/core/tests/miri_stress.rs +++ b/packages/core/tests/miri_stress.rs @@ -314,36 +314,117 @@ fn test_pass_thru() { #[inline_props] fn Router<'a>(cx: Scope, children: Element<'a>) -> Element { cx.render(rsx! { - &cx.props.children + div { + &cx.props.children + } }) } - fn MemoizedThing(cx: Scope) -> Element { - rsx!(cx, div { "memoized" }) + #[inline_props] + fn NavContainer<'a>(cx: Scope, children: Element<'a>) -> Element { + cx.render(rsx! { + header { + nav { + &cx.props.children + } + } + }) } - fn app(cx: Scope) -> Element { - let thing = cx.use_hook(|_| "asd"); + fn NavMenu(cx: Scope) -> Element { rsx!(cx, - Router { - MemoizedThing { - } + NavBrand {} + div { + NavStart {} + NavEnd {} } ) } - let mut dom = new_dom(app, ()); - let _ = dom.rebuild(); + fn NavBrand(cx: Scope) -> Element { + rsx!(cx, div {}) + } - dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); - dom.work_with_deadline(|| false); + fn NavStart(cx: Scope) -> Element { + rsx!(cx, div {}) + } - dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); - dom.work_with_deadline(|| false); + fn NavEnd(cx: Scope) -> Element { + rsx!(cx, div {}) + } - dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); - dom.work_with_deadline(|| false); + #[inline_props] + fn MainContainer<'a>( + cx: Scope, + nav: Element<'a>, + body: Element<'a>, + footer: Element<'a>, + ) -> Element { + cx.render(rsx! { + div { + class: "columns is-mobile", + div { + class: "column is-full", + &cx.props.nav, + &cx.props.body, + &cx.props.footer, + } + } + }) + } - dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); - dom.work_with_deadline(|| false); + fn app(cx: Scope) -> Element { + let nav = cx.render(rsx! { + NavContainer { + NavMenu {} + } + }); + let body = cx.render(rsx! { + div {} + }); + let footer = cx.render(rsx! { + div {} + }); + + cx.render(rsx! { + MainContainer { + nav: nav, + body: body, + footer: footer, + } + }) + } + + let mut dom = new_dom(app, ()); + let _ = dom.rebuild(); + + for x in 0..40 { + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(1))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(1))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(1))); + dom.work_with_deadline(|| false); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(2))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(2))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(2))); + dom.work_with_deadline(|| false); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(3))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(3))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(3))); + dom.work_with_deadline(|| false); + } } diff --git a/packages/core/tests/passthru.rs b/packages/core/tests/passthru.rs new file mode 100644 index 0000000000..e00aa545c4 --- /dev/null +++ b/packages/core/tests/passthru.rs @@ -0,0 +1,108 @@ +#![allow(unused, non_upper_case_globals)] + +//! Diffing Tests +//! +//! These tests only verify that the diffing algorithm works properly for single components. +//! +//! It does not validated that component lifecycles work properly. This is done in another test file. + +use dioxus::{prelude::*, DomEdit, ScopeId}; +use dioxus_core as dioxus; +use dioxus_core_macro::*; +use dioxus_html as dioxus_elements; + +mod test_logging; + +fn new_dom() -> VirtualDom { + const IS_LOGGING_ENABLED: bool = false; + test_logging::set_up_logging(IS_LOGGING_ENABLED); + VirtualDom::new(|cx| rsx!(cx, "hi")) +} + +use DomEdit::*; + +/// Should push the text node onto the stack and modify it +#[test] +fn nested_passthru_creates() { + fn app(cx: Scope) -> Element { + cx.render(rsx! { + Child { + Child { + Child { + div { + "hi" + } + } + } + } + }) + }; + + #[inline_props] + fn Child<'a>(cx: Scope, children: Element<'a>) -> Element { + cx.render(rsx! { + children + }) + }; + + let mut dom = VirtualDom::new(app); + let mut channel = dom.get_scheduler_channel(); + assert!(dom.has_work()); + + let edits = dom.rebuild(); + assert_eq!( + edits.edits, + [ + CreateElement { tag: "div", root: 1 }, + CreateTextNode { text: "hi", root: 2 }, + AppendChildren { many: 1 }, + AppendChildren { many: 1 }, + ] + ) +} + +/// Should push the text node onto the stack and modify it +#[test] +fn nested_passthru_creates_add() { + fn app(cx: Scope) -> Element { + cx.render(rsx! { + Child { + "1" + Child { + "2" + Child { + "3" + div { + "hi" + } + } + } + } + }) + }; + + #[inline_props] + fn Child<'a>(cx: Scope, children: Element<'a>) -> Element { + cx.render(rsx! { + children + }) + }; + + let mut dom = VirtualDom::new(app); + let mut channel = dom.get_scheduler_channel(); + assert!(dom.has_work()); + + let edits = dom.rebuild(); + assert_eq!( + edits.edits, + [ + CreateTextNode { text: "1", root: 1 }, + CreateTextNode { text: "2", root: 2 }, + CreateTextNode { text: "3", root: 3 }, + CreateElement { tag: "div", root: 4 }, + CreateTextNode { text: "hi", root: 5 }, + AppendChildren { many: 1 }, + AppendChildren { many: 4 }, + ] + ) +} diff --git a/packages/core/tests/sharedstate.rs b/packages/core/tests/sharedstate.rs index 38195ccd82..70e8235692 100644 --- a/packages/core/tests/sharedstate.rs +++ b/packages/core/tests/sharedstate.rs @@ -1,6 +1,6 @@ #![allow(unused, non_upper_case_globals)] -use dioxus::{prelude::*, DomEdit, Mutations}; +use dioxus::{prelude::*, DomEdit, Mutations, SchedulerMsg, ScopeId}; use dioxus_core as dioxus; use dioxus_core_macro::*; use dioxus_html as dioxus_elements; @@ -34,3 +34,97 @@ fn shared_state_test() { ] ); } + +#[test] +fn swap_test() { + struct MySharedState(&'static str); + + fn app(cx: Scope) -> Element { + let val = cx.use_hook(|_| 0); + *val += 1; + + cx.provide_context(MySharedState("world!")); + + let child = match *val % 2 { + 0 => rsx!( + Child1 { + Child1 { } + Child2 { } + } + ), + _ => rsx!( + Child2 { + Child2 { } + Child2 { } + } + ), + }; + + cx.render(rsx!( + Router { + div { child } + } + )) + } + + #[inline_props] + fn Router<'a>(cx: Scope, children: Element<'a>) -> Element<'a> { + cx.render(rsx!(div { children })) + } + + #[inline_props] + fn Child1<'a>(cx: Scope, children: Element<'a>) -> Element { + let shared = cx.consume_context::().unwrap(); + println!("Child1: {}", shared.0); + cx.render(rsx! { + div { + "{shared.0}", + children + } + }) + } + + #[inline_props] + fn Child2<'a>(cx: Scope, children: Element<'a>) -> Element { + let shared = cx.consume_context::().unwrap(); + println!("Child2: {}", shared.0); + cx.render(rsx! { + h1 { + "{shared.0}", + children + } + }) + } + + let mut dom = VirtualDom::new(app); + let Mutations { edits, .. } = dom.rebuild(); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + + // dom.handle_message(SchedulerMsg::Immediate(ScopeId(1))); + // dom.work_with_deadline(|| false); + + // dom.handle_message(SchedulerMsg::Immediate(ScopeId(2))); + // dom.work_with_deadline(|| false); + + // dom.handle_message(SchedulerMsg::Immediate(ScopeId(3))); + // dom.work_with_deadline(|| false); + + // dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + // dom.work_with_deadline(|| false); + // dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + // dom.work_with_deadline(|| false); + // dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + // dom.work_with_deadline(|| false); +} diff --git a/packages/router/src/components/link.rs b/packages/router/src/components/link.rs index fd8eea66d6..18e9fd92ee 100644 --- a/packages/router/src/components/link.rs +++ b/packages/router/src/components/link.rs @@ -41,7 +41,7 @@ pub struct LinkProps<'a> { } pub fn Link<'a>(cx: Scope<'a, LinkProps<'a>>) -> Element { - log::trace!("render Link to {}", cx.props.to); + // log::trace!("render Link to {}", cx.props.to); if let Some(service) = cx.consume_context::() { return cx.render(rsx! { a { diff --git a/packages/router/src/components/route.rs b/packages/router/src/components/route.rs index 129953eea4..e4dd2f9c8c 100644 --- a/packages/router/src/components/route.rs +++ b/packages/router/src/components/route.rs @@ -30,7 +30,7 @@ pub fn Route<'a>(cx: Scope<'a, RouteProps<'a>>) -> Element { Some(ctx) => ctx.total_route.to_string(), None => cx.props.to.to_string(), }; - log::trace!("total route for {} is {}", cx.props.to, total_route); + // log::trace!("total route for {} is {}", cx.props.to, total_route); // provide our route context let route_context = cx.provide_context(RouteContext { @@ -48,7 +48,7 @@ pub fn Route<'a>(cx: Scope<'a, RouteProps<'a>>) -> Element { Some(RouteInner {}) }); - log::trace!("Checking route {}", cx.props.to); + // log::trace!("Checking route {}", cx.props.to); if router_root.should_render(cx.scope_id()) { cx.render(rsx!(&cx.props.children)) diff --git a/packages/router/src/components/router.rs b/packages/router/src/components/router.rs index 240c3806ae..bf23645f59 100644 --- a/packages/router/src/components/router.rs +++ b/packages/router/src/components/router.rs @@ -17,6 +17,7 @@ pub struct RouterProps<'a> { #[allow(non_snake_case)] pub fn Router<'a>(cx: Scope<'a, RouterProps<'a>>) -> Element { + log::debug!("running router {:?}", cx.scope_id()); let svc = cx.use_hook(|_| { let update = cx.schedule_update_any(); cx.provide_context(RouterService::new(update, cx.scope_id())) diff --git a/packages/router/src/platform/mod.rs b/packages/router/src/platform/mod.rs index 7afc1316a6..ec59a4703c 100644 --- a/packages/router/src/platform/mod.rs +++ b/packages/router/src/platform/mod.rs @@ -1,9 +1,4 @@ pub trait RouterProvider { fn get_current_route(&self) -> String; - fn subscribe_to_route_changes(&self, callback: Box); -} - -enum RouteChange { - LinkTo(String), - Back(String), + fn listen(&self, callback: Box); } diff --git a/packages/router/src/service.rs b/packages/router/src/service.rs index b62bc78d56..a4ab4dc0d5 100644 --- a/packages/router/src/service.rs +++ b/packages/router/src/service.rs @@ -7,14 +7,18 @@ use std::{ use dioxus_core::ScopeId; +use crate::platform::RouterProvider; + pub struct RouterService { pub(crate) regen_route: Rc, pub(crate) pending_events: Rc>>, - history: Rc>, slots: Rc>>, onchange_listeners: Rc>>, root_found: Rc>>, cur_path_params: Rc>>, + + // history: Rc, + history: Rc>, listener: HistoryListener, } @@ -58,12 +62,10 @@ impl RouterService { root_found.set(None); // checking if the route is valid is cheap, so we do it for (slot, root) in slots.borrow_mut().iter().rev() { - log::trace!("regenerating slot {:?} for root '{}'", slot, root); regen_route(*slot); } for listener in onchange_listeners.borrow_mut().iter() { - log::trace!("regenerating listener {:?}", listener); regen_route(*listener); } @@ -87,31 +89,24 @@ impl RouterService { } pub fn push_route(&self, route: &str) { - log::trace!("Pushing route: {}", route); self.history.borrow_mut().push(route); } pub fn register_total_route(&self, route: String, scope: ScopeId, fallback: bool) { let clean = clean_route(route); - log::trace!("Registered route '{}' with scope id {:?}", clean, scope); self.slots.borrow_mut().push((scope, clean)); } pub fn should_render(&self, scope: ScopeId) -> bool { - log::trace!("Should render scope id {:?}?", scope); if let Some(root_id) = self.root_found.get() { - log::trace!(" we already found a root with scope id {:?}", root_id); if root_id == scope { - log::trace!(" yes - it's a match"); return true; } - log::trace!(" no - it's not a match"); return false; } let location = self.history.borrow().location(); let path = location.path(); - log::trace!(" current path is '{}'", path); let roots = self.slots.borrow(); @@ -120,23 +115,15 @@ impl RouterService { // fallback logic match root { Some((id, route)) => { - log::trace!( - " matched given scope id {:?} with route root '{}'", - scope, - route, - ); if let Some(params) = route_matches_path(route, path) { - log::trace!(" and it matches the current path '{}'", path); self.root_found.set(Some(*id)); *self.cur_path_params.borrow_mut() = params; true } else { if route == "" { - log::trace!(" and the route is the root, so we will use that without a better match"); self.root_found.set(Some(*id)); true } else { - log::trace!(" and the route '{}' is not the root nor does it match the current path", route); false } } @@ -154,12 +141,10 @@ impl RouterService { } pub fn subscribe_onchange(&self, id: ScopeId) { - log::trace!("Subscribing onchange for scope id {:?}", id); self.onchange_listeners.borrow_mut().insert(id); } pub fn unsubscribe_onchange(&self, id: ScopeId) { - log::trace!("Subscribing onchange for scope id {:?}", id); self.onchange_listeners.borrow_mut().remove(&id); } } @@ -182,36 +167,20 @@ fn route_matches_path(route: &str, path: &str) -> Option let route_pieces = route.split('/').collect::>(); let path_pieces = clean_path(path).split('/').collect::>(); - log::trace!( - " checking route pieces {:?} vs path pieces {:?}", - route_pieces, - path_pieces, - ); - if route_pieces.len() != path_pieces.len() { - log::trace!(" the routes are different lengths"); return None; } let mut matches = HashMap::new(); for (i, r) in route_pieces.iter().enumerate() { - log::trace!(" checking route piece '{}' vs path", r); // If this is a parameter then it matches as long as there's // _any_thing in that spot in the path. if r.starts_with(':') { - log::trace!( - " route piece '{}' starts with a colon so it matches anything", - r, - ); let param = &r[1..]; matches.insert(param.to_string(), path_pieces[i].to_string()); continue; } - log::trace!( - " route piece '{}' must be an exact match for path piece '{}'", - r, - path_pieces[i], - ); + if path_pieces[i] != *r { return None; }