From 113b2c1529ca95908590359bff0f7226165bb76e Mon Sep 17 00:00:00 2001
From: Evan Almloff <evanalmloff@gmail.com>
Date: Fri, 5 Jan 2024 12:07:29 -0600
Subject: [PATCH] port over more diffing + creation logic

---
 packages/core/src/create.rs      | 10 ++---
 packages/core/src/diff.rs        | 67 ++++++++++++++------------------
 packages/core/src/mutations.rs   |  8 ++--
 packages/core/src/nodes.rs       | 16 ++++----
 packages/core/src/scopes.rs      |  6 +--
 packages/core/src/virtual_dom.rs | 28 +++++++++----
 packages/core/tests/fuzzing.rs   |  2 +-
 7 files changed, 69 insertions(+), 68 deletions(-)

diff --git a/packages/core/src/create.rs b/packages/core/src/create.rs
index 3da568634..06cb07603 100644
--- a/packages/core/src/create.rs
+++ b/packages/core/src/create.rs
@@ -408,11 +408,7 @@ impl VirtualDom {
     }
 
     /// Insert a new template into the VirtualDom's template registry
-    pub(crate) fn register_template_first_byte_index(
-        &mut self,
-        mut template: Template<'static>,
-        to: &mut impl WriteMutations,
-    ) {
+    pub(crate) fn register_template_first_byte_index(&mut self, mut template: Template) {
         // First, make sure we mark the template as seen, regardless if we process it
         let (path, _) = template.name.rsplit_once(':').unwrap();
         if let Some((_, old_template)) = self
@@ -435,7 +431,7 @@ impl VirtualDom {
 
         // If it's all dynamic nodes, then we don't need to register it
         if !template.is_completely_dynamic() {
-            to.register_template(template);
+            self.queued_templates.push(template);
         }
     }
 
@@ -444,7 +440,7 @@ impl VirtualDom {
     #[allow(unused_mut)]
     pub(crate) fn register_template(
         &mut self,
-        mut template: Template<'static>,
+        mut template: Template,
         to: &mut impl WriteMutations,
     ) {
         let (path, byte_index) = template.name.rsplit_once(':').unwrap();
diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs
index 0b41f4916..61cb8e178 100644
--- a/packages/core/src/diff.rs
+++ b/packages/core/src/diff.rs
@@ -6,7 +6,6 @@ use crate::{
     innerlude::{
         DirtyScope, ElementPath, ElementRef, VComponent, VPlaceholder, VText, WriteMutations,
     },
-    mutations::Mutation,
     nodes::RenderReturn,
     nodes::{DynamicNode, VNode},
     scopes::ScopeId,
@@ -63,15 +62,10 @@ impl VirtualDom {
         *p.parent.borrow_mut() = l.parent.borrow().clone();
         to.create_placeholder(id);
 
-        self.remove_node(l, true, to);
+        to.insert_nodes_before(id, 1);
 
-        // We want to optimize the replace case to use one less mutation if possible
-        // Since mutations are done in reverse, the last node removed will be the first in the stack
-        // Instead of *just* removing it, we can use the replace mutation
-        match self.mutations.edits.pop().unwrap() {
-            Mutation::Remove { id } => self.mutations.push(Mutation::ReplaceWith { id, m: 1 }),
-            _ => panic!("Expected remove mutation from remove_node"),
-        };
+        // TODO: Instead of *just* removing it, we can use the replace mutation
+        self.remove_node(l, true, to);
     }
 
     fn diff_node(
@@ -250,17 +244,12 @@ impl VirtualDom {
         parent: Option<ElementRef>,
         to: &mut impl WriteMutations,
     ) {
-        let m = self.create_component_node(parent, right);
+        let m = self.create_component_node(parent, right, to);
 
+        // TODO: Instead of *just* removing it, we can use the replace mutation
         self.remove_component_node(left, true, to);
 
-        // We want to optimize the replace case to use one less mutation if possible
-        // Since mutations are done in reverse, the last node removed will be the first in the stack
-        // Instead of *just* removing it, we can use the replace mutation
-        match self.mutations.edits.pop().unwrap() {
-            Mutation::Remove { id } => self.mutations.push(Mutation::ReplaceWith { id, m }),
-            at => panic!("Expected remove mutation from remove_node {:#?}", at),
-        };
+        todo!()
     }
 
     /// Lightly diff the two templates, checking only their roots.
@@ -656,7 +645,7 @@ impl VirtualDom {
                 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(new_node);
+                    nodes_created += self.create(new_node, to);
                 } else {
                     self.diff_node(&old[old_index], new_node, to);
                     nodes_created += self.push_all_real_nodes(new_node, to);
@@ -679,7 +668,7 @@ impl VirtualDom {
                     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(new_node);
+                        nodes_created += self.create(new_node, to);
                     } else {
                         self.diff_node(&old[old_index], new_node, to);
                         nodes_created += self.push_all_real_nodes(new_node, to);
@@ -702,7 +691,7 @@ impl VirtualDom {
             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(new_node);
+                    nodes_created += self.create(new_node, to);
                 } else {
                     self.diff_node(&old[old_index], new_node, to);
                     nodes_created += self.push_all_real_nodes(new_node, to);
@@ -768,7 +757,7 @@ impl VirtualDom {
             .into_iter()
             .map(|child| {
                 self.assign_boundary_ref(parent, child);
-                self.create(child)
+                self.create(child, to)
             })
             .sum()
     }
@@ -807,7 +796,7 @@ impl VirtualDom {
     ) {
         let m = self.create_children(r, Some(parent), to);
         let id = l.id.get().unwrap();
-        to.replace_with(id, m);
+        to.replace_node_with(id, m);
         self.reclaim(id);
     }
 
@@ -820,15 +809,10 @@ impl VirtualDom {
     ) {
         let m = self.create_children(right, parent, to);
 
-        self.remove_node(left, true, to);
+        // TODO: Instead of *just* removing it, we can use the replace mutation
+        to.insert_nodes_before(self.find_first_element(left), m);
 
-        // We want to optimize the replace case to use one less mutation if possible
-        // Since mutations are done in reverse, the last node removed will be the first in the stack
-        // Instead of *just* removing it, we can use the replace mutation
-        match self.mutations.edits.pop().unwrap() {
-            Mutation::Remove { id } => self.mutations.push(Mutation::ReplaceWith { id, m }),
-            _ => panic!("Expected remove mutation from remove_node"),
-        };
+        self.remove_node(left, true, to);
     }
 
     fn node_to_placeholder(
@@ -846,15 +830,22 @@ impl VirtualDom {
 
         to.create_placeholder(placeholder);
 
-        self.remove_nodes(l, to);
+        self.replace_nodes(l, 1, to);
+    }
 
+    /// Replace many nodes with a number of nodes on the stack
+    fn replace_nodes(&mut self, nodes: &[VNode], m: usize, to: &mut impl WriteMutations) {
         // We want to optimize the replace case to use one less mutation if possible
         // Since mutations are done in reverse, the last node removed will be the first in the stack
-        // Instead of *just* removing it, we can use the replace mutation
-        match self.mutations.edits.pop().unwrap() {
-            Mutation::Remove { id } => self.mutations.push(Mutation::ReplaceWith { id, m: 1 }),
-            _ => panic!("Expected remove mutation from remove_node"),
-        };
+        // TODO: Instead of *just* removing it, we can use the replace mutation
+        to.insert_nodes_before(self.find_first_element(&nodes[0]), m);
+
+        debug_assert!(
+            !nodes.is_empty(),
+            "replace_nodes must have at least one node"
+        );
+
+        self.remove_nodes(&nodes, to);
     }
 
     /// Remove these nodes from the dom
@@ -869,7 +860,7 @@ impl VirtualDom {
     fn remove_node(&mut self, node: &VNode, gen_muts: bool, to: &mut impl WriteMutations) {
         // Clean up any attributes that have claimed a static node as dynamic for mount/unmounta
         // Will not generate mutations!
-        self.reclaim_attributes(node, to);
+        self.reclaim_attributes(node);
 
         // Remove the nested dynamic nodes
         // We don't generate mutations for these, as they will be removed by the parent (in the next line)
@@ -895,7 +886,7 @@ impl VirtualDom {
         }
     }
 
-    fn reclaim_attributes(&mut self, node: &VNode, to: &mut impl WriteMutations) {
+    fn reclaim_attributes(&mut self, node: &VNode) {
         let mut id = None;
         for (idx, attr) in node.dynamic_attrs.iter().enumerate() {
             // We'll clean up the root nodes either way, so don't worry
diff --git a/packages/core/src/mutations.rs b/packages/core/src/mutations.rs
index 6ac45b690..0493194f2 100644
--- a/packages/core/src/mutations.rs
+++ b/packages/core/src/mutations.rs
@@ -15,7 +15,7 @@ use crate::{arena::ElementId, AttributeValue, ScopeId, Template};
 /// Mutations are the only link between the RealDOM and the VirtualDOM.
 pub trait WriteMutations {
     /// Register a template with the renderer
-    fn register_template(&mut self, template: Template<'static>);
+    fn register_template(&mut self, template: Template);
 
     /// Add these m children to the target element
     ///
@@ -333,7 +333,7 @@ pub struct MutationsVec {
     /// Any templates encountered while diffing the DOM.
     ///
     /// These must be loaded into a cache before applying the edits
-    pub templates: Vec<Template<'static>>,
+    pub templates: Vec<Template>,
 
     /// Any mutations required to patch the renderer to match the layout of the VirtualDom
     pub edits: Vec<Mutation>,
@@ -355,7 +355,7 @@ impl MutationsVec {
 }
 
 impl WriteMutations for MutationsVec {
-    fn register_template(&mut self, template: Template<'static>) {
+    fn register_template(&mut self, template: Template) {
         self.templates.push(template)
     }
 
@@ -468,7 +468,7 @@ impl WriteMutations for MutationsVec {
 pub struct NoOpMutations;
 
 impl WriteMutations for NoOpMutations {
-    fn register_template(&mut self, template: Template<'static>) {}
+    fn register_template(&mut self, template: Template) {}
 
     fn append_children(&mut self, id: ElementId, m: usize) {}
 
diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs
index e25a69209..ae310ab54 100644
--- a/packages/core/src/nodes.rs
+++ b/packages/core/src/nodes.rs
@@ -55,7 +55,7 @@ pub struct VNodeInner {
     pub root_ids: RefCell<Vec<ElementId>>,
 
     /// The static nodes and static descriptor of the template
-    pub template: Cell<Template<'static>>,
+    pub template: Cell<Template>,
 
     /// The dynamic parts of the template
     pub dynamic_nodes: Vec<DynamicNode>,
@@ -106,7 +106,7 @@ impl VNode {
     /// Create a new VNode
     pub fn new(
         key: Option<String>,
-        template: Template<'static>,
+        template: Template,
         root_ids: Vec<ElementId>,
         dynamic_nodes: Vec<DynamicNode>,
         dynamic_attrs: Vec<Attribute>,
@@ -144,7 +144,7 @@ impl VNode {
 /// ways, with the suggested approach being the unique code location (file, line, col, etc).
 #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))]
 #[derive(Debug, Clone, Copy, PartialEq, Hash, Eq, PartialOrd, Ord)]
-pub struct Template<'a> {
+pub struct Template {
     /// The name of the template. This must be unique across your entire program for template diffing to work properly
     ///
     /// If two templates have the same name, it's likely that Dioxus will panic when diffing.
@@ -152,13 +152,13 @@ pub struct Template<'a> {
         feature = "serialize",
         serde(deserialize_with = "deserialize_string_leaky")
     )]
-    pub name: &'a str,
+    pub name: &'static str,
 
     /// The list of template nodes that make up the template
     ///
     /// Unlike react, calls to `rsx!` can have multiple roots. This list supports that paradigm.
     #[cfg_attr(feature = "serialize", serde(deserialize_with = "deserialize_leaky"))]
-    pub roots: &'a [TemplateNode],
+    pub roots: &'static [TemplateNode],
 
     /// The paths of each node relative to the root of the template.
     ///
@@ -168,7 +168,7 @@ pub struct Template<'a> {
         feature = "serialize",
         serde(deserialize_with = "deserialize_bytes_leaky")
     )]
-    pub node_paths: &'a [&'a [u8]],
+    pub node_paths: &'static [&'static [u8]],
 
     /// The paths of each dynamic attribute relative to the root of the template
     ///
@@ -178,7 +178,7 @@ pub struct Template<'a> {
         feature = "serialize",
         serde(deserialize_with = "deserialize_bytes_leaky")
     )]
-    pub attr_paths: &'a [&'a [u8]],
+    pub attr_paths: &'static [&'static [u8]],
 }
 
 #[cfg(feature = "serialize")]
@@ -232,7 +232,7 @@ where
     Ok(deserialized.map(|deserialized| &*Box::leak(deserialized.into_boxed_str())))
 }
 
-impl<'a> Template<'a> {
+impl Template {
     /// Is this template worth caching at all, since it's completely runtime?
     ///
     /// There's no point in saving templates that are completely dynamic, since they'll be recreated every time anyway.
diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs
index 5537553a5..3aba1f912 100644
--- a/packages/core/src/scopes.rs
+++ b/packages/core/src/scopes.rs
@@ -310,7 +310,7 @@ impl<'src> ScopeState {
     }
 
     /// Create a new [`EventHandler`] from an [`FnMut`]
-    pub fn event_handler<T>(&self, f: impl FnMut(T)) -> EventHandler<T> {
+    pub fn event_handler<T>(&self, f: impl FnMut(T) + 'static) -> EventHandler<T> {
         let callback = RefCell::new(Some(Box::new(move |event: T| {
             f(event);
         }) as Box<dyn FnMut(T)>));
@@ -324,8 +324,8 @@ impl<'src> ScopeState {
     ///
     /// The callback must be confined to the lifetime of the ScopeState
     pub fn listener<T: 'static>(
-        &'src self,
-        mut callback: impl FnMut(Event<T>) + 'src,
+        &self,
+        mut callback: impl FnMut(Event<T>) + 'static,
     ) -> AttributeValue {
         AttributeValue::Listener(RefCell::new(Box::new(move |event: Event<dyn Any>| {
             if let Ok(data) = event.data.downcast::<T>() {
diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs
index b4209e7aa..d445c3077 100644
--- a/packages/core/src/virtual_dom.rs
+++ b/packages/core/src/virtual_dom.rs
@@ -6,10 +6,9 @@ use crate::{
     any_props::{BoxedAnyProps, VProps},
     arena::ElementId,
     innerlude::{
-        DirtyScope, ElementRef, ErrorBoundary, Mutations, NoOpMutations, Scheduler, SchedulerMsg,
+        DirtyScope, ElementRef, ErrorBoundary, NoOpMutations, Scheduler, SchedulerMsg,
         WriteMutations,
     },
-    mutations::Mutation,
     nodes::RenderReturn,
     nodes::{Template, TemplateId},
     runtime::{Runtime, RuntimeGuard},
@@ -186,7 +185,10 @@ pub struct VirtualDom {
     pub(crate) dirty_scopes: BTreeSet<DirtyScope>,
 
     // Maps a template path to a map of byteindexes to templates
-    pub(crate) templates: FxHashMap<TemplateId, FxHashMap<usize, Template<'static>>>,
+    pub(crate) templates: FxHashMap<TemplateId, FxHashMap<usize, Template>>,
+
+    // Templates changes that are queued for the next render
+    pub(crate) queued_templates: Vec<Template>,
 
     // The element ids that are used in the renderer
     pub(crate) elements: Slab<Option<ElementRef>>,
@@ -263,6 +265,7 @@ impl VirtualDom {
             scopes: Default::default(),
             dirty_scopes: Default::default(),
             templates: Default::default(),
+            queued_templates: Default::default(),
             elements: Default::default(),
             suspended_scopes: Default::default(),
         };
@@ -361,7 +364,6 @@ impl VirtualDom {
             _ => return,
         };
         let mut parent_node = Some(parent_path);
-        let mut listeners = vec![];
 
         // We will clone this later. The data itself is wrapped in RC to be used in callbacks if required
         let uievent = Event {
@@ -373,6 +375,8 @@ impl VirtualDom {
         if bubbles {
             // Loop through each dynamic attribute (in a depth first order) in this template before moving up to the template's parent.
             while let Some(path) = parent_node {
+                let mut listeners = vec![];
+
                 let el_ref = &path.element;
                 let node_template = el_ref.template.get();
                 let target_path = path.path;
@@ -397,7 +401,7 @@ impl VirtualDom {
 
                 // Now that we've accumulated all the parent attributes for the target element, call them in reverse order
                 // We check the bubble state between each call to see if the event has been stopped from bubbling
-                for listener in listeners.drain(..).rev() {
+                for listener in listeners.into_iter().rev() {
                     if let AttributeValue::Listener(listener) = listener {
                         let origin = path.scope;
                         self.runtime.scope_stack.borrow_mut().push(origin);
@@ -505,7 +509,7 @@ impl VirtualDom {
     /// The caller must ensure that the template refrences the same dynamic attributes and nodes as the original template.
     ///
     /// This will only replace the the parent template, not any nested templates.
-    pub fn replace_template(&mut self, template: Template<'static>) {
+    pub fn replace_template(&mut self, template: Template) {
         self.register_template_first_byte_index(template);
         // iterating a slab is very inefficient, but this is a rare operation that will only happen during development so it's fine
         for (_, scope) in self.scopes.iter() {
@@ -545,8 +549,9 @@ impl VirtualDom {
     /// apply_edits(edits);
     /// ```
     pub fn rebuild(&mut self, to: &mut impl WriteMutations) {
+        self.flush_templates(to);
         let _runtime = RuntimeGuard::new(self.runtime.clone());
-        match unsafe { self.run_scope(ScopeId::ROOT) } {
+        match self.run_scope(ScopeId::ROOT) {
             // Rebuilding implies we append the created elements to the root
             RenderReturn::Ready(node) => {
                 let m = self.create_scope(ScopeId::ROOT, &node, to);
@@ -565,6 +570,7 @@ impl VirtualDom {
     /// Render whatever the VirtualDom has ready as fast as possible without requiring an executor to progress
     /// suspended subtrees.
     pub fn render_immediate(&mut self, to: &mut impl WriteMutations) {
+        self.flush_templates(to);
         // Build a waker that won't wake up since our deadline is already expired when it's polled
         let waker = futures_util::task::noop_waker();
         let mut cx = std::task::Context::from_waker(&waker);
@@ -605,6 +611,7 @@ impl VirtualDom {
         deadline: impl Future<Output = ()>,
         to: &mut impl WriteMutations,
     ) {
+        self.flush_templates(to);
         pin_mut!(deadline);
 
         self.process_events();
@@ -653,6 +660,13 @@ impl VirtualDom {
     pub fn runtime(&self) -> Rc<Runtime> {
         self.runtime.clone()
     }
+
+    /// Flush any queued template changes
+    pub fn flush_templates(&mut self, to: &mut impl WriteMutations) {
+        for template in self.queued_templates.drain(..) {
+            to.register_template(template);
+        }
+    }
 }
 
 impl Drop for VirtualDom {
diff --git a/packages/core/tests/fuzzing.rs b/packages/core/tests/fuzzing.rs
index 02017307f..3f0b95e48 100644
--- a/packages/core/tests/fuzzing.rs
+++ b/packages/core/tests/fuzzing.rs
@@ -130,7 +130,7 @@ enum DynamicNodeType {
     Other,
 }
 
-fn create_random_template(name: &'static str) -> (Template<'static>, Vec<DynamicNodeType>) {
+fn create_random_template(name: &'static str) -> (Template, Vec<DynamicNodeType>) {
     let mut dynamic_node_type = Vec::new();
     let mut template_idx = 0;
     let mut attr_idx = 0;