chore: dynamic attributes cleanup

packages/core/src/arena.rs

@@ -1,4 +1,4 @@
-use crate::{nodes::VNode, virtual_dom::VirtualDom};
+use crate::{nodes::VNode, virtual_dom::VirtualDom, Mutations, ScopeId};
 
 #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))]
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
@@ -21,7 +21,7 @@ impl ElementRef {
     }
 }
 
-impl VirtualDom {
+impl<'b> VirtualDom {
     pub fn next_element(&mut self, template: &VNode, path: &'static [u8]) -> ElementId {
         let entry = self.elements.vacant_entry();
         let id = entry.key();
@@ -39,11 +39,40 @@ impl VirtualDom {
     pub fn cleanup_element(&mut self, id: ElementId) {
         self.elements.remove(id.0);
     }
-}
 
-/*
-now......
+    pub fn drop_scope(&mut self, id: ScopeId) {
+        // let scope = self.scopes.get(id.0).unwrap();
+
+        // let root = scope.root_node();
+        // let root = unsafe { std::mem::transmute(root) };
+
+        // self.drop_template(root, false);
+        todo!()
+    }
+
+    pub fn reclaim(&mut self, el: ElementId) {
+        assert_ne!(el, ElementId(0));
+        self.elements.remove(el.0);
+    }
+
+    pub fn drop_template(
+        &mut self,
+        mutations: &mut Mutations,
+        template: &'b VNode<'b>,
+        gen_roots: bool,
+    ) {
+        // for node in template.dynamic_nodes.iter() {
+        //     match node {
+        //         DynamicNode::Text { id, .. } => {}
+
+        //         DynamicNode::Component { .. } => {
+        //             todo!()
+        //         }
 
-an ID is mostly a pointer to a node in the real dom.
-We need to
-*/
+        //         DynamicNode::Fragment { inner, nodes } => {}
+        //         DynamicNode::Placeholder(_) => todo!(),
+        //         _ => todo!(),
+        //     }
+        // }
+    }
+}

packages/core/src/create.rs

@@ -80,7 +80,15 @@ impl<'b: 'static> VirtualDom {
                         // Else, it's deep in the template and we should create a new id for it
                         let id = match path.len() {
                             1 => this_id,
-                            _ => self.next_element(template, template.template.attr_paths[attr_id]),
+                            _ => {
+                                let id = self
+                                    .next_element(template, template.template.attr_paths[attr_id]);
+                                self.mutations.push(Mutation::AssignId {
+                                    path: &path[1..],
+                                    id,
+                                });
+                                id
+                            }
                         };
 
                         loop {

packages/core/src/diff.rs

@@ -301,14 +301,24 @@ 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!(),
-        //     }
-        // }
+        // we clean up nodes with dynamic attributes, provided the node is unique and not a root node
+        let mut id = None;
+        for (idx, attr) in node.dynamic_attrs.into_iter().enumerate() {
+            // We'll clean up the root nodes either way, so don't worry
+            if node.template.attr_paths[idx].len() == 1 {
+                continue;
+            }
+
+            let next_id = attr.mounted_element.get();
+
+            if id == Some(next_id) {
+                continue;
+            }
+
+            id = Some(next_id);
+
+            self.reclaim(next_id);
+        }
     }
 
     fn remove_root_node(&mut self, node: &'b VNode<'b>, idx: usize) {

packages/core/src/garbage.rs

@@ -1,42 +0,0 @@
-use crate::{
-    nodes::VNode, scopes::ScopeId, virtual_dom::VirtualDom, DynamicNode, ElementId, Mutations,
-};
-
-impl<'b> VirtualDom {
-    pub fn drop_scope(&mut self, id: ScopeId) {
-        // let scope = self.scopes.get(id.0).unwrap();
-
-        // let root = scope.root_node();
-        // let root = unsafe { std::mem::transmute(root) };
-
-        // self.drop_template(root, false);
-        todo!()
-    }
-
-    pub fn reclaim(&mut self, el: ElementId) {
-        assert_ne!(el, ElementId(0));
-        println!("reclaiming {}", el.0);
-        self.elements.remove(el.0);
-    }
-
-    pub fn drop_template(
-        &mut self,
-        mutations: &mut Mutations,
-        template: &'b VNode<'b>,
-        gen_roots: bool,
-    ) {
-        // for node in template.dynamic_nodes.iter() {
-        //     match node {
-        //         DynamicNode::Text { id, .. } => {}
-
-        //         DynamicNode::Component { .. } => {
-        //             todo!()
-        //         }
-
-        //         DynamicNode::Fragment { inner, nodes } => {}
-        //         DynamicNode::Placeholder(_) => todo!(),
-        //         _ => todo!(),
-        //     }
-        // }
-    }
-}

packages/core/src/lib.rs

@@ -8,7 +8,6 @@ mod error_boundary;
 mod events;
 mod factory;
 mod fragment;
-mod garbage;
 mod lazynodes;
 mod mutations;
 mod nodes;

packages/core/tests/attr_cleanup.rs

@@ -0,0 +1,65 @@
+//! dynamic attributes in dioxus necessitate an allocated node ID.
+//!
+//! This tests to ensure we clean it up
+
+use dioxus::core::{ElementId, Mutation::*};
+use dioxus::prelude::*;
+
+#[test]
+fn attrs_cycle() {
+    let mut dom = VirtualDom::new(|cx| {
+        let id = cx.generation();
+        match cx.generation() % 2 {
+            0 => cx.render(rsx! {
+                div {}
+            }),
+            1 => cx.render(rsx! {
+                div {
+                    h1 { class: "{id}", id: "{id}" }
+                }
+            }),
+            _ => unreachable!(),
+        }
+    });
+
+    assert_eq!(
+        dom.rebuild().santize().edits,
+        [
+            LoadTemplate { name: "template", index: 0, id: ElementId(1,) },
+            AppendChildren { m: 1 },
+        ]
+    );
+
+    dom.mark_dirty_scope(ScopeId(0));
+    assert_eq!(
+        dom.render_immediate().santize().edits,
+        [
+            LoadTemplate { name: "template", index: 0, id: ElementId(2,) },
+            AssignId { path: &[0,], id: ElementId(3,) },
+            SetAttribute { name: "class", value: "1", id: ElementId(3,), ns: None },
+            SetAttribute { name: "id", value: "1", id: ElementId(3,), ns: None },
+            ReplaceWith { id: ElementId(1,), m: 1 },
+        ]
+    );
+
+    dom.mark_dirty_scope(ScopeId(0));
+    dbg!(
+        dom.render_immediate().santize(),
+        [
+            LoadTemplate { name: "template", index: 0, id: ElementId(1,) },
+            ReplaceWith { id: ElementId(2,), m: 1 },
+        ]
+    );
+
+    dom.mark_dirty_scope(ScopeId(0));
+    assert_eq!(
+        dom.render_immediate().santize().edits,
+        [
+            LoadTemplate { name: "template", index: 0, id: ElementId(2) },
+            AssignId { path: &[0], id: ElementId(1) },
+            SetAttribute { name: "class", value: "3", id: ElementId(1), ns: None },
+            SetAttribute { name: "id", value: "3", id: ElementId(1), ns: None },
+            ReplaceWith { id: ElementId(3), m: 1 }
+        ]
+    );
+}