From 62bfab4ee89e10e0fba90020eb9b5d3e15bf16c9 Mon Sep 17 00:00:00 2001 From: Jovansonlee Cesar Date: Sat, 20 Apr 2024 07:48:13 +0800 Subject: [PATCH] refactor: remove the use of parent_node in the DomNode, as it can be obtain using tree_path.backtrack() --- ; | 514 ++++++++++++++++++++++ crates/core/src/dom/component/template.rs | 7 +- crates/core/src/dom/dom_node.rs | 97 ++-- crates/core/src/dom/dom_patch.rs | 87 ++-- 4 files changed, 582 insertions(+), 123 deletions(-) create mode 100644 ; diff --git a/; b/; new file mode 100644 index 00000000..e234656c --- /dev/null +++ b/; @@ -0,0 +1,514 @@ +use crate::dom; +use crate::dom::dom_node; +use crate::dom::dom_node::DomInner; +use crate::dom::DomAttr; +use crate::dom::DomAttrValue; +use crate::dom::DomNode; +use crate::dom::{Application, Program}; +use crate::vdom::ComponentEventCallback; +use crate::vdom::EventCallback; +use crate::vdom::TreePath; +use crate::vdom::{Attribute, AttributeValue, Patch, PatchType}; +use indexmap::IndexMap; +use std::rc::Rc; +use wasm_bindgen::closure::Closure; +use wasm_bindgen::JsValue; + +/// a Patch where the virtual nodes are all created in the document. +/// This is necessary since the created Node doesn't contain references +/// as opposed to Patch which contains reference to the vdom, which makes it hard +/// to be included in a struct +#[derive(Debug)] +pub struct DomPatch { + /// The path to traverse to get to the target_element + pub patch_path: TreePath, + /// the target node + pub target_element: DomNode, + /// the parent element of the target node + pub target_parent: DomNode, + /// the patch variant + pub patch_variant: PatchVariant, +} + +/// patch variant +#[derive(Debug)] +pub enum PatchVariant { + /// Insert nodes before the target node + InsertBeforeNode { + /// nodes to be inserted before the target node + nodes: Vec, + }, + /// Insert nodes after the target node + InsertAfterNode { + /// the nodes to be inserted after the target node + nodes: Vec, + }, + /// Append nodes into the target node + AppendChildren { + /// the children nodes to be appended into the target node + children: Vec, + }, + /// Add attributes to the target node + AddAttributes { + /// the attributes to be added to the target node + attrs: Vec, + }, + /// Remove attributes from the target node + RemoveAttributes { + /// the attributes names to be removed + attrs: Vec, + }, + /// Replace the target node with the replacement node + ReplaceNode { + /// the replacement node + replacement: Vec, + }, + /// Remove the target node + RemoveNode, + /// Clear the children of the target node + ClearChildren, + /// Move the target node before the node specified in the path location + MoveBeforeNode { + /// before the node at this location + for_moving: Vec, + }, + /// Move the target node after the node specified in the path location + MoveAfterNode { + /// after the node at this location + for_moving: Vec, + }, +} + +impl DomNode { + pub(crate) fn find_node(&self, path: &mut TreePath) -> Option { + match &self.inner { + DomInner::StatefulComponent {.. } => { + log::info!( + "This is a stateful component, should return the element + inside relative to the child container at this path: {:?}", + path + ); + // just return self and handle its own patches + Some(self.clone()) + } + _ => { + if path.is_empty() { + Some(self.clone()) + } else { + let idx = path.remove_first(); + if let Some(children) = self.children() { + if let Some(child) = children.get(idx) { + child.find_node(path) + } else { + log::warn!("There is no child at index: {idx}"); + None + } + } else { + log::warn!("Traversing to a childless node.."); + None + } + } + } + } + } + + pub(crate) fn find_all_nodes( + &self, + nodes_to_find: &[(&TreePath, Option<&&'static str>)], + ) -> IndexMap { + let mut nodes_to_patch = + IndexMap::with_capacity(nodes_to_find.len()); + for (path, tag) in nodes_to_find { + let mut traverse_path: TreePath = (*path).clone(); + if let Some(found) = self.find_node(&mut traverse_path) { + let mut parent_path = path.backtrack(); + let target_parent = self.find_node(&mut parent_path).expect("must find the parent"); + nodes_to_patch.insert((*path).clone(), (found, target_parent)); + } else { + log::warn!( + "can not find: {:?} {:?} target_node: {:?}", + path, + tag, + &self + ); + log::info!( + "real entire dom: {:#?}", + dom_node::render_real_dom_to_string(&self.as_node()) + ); + log::warn!("entire dom: {}", self.render_to_string()); + } + } + nodes_to_patch + } +} + +impl Program +where + APP: Application + 'static, +{ + pub(crate) fn convert_attr(&self, attr: &Attribute) -> DomAttr { + DomAttr { + namespace: attr.namespace, + name: attr.name, + value: attr + .value + .iter() + .filter_map(|v| self.convert_attr_value(v)) + .collect(), + } + } + + fn convert_attr_value(&self, attr_value: &AttributeValue) -> Option { + match attr_value { + AttributeValue::Simple(v) => Some(DomAttrValue::Simple(v.clone())), + AttributeValue::Style(v) => Some(DomAttrValue::Style(v.clone())), + AttributeValue::EventListener(v) => { + Some(DomAttrValue::EventListener(self.convert_event_listener(v))) + } + AttributeValue::ComponentEventListener(v) => Some(DomAttrValue::EventListener( + self.convert_component_event_listener(v), + )), + AttributeValue::Empty => None, + } + } + + fn convert_event_listener( + &self, + event_listener: &EventCallback, + ) -> Closure { + let program = self.downgrade(); + let event_listener = event_listener.clone(); + let closure: Closure = + Closure::new(move |event: web_sys::Event| { + let msg = event_listener.emit(dom::Event::from(event)); + let mut program = program.upgrade().expect("must upgrade"); + program.dispatch(msg); + }); + closure + } + + fn convert_component_event_listener( + &self, + component_callback: &ComponentEventCallback, + ) -> Closure { + let component_callback = component_callback.clone(); + let closure: Closure = + Closure::new(move |event: web_sys::Event| { + component_callback.emit(dom::Event::from(event)); + }); + closure + } + /// get the real DOM target node and make a DomPatch object for each of the Patch + pub(crate) fn convert_patches( + &self, + target_node: &DomNode, + patches: &[Patch], + ) -> Result, JsValue> { + let nodes_to_find: Vec<(&TreePath, Option<&&'static str>)> = patches + .iter() + .map(|patch| (patch.path(), patch.tag())) + .chain( + patches + .iter() + .flat_map(|patch| patch.node_paths()) + .map(|path| (path, None)), + ) + .collect(); + + let nodes_lookup = target_node.find_all_nodes(&nodes_to_find); + + let dom_patches:Vec = patches.iter().map(|patch|{ + let patch_path = patch.path(); + let patch_tag = patch.tag(); + if let Some((target_node, target_parent)) = nodes_lookup.get(patch_path) { + let target_tag = target_node.tag(); + if let (Some(patch_tag), Some(target_tag)) = (patch_tag, target_tag) { + if **patch_tag != target_tag{ + panic!( + "expecting a tag: {patch_tag:?}, but found: {target_tag:?}" + ); + } + } + self.convert_patch(&nodes_lookup, target_node, target_parent, patch) + } else { + unreachable!("Getting here means we didn't find the element of next node that we are supposed to patch, patch_path: {:?}, with tag: {:?}", patch_path, patch_tag); + } + }).collect(); + + Ok(dom_patches) + } + /// convert a virtual DOM Patch into a created DOM node Patch + pub fn convert_patch( + &self, + nodes_lookup: &IndexMap, + target_element: &DomNode, + target_parent: &DomNode, + patch: &Patch, + ) -> DomPatch { + let target_element = target_element.clone(); + let target_parent = target_parent.clone(); + let Patch { + patch_path, + patch_type, + .. + } = patch; + + let patch_path = patch_path.clone(); + + match patch_type { + PatchType::InsertBeforeNode { nodes } => { + let nodes = nodes + .iter() + .map(|for_insert| self.create_dom_node(Rc::new(None), for_insert)) + .collect(); + DomPatch { + patch_path, + target_element, + target_parent, + patch_variant: PatchVariant::InsertBeforeNode { nodes }, + } + } + PatchType::InsertAfterNode { nodes } => { + let nodes = nodes + .iter() + .map(|for_insert| self.create_dom_node(Rc::new(None), for_insert)) + .collect(); + DomPatch { + patch_path, + target_element, + target_parent, + patch_variant: PatchVariant::InsertAfterNode { nodes }, + } + } + + PatchType::AddAttributes { attrs } => { + // we merge the attributes here prior to conversion + let attrs = Attribute::merge_attributes_of_same_name(attrs.iter().copied()); + DomPatch { + patch_path, + target_element, + target_parent, + patch_variant: PatchVariant::AddAttributes { + attrs: attrs.iter().map(|a| self.convert_attr(a)).collect(), + }, + } + } + PatchType::RemoveAttributes { attrs } => DomPatch { + patch_path, + target_element, + target_parent, + patch_variant: PatchVariant::RemoveAttributes { + attrs: attrs.iter().map(|a| self.convert_attr(a)).collect(), + }, + }, + + PatchType::ReplaceNode { replacement } => { + let replacement = replacement + .iter() + .map(|node| self.create_dom_node(Rc::new(None), node)) + .collect(); + DomPatch { + patch_path, + target_element, + target_parent, + patch_variant: PatchVariant::ReplaceNode { replacement }, + } + } + PatchType::RemoveNode => DomPatch { + patch_path, + target_element, + target_parent, + patch_variant: PatchVariant::RemoveNode, + }, + PatchType::ClearChildren => DomPatch { + patch_path, + target_element, + target_parent, + patch_variant: PatchVariant::ClearChildren, + }, + PatchType::MoveBeforeNode { nodes_path } => { + let for_moving = nodes_path + .iter() + .map(|path| { + let (node,_) = nodes_lookup + .get(path).expect("must have found the node"); + node.clone() + }) + .collect(); + DomPatch { + patch_path, + target_element, + target_parent, + patch_variant: PatchVariant::MoveBeforeNode { for_moving }, + } + } + PatchType::MoveAfterNode { nodes_path } => { + let for_moving = nodes_path + .iter() + .map(|path| { + let (node,_) = nodes_lookup + .get(path) + .expect("must have found the node"); + node.clone() + }) + .collect(); + DomPatch { + patch_path, + target_element, + target_parent, + patch_variant: PatchVariant::MoveAfterNode { for_moving }, + } + } + PatchType::AppendChildren { children } => { + let children = children + .iter() + .map(|for_insert| self.create_dom_node(Rc::new(None), for_insert)) + .collect(); + + DomPatch { + patch_path, + target_element, + target_parent, + patch_variant: PatchVariant::AppendChildren { children }, + } + } + } + } + + /// TODO: this should not have access to root_node, so it can generically + /// apply patch to any dom node + pub(crate) fn apply_dom_patches( + &self, + dom_patches: impl IntoIterator, + ) -> Result<(), JsValue> { + for dom_patch in dom_patches { + self.apply_dom_patch(dom_patch)?; + } + Ok(()) + } + + /// apply a dom patch to this root node, + /// return a new root_node if it would replace the original root_node + /// TODO: this should have no access to root_node, so it can be used in general sense + pub(crate) fn apply_dom_patch(&self, dom_patch: DomPatch) -> Result<(), JsValue> { + let DomPatch { + patch_path, + target_element, + target_parent, + patch_variant, + } = dom_patch; + + match patch_variant { + PatchVariant::InsertBeforeNode { nodes } => { + target_element.insert_before(nodes); + } + + PatchVariant::InsertAfterNode { nodes } => { + target_element.insert_after(nodes); + } + PatchVariant::AppendChildren { children } => { + target_element.append_children(children); + } + + PatchVariant::AddAttributes { attrs } => { + target_element.set_dom_attrs(attrs).unwrap(); + } + PatchVariant::RemoveAttributes { attrs } => { + for attr in attrs.iter() { + for att_value in attr.value.iter() { + match att_value { + DomAttrValue::Simple(_) => { + target_element.remove_dom_attr(attr)?; + } + // it is an event listener + DomAttrValue::EventListener(_) => { + let DomInner::Element { listeners, .. } = &target_element.inner + else { + unreachable!("must be an element"); + }; + if let Some(listener) = listeners.borrow_mut().as_mut() { + listener.retain(|event, _| *event != attr.name) + } + } + DomAttrValue::Style(_) => { + target_element.remove_dom_attr(attr)?; + } + DomAttrValue::Empty => (), + } + } + } + } + + // This also removes the associated closures and event listeners to the node being replaced + // including the associated closures of the descendant of replaced node + // before it is actully replaced in the DOM + // TODO: make root node a Vec + PatchVariant::ReplaceNode { mut replacement } => { + let mut first_node = replacement.remove(0); + + let parent_node = if patch_path.path.is_empty() { + let mount_node = self.mount_node.borrow(); + let mount_node = mount_node.as_ref().expect("must have a mount node"); + Rc::new(Some(mount_node.clone())) + } else if let Some(parent_target) = target_element.parent.as_ref() { + Rc::new(Some(parent_target.clone())) + } else { + unreachable!("target element should have a parent"); + }; + + first_node.parent = Rc::clone(&parent_node); + for replace_node in replacement.iter_mut() { + replace_node.parent = Rc::clone(&parent_node); + } + + if target_element.is_fragment() { + assert!( + patch_path.is_empty(), + "this should only happen to root node" + ); + let mut mount_node = self.mount_node.borrow_mut(); + let mount_node = mount_node.as_mut().expect("must have a mount node"); + mount_node.append_children(vec![first_node.clone()]); + mount_node.append_children(replacement); + } else { + if patch_path.path.is_empty() { + let mut mount_node = self.mount_node.borrow_mut(); + let mount_node = mount_node.as_mut().expect("must have a mount node"); + mount_node.replace_child(&target_element, first_node.clone()); + } else { + target_element.replace_node(first_node.clone()); + } + //insert the rest + first_node.insert_after(replacement); + } + if patch_path.path.is_empty() { + *self.root_node.borrow_mut() = Some(first_node); + } + } + PatchVariant::RemoveNode => { + target_element.remove_node(); + } + PatchVariant::ClearChildren => { + target_element.clear_children(); + } + PatchVariant::MoveBeforeNode { for_moving } => { + //if let Some(target_parent) = target_element.parent.as_ref() { + target_parent.remove_children(&for_moving.iter().collect::>()); + target_element.insert_before(for_moving); + /* + } else { + panic!("unable to get the parent node of the target element"); + } + */ + } + + PatchVariant::MoveAfterNode { for_moving } => { + //if let Some(target_parent) = target_element.parent.as_ref() { + target_parent.remove_children(&for_moving.iter().collect::>()); + target_element.insert_after(for_moving); + //} + } + } + Ok(()) + } +} diff --git a/crates/core/src/dom/component/template.rs b/crates/core/src/dom/component/template.rs index b843f447..eaa1e7a7 100644 --- a/crates/core/src/dom/component/template.rs +++ b/crates/core/src/dom/component/template.rs @@ -1,3 +1,4 @@ +#![allow(unused)] use std::{any::TypeId, cell::RefCell, collections::hash_map, collections::HashMap, rc::Rc}; use wasm_bindgen::JsCast; @@ -144,7 +145,6 @@ fn create_fragment_node_no_listeners( fragment, children: Rc::new(RefCell::new(vec![])), }, - parent: parent_node, }; let dom_node_rc = Rc::new(Some(dom_node.clone())); let children = nodes @@ -162,15 +162,12 @@ fn create_leaf_node_no_listeners( match leaf { Leaf::Text(txt) => DomNode { inner: DomInner::Text(document().create_text_node(txt)), - parent: parent_node, }, Leaf::Symbol(symbol) => DomNode { inner: DomInner::Symbol(symbol.clone()), - parent: parent_node, }, Leaf::Comment(comment) => DomNode { inner: DomInner::Comment(document().create_comment(comment)), - parent: parent_node, }, Leaf::DocType(_doctype) => { panic!( @@ -226,7 +223,6 @@ fn create_element_node_no_listeners( children: Rc::new(RefCell::new(vec![])), has_mount_callback: elm.has_mount_callback(), }, - parent: parent_node, }; let dom_node_rc = Rc::new(Some(dom_node.clone())); let children = elm @@ -365,7 +361,6 @@ impl DomNode { pub(crate) fn deep_clone(&self) -> DomNode { DomNode { inner: self.inner.deep_clone(), - parent: self.parent.clone(), } } } diff --git a/crates/core/src/dom/dom_node.rs b/crates/core/src/dom/dom_node.rs index 203fa625..ccf4b1e5 100644 --- a/crates/core/src/dom/dom_node.rs +++ b/crates/core/src/dom/dom_node.rs @@ -1,3 +1,4 @@ +#![allow(unused)] use crate::dom::component::StatelessModel; use crate::dom::DomAttr; use crate::dom::GroupedDomAttrValues; @@ -32,8 +33,6 @@ pub type NamedEventClosures = IndexMap<&'static str, EventClosure>; #[derive(Clone, Debug)] pub struct DomNode { pub(crate) inner: DomInner, - //TODO: parent needs to be a weak reference - pub(crate) parent: Rc>, } #[derive(Clone)] pub enum DomInner { @@ -120,21 +119,18 @@ impl From for DomNode { children: Rc::new(RefCell::new(children)), has_mount_callback: false, }, - parent: Rc::new(None), } } Node::TEXT_NODE => { let text_node: web_sys::Text = node.unchecked_into(); DomNode { inner: DomInner::Text(text_node), - parent: Rc::new(None), } } Node::COMMENT_NODE => { let comment: web_sys::Comment = node.unchecked_into(); DomNode { inner: DomInner::Comment(comment), - parent: Rc::new(None), } } Node::DOCUMENT_FRAGMENT_NODE => { @@ -144,7 +140,6 @@ impl From for DomNode { fragment, children: Rc::new(RefCell::new(vec![])), }, - parent: Rc::new(None), } } _node_type => todo!("for: {_node_type:?}"), @@ -275,7 +270,6 @@ impl DomNode { .expect("append child"); child.dispatch_mount_event(); } - child.parent = Rc::new(Some(self.clone())); children.borrow_mut().push(child); } } @@ -287,7 +281,6 @@ impl DomNode { .append_child(&child.as_node()) .expect("append child"); child.dispatch_mount_event(); - child.parent = Rc::new(Some(self.clone())); children.borrow_mut().push(child); } } @@ -299,30 +292,23 @@ impl DomNode { } /// Insert the DomNode `for_insert` before `self` DomNode - pub(crate) fn insert_before(&self, mut for_insert: Vec) { - let parent_target = self.parent.as_ref().as_ref().expect("must have a parent"); - let DomInner::Element { - children: parent_children, - .. - } = &parent_target.inner - else { + pub(crate) fn insert_before(&self, target_element: &DomNode, for_insert: Vec) { + let DomInner::Element { children, .. } = &self.inner else { unreachable!("parent must be an element"); }; - let mut self_index = None; - for (i, child) in parent_children.borrow().iter().enumerate() { - if self == child { - self_index = Some(i); + let mut target_index = None; + for (i, child) in children.borrow().iter().enumerate() { + if target_element == child { + target_index = Some(i); break; } } - for insert_node in for_insert.iter_mut() { - insert_node.parent = Rc::clone(&self.parent); - } // NOTE: This is not reverse since inserting the last insert_node will always be next // before the target element for insert_node in for_insert.iter() { - self.as_element() + target_element + .as_element() .insert_adjacent_element(intern("beforebegin"), &insert_node.as_element()) .expect("must insert before this element"); insert_node.dispatch_mount_event(); @@ -331,8 +317,8 @@ impl DomNode { // NOTE: It is important that we reverse the insertion to the wrapper DomNode since it is // just a Vec where inserting from the last will preserve the index to insert into for insert_node in for_insert.into_iter().rev() { - if let Some(self_index) = self_index { - parent_children.borrow_mut().insert(self_index, insert_node); + if let Some(target_index) = target_index { + children.borrow_mut().insert(target_index, insert_node); } else { unreachable!("should have a self index"); } @@ -340,35 +326,26 @@ impl DomNode { } /// Insert the DomNode `for_insert` after `self` DomNode - pub(crate) fn insert_after(&self, mut for_insert: Vec) { - let parent_target = self.parent.as_ref().as_ref().expect("must have a parent"); - let DomInner::Element { - children: parent_children, - .. - } = &parent_target.inner - else { + pub(crate) fn insert_after(&self, target_element: &DomNode, for_insert: Vec) { + let DomInner::Element { children, .. } = &self.inner else { unreachable!("parent must be an element"); }; - let mut self_index = None; - for (i, child) in parent_children.borrow().iter().enumerate() { - if self == child { - self_index = Some(i); + let mut target_index = None; + for (i, child) in children.borrow().iter().enumerate() { + if target_element == child { + target_index = Some(i); break; } } - for insert_node in for_insert.iter_mut() { - insert_node.parent = Rc::clone(&self.parent); - } for insert_node in for_insert.into_iter().rev() { - self.as_element() + target_element + .as_element() .insert_adjacent_element(intern("afterend"), &insert_node.as_element()) .expect("must insert after this element"); insert_node.dispatch_mount_event(); - if let Some(self_index) = self_index { - parent_children - .borrow_mut() - .insert(self_index + 1, insert_node); + if let Some(target_index) = target_index { + children.borrow_mut().insert(target_index + 1, insert_node); } else { unreachable!("should have a self index"); } @@ -386,7 +363,6 @@ impl DomNode { break; } } - replacement.parent = Rc::new(Some(self.clone())); if let Some(child_index) = child_index { children.borrow_mut().remove(child_index); target_child @@ -459,23 +435,11 @@ impl DomNode { } } - pub(crate) fn remove_node(&self) { - if let Some(parent) = self.parent.as_ref() { - parent.remove_children(&[self]); - } else { - unreachable!("this has no parent node"); - } - } - pub(crate) fn replace_node(&self, replacement: DomNode) { - if let Some(parent) = self.parent.as_ref() { - parent.replace_child(self, replacement); - } else { - //NOTE: This must be replacing a mount node - self.as_element() - .replace_with_with_node_1(&replacement.as_node()) - .expect("must replace child"); - } + //NOTE: This must be replacing a mount node + self.as_element() + .replace_with_with_node_1(&replacement.as_node()) + .expect("must replace child"); } /// set the attributes of the dom element @@ -524,7 +488,7 @@ impl DomNode { plain_values, ); } - DomInner::StatefulComponent{comp,.. } => { + DomInner::StatefulComponent { comp, .. } => { log::info!("applying attribute change for stateful component...{attr:?}"); comp.borrow_mut().attribute_changed(attr); } @@ -721,9 +685,8 @@ where children: Rc::new(RefCell::new(vec![])), has_mount_callback: elm.has_mount_callback(), }, - parent: parent_node, }; - let dom_attrs = attrs.iter().map(|a|self.convert_attr(a)); + let dom_attrs = attrs.iter().map(|a| self.convert_attr(a)); dom_node.set_dom_attrs(dom_attrs).expect("set dom attrs"); let dom_node_rc = Rc::new(Some(dom_node.clone())); let children: Vec = elm @@ -743,15 +706,12 @@ where match leaf { Leaf::Text(txt) => DomNode { inner: DomInner::Text(document().create_text_node(txt)), - parent: parent_node, }, Leaf::Symbol(symbol) => DomNode { inner: DomInner::Symbol(symbol.clone()), - parent: parent_node, }, Leaf::Comment(comment) => DomNode { inner: DomInner::Comment(document().create_comment(comment)), - parent: parent_node, }, Leaf::Fragment(nodes) => self.create_fragment_node(parent_node, nodes), // NodeList that goes here is only possible when it is the root_node, @@ -767,7 +727,6 @@ where self.create_stateful_component(Rc::clone(&parent_node), comp), ), }, - parent: parent_node, } } Leaf::StatelessComponent(comp) => { @@ -798,7 +757,6 @@ where fragment, children: Rc::new(RefCell::new(vec![])), }, - parent: parent_node, }; let dom_node_rc = Rc::new(Some(dom_node.clone())); let children = nodes @@ -871,7 +829,6 @@ where let real_comp_view = comp_view.unwrap_template_ref(); self.create_dom_node(parent_node, real_comp_view) } - } /// render the underlying real dom node into string diff --git a/crates/core/src/dom/dom_patch.rs b/crates/core/src/dom/dom_patch.rs index 223c4337..8c72d8d8 100644 --- a/crates/core/src/dom/dom_patch.rs +++ b/crates/core/src/dom/dom_patch.rs @@ -24,6 +24,8 @@ pub struct DomPatch { pub patch_path: TreePath, /// the target node pub target_element: DomNode, + /// the parent element of the target node + pub target_parent: DomNode, /// the patch variant pub patch_variant: PatchVariant, } @@ -80,7 +82,7 @@ pub enum PatchVariant { impl DomNode { pub(crate) fn find_node(&self, path: &mut TreePath) -> Option { match &self.inner { - DomInner::StatefulComponent {.. } => { + DomInner::StatefulComponent { .. } => { log::info!( "This is a stateful component, should return the element inside relative to the child container at this path: {:?}", @@ -113,13 +115,16 @@ impl DomNode { pub(crate) fn find_all_nodes( &self, nodes_to_find: &[(&TreePath, Option<&&'static str>)], - ) -> IndexMap { - let mut nodes_to_patch: IndexMap = - IndexMap::with_capacity(nodes_to_find.len()); + ) -> IndexMap { + let mut nodes_to_patch = IndexMap::with_capacity(nodes_to_find.len()); for (path, tag) in nodes_to_find { let mut traverse_path: TreePath = (*path).clone(); if let Some(found) = self.find_node(&mut traverse_path) { - nodes_to_patch.insert((*path).clone(), found); + let mut parent_path = path.backtrack(); + let target_parent = self + .find_node(&mut parent_path) + .expect("must find the parent"); + nodes_to_patch.insert((*path).clone(), (found, target_parent)); } else { log::warn!( "can not find: {:?} {:?} target_node: {:?}", @@ -216,7 +221,7 @@ where let dom_patches:Vec = patches.iter().map(|patch|{ let patch_path = patch.path(); let patch_tag = patch.tag(); - if let Some(target_node) = nodes_lookup.get(patch_path) { + if let Some((target_node, target_parent)) = nodes_lookup.get(patch_path) { let target_tag = target_node.tag(); if let (Some(patch_tag), Some(target_tag)) = (patch_tag, target_tag) { if **patch_tag != target_tag{ @@ -225,7 +230,7 @@ where ); } } - self.convert_patch(&nodes_lookup, target_node, patch) + self.convert_patch(&nodes_lookup, target_node, target_parent, patch) } else { unreachable!("Getting here means we didn't find the element of next node that we are supposed to patch, patch_path: {:?}, with tag: {:?}", patch_path, patch_tag); } @@ -236,11 +241,13 @@ where /// convert a virtual DOM Patch into a created DOM node Patch pub fn convert_patch( &self, - nodes_lookup: &IndexMap, + nodes_lookup: &IndexMap, target_element: &DomNode, + target_parent: &DomNode, patch: &Patch, ) -> DomPatch { let target_element = target_element.clone(); + let target_parent = target_parent.clone(); let Patch { patch_path, patch_type, @@ -258,6 +265,7 @@ where DomPatch { patch_path, target_element, + target_parent, patch_variant: PatchVariant::InsertBeforeNode { nodes }, } } @@ -269,6 +277,7 @@ where DomPatch { patch_path, target_element, + target_parent, patch_variant: PatchVariant::InsertAfterNode { nodes }, } } @@ -279,6 +288,7 @@ where DomPatch { patch_path, target_element, + target_parent, patch_variant: PatchVariant::AddAttributes { attrs: attrs.iter().map(|a| self.convert_attr(a)).collect(), }, @@ -287,6 +297,7 @@ where PatchType::RemoveAttributes { attrs } => DomPatch { patch_path, target_element, + target_parent, patch_variant: PatchVariant::RemoveAttributes { attrs: attrs.iter().map(|a| self.convert_attr(a)).collect(), }, @@ -300,32 +311,34 @@ where DomPatch { patch_path, target_element, + target_parent, patch_variant: PatchVariant::ReplaceNode { replacement }, } } PatchType::RemoveNode => DomPatch { patch_path, target_element, + target_parent, patch_variant: PatchVariant::RemoveNode, }, PatchType::ClearChildren => DomPatch { patch_path, target_element, + target_parent, patch_variant: PatchVariant::ClearChildren, }, PatchType::MoveBeforeNode { nodes_path } => { let for_moving = nodes_path .iter() .map(|path| { - nodes_lookup - .get(path) - .expect("must have found the node") - .clone() + let (node, _) = nodes_lookup.get(path).expect("must have found the node"); + node.clone() }) .collect(); DomPatch { patch_path, target_element, + target_parent, patch_variant: PatchVariant::MoveBeforeNode { for_moving }, } } @@ -333,15 +346,14 @@ where let for_moving = nodes_path .iter() .map(|path| { - nodes_lookup - .get(path) - .expect("must have found the node") - .clone() + let (node, _) = nodes_lookup.get(path).expect("must have found the node"); + node.clone() }) .collect(); DomPatch { patch_path, target_element, + target_parent, patch_variant: PatchVariant::MoveAfterNode { for_moving }, } } @@ -354,6 +366,7 @@ where DomPatch { patch_path, target_element, + target_parent, patch_variant: PatchVariant::AppendChildren { children }, } } @@ -379,16 +392,17 @@ where let DomPatch { patch_path, target_element, + target_parent, patch_variant, } = dom_patch; match patch_variant { PatchVariant::InsertBeforeNode { nodes } => { - target_element.insert_before(nodes); + target_parent.insert_before(&target_element, nodes); } PatchVariant::InsertAfterNode { nodes } => { - target_element.insert_after(nodes); + target_parent.insert_after(&target_element, nodes); } PatchVariant::AppendChildren { children } => { target_element.append_children(children); @@ -428,22 +442,7 @@ where // before it is actully replaced in the DOM // TODO: make root node a Vec PatchVariant::ReplaceNode { mut replacement } => { - let mut first_node = replacement.remove(0); - - let parent_node = if patch_path.path.is_empty() { - let mount_node = self.mount_node.borrow(); - let mount_node = mount_node.as_ref().expect("must have a mount node"); - Rc::new(Some(mount_node.clone())) - } else if let Some(parent_target) = target_element.parent.as_ref() { - Rc::new(Some(parent_target.clone())) - } else { - unreachable!("target element should have a parent"); - }; - - first_node.parent = Rc::clone(&parent_node); - for replace_node in replacement.iter_mut() { - replace_node.parent = Rc::clone(&parent_node); - } + let first_node = replacement.remove(0); if target_element.is_fragment() { assert!( @@ -460,35 +459,29 @@ where let mount_node = mount_node.as_mut().expect("must have a mount node"); mount_node.replace_child(&target_element, first_node.clone()); } else { - target_element.replace_node(first_node.clone()); + target_parent.replace_child(&target_element, first_node.clone()); } //insert the rest - first_node.insert_after(replacement); + target_parent.insert_after(&first_node, replacement); } if patch_path.path.is_empty() { *self.root_node.borrow_mut() = Some(first_node); } } PatchVariant::RemoveNode => { - target_element.remove_node(); + target_parent.remove_children(&[&target_element]); } PatchVariant::ClearChildren => { target_element.clear_children(); } PatchVariant::MoveBeforeNode { for_moving } => { - if let Some(target_parent) = target_element.parent.as_ref() { - target_parent.remove_children(&for_moving.iter().collect::>()); - target_element.insert_before(for_moving); - } else { - panic!("unable to get the parent node of the target element"); - } + target_parent.remove_children(&for_moving.iter().collect::>()); + target_parent.insert_before(&target_element, for_moving); } PatchVariant::MoveAfterNode { for_moving } => { - if let Some(target_parent) = target_element.parent.as_ref() { - target_parent.remove_children(&for_moving.iter().collect::>()); - target_element.insert_after(for_moving); - } + target_parent.remove_children(&for_moving.iter().collect::>()); + target_parent.insert_after(&target_element, for_moving); } } Ok(())