diff --git a/packages/core/src/diff/component.rs b/packages/core/src/diff/component.rs index d79c1da5a..047951dfa 100644 --- a/packages/core/src/diff/component.rs +++ b/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 - dom.remove_component_node(to, scope, true); - - todo!() + // Instead of *just* removing it, we can use the replace mutation + dom.remove_component_node(to, scope, Some(m), true); } pub(super) fn create_component_node( diff --git a/packages/core/src/diff/iterator.rs b/packages/core/src/diff/iterator.rs index 7c3555e15..fefab3c93 100644 --- a/packages/core/src/diff/iterator.rs +++ b/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); } } diff --git a/packages/core/src/diff/mod.rs b/packages/core/src/diff/mod.rs index db6b25e9b..9f439d15a 100644 --- a/packages/core/src/diff/mod.rs +++ b/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]) { - nodes - .iter() - .rev() - .for_each(|node| node.remove_node(self, to, true)); + fn remove_nodes( + &mut self, + to: &mut impl WriteMutations, + nodes: &[VNode], + replace_with: Option, + ) { + 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, 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 diff --git a/packages/core/src/diff/node.rs b/packages/core/src/diff/node.rs index b8777f2eb..ce9c02f44 100644 --- a/packages/core/src/diff/node.rs +++ b/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 - let first_element = self.find_first_element(dom); - to.insert_nodes_before(first_element, m); - - self.remove_node(dom, to, true) + // Instead of *just* removing it, we can use the replace mutation + self.remove_node(dom, to, Some(m), true) } pub(crate) fn remove_node( &self, dom: &mut VirtualDom, to: &mut impl WriteMutations, + replace_with: Option, 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, 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, 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)), }; } diff --git a/packages/core/tests/attr_cleanup.rs b/packages/core/tests/attr_cleanup.rs index d545f414e..e84beb7e5 100644 --- a/packages/core/tests/attr_cleanup.rs +++ b/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}" } }