restore replace optimization

packages/core/src/diff/component.rs

@@ -112,12 +112,10 @@ impl VNode {
     ) {
         let scope = ScopeId(dom.mounts[mount.0].mounted_dynamic_nodes[idx]);
 
-        let _m = self.create_component_node(mount, idx, new, parent, dom, to);
+        let m = self.create_component_node(mount, idx, new, parent, dom, to);
 
-        // TODO: Instead of *just* removing it, we can use the replace mutation
+        // Instead of *just* removing it, we can use the replace mutation
-        dom.remove_component_node(to, scope, true);
+        dom.remove_component_node(to, scope, Some(m), true);
-
-        todo!()
     }
 
     pub(super) fn create_component_node(

packages/core/src/diff/iterator.rs

@@ -54,7 +54,7 @@ impl VirtualDom {
         debug_assert!(!old.is_empty());
 
         match old.len().cmp(&new.len()) {
-            Ordering::Greater => self.remove_nodes(to, &old[new.len()..]),
+            Ordering::Greater => self.remove_nodes(to, &old[new.len()..], None),
             Ordering::Less => {
                 self.create_and_insert_after(to, &new[old.len()..], old.last().unwrap(), parent)
             }
@@ -135,7 +135,7 @@ impl VirtualDom {
 
         if new_middle.is_empty() {
             // remove the old elements
-            self.remove_nodes(to, old_middle);
+            self.remove_nodes(to, old_middle, None);
         } else if old_middle.is_empty() {
             // there were no old elements, so just create the new elements
             // we need to find the right "foothold" though - we shouldn't use the "append" at all
@@ -190,7 +190,7 @@ impl VirtualDom {
         // And if that was all of the new children, then remove all of the remaining
         // old children and we're finished.
         if left_offset == new.len() {
-            self.remove_nodes(to, &old[left_offset..]);
+            self.remove_nodes(to, &old[left_offset..], None);
             return None;
         }
 
@@ -284,7 +284,7 @@ impl VirtualDom {
         // create the new children afresh.
         if shared_keys.is_empty() {
             if !old.is_empty() {
-                self.remove_nodes(to, &old[1..]);
+                self.remove_nodes(to, &old[1..], None);
                 old[0].replace(new, parent, self, to);
             } else {
                 // I think this is wrong - why are we appending?
@@ -301,7 +301,7 @@ impl VirtualDom {
         for child in old {
             let key = child.key.as_ref().unwrap();
             if !shared_keys.contains(&key) {
-                child.remove_node(self, to, true);
+                child.remove_node(self, to, None, true);
             }
         }
 

packages/core/src/diff/mod.rs

@@ -26,13 +26,6 @@ impl VirtualDom {
             .sum()
     }
 
-    fn remove_element_id(&mut self, to: &mut impl WriteMutations, id: ElementId, gen_muts: bool) {
-        if gen_muts {
-            to.remove_node(id);
-        }
-        self.reclaim(id)
-    }
-
     /// Simply replace a placeholder with a list of nodes
     fn replace_placeholder<'a>(
         &mut self,
@@ -66,38 +59,40 @@ impl VirtualDom {
 
     /// Replace many nodes with a number of nodes on the stack
     fn replace_nodes(&mut self, to: &mut impl WriteMutations, nodes: &[VNode], m: usize) {
-        // 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
-        // TODO: Instead of *just* removing it, we can use the replace mutation
-        let first_element = nodes[0].find_first_element(self);
-        to.insert_nodes_before(first_element, m);
-
         debug_assert!(
             !nodes.is_empty(),
             "replace_nodes must have at least one node"
         );
 
-        self.remove_nodes(to, nodes);
+        // We want to optimize the replace case to use one less mutation if possible
+        // Instead of *just* removing it, we can use the replace mutation
+        self.remove_nodes(to, nodes, Some(m));
     }
 
     /// Remove these nodes from the dom
     /// Wont generate mutations for the inner nodes
-    fn remove_nodes(&mut self, to: &mut impl WriteMutations, nodes: &[VNode]) {
+    fn remove_nodes(
-        nodes
+        &mut self,
-            .iter()
+        to: &mut impl WriteMutations,
-            .rev()
+        nodes: &[VNode],
-            .for_each(|node| node.remove_node(self, to, true));
+        replace_with: Option<usize>,
+    ) {
+        for (i, node) in nodes.iter().rev().enumerate() {
+            let last_node = i == nodes.len() - 1;
+            node.remove_node(self, to, replace_with.filter(|_| last_node), true);
+        }
     }
 
     pub(crate) fn remove_component_node(
         &mut self,
         to: &mut impl WriteMutations,
         scope: ScopeId,
+        replace_with: Option<usize>,
         gen_muts: bool,
     ) {
         // Remove the component from the dom
         if let Some(node) = self.scopes[scope.0].last_rendered_node.take() {
-            node.remove_node(self, to, gen_muts)
+            node.remove_node(self, to, replace_with, gen_muts)
         };
 
         // Now drop all the resources

packages/core/src/diff/node.rs

@@ -179,17 +179,15 @@ impl VNode {
     ) {
         let m = dom.create_children(to, right, parent);
 
-        // TODO: Instead of *just* removing it, we can use the replace mutation
+        // Instead of *just* removing it, we can use the replace mutation
-        let first_element = self.find_first_element(dom);
+        self.remove_node(dom, to, Some(m), true)
-        to.insert_nodes_before(first_element, m);
-
-        self.remove_node(dom, to, true)
     }
 
     pub(crate) fn remove_node(
         &self,
         dom: &mut VirtualDom,
         to: &mut impl WriteMutations,
+        replace_with: Option<usize>,
         gen_muts: bool,
     ) {
         let mount = self.mount.get();
@@ -205,7 +203,7 @@ impl VNode {
 
         // Clean up the roots, assuming we need to generate mutations for these
         // This is done last in order to preserve Node ID reclaim order (reclaim in reverse order of claim)
-        self.reclaim_roots(mount, dom, to, gen_muts);
+        self.reclaim_roots(mount, dom, to, replace_with, gen_muts);
 
         // Remove the mount information
         dom.mounts.remove(mount.0);
@@ -218,17 +216,32 @@ impl VNode {
         mount: MountId,
         dom: &mut VirtualDom,
         to: &mut impl WriteMutations,
+        replace_with: Option<usize>,
         gen_muts: bool,
     ) {
-        for (idx, node) in self.template.get().roots.iter().enumerate() {
+        let roots = self.template.get().roots;
+        for (idx, node) in roots.iter().enumerate() {
+            let last_node = idx == roots.len() - 1;
             if let Some(id) = node.dynamic_id() {
                 let dynamic_node = &self.dynamic_nodes[id];
-                self.remove_dynamic_node(mount, dom, to, idx, dynamic_node, gen_muts);
+                self.remove_dynamic_node(
+                    mount,
+                    dom,
+                    to,
+                    idx,
+                    dynamic_node,
+                    replace_with.filter(|_| last_node),
+                    gen_muts,
+                );
             } else {
                 let mount = &dom.mounts[mount.0];
                 let id = mount.root_ids[idx];
                 if gen_muts {
-                    to.remove_node(id);
+                    if let (true, Some(replace_with)) = (last_node, replace_with) {
+                        to.replace_node_with(id, replace_with);
+                    } else {
+                        to.remove_node(id);
+                    }
                 }
                 dom.reclaim(id);
             }
@@ -246,7 +259,7 @@ impl VNode {
             let path_len = template.node_paths.get(idx).map(|path| path.len());
             // Roots are cleaned up automatically above and nodes with a empty path are placeholders
             if let Some(2..) = path_len {
-                self.remove_dynamic_node(mount, dom, to, idx, dyn_node, false)
+                self.remove_dynamic_node(mount, dom, to, idx, dyn_node, None, false)
             }
         }
     }
@@ -258,20 +271,28 @@ impl VNode {
         to: &mut impl WriteMutations,
         idx: usize,
         node: &DynamicNode,
+        replace_with: Option<usize>,
         gen_muts: bool,
     ) {
         match node {
             Component(_comp) => {
                 let scope_id = ScopeId(dom.mounts[mount.0].mounted_dynamic_nodes[idx]);
-                dom.remove_component_node(to, scope_id, gen_muts);
+                dom.remove_component_node(to, scope_id, replace_with, gen_muts);
             }
             Text(_) | Placeholder(_) => {
                 let id = ElementId(dom.mounts[mount.0].mounted_dynamic_nodes[idx]);
-                dom.remove_element_id(to, id, gen_muts)
+                if gen_muts {
+                    if let Some(replace_with) = replace_with {
+                        to.replace_node_with(id, replace_with);
+                    } else {
+                        to.remove_node(id);
+                    }
+                }
+                dom.reclaim(id)
             }
             Fragment(nodes) => nodes
                 .iter()
-                .for_each(|_node| self.remove_node(dom, to, gen_muts)),
+                .for_each(|_node| self.remove_node(dom, to, replace_with, gen_muts)),
         };
     }
 

packages/core/tests/attr_cleanup.rs

@@ -9,7 +9,7 @@ use dioxus::prelude::*;
 fn attrs_cycle() {
     let mut dom = VirtualDom::new(|| {
         let id = generation();
-        match generation() % 2 {
+        match id % 2 {
             0 => render! { div {} },
             1 => render! {
                 div { h1 { class: "{id}", id: "{id}" } }