fix: node reclaimation

packages/core/src/arena.rs

@@ -31,6 +31,8 @@ impl VirtualDom {
             path,
         });
 
+        println!("Claiming {}", id);
+
         ElementId(id)
     }
 

packages/core/src/diff.rs

@@ -227,17 +227,16 @@ impl<'b: 'static> VirtualDom {
     }
 
     fn replace_placeholder_with_nodes(&mut self, l: &'b Cell<ElementId>, r: &'b [VNode<'b>]) {
-        let created = self.create_children(r);
-        self.mutations.push(Mutation::ReplaceWith {
-            id: l.get(),
-            m: created,
-        })
+        let m = self.create_children(r);
+        let id = l.get();
+        self.mutations.push(Mutation::ReplaceWith { id, m });
+        self.reclaim(id);
     }
 
     fn replace_nodes_with_placeholder(&mut self, l: &'b [VNode<'b>], r: &'b Cell<ElementId>) {
         // Remove the old nodes, except for onea
-        self.remove_nodes(&l[1..]);
         let first = self.replace_inner(&l[0]);
+        self.remove_nodes(&l[1..]);
 
         let placeholder = self.next_element(&l[0], &[]);
         r.set(placeholder);
@@ -252,24 +251,20 @@ impl<'b: 'static> VirtualDom {
 
     // Remove all the top-level nodes, returning the firstmost root ElementId
     fn replace_inner(&mut self, node: &'b VNode<'b>) -> ElementId {
-        let id = match &node.template.roots[0] {
-            TemplateNode::Text(_) | TemplateNode::Element { .. } => node.root_ids[0].get(),
-            TemplateNode::DynamicText(id) | TemplateNode::Dynamic(id) => {
-                match &node.dynamic_nodes[*id] {
-                    Text(t) => t.id.get(),
-                    Fragment(VFragment::Empty(e)) => e.get(),
-                    Fragment(VFragment::NonEmpty(nodes)) => {
-                        let id = self.replace_inner(&nodes[0]);
-                        self.remove_nodes(&nodes[1..]);
-                        id
-                    }
-                    Component(comp) => {
-                        let scope = comp.scope.get().unwrap();
-                        match unsafe { self.scopes[scope.0].root_node().extend_lifetime_ref() } {
-                            RenderReturn::Sync(Ok(t)) => self.replace_inner(t),
-                            _ => todo!("cannot handle nonstandard nodes"),
-                        }
-                    }
+        let id = match node.dynamic_root(0) {
+            None => node.root_ids[0].get(),
+            Some(Text(t)) => t.id.get(),
+            Some(Fragment(VFragment::Empty(e))) => e.get(),
+            Some(Fragment(VFragment::NonEmpty(nodes))) => {
+                let id = self.replace_inner(&nodes[0]);
+                self.remove_nodes(&nodes[1..]);
+                id
+            }
+            Some(Component(comp)) => {
+                let scope = comp.scope.get().unwrap();
+                match unsafe { self.scopes[scope.0].root_node().extend_lifetime_ref() } {
+                    RenderReturn::Sync(Ok(t)) => self.replace_inner(t),
+                    _ => todo!("cannot handle nonstandard nodes"),
                 }
             }
         };
@@ -305,40 +300,43 @@ impl<'b: 'static> VirtualDom {
                 }
             };
         }
+
+        // we also need to clean up dynamic attribute roots
+        // let last_node = None;
+        // for attr in node.dynamic_attrs {
+        //     match last_node {
+        //         Some(node) => todo!(),
+        //         None => todo!(),
+        //     }
+        // }
     }
 
     fn remove_root_node(&mut self, node: &'b VNode<'b>, idx: usize) {
-        let root = node.template.roots[idx];
-        match root {
-            TemplateNode::Element { .. } | TemplateNode::Text(_) => {
+        match node.dynamic_root(idx) {
+            Some(Text(i)) => {
+                let id = i.id.get();
+                self.mutations.push(Mutation::Remove { id });
+                self.reclaim(id);
+            }
+            Some(Fragment(VFragment::Empty(e))) => {
+                let id = e.get();
+                self.mutations.push(Mutation::Remove { id });
+                self.reclaim(id);
+            }
+            Some(Fragment(VFragment::NonEmpty(nodes))) => self.remove_nodes(nodes),
+            Some(Component(comp)) => {
+                let scope = comp.scope.get().unwrap();
+                match unsafe { self.scopes[scope.0].root_node().extend_lifetime_ref() } {
+                    RenderReturn::Sync(Ok(t)) => self.remove_node(t),
+                    _ => todo!("cannot handle nonstandard nodes"),
+                };
+            }
+            None => {
                 let id = node.root_ids[idx].get();
                 self.mutations.push(Mutation::Remove { id });
                 self.reclaim(id);
             }
-
-            TemplateNode::DynamicText(id) | TemplateNode::Dynamic(id) => {
-                match &node.dynamic_nodes[id] {
-                    Text(i) => {
-                        let id = i.id.get();
-                        self.mutations.push(Mutation::Remove { id });
-                        self.reclaim(id);
-                    }
-                    Fragment(VFragment::Empty(e)) => {
-                        let id = e.get();
-                        self.mutations.push(Mutation::Remove { id });
-                        self.reclaim(id);
-                    }
-                    Fragment(VFragment::NonEmpty(nodes)) => self.remove_nodes(nodes),
-                    Component(comp) => {
-                        let scope = comp.scope.get().unwrap();
-                        match unsafe { self.scopes[scope.0].root_node().extend_lifetime_ref() } {
-                            RenderReturn::Sync(Ok(t)) => self.remove_node(t),
-                            _ => todo!("cannot handle nonstandard nodes"),
-                        };
-                    }
-                };
-            }
-        }
+        };
     }
 
     fn diff_non_empty_fragment(&mut self, new: &'b [VNode<'b>], old: &'b [VNode<'b>]) {
@@ -729,21 +727,26 @@ impl<'b: 'static> VirtualDom {
     }
 
     fn remove_node(&mut self, node: &'b VNode<'b>) {
-        for (idx, root) in node.template.roots.iter().enumerate() {
-            let id = match root {
-                TemplateNode::Text(_) | TemplateNode::Element { .. } => node.root_ids[idx].get(),
-                TemplateNode::DynamicText(id) | TemplateNode::Dynamic(id) => {
-                    match &node.dynamic_nodes[*id] {
-                        Text(t) => t.id.get(),
-                        Fragment(VFragment::Empty(t)) => t.get(),
-                        Fragment(VFragment::NonEmpty(t)) => return self.remove_nodes(t),
-                        Component(comp) => return self.remove_component(comp.scope.get().unwrap()),
-                    }
-                }
+        for (idx, _) in node.template.roots.iter().enumerate() {
+            let id = match node.dynamic_root(idx) {
+                Some(Text(t)) => t.id.get(),
+                Some(Fragment(VFragment::Empty(t))) => t.get(),
+                Some(Fragment(VFragment::NonEmpty(t))) => return self.remove_nodes(t),
+                Some(Component(comp)) => return self.remove_component(comp.scope.get().unwrap()),
+                None => node.root_ids[idx].get(),
             };
 
             self.mutations.push(Mutation::Remove { id })
         }
+
+        self.clean_up_node(node);
+
+        for root in node.root_ids {
+            let id = root.get();
+            if id.0 != 0 {
+                self.reclaim(id);
+            }
+        }
     }
 
     fn remove_component(&mut self, scope_id: ScopeId) {
@@ -782,42 +785,32 @@ impl<'b: 'static> VirtualDom {
     }
 
     fn find_first_element(&self, node: &VNode) -> ElementId {
-        match &node.template.roots[0] {
-            TemplateNode::Element { .. } | TemplateNode::Text(_) => node.root_ids[0].get(),
-            TemplateNode::DynamicText(id) | TemplateNode::Dynamic(id) => {
-                match &node.dynamic_nodes[*id] {
-                    Text(t) => t.id.get(),
-                    Fragment(VFragment::NonEmpty(t)) => self.find_first_element(&t[0]),
-                    Fragment(VFragment::Empty(t)) => t.get(),
-                    Component(comp) => {
-                        let scope = comp.scope.get().unwrap();
-                        match self.scopes[scope.0].root_node() {
-                            RenderReturn::Sync(Ok(t)) => self.find_first_element(t),
-                            _ => todo!("cannot handle nonstandard nodes"),
-                        }
-                    }
+        match node.dynamic_root(0) {
+            None => node.root_ids[0].get(),
+            Some(Text(t)) => t.id.get(),
+            Some(Fragment(VFragment::NonEmpty(t))) => self.find_first_element(&t[0]),
+            Some(Fragment(VFragment::Empty(t))) => t.get(),
+            Some(Component(comp)) => {
+                let scope = comp.scope.get().unwrap();
+                match self.scopes[scope.0].root_node() {
+                    RenderReturn::Sync(Ok(t)) => self.find_first_element(t),
+                    _ => todo!("cannot handle nonstandard nodes"),
                 }
             }
         }
     }
 
     fn find_last_element(&self, node: &VNode) -> ElementId {
-        match node.template.roots.last().unwrap() {
-            TemplateNode::Element { .. } | TemplateNode::Text(_) => {
-                node.root_ids.last().unwrap().get()
-            }
-            TemplateNode::DynamicText(id) | TemplateNode::Dynamic(id) => {
-                match &node.dynamic_nodes[*id] {
-                    Text(t) => t.id.get(),
-                    Fragment(VFragment::NonEmpty(t)) => self.find_last_element(t.last().unwrap()),
-                    Fragment(VFragment::Empty(t)) => t.get(),
-                    Component(comp) => {
-                        let scope = comp.scope.get().unwrap();
-                        match self.scopes[scope.0].root_node() {
-                            RenderReturn::Sync(Ok(t)) => self.find_last_element(t),
-                            _ => todo!("cannot handle nonstandard nodes"),
-                        }
-                    }
+        match node.dynamic_root(node.template.roots.len() - 1) {
+            None => node.root_ids.last().unwrap().get(),
+            Some(Text(t)) => t.id.get(),
+            Some(Fragment(VFragment::NonEmpty(t))) => self.find_last_element(t.last().unwrap()),
+            Some(Fragment(VFragment::Empty(t))) => t.get(),
+            Some(Component(comp)) => {
+                let scope = comp.scope.get().unwrap();
+                match self.scopes[scope.0].root_node() {
+                    RenderReturn::Sync(Ok(t)) => self.find_last_element(t),
+                    _ => todo!("cannot handle nonstandard nodes"),
                 }
             }
         }

packages/core/src/garbage.rs

@@ -15,6 +15,7 @@ impl<'b> VirtualDom {
 
     pub fn reclaim(&mut self, el: ElementId) {
         assert_ne!(el, ElementId(0));
+        println!("reclaiming {}", el.0);
         self.elements.remove(el.0);
     }
 

packages/core/src/nodes.rs

@@ -54,6 +54,15 @@ impl<'a> VNode<'a> {
             },
         })
     }
+
+    pub fn dynamic_root(&self, idx: usize) -> Option<&'a DynamicNode<'a>> {
+        match &self.template.roots[idx] {
+            TemplateNode::Element { .. } | TemplateNode::Text(_) => None,
+            TemplateNode::Dynamic(id) | TemplateNode::DynamicText(id) => {
+                Some(&self.dynamic_nodes[*id])
+            }
+        }
+    }
 }
 
 #[derive(Debug, Clone, Copy)]

packages/core/tests/diff_element.rs

@@ -59,7 +59,7 @@ fn element_swap() {
     assert_eq!(
         vdom.render_immediate().santize().edits,
         [
-            LoadTemplate { name: "template", index: 0, id: ElementId(3,) },
+            LoadTemplate { name: "template", index: 0, id: ElementId(1,) },
             ReplaceWith { id: ElementId(2,), m: 1 },
         ]
     );
@@ -68,8 +68,8 @@ fn element_swap() {
     assert_eq!(
         vdom.render_immediate().santize().edits,
         [
-            LoadTemplate { name: "template", index: 0, id: ElementId(4,) },
-            ReplaceWith { id: ElementId(3,), m: 1 },
+            LoadTemplate { name: "template", index: 0, id: ElementId(2,) },
+            ReplaceWith { id: ElementId(1,), m: 1 },
         ]
     );
 
@@ -77,8 +77,8 @@ fn element_swap() {
     assert_eq!(
         vdom.render_immediate().santize().edits,
         [
-            LoadTemplate { name: "template", index: 0, id: ElementId(5,) },
-            ReplaceWith { id: ElementId(4,), m: 1 },
+            LoadTemplate { name: "template", index: 0, id: ElementId(1,) },
+            ReplaceWith { id: ElementId(2,), m: 1 },
         ]
     );
 }

packages/core/tests/diff_unkeyed_list.rs

@@ -41,8 +41,8 @@ fn list_creates_one_by_one() {
     assert_eq!(
         dom.render_immediate().santize().edits,
         [
-            LoadTemplate { name: "template", index: 0, id: ElementId(5,) },
-            HydrateText { path: &[0], value: "1", id: ElementId(6,) },
+            LoadTemplate { name: "template", index: 0, id: ElementId(2,) },
+            HydrateText { path: &[0], value: "1", id: ElementId(5,) },
             InsertAfter { id: ElementId(3,), m: 1 },
         ]
     );
@@ -52,9 +52,9 @@ fn list_creates_one_by_one() {
     assert_eq!(
         dom.render_immediate().santize().edits,
         [
-            LoadTemplate { name: "template", index: 0, id: ElementId(7,) },
-            HydrateText { path: &[0], value: "2", id: ElementId(8,) },
-            InsertAfter { id: ElementId(5,), m: 1 },
+            LoadTemplate { name: "template", index: 0, id: ElementId(6,) },
+            HydrateText { path: &[0], value: "2", id: ElementId(7,) },
+            InsertAfter { id: ElementId(2,), m: 1 },
         ]
     );
 
@@ -63,9 +63,9 @@ fn list_creates_one_by_one() {
     assert_eq!(
         dom.render_immediate().santize().edits,
         [
-            LoadTemplate { name: "template", index: 0, id: ElementId(9,) },
-            HydrateText { path: &[0], value: "3", id: ElementId(10,) },
-            InsertAfter { id: ElementId(7,), m: 1 },
+            LoadTemplate { name: "template", index: 0, id: ElementId(8,) },
+            HydrateText { path: &[0], value: "3", id: ElementId(9,) },
+            InsertAfter { id: ElementId(6,), m: 1 },
         ]
     );
 }
@@ -125,7 +125,7 @@ fn removes_one_by_one() {
     assert_eq!(
         dom.render_immediate().santize().edits,
         [
-            CreatePlaceholder { id: ElementId(8) },
+            CreatePlaceholder { id: ElementId(3) },
             ReplaceWith { id: ElementId(2), m: 1 }
         ]
     );
@@ -136,13 +136,13 @@ fn removes_one_by_one() {
     assert_eq!(
         dom.render_immediate().santize().edits,
         [
-            LoadTemplate { name: "template", index: 0, id: ElementId(9) },
-            HydrateText { path: &[0], value: "0", id: ElementId(10) },
-            LoadTemplate { name: "template", index: 0, id: ElementId(11) },
-            HydrateText { path: &[0], value: "1", id: ElementId(12) },
-            LoadTemplate { name: "template", index: 0, id: ElementId(13) },
-            HydrateText { path: &[0], value: "2", id: ElementId(14) },
-            ReplaceWith { id: ElementId(8), m: 3 }
+            LoadTemplate { name: "template", index: 0, id: ElementId(2) },
+            HydrateText { path: &[0], value: "0", id: ElementId(4) },
+            LoadTemplate { name: "template", index: 0, id: ElementId(5) },
+            HydrateText { path: &[0], value: "1", id: ElementId(6) },
+            LoadTemplate { name: "template", index: 0, id: ElementId(7) },
+            HydrateText { path: &[0], value: "2", id: ElementId(8) },
+            ReplaceWith { id: ElementId(3), m: 3 }
         ]
     );
 }
@@ -150,10 +150,9 @@ fn removes_one_by_one() {
 #[test]
 fn list_shrink_multiroot() {
     let mut dom = VirtualDom::new(|cx| {
-        let gen = cx.generation();
         cx.render(rsx! {
             div {
-                (0..gen).map(|i| rsx! {
+                (0..cx.generation()).map(|i| rsx! {
                     div { "{i}" }
                     div { "{i}" }
                 })
@@ -186,10 +185,10 @@ fn list_shrink_multiroot() {
     assert_eq!(
         dom.render_immediate().santize().edits,
         [
-            LoadTemplate { name: "template", index: 0, id: ElementId(7) },
-            HydrateText { path: &[0], value: "1", id: ElementId(8) },
-            LoadTemplate { name: "template", index: 1, id: ElementId(9) },
-            HydrateText { path: &[0], value: "1", id: ElementId(10) },
+            LoadTemplate { name: "template", index: 0, id: ElementId(2) },
+            HydrateText { path: &[0], value: "1", id: ElementId(7) },
+            LoadTemplate { name: "template", index: 1, id: ElementId(8) },
+            HydrateText { path: &[0], value: "1", id: ElementId(9) },
             InsertAfter { id: ElementId(5), m: 2 }
         ]
     );
@@ -198,11 +197,11 @@ fn list_shrink_multiroot() {
     assert_eq!(
         dom.render_immediate().santize().edits,
         [
-            LoadTemplate { name: "template", index: 0, id: ElementId(11) },
-            HydrateText { path: &[0], value: "2", id: ElementId(12) },
-            LoadTemplate { name: "template", index: 1, id: ElementId(13) },
-            HydrateText { path: &[0], value: "2", id: ElementId(14) },
-            InsertAfter { id: ElementId(9), m: 2 }
+            LoadTemplate { name: "template", index: 0, id: ElementId(10) },
+            HydrateText { path: &[0], value: "2", id: ElementId(11) },
+            LoadTemplate { name: "template", index: 1, id: ElementId(12) },
+            HydrateText { path: &[0], value: "2", id: ElementId(13) },
+            InsertAfter { id: ElementId(8), m: 2 }
         ]
     );
 }
@@ -266,7 +265,7 @@ fn removes_one_by_one_multiroot() {
         dom.render_immediate().santize().edits,
         [
             Remove { id: ElementId(4) },
-            CreatePlaceholder { id: ElementId(14) },
+            CreatePlaceholder { id: ElementId(5) },
             ReplaceWith { id: ElementId(2), m: 1 }
         ]
     );