From 399169800db7bbc6e58c3e4b02fe22048057e26e Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sun, 27 Nov 2022 00:22:39 -0500 Subject: [PATCH] chore: dynamic attributes cleanup --- packages/core/src/arena.rs | 47 +++++++++++++++++---- packages/core/src/create.rs | 10 ++++- packages/core/src/diff.rs | 26 ++++++++---- packages/core/src/garbage.rs | 42 ------------------- packages/core/src/lib.rs | 1 - packages/core/tests/attr_cleanup.rs | 65 +++++++++++++++++++++++++++++ 6 files changed, 130 insertions(+), 61 deletions(-) delete mode 100644 packages/core/src/garbage.rs create mode 100644 packages/core/tests/attr_cleanup.rs diff --git a/packages/core/src/arena.rs b/packages/core/src/arena.rs index 98fe478e6..ec4ff862c 100644 --- a/packages/core/src/arena.rs +++ b/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); } + + 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!() + // } + + // DynamicNode::Fragment { inner, nodes } => {} + // DynamicNode::Placeholder(_) => todo!(), + // _ => todo!(), + // } + // } + } } - -/* -now...... - -an ID is mostly a pointer to a node in the real dom. -We need to -*/ diff --git a/packages/core/src/create.rs b/packages/core/src/create.rs index b0c1a10a9..ccc95887a 100644 --- a/packages/core/src/create.rs +++ b/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 { diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index 21091f961..bb6a66efc 100644 --- a/packages/core/src/diff.rs +++ b/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) { diff --git a/packages/core/src/garbage.rs b/packages/core/src/garbage.rs deleted file mode 100644 index 66a54c2d9..000000000 --- a/packages/core/src/garbage.rs +++ /dev/null @@ -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!(), - // } - // } - } -} diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index d88e73986..ac9b4e10f 100644 --- a/packages/core/src/lib.rs +++ b/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; diff --git a/packages/core/tests/attr_cleanup.rs b/packages/core/tests/attr_cleanup.rs new file mode 100644 index 000000000..f8aa39835 --- /dev/null +++ b/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 } + ] + ); +}