From 7b00d04ea9128cbd047469ca42abb5953906af84 Mon Sep 17 00:00:00 2001 From: bakape Date: Sun, 13 Sep 2020 16:49:36 +0300 Subject: [PATCH 01/15] yew/vlist: optimize diffing and patching --- yew/src/html/mod.rs | 14 +- yew/src/utils.rs | 20 +- yew/src/virtual_dom/mod.rs | 8 +- yew/src/virtual_dom/vcomp.rs | 5 +- yew/src/virtual_dom/vlist.rs | 380 ++++++++++++++++++++++------------- yew/src/virtual_dom/vnode.rs | 8 +- yew/src/virtual_dom/vtag.rs | 4 +- yew/src/virtual_dom/vtext.rs | 18 +- 8 files changed, 300 insertions(+), 157 deletions(-) diff --git a/yew/src/html/mod.rs b/yew/src/html/mod.rs index 481a0c4a18a..a491d3a754e 100644 --- a/yew/src/html/mod.rs +++ b/yew/src/html/mod.rs @@ -395,7 +395,7 @@ impl IntoIterator for ChildrenRenderer { /// } /// } /// } -#[derive(Debug, Default, Clone)] +#[derive(Default, Clone)] pub struct NodeRef(Rc>); impl PartialEq for NodeRef { @@ -404,12 +404,22 @@ impl PartialEq for NodeRef { } } -#[derive(PartialEq, Debug, Default, Clone)] +#[derive(PartialEq, Default, Clone)] struct NodeRefInner { node: Option, link: Option, } +impl std::fmt::Debug for NodeRef { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "NodeRef {{ references: {:?} }}", + self.get().map(|n| crate::utils::print_node(&n)) + ) + } +} + impl NodeRef { /// Get the wrapped Node reference if it exists pub fn get(&self) -> Option { diff --git a/yew/src/utils.rs b/yew/src/utils.rs index 998f41aebe9..b8b08e26248 100644 --- a/yew/src/utils.rs +++ b/yew/src/utils.rs @@ -7,9 +7,10 @@ use std::marker::PhantomData; use yew::html::ChildrenRenderer; cfg_if! { if #[cfg(feature = "std_web")] { - use stdweb::web::{Document, Window}; + use stdweb::web::{Document, Window, Node}; } else if #[cfg(feature = "web_sys")] { - use web_sys::{Document, Window}; + use wasm_bindgen::JsCast; + use web_sys::{Document, Window, Node, Element}; } } @@ -114,3 +115,18 @@ impl IntoIterator for NodeSeq { self.0.into_iter() } } + +/// Print Node contents as a string for debugging purposes +pub fn print_node(n: &Node) -> String { + #[cfg(feature = "std_web")] + { + format!("{:?}", n) + } + #[cfg(feature = "web_sys")] + { + match n.dyn_ref::() { + Some(el) => el.outer_html(), + None => n.text_content().unwrap_or_default(), + } + } +} diff --git a/yew/src/virtual_dom/mod.rs b/yew/src/virtual_dom/mod.rs index 59064c4b60a..91e4bdd5f0c 100644 --- a/yew/src/virtual_dom/mod.rs +++ b/yew/src/virtual_dom/mod.rs @@ -479,20 +479,20 @@ pub(crate) trait VDiff { } #[cfg(feature = "web_sys")] -fn insert_node(node: &Node, parent: &Element, next_sibling: Option) { +fn insert_node(node: &Node, parent: &Element, next_sibling: &Option) { match next_sibling { Some(next_sibling) => parent - .insert_before(&node, Some(&next_sibling)) + .insert_before(&node, Some(next_sibling)) .expect("failed to insert tag before next sibling"), None => parent.append_child(node).expect("failed to append child"), }; } #[cfg(feature = "std_web")] -fn insert_node(node: &impl INode, parent: &impl INode, next_sibling: Option) { +fn insert_node(node: &impl INode, parent: &impl INode, next_sibling: &Option) { if let Some(next_sibling) = next_sibling { parent - .insert_before(node, &next_sibling) + .insert_before(node, next_sibling) .expect("failed to insert tag before next sibling"); } else { parent.append_child(node); diff --git a/yew/src/virtual_dom/vcomp.rs b/yew/src/virtual_dom/vcomp.rs index e9dcaa7d0a6..7606fed6ff1 100644 --- a/yew/src/virtual_dom/vcomp.rs +++ b/yew/src/virtual_dom/vcomp.rs @@ -107,7 +107,6 @@ impl VComp { } } - #[allow(unused)] pub(crate) fn root_vnode(&self) -> Option + '_> { self.scope.as_ref().and_then(|scope| scope.root_vnode()) } @@ -198,7 +197,7 @@ impl VDiff for VComp { } let placeholder: Node = document().create_text_node("").into(); - super::insert_node(&placeholder, parent, next_sibling.get()); + super::insert_node(&placeholder, parent, &next_sibling.get()); self.node_ref.set(Some(placeholder)); let scope = mountable.mount( self.node_ref.clone(), @@ -267,7 +266,7 @@ impl PartialEq for VComp { impl fmt::Debug for VComp { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("VComp") + write!(f, "VComp {{ root: {:?} }}", self.root_vnode().as_deref()) } } diff --git a/yew/src/virtual_dom/vlist.rs b/yew/src/virtual_dom/vlist.rs index 6c2e9c7ef5b..6dc0e93513d 100644 --- a/yew/src/virtual_dom/vlist.rs +++ b/yew/src/virtual_dom/vlist.rs @@ -2,7 +2,7 @@ use super::{Key, VDiff, VNode, VText}; use crate::html::{AnyScope, NodeRef}; use cfg_if::cfg_if; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::ops::{Deref, DerefMut}; cfg_if! { if #[cfg(feature = "std_web")] { @@ -34,6 +34,27 @@ impl DerefMut for VList { } } +/// Log an operation during tests for debugging purposes +/// Set RUSTFLAGS="--cfg verbose_tests" environment variable to activate. +macro_rules! test_log { + ($fmt:literal, $($arg:expr),* $(,)?) => { + #[cfg(all(feature = "wasm_test", verbose_tests))] + ::wasm_bindgen_test::console_log!(concat!("\t ", $fmt), $($arg),*); + }; +} + +/// Advance the next sibling reference (from right to left) and log it for testing purposes +/// Set RUSTFLAGS="--cfg verbose_tests" environment variable to activate. +macro_rules! advance_next_sibling { + ($current:expr, $advance:expr) => {{ + #[cfg(all(feature = "wasm_test", verbose_tests))] + let current = format!("{:?}", $current); + let next = $advance(); + test_log!("advance next_sibling: {} -> {:?}", current, next); + next + }}; +} + impl VList { /// Creates a new empty `VList` instance. pub fn new() -> Self { @@ -55,16 +76,211 @@ impl VList { self.children.extend(children); } - fn children_keys(&self, warn: bool) -> HashSet { - let mut hash_set = HashSet::with_capacity(self.children.len()); - for l in self.children.iter() { - if let Some(k) = l.key() { - if !hash_set.insert(k.clone()) && warn { - log::warn!("Key '{}' is not unique in list but must be.", k); + /// Diff and patch unkeyed child lists + fn apply_unkeyed( + parent_scope: &AnyScope, + parent: &Element, + mut next_sibling: NodeRef, + lefts: &mut [VNode], + rights: Vec, + ) -> NodeRef { + let mut diff = lefts.len() as isize - rights.len() as isize; + let mut lefts_it = lefts.iter_mut().rev(); + let mut rights_it = rights.into_iter().rev(); + + macro_rules! apply { + ($l:expr, $r:expr) => { + test_log!("parent={:?}", parent.outer_html()); + next_sibling = advance_next_sibling!(next_sibling, || $l.apply( + parent_scope, + parent, + next_sibling, + $r + )); + }; + } + + // Add missing nodes + while diff > 0 { + let l = lefts_it.next().unwrap(); + test_log!("adding: {:?}", l); + apply!(l, None); + diff -= 1; + } + // Remove extra nodes + while diff < 0 { + let mut r = rights_it.next().unwrap(); + test_log!("removing: {:?}", r); + r.detach(parent); + diff += 1; + } + + for (l, r) in lefts_it.zip(rights_it) { + test_log!("patching: {:?} -> {:?}", r, l); + apply!(l, r.into()); + } + + next_sibling + } + + /// Diff and patch fully keyed child lists. + /// + /// Optimized for node addition or removal from either end of the list and small changes in the + /// middle. + fn apply_keyed( + parent_scope: &AnyScope, + parent: &Element, + mut next_sibling: NodeRef, + lefts: &mut [VNode], + lefts_keys: &[Key], + mut rights: Vec, + rights_keys: &[Key], + ) -> NodeRef { + /// Find the first differing key in 2 iterators + fn diff_i<'a, 'b>( + a: impl Iterator, + b: impl Iterator, + ) -> usize { + a.zip(b).filter(|(a, b)| a == b).count() + } + + // Find first key mismatch from the front + let from_start = diff_i(lefts_keys.iter(), rights_keys.iter()); + + if from_start == std::cmp::min(lefts.len(), rights.len()) { + // No key changes + return Self::apply_unkeyed(parent_scope, parent, next_sibling, lefts, rights); + } + + // Find first key mismatch from the back + let from_end = diff_i( + lefts_keys[from_start..].iter().rev(), + rights_keys[from_start..].iter().rev(), + ); + + macro_rules! apply { + ($l:expr, $r:expr) => { + test_log!("patching: {:?} -> {:?}", $r, $l); + apply!(std::mem::take($r).into() => $l); + }; + ($l:expr) => { + test_log!("adding: {:?}", $l); + apply!(None => $l); + }; + ($ancestor:expr => $l:expr) => { + test_log!("parent={:?}", parent.outer_html()); + next_sibling = advance_next_sibling!( + next_sibling, + || $l.apply(parent_scope, parent, next_sibling, $ancestor) + ); + }; + } + + // Diff matching children at the end + let lefts_to = lefts_keys.len() - from_end; + let rights_to = rights_keys.len() - from_end; + for (l, r) in lefts[lefts_to..] + .iter_mut() + .rev() + .zip(rights[rights_to..].iter_mut().rev()) + { + apply!(l, r); + } + + // Diff mismatched children in the middle + let mut rights_diff: HashMap = rights_keys[from_start..rights_to] + .iter() + .zip(rights[from_start..rights_to].iter_mut()) + .map(|(k, v)| (k.clone(), v)) + .collect(); + for (l_k, l) in lefts_keys[from_start..lefts_to] + .iter() + .rev() + .zip(lefts[from_start..lefts_to].iter_mut().rev()) + { + match rights_diff.remove(l_k) { + // Reorder and diff any existing children + Some(r) => { + test_log!("moving as next: {:?}", r); + r.move_before(parent, &next_sibling.get()); + apply!(l, r); + } + // Add new children + None => { + apply!(l); } } } - hash_set + + // Remove any extra rights + for (_, r) in rights_diff.drain() { + test_log!("removing: {:?}", r); + r.detach(parent); + } + + // Diff matching children at the start + for (l, r) in lefts[..from_start] + .iter_mut() + .rev() + .zip(rights[..from_start].iter_mut().rev()) + { + apply!(l, r); + } + + next_sibling + } + + /// Diff and patch child lists, if the are fully keyed. + /// Returns Err(rights, next_sibling) back, if diff and patch not performed. + fn try_apply_keyed( + parent_scope: &AnyScope, + parent: &Element, + next_sibling: NodeRef, + lefts: &mut [VNode], + rights: Vec, + ) -> Result, NodeRef)> { + macro_rules! fail { + () => {{ + return Err((rights, next_sibling)); + }}; + } + + /// First perform a cheap check on the first nodes to avoid allocations + macro_rules! no_key { + ($list:expr) => { + $list.get(0).map(|n| n.key().is_none()).unwrap_or(false) + }; + } + if no_key!(lefts) || no_key!(rights) { + fail!(); + } + + // Build key vectors ahead of time to prevent heavy branching in keyed diffing, in case + // the child lists are not fully keyed, and to memoize key lookup + macro_rules! map_keys { + ($src:expr) => { + match $src + .iter() + .map(|v| v.key().ok_or(())) + .collect::, ()>>() + { + Ok(vec) => vec, + Err(_) => fail!(), + } + }; + } + let lefts_keys = map_keys!(lefts); + let rights_keys = map_keys!(rights); + + Ok(Self::apply_keyed( + parent_scope, + parent, + next_sibling, + lefts, + &lefts_keys, + rights, + &rights_keys, + )) } } @@ -99,139 +315,27 @@ impl VDiff for VList { self.children.push(placeholder.into()); } - let left_keys = self.children_keys(true); - let lefts_keyed = left_keys.len() == self.children.len(); - - let right_keys = if let Some(VNode::VList(vlist)) = &ancestor { - vlist.children_keys(false) - } else { - HashSet::new() - }; - - let mut right_children = match ancestor { - // If the ancestor is also a VList, then the "right" list is the - // previously rendered items. - Some(VNode::VList(vlist)) => vlist.children, + let lefts = &mut self.children; + let rights = match ancestor { + // If the ancestor is also a VList, then the "right" list is the previously + // rendered items. + Some(VNode::VList(v)) => v.children, // If the ancestor was not a VList, then the "right" list is a single node - Some(vnode) => vec![vnode], - None => vec![], + Some(v) => vec![v], + _ => vec![], }; - let rights_keyed = right_keys.len() == right_children.len(); - - // If the existing list and the new list are both properly keyed, - // then move existing list nodes into the new list's order before diffing - if lefts_keyed && rights_keyed { - // Find the intersection of keys to determine which right nodes can be reused - let matched_keys: HashSet<_> = left_keys.intersection(&right_keys).collect(); - - // Detach any right nodes that were not matched with a left node - right_children = right_children - .into_iter() - .filter_map(|mut right| { - if matched_keys.contains(right.key().as_ref().unwrap()) { - Some(right) - } else { - right.detach(parent); - None - } - }) - .collect(); - - // Determine which rights are already in correct order and which - // rights need to be moved in the DOM before being reused - let mut rights_to_move = HashMap::with_capacity(right_children.len()); - let mut matched_lefts = self - .children - .iter() - .filter(|left| matched_keys.contains(left.key().as_ref().unwrap())) - .peekable(); - let mut left = matched_lefts.next(); - - // Note: `filter_map` is used to move rights into `rights_to_move` - #[allow(clippy::unnecessary_filter_map)] - let rights_in_place: Vec<_> = right_children - .into_iter() - .filter_map(|right| { - if right.key() == left.and_then(|l| l.key()) { - left = matched_lefts.next(); - return Some(right); - } else if right.key() == matched_lefts.peek().and_then(|l| l.key()) { - matched_lefts.next(); - left = matched_lefts.next(); - return Some(right); - } - - rights_to_move.insert(right.key().unwrap(), right); - None - }) - .collect(); - - // Move rights into correct order and build `right_children` - right_children = Vec::with_capacity(matched_keys.len()); - let mut matched_lefts = self - .children - .iter() - .filter(|left| matched_keys.contains(left.key().as_ref().unwrap())); - - for right in rights_in_place.into_iter() { - let mut left = matched_lefts.next().unwrap(); - while right.key() != left.key() { - let right_to_move = rights_to_move.remove(&left.key().unwrap()).unwrap(); - right_to_move.move_before(parent, Some(right.first_node())); - right_children.push(right_to_move); - left = matched_lefts.next().unwrap(); - } - right_children.push(right); - } - - for left in matched_lefts { - let right_to_move = rights_to_move.remove(&left.key().unwrap()).unwrap(); - right_to_move.move_before(parent, next_sibling.get()); - right_children.push(right_to_move); + test_log!("lefts: {:?}", lefts); + test_log!("rights: {:?}", rights); + + #[allow(clippy::let_and_return)] + let first = match Self::try_apply_keyed(parent_scope, parent, next_sibling, lefts, rights) { + Ok(n) => n, + Err((rights, next_sibling)) => { + Self::apply_unkeyed(parent_scope, parent, next_sibling, lefts, rights) } - - assert!(rights_to_move.is_empty()) - } - - let mut rights = right_children.into_iter().peekable(); - let mut last_next_sibling = NodeRef::default(); - let mut nodes: Vec = self - .children - .iter_mut() - .map(|left| { - let ancestor = rights.next(); - - // Create a new `next_sibling` reference which points to the next `right` or - // the outer list's `next_sibling` if there are no more `rights`. - let new_next_sibling = NodeRef::default(); - if let Some(next_right) = rights.peek() { - new_next_sibling.set(Some(next_right.first_node())); - } else { - new_next_sibling.link(next_sibling.clone()); - } - - // Update the next list item and then link the previous left's `next_sibling` to the - // returned `node` reference so that the previous left has an up-to-date `next_sibling`. - // This is important for rendering a `VComp` because each `VComp` keeps track of its - // `next_sibling` to properly render its children. - let node = left.apply(parent_scope, parent, new_next_sibling.clone(), ancestor); - last_next_sibling.link(node.clone()); - last_next_sibling = new_next_sibling; - node - }) - .collect(); - - // If there are more `rights` than `lefts`, we need to make sure to link the last left's `next_sibling` - // to the outer list's `next_sibling` so that it doesn't point at a `right` that is detached. - last_next_sibling.link(next_sibling); - - // Detach all extra rights - for mut right in rights { - right.detach(parent); - } - - assert!(!nodes.is_empty(), "VList should have at least one child"); - nodes.swap_remove(0) + }; + test_log!("result: {:?}", lefts); + first } } diff --git a/yew/src/virtual_dom/vnode.rs b/yew/src/virtual_dom/vnode.rs index 48942f6ae97..13079d76975 100644 --- a/yew/src/virtual_dom/vnode.rs +++ b/yew/src/virtual_dom/vnode.rs @@ -69,11 +69,11 @@ impl VNode { } } - pub(crate) fn move_before(&self, parent: &Element, next_sibling: Option) { + pub(crate) fn move_before(&self, parent: &Element, next_sibling: &Option) { match self { VNode::VList(vlist) => { for node in vlist.children.iter() { - node.move_before(parent, next_sibling.clone()); + node.move_before(parent, next_sibling); } } VNode::VComp(vcomp) => { @@ -130,7 +130,7 @@ impl VDiff for VNode { } ancestor.detach(parent); } - super::insert_node(node, parent, next_sibling.get()); + super::insert_node(node, parent, &next_sibling.get()); NodeRef::new(node.clone()) } } @@ -199,7 +199,7 @@ impl fmt::Debug for VNode { VNode::VText(ref vtext) => vtext.fmt(f), VNode::VComp(ref vcomp) => vcomp.fmt(f), VNode::VList(ref vlist) => vlist.fmt(f), - VNode::VRef(ref vref) => vref.fmt(f), + VNode::VRef(ref vref) => write!(f, "VRef ( \"{}\" )", crate::utils::print_node(vref)), } } } diff --git a/yew/src/virtual_dom/vtag.rs b/yew/src/virtual_dom/vtag.rs index 986251e7119..7b0ded5aed2 100644 --- a/yew/src/virtual_dom/vtag.rs +++ b/yew/src/virtual_dom/vtag.rs @@ -473,7 +473,7 @@ impl VDiff for VTag { VNode::VTag(vtag) if self.tag() == vtag.tag() && self.key == vtag.key => Some(vtag), _ => { let element = self.create_element(parent); - super::insert_node(&element, parent, Some(ancestor.first_node())); + super::insert_node(&element, parent, &Some(ancestor.first_node())); self.reference = Some(element); ancestor.detach(parent); None @@ -489,7 +489,7 @@ impl VDiff for VTag { self.reference = ancestor_tag.reference.take(); } else if self.reference.is_none() { let element = self.create_element(parent); - super::insert_node(&element, parent, next_sibling.get()); + super::insert_node(&element, parent, &next_sibling.get()); self.reference = Some(element); } diff --git a/yew/src/virtual_dom/vtext.rs b/yew/src/virtual_dom/vtext.rs index 6c0e914ebc3..5e2becbb3be 100644 --- a/yew/src/virtual_dom/vtext.rs +++ b/yew/src/virtual_dom/vtext.rs @@ -18,7 +18,7 @@ cfg_if! { /// A type for a virtual /// [`TextNode`](https://developer.mozilla.org/en-US/docs/Web/API/Document/createTextNode) /// representation. -#[derive(Clone, Debug)] +#[derive(Clone)] pub struct VText { /// Contains a text of the node. pub text: Cow<'static, str>, @@ -36,6 +36,20 @@ impl VText { } } +impl std::fmt::Debug for VText { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "VText {{ text: \"{}\", reference: {} }}", + self.text, + match &self.reference { + Some(_) => "Some(...)", + None => "None", + } + ) + } +} + impl VDiff for VText { /// Remove VText from parent. fn detach(&mut self, parent: &Element) { @@ -74,7 +88,7 @@ impl VDiff for VText { } let text_node = document().create_text_node(&self.text); - super::insert_node(&text_node, parent, next_sibling.get()); + super::insert_node(&text_node, parent, &next_sibling.get()); self.reference = Some(text_node.clone()); NodeRef::new(text_node.into()) } From 8d9c81821dff5a5396cbd22d8be922659c5129af Mon Sep 17 00:00:00 2001 From: bakape Date: Fri, 18 Sep 2020 21:46:24 +0300 Subject: [PATCH 02/15] yew/vlist: fix key difference point detection --- yew/src/virtual_dom/vlist.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yew/src/virtual_dom/vlist.rs b/yew/src/virtual_dom/vlist.rs index 6dc0e93513d..aa05deea10b 100644 --- a/yew/src/virtual_dom/vlist.rs +++ b/yew/src/virtual_dom/vlist.rs @@ -141,7 +141,7 @@ impl VList { a: impl Iterator, b: impl Iterator, ) -> usize { - a.zip(b).filter(|(a, b)| a == b).count() + a.zip(b).take_while(|(a, b)| a == b).count() } // Find first key mismatch from the front From 357f5b4a724a8717ab03c8d6fecb0174719bc8c4 Mon Sep 17 00:00:00 2001 From: bakape Date: Sun, 8 Nov 2020 16:17:47 +0200 Subject: [PATCH 03/15] yew-macro: update stderr --- yew-macro/tests/macro/html-block-fail.stderr | 4 ++-- yew-macro/tests/macro/html-iterable-fail.stderr | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/yew-macro/tests/macro/html-block-fail.stderr b/yew-macro/tests/macro/html-block-fail.stderr index 3d07a7c7d6d..354d3fdff21 100644 --- a/yew-macro/tests/macro/html-block-fail.stderr +++ b/yew-macro/tests/macro/html-block-fail.stderr @@ -34,9 +34,9 @@ error[E0277]: `()` doesn't implement `std::fmt::Display` 15 | <>{ for (0..3).map(|_| not_tree()) } | ^^^^^^ `()` cannot be formatted with the default formatter | - ::: $WORKSPACE/yew/src/utils.rs:76:8 + ::: $WORKSPACE/yew/src/utils.rs:77:8 | -76 | T: Into, +77 | T: Into, | ------- required by this bound in `yew::utils::into_node_iter` | = help: the trait `std::fmt::Display` is not implemented for `()` diff --git a/yew-macro/tests/macro/html-iterable-fail.stderr b/yew-macro/tests/macro/html-iterable-fail.stderr index b6abf51cdfe..f19741caa39 100644 --- a/yew-macro/tests/macro/html-iterable-fail.stderr +++ b/yew-macro/tests/macro/html-iterable-fail.stderr @@ -71,9 +71,9 @@ error[E0277]: `()` is not an iterator 18 | { for () } | ^^ `()` is not an iterator | - ::: $WORKSPACE/yew/src/utils.rs:75:9 + ::: $WORKSPACE/yew/src/utils.rs:76:9 | -75 | IT: IntoIterator, +76 | IT: IntoIterator, | ---------------------- required by this bound in `yew::utils::into_node_iter` | = help: the trait `std::iter::Iterator` is not implemented for `()` From a40faf43beb58193c7ff420093444acd3f230737 Mon Sep 17 00:00:00 2001 From: bakape Date: Sun, 8 Nov 2020 17:46:42 +0200 Subject: [PATCH 04/15] vlist: store on the list, if all the children are keyed --- yew/src/virtual_dom/vlist.rs | 138 +++++++++++++++++------------------ yew/src/virtual_dom/vnode.rs | 18 +++-- 2 files changed, 79 insertions(+), 77 deletions(-) diff --git a/yew/src/virtual_dom/vlist.rs b/yew/src/virtual_dom/vlist.rs index aa05deea10b..8decc032215 100644 --- a/yew/src/virtual_dom/vlist.rs +++ b/yew/src/virtual_dom/vlist.rs @@ -13,13 +13,27 @@ cfg_if! { } /// This struct represents a fragment of the Virtual DOM tree. -#[derive(Clone, Debug, PartialEq, Default)] +#[derive(Clone, Debug, PartialEq)] pub struct VList { /// The list of children nodes. - pub children: Vec, + children: Vec, + + // All Nodes in the VList have keys + fully_keyed: bool, + pub key: Option, } +impl Default for VList { + fn default() -> Self { + Self { + children: Default::default(), + key: None, + fully_keyed: true, + } + } +} + impl Deref for VList { type Target = Vec; @@ -30,10 +44,13 @@ impl Deref for VList { impl DerefMut for VList { fn deref_mut(&mut self) -> &mut Self::Target { + // Caller might change the keys of the VList or add unkeyed children. + // Defensively assume they will. + self.fully_keyed = false; + &mut self.children } } - /// Log an operation during tests for debugging purposes /// Set RUSTFLAGS="--cfg verbose_tests" environment variable to activate. macro_rules! test_log { @@ -63,17 +80,37 @@ impl VList { /// Creates a new `VList` instance with children. pub fn new_with_children(children: Vec, key: Option) -> Self { - VList { children, key } + VList { + fully_keyed: children.iter().all(|ch| ch.has_key()), + children, + key, + } } /// Add `VNode` child. pub fn add_child(&mut self, child: VNode) { + if self.fully_keyed && !child.has_key() { + self.fully_keyed = false; + } self.children.push(child); } /// Add multiple `VNode` children. pub fn add_children(&mut self, children: impl IntoIterator) { - self.children.extend(children); + let it = children.into_iter(); + let bound = it.size_hint(); + self.children.reserve(bound.1.unwrap_or(bound.0)); + for ch in it { + self.add_child(ch); + } + } + + /// Recheck, if the all the children have keys. + /// + /// Run this, after modifying the child list that contained only keyed children prior to the + /// mutable dereference. + pub fn recheck_fully_keyed(&mut self) { + self.fully_keyed = self.children.iter().all(|ch| ch.has_key()); } /// Diff and patch unkeyed child lists @@ -132,10 +169,18 @@ impl VList { parent: &Element, mut next_sibling: NodeRef, lefts: &mut [VNode], - lefts_keys: &[Key], mut rights: Vec, - rights_keys: &[Key], ) -> NodeRef { + macro_rules! map_keys { + ($src:expr) => { + $src.iter() + .map(|v| v.key().expect("unkeyed child in fully keyed list")) + .collect::>() + }; + } + let lefts_keys = map_keys!(lefts); + let rights_keys = map_keys!(rights); + /// Find the first differing key in 2 iterators fn diff_i<'a, 'b>( a: impl Iterator, @@ -229,59 +274,6 @@ impl VList { next_sibling } - - /// Diff and patch child lists, if the are fully keyed. - /// Returns Err(rights, next_sibling) back, if diff and patch not performed. - fn try_apply_keyed( - parent_scope: &AnyScope, - parent: &Element, - next_sibling: NodeRef, - lefts: &mut [VNode], - rights: Vec, - ) -> Result, NodeRef)> { - macro_rules! fail { - () => {{ - return Err((rights, next_sibling)); - }}; - } - - /// First perform a cheap check on the first nodes to avoid allocations - macro_rules! no_key { - ($list:expr) => { - $list.get(0).map(|n| n.key().is_none()).unwrap_or(false) - }; - } - if no_key!(lefts) || no_key!(rights) { - fail!(); - } - - // Build key vectors ahead of time to prevent heavy branching in keyed diffing, in case - // the child lists are not fully keyed, and to memoize key lookup - macro_rules! map_keys { - ($src:expr) => { - match $src - .iter() - .map(|v| v.key().ok_or(())) - .collect::, ()>>() - { - Ok(vec) => vec, - Err(_) => fail!(), - } - }; - } - let lefts_keys = map_keys!(lefts); - let rights_keys = map_keys!(rights); - - Ok(Self::apply_keyed( - parent_scope, - parent, - next_sibling, - lefts, - &lefts_keys, - rights, - &rights_keys, - )) - } } impl VDiff for VList { @@ -311,28 +303,32 @@ impl VDiff for VList { // Without a placeholder the next element becomes first // and corrupts the order of rendering // We use empty text element to stake out a place - let placeholder = VText::new(""); - self.children.push(placeholder.into()); + self.add_child(VText::new("").into()); } let lefts = &mut self.children; - let rights = match ancestor { + let (rights, rights_fully_keyed) = match ancestor { // If the ancestor is also a VList, then the "right" list is the previously // rendered items. - Some(VNode::VList(v)) => v.children, + Some(VNode::VList(v)) => (v.children, v.fully_keyed), + // If the ancestor was not a VList, then the "right" list is a single node - Some(v) => vec![v], - _ => vec![], + Some(v) => { + let has_key = v.has_key(); + (vec![v], has_key) + } + + // No unkeyed nodes in an empty VList + _ => (vec![], true), }; test_log!("lefts: {:?}", lefts); test_log!("rights: {:?}", rights); #[allow(clippy::let_and_return)] - let first = match Self::try_apply_keyed(parent_scope, parent, next_sibling, lefts, rights) { - Ok(n) => n, - Err((rights, next_sibling)) => { - Self::apply_unkeyed(parent_scope, parent, next_sibling, lefts, rights) - } + let first = if self.fully_keyed && rights_fully_keyed { + Self::apply_keyed(parent_scope, parent, next_sibling, lefts, rights) + } else { + Self::apply_unkeyed(parent_scope, parent, next_sibling, lefts, rights) }; test_log!("result: {:?}", lefts); first diff --git a/yew/src/virtual_dom/vnode.rs b/yew/src/virtual_dom/vnode.rs index 13079d76975..20f64f1879a 100644 --- a/yew/src/virtual_dom/vnode.rs +++ b/yew/src/virtual_dom/vnode.rs @@ -43,6 +43,16 @@ impl VNode { } } + /// Returns, if the VNode has a key without needlessly cloning the key + pub fn has_key(&self) -> bool { + match self { + VNode::VComp(vcomp) => vcomp.key.is_some(), + VNode::VList(vlist) => vlist.key.is_some(), + VNode::VRef(_) | VNode::VText(_) => false, + VNode::VTag(vtag) => vtag.key.is_some(), + } + } + /// Returns the first DOM node that is used to designate the position of the virtual DOM node. pub(crate) fn first_node(&self) -> Node { match self { @@ -60,11 +70,7 @@ impl VNode { } } VNode::VComp(vcomp) => vcomp.node_ref.get().expect("VComp is not mounted"), - VNode::VList(vlist) => vlist - .children - .get(0) - .expect("VList is not mounted") - .first_node(), + VNode::VList(vlist) => vlist.get(0).expect("VList is not mounted").first_node(), VNode::VRef(node) => node.clone(), } } @@ -72,7 +78,7 @@ impl VNode { pub(crate) fn move_before(&self, parent: &Element, next_sibling: &Option) { match self { VNode::VList(vlist) => { - for node in vlist.children.iter() { + for node in vlist.iter() { node.move_before(parent, next_sibling); } } From 5bbef6525a902126ba76120f742958c22978499c Mon Sep 17 00:00:00 2001 From: bakape Date: Wed, 18 Nov 2020 22:34:11 +0200 Subject: [PATCH 05/15] yew/vlist: simplify iterators --- yew/src/virtual_dom/vlist.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yew/src/virtual_dom/vlist.rs b/yew/src/virtual_dom/vlist.rs index 8decc032215..902c917e7d8 100644 --- a/yew/src/virtual_dom/vlist.rs +++ b/yew/src/virtual_dom/vlist.rs @@ -226,8 +226,8 @@ impl VList { let rights_to = rights_keys.len() - from_end; for (l, r) in lefts[lefts_to..] .iter_mut() + .zip(rights[rights_to..].iter_mut()) .rev() - .zip(rights[rights_to..].iter_mut().rev()) { apply!(l, r); } @@ -240,8 +240,8 @@ impl VList { .collect(); for (l_k, l) in lefts_keys[from_start..lefts_to] .iter() + .zip(lefts[from_start..lefts_to].iter_mut()) .rev() - .zip(lefts[from_start..lefts_to].iter_mut().rev()) { match rights_diff.remove(l_k) { // Reorder and diff any existing children @@ -266,8 +266,8 @@ impl VList { // Diff matching children at the start for (l, r) in lefts[..from_start] .iter_mut() + .zip(rights[..from_start].iter_mut()) .rev() - .zip(rights[..from_start].iter_mut().rev()) { apply!(l, r); } From b51eb39ef053c024754d624e71c0b4c96d88b307 Mon Sep 17 00:00:00 2001 From: bakape Date: Thu, 19 Nov 2020 00:20:49 +0200 Subject: [PATCH 06/15] yew/vlist: more short-circuiting during diff --- yew/src/virtual_dom/vlist.rs | 84 ++++++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 32 deletions(-) diff --git a/yew/src/virtual_dom/vlist.rs b/yew/src/virtual_dom/vlist.rs index 902c917e7d8..bd097f68726 100644 --- a/yew/src/virtual_dom/vlist.rs +++ b/yew/src/virtual_dom/vlist.rs @@ -51,6 +51,7 @@ impl DerefMut for VList { &mut self.children } } + /// Log an operation during tests for debugging purposes /// Set RUSTFLAGS="--cfg verbose_tests" environment variable to activate. macro_rules! test_log { @@ -221,7 +222,7 @@ impl VList { }; } - // Diff matching children at the end + // Diff matching children at the end (or the entire list, if they match) let lefts_to = lefts_keys.len() - from_end; let rights_to = rights_keys.len() - from_end; for (l, r) in lefts[lefts_to..] @@ -233,43 +234,62 @@ impl VList { } // Diff mismatched children in the middle - let mut rights_diff: HashMap = rights_keys[from_start..rights_to] - .iter() - .zip(rights[from_start..rights_to].iter_mut()) - .map(|(k, v)| (k.clone(), v)) - .collect(); - for (l_k, l) in lefts_keys[from_start..lefts_to] - .iter() - .zip(lefts[from_start..lefts_to].iter_mut()) - .rev() - { - match rights_diff.remove(l_k) { - // Reorder and diff any existing children - Some(r) => { - test_log!("moving as next: {:?}", r); - r.move_before(parent, &next_sibling.get()); - apply!(l, r); - } - // Add new children - None => { - apply!(l); + if from_end != lefts.len() || from_start != lefts.len() { + let mut next: Option<&Key> = None; + let mut rights_diff: HashMap<&Key, (&mut VNode, Option<&Key>)> = + HashMap::with_capacity(rights_to - from_start); + for (k, v) in rights_keys[from_start..rights_to] + .iter() + .zip(rights[from_start..rights_to].iter_mut()) + .rev() + { + rights_diff.insert(k, (v, next)); + next = Some(k); + } + next = None; + for (l_k, l) in lefts_keys[from_start..lefts_to] + .iter() + .zip(lefts[from_start..lefts_to].iter_mut()) + .rev() + { + match rights_diff.remove(l_k) { + // Reorder and diff any existing children + Some((r, r_next)) => { + match (r_next, next) { + // If the next sibling was already the same, we don't need to move the + // node + (Some(r), Some(l)) if l == r => (), + _ => { + test_log!("moving as next: {:?}", r); + r.move_before(parent, &next_sibling.get()); + } + } + apply!(l, r); + } + // Add new children + None => { + apply!(l); + } } + next = Some(l_k); } - } - // Remove any extra rights - for (_, r) in rights_diff.drain() { - test_log!("removing: {:?}", r); - r.detach(parent); + // Remove any extra rights + for (_, (r, _)) in rights_diff.drain() { + test_log!("removing: {:?}", r); + r.detach(parent); + } } // Diff matching children at the start - for (l, r) in lefts[..from_start] - .iter_mut() - .zip(rights[..from_start].iter_mut()) - .rev() - { - apply!(l, r); + if from_start != 0 { + for (l, r) in lefts[..from_start] + .iter_mut() + .zip(rights[..from_start].iter_mut()) + .rev() + { + apply!(l, r); + } } next_sibling From cb6d9f7b4dc2e16c712f2f318b3b49132c72fe0c Mon Sep 17 00:00:00 2001 From: bakape Date: Thu, 19 Nov 2020 01:30:49 +0200 Subject: [PATCH 07/15] yew/vlist: clone Rcs instead of referencing --- yew/src/virtual_dom/vlist.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/yew/src/virtual_dom/vlist.rs b/yew/src/virtual_dom/vlist.rs index bd097f68726..5588591dd26 100644 --- a/yew/src/virtual_dom/vlist.rs +++ b/yew/src/virtual_dom/vlist.rs @@ -235,16 +235,16 @@ impl VList { // Diff mismatched children in the middle if from_end != lefts.len() || from_start != lefts.len() { - let mut next: Option<&Key> = None; - let mut rights_diff: HashMap<&Key, (&mut VNode, Option<&Key>)> = + let mut next: Option = None; + let mut rights_diff: HashMap)> = HashMap::with_capacity(rights_to - from_start); for (k, v) in rights_keys[from_start..rights_to] .iter() .zip(rights[from_start..rights_to].iter_mut()) .rev() { - rights_diff.insert(k, (v, next)); - next = Some(k); + rights_diff.insert(k.clone(), (v, next.take())); + next = Some(k.clone()); } next = None; for (l_k, l) in lefts_keys[from_start..lefts_to] @@ -271,7 +271,7 @@ impl VList { apply!(l); } } - next = Some(l_k); + next = Some(l_k.clone()); } // Remove any extra rights From e924f1d3a7de4d67e6de3d42a5c05b0992404e6c Mon Sep 17 00:00:00 2001 From: bakape Date: Thu, 19 Nov 2020 01:35:12 +0200 Subject: [PATCH 08/15] yew/vlist: remove redundant branching --- yew/src/virtual_dom/vlist.rs | 92 +++++++++++++++++------------------- 1 file changed, 44 insertions(+), 48 deletions(-) diff --git a/yew/src/virtual_dom/vlist.rs b/yew/src/virtual_dom/vlist.rs index 5588591dd26..895aff3d481 100644 --- a/yew/src/virtual_dom/vlist.rs +++ b/yew/src/virtual_dom/vlist.rs @@ -234,62 +234,58 @@ impl VList { } // Diff mismatched children in the middle - if from_end != lefts.len() || from_start != lefts.len() { - let mut next: Option = None; - let mut rights_diff: HashMap)> = - HashMap::with_capacity(rights_to - from_start); - for (k, v) in rights_keys[from_start..rights_to] - .iter() - .zip(rights[from_start..rights_to].iter_mut()) - .rev() - { - rights_diff.insert(k.clone(), (v, next.take())); - next = Some(k.clone()); - } - next = None; - for (l_k, l) in lefts_keys[from_start..lefts_to] - .iter() - .zip(lefts[from_start..lefts_to].iter_mut()) - .rev() - { - match rights_diff.remove(l_k) { - // Reorder and diff any existing children - Some((r, r_next)) => { - match (r_next, next) { - // If the next sibling was already the same, we don't need to move the - // node - (Some(r), Some(l)) if l == r => (), - _ => { - test_log!("moving as next: {:?}", r); - r.move_before(parent, &next_sibling.get()); - } + let mut next: Option = None; + let mut rights_diff: HashMap)> = + HashMap::with_capacity(rights_to - from_start); + for (k, v) in rights_keys[from_start..rights_to] + .iter() + .zip(rights[from_start..rights_to].iter_mut()) + .rev() + { + rights_diff.insert(k.clone(), (v, next.take())); + next = Some(k.clone()); + } + next = None; + for (l_k, l) in lefts_keys[from_start..lefts_to] + .iter() + .zip(lefts[from_start..lefts_to].iter_mut()) + .rev() + { + match rights_diff.remove(l_k) { + // Reorder and diff any existing children + Some((r, r_next)) => { + match (r_next, next) { + // If the next sibling was already the same, we don't need to move the + // node + (Some(r), Some(l)) if l == r => (), + _ => { + test_log!("moving as next: {:?}", r); + r.move_before(parent, &next_sibling.get()); } - apply!(l, r); - } - // Add new children - None => { - apply!(l); } + apply!(l, r); + } + // Add new children + None => { + apply!(l); } - next = Some(l_k.clone()); } + next = Some(l_k.clone()); + } - // Remove any extra rights - for (_, (r, _)) in rights_diff.drain() { - test_log!("removing: {:?}", r); - r.detach(parent); - } + // Remove any extra rights + for (_, (r, _)) in rights_diff.drain() { + test_log!("removing: {:?}", r); + r.detach(parent); } // Diff matching children at the start - if from_start != 0 { - for (l, r) in lefts[..from_start] - .iter_mut() - .zip(rights[..from_start].iter_mut()) - .rev() - { - apply!(l, r); - } + for (l, r) in lefts[..from_start] + .iter_mut() + .zip(rights[..from_start].iter_mut()) + .rev() + { + apply!(l, r); } next_sibling From c2f9bca1cc0e2e4bea1b28da0b49873f75ac57aa Mon Sep 17 00:00:00 2001 From: bakape Date: Thu, 19 Nov 2020 02:07:10 +0200 Subject: [PATCH 09/15] yew/vlist: remove newline --- yew/src/virtual_dom/vlist.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/yew/src/virtual_dom/vlist.rs b/yew/src/virtual_dom/vlist.rs index 895aff3d481..8c3924adfaf 100644 --- a/yew/src/virtual_dom/vlist.rs +++ b/yew/src/virtual_dom/vlist.rs @@ -255,8 +255,7 @@ impl VList { // Reorder and diff any existing children Some((r, r_next)) => { match (r_next, next) { - // If the next sibling was already the same, we don't need to move the - // node + // If the next sibling was already the same, we don't need to move the node (Some(r), Some(l)) if l == r => (), _ => { test_log!("moving as next: {:?}", r); From 85320a83c1d27849fe19bbe10961c87431c1ebdf Mon Sep 17 00:00:00 2001 From: bakape Date: Thu, 19 Nov 2020 02:15:09 +0200 Subject: [PATCH 10/15] yew/vlist: remove false comment --- yew/src/virtual_dom/vlist.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yew/src/virtual_dom/vlist.rs b/yew/src/virtual_dom/vlist.rs index 8c3924adfaf..3415245cc38 100644 --- a/yew/src/virtual_dom/vlist.rs +++ b/yew/src/virtual_dom/vlist.rs @@ -222,7 +222,7 @@ impl VList { }; } - // Diff matching children at the end (or the entire list, if they match) + // Diff matching children at the end let lefts_to = lefts_keys.len() - from_end; let rights_to = rights_keys.len() - from_end; for (l, r) in lefts[lefts_to..] From c77a25ca60455d87aba5bceb9ba96a24ef208979 Mon Sep 17 00:00:00 2001 From: bakape Date: Thu, 19 Nov 2020 09:58:41 +0200 Subject: [PATCH 11/15] yew: partial vector deconstruction --- yew/src/virtual_dom/vlist.rs | 44 +++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/yew/src/virtual_dom/vlist.rs b/yew/src/virtual_dom/vlist.rs index 3415245cc38..7c1ee96273c 100644 --- a/yew/src/virtual_dom/vlist.rs +++ b/yew/src/virtual_dom/vlist.rs @@ -170,8 +170,10 @@ impl VList { parent: &Element, mut next_sibling: NodeRef, lefts: &mut [VNode], - mut rights: Vec, + rights: Vec, ) -> NodeRef { + use std::mem::{transmute, MaybeUninit}; + macro_rules! map_keys { ($src:expr) => { $src.iter() @@ -207,7 +209,7 @@ impl VList { macro_rules! apply { ($l:expr, $r:expr) => { test_log!("patching: {:?} -> {:?}", $r, $l); - apply!(std::mem::take($r).into() => $l); + apply!(Some($r) => $l); }; ($l:expr) => { test_log!("adding: {:?}", $l); @@ -222,6 +224,26 @@ impl VList { }; } + // We partially deconstruct the rights vector in several steps. + // This can not be done without default swapping or 2 big vector reallocation overhead in + // safe Rust. + let mut rights: Vec> = unsafe { transmute(rights) }; + + // Takes a value from the rights vector. + // + // We guarantee this is only done once because the consumer routine ranges don't overlap. + // We guarantee this is done for all nodes because all ranges are processed. There are no + // early returns (except panics) possible before full vector depletion. + macro_rules! take { + ($src:expr) => { + unsafe { + let mut dst = MaybeUninit::::uninit(); + std::ptr::copy_nonoverlapping($src.as_mut_ptr(), dst.as_mut_ptr(), 1); + dst.assume_init() + } + }; + } + // Diff matching children at the end let lefts_to = lefts_keys.len() - from_end; let rights_to = rights_keys.len() - from_end; @@ -230,20 +252,20 @@ impl VList { .zip(rights[rights_to..].iter_mut()) .rev() { - apply!(l, r); + apply!(l, take!(r)); } // Diff mismatched children in the middle - let mut next: Option = None; - let mut rights_diff: HashMap)> = + let mut next: Option<&Key> = None; + let mut rights_diff: HashMap<&Key, (VNode, Option<&Key>)> = HashMap::with_capacity(rights_to - from_start); for (k, v) in rights_keys[from_start..rights_to] .iter() .zip(rights[from_start..rights_to].iter_mut()) .rev() { - rights_diff.insert(k.clone(), (v, next.take())); - next = Some(k.clone()); + rights_diff.insert(k, (take!(v), next.take())); + next = Some(k); } next = None; for (l_k, l) in lefts_keys[from_start..lefts_to] @@ -256,7 +278,7 @@ impl VList { Some((r, r_next)) => { match (r_next, next) { // If the next sibling was already the same, we don't need to move the node - (Some(r), Some(l)) if l == r => (), + (Some(r_next), Some(l_next)) if r_next == l_next => (), _ => { test_log!("moving as next: {:?}", r); r.move_before(parent, &next_sibling.get()); @@ -269,11 +291,11 @@ impl VList { apply!(l); } } - next = Some(l_k.clone()); + next = Some(l_k); } // Remove any extra rights - for (_, (r, _)) in rights_diff.drain() { + for (_, (mut r, _)) in rights_diff.drain() { test_log!("removing: {:?}", r); r.detach(parent); } @@ -284,7 +306,7 @@ impl VList { .zip(rights[..from_start].iter_mut()) .rev() { - apply!(l, r); + apply!(l, take!(r)); } next_sibling From 7b8761a176ef4c7d9dd2b496850cd44c8e6695fc Mon Sep 17 00:00:00 2001 From: bakape Date: Sun, 18 Jul 2021 19:19:07 +0300 Subject: [PATCH 12/15] Apply suggestions from code review Co-authored-by: Teymour Aldridge Co-authored-by: Cecile Tonglet --- packages/yew/src/utils.rs | 2 +- packages/yew/src/virtual_dom/vnode.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/yew/src/utils.rs b/packages/yew/src/utils.rs index af2b07d1910..7f6d96fcd5f 100644 --- a/packages/yew/src/utils.rs +++ b/packages/yew/src/utils.rs @@ -90,7 +90,7 @@ impl IntoIterator for NodeSeq { } } -/// Print Node contents as a string for debugging purposes +/// Print the `Node`'s contents as a string for debugging purposes pub fn print_node(n: &web_sys::Node) -> String { use wasm_bindgen::JsCast; diff --git a/packages/yew/src/virtual_dom/vnode.rs b/packages/yew/src/virtual_dom/vnode.rs index 41ca0e5b601..440e262c28f 100644 --- a/packages/yew/src/virtual_dom/vnode.rs +++ b/packages/yew/src/virtual_dom/vnode.rs @@ -35,7 +35,7 @@ impl VNode { } } - /// Returns, if the VNode has a key without needlessly cloning the key + /// Returns true if the `VNode` has a key without needlessly cloning the key. pub fn has_key(&self) -> bool { match self { VNode::VComp(vcomp) => vcomp.key.is_some(), From da725f4a5c2c5b45131c7139fdbc1fc4e1a012a7 Mon Sep 17 00:00:00 2001 From: bakape Date: Sun, 18 Jul 2021 19:29:05 +0300 Subject: [PATCH 13/15] yew: use links in doc comments --- packages/yew/src/utils/mod.rs | 2 +- packages/yew/src/virtual_dom/vlist.rs | 12 ++++++------ packages/yew/src/virtual_dom/vnode.rs | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/yew/src/utils/mod.rs b/packages/yew/src/utils/mod.rs index c8baccd6c90..c9c0c52a8d8 100644 --- a/packages/yew/src/utils/mod.rs +++ b/packages/yew/src/utils/mod.rs @@ -95,7 +95,7 @@ impl IntoIterator for NodeSeq { } } -/// Print the `Node`'s contents as a string for debugging purposes +/// Print the [web_sys::Node]'s contents as a string for debugging purposes pub fn print_node(n: &web_sys::Node) -> String { use wasm_bindgen::JsCast; diff --git a/packages/yew/src/virtual_dom/vlist.rs b/packages/yew/src/virtual_dom/vlist.rs index 61ecaf64537..b1e81ca287c 100644 --- a/packages/yew/src/virtual_dom/vlist.rs +++ b/packages/yew/src/virtual_dom/vlist.rs @@ -8,10 +8,10 @@ use web_sys::Element; /// This struct represents a fragment of the Virtual DOM tree. #[derive(Clone, Debug, PartialEq)] pub struct VList { - /// The list of children nodes. + /// The list of child [VNode]s children: Vec, - // All Nodes in the VList have keys + /// All [VNode]s in the VList have keys fully_keyed: bool, pub key: Option, @@ -67,12 +67,12 @@ macro_rules! advance_next_sibling { } impl VList { - /// Creates a new empty `VList` instance. + /// Creates a new empty [VList] instance. pub fn new() -> Self { Self::default() } - /// Creates a new `VList` instance with children. + /// Creates a new [VList] instance with children. pub fn new_with_children(children: Vec, key: Option) -> Self { VList { fully_keyed: children.iter().all(|ch| ch.has_key()), @@ -81,7 +81,7 @@ impl VList { } } - /// Add `VNode` child. + /// Add [VNode] child. pub fn add_child(&mut self, child: VNode) { if self.fully_keyed && !child.has_key() { self.fully_keyed = false; @@ -89,7 +89,7 @@ impl VList { self.children.push(child); } - /// Add multiple `VNode` children. + /// Add multiple [VNode] children. pub fn add_children(&mut self, children: impl IntoIterator) { let it = children.into_iter(); let bound = it.size_hint(); diff --git a/packages/yew/src/virtual_dom/vnode.rs b/packages/yew/src/virtual_dom/vnode.rs index 440e262c28f..d645042f983 100644 --- a/packages/yew/src/virtual_dom/vnode.rs +++ b/packages/yew/src/virtual_dom/vnode.rs @@ -35,7 +35,7 @@ impl VNode { } } - /// Returns true if the `VNode` has a key without needlessly cloning the key. + /// Returns true if the [VNode] has a key without needlessly cloning the key. pub fn has_key(&self) -> bool { match self { VNode::VComp(vcomp) => vcomp.key.is_some(), From 0aec74c8619b8062757bd27a18b2f9f1fa8366b3 Mon Sep 17 00:00:00 2001 From: bakape Date: Sun, 18 Jul 2021 19:56:29 +0300 Subject: [PATCH 14/15] Update packages/yew/src/virtual_dom/mod.rs Co-authored-by: Muhammad Hamza --- packages/yew/src/virtual_dom/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/yew/src/virtual_dom/mod.rs b/packages/yew/src/virtual_dom/mod.rs index 66d7b3a3088..a6ee9d32ee6 100644 --- a/packages/yew/src/virtual_dom/mod.rs +++ b/packages/yew/src/virtual_dom/mod.rs @@ -422,7 +422,7 @@ pub(crate) trait VDiff { ) -> NodeRef; } -pub(crate) fn insert_node(node: &Node, parent: &Element, next_sibling: &Option) { +pub(crate) fn insert_node(node: &Node, parent: &Element, next_sibling: Option<&Node>) { match next_sibling { Some(next_sibling) => parent .insert_before(&node, Some(next_sibling)) From 1565044ec90eccdbd5f079750f9ba154551d8e2a Mon Sep 17 00:00:00 2001 From: bakape Date: Sun, 18 Jul 2021 19:59:05 +0300 Subject: [PATCH 15/15] yew: update insert_node() calls --- packages/yew/src/html/component/scope.rs | 2 +- packages/yew/src/virtual_dom/vnode.rs | 4 ++-- packages/yew/src/virtual_dom/vtag.rs | 4 ++-- packages/yew/src/virtual_dom/vtext.rs | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/yew/src/html/component/scope.rs b/packages/yew/src/html/component/scope.rs index 4bd67f9b977..1916f1a94d1 100644 --- a/packages/yew/src/html/component/scope.rs +++ b/packages/yew/src/html/component/scope.rs @@ -172,7 +172,7 @@ impl Scope { ) { let placeholder = { let placeholder: Node = document().create_text_node("").into(); - insert_node(&placeholder, &parent, &next_sibling.get()); + insert_node(&placeholder, &parent, next_sibling.get().as_ref()); node_ref.set(Some(placeholder.clone())); VNode::VRef(placeholder) }; diff --git a/packages/yew/src/virtual_dom/vnode.rs b/packages/yew/src/virtual_dom/vnode.rs index f54a98290ef..c259adfd99e 100644 --- a/packages/yew/src/virtual_dom/vnode.rs +++ b/packages/yew/src/virtual_dom/vnode.rs @@ -76,7 +76,7 @@ impl VNode { .expect("VComp has no root vnode") .move_before(parent, next_sibling); } - _ => super::insert_node(&self.first_node(), parent, next_sibling), + _ => super::insert_node(&self.first_node(), parent, next_sibling.as_ref()), }; } } @@ -124,7 +124,7 @@ impl VDiff for VNode { } ancestor.detach(parent); } - super::insert_node(node, parent, &next_sibling.get()); + super::insert_node(node, parent, next_sibling.get().as_ref()); NodeRef::new(node.clone()) } } diff --git a/packages/yew/src/virtual_dom/vtag.rs b/packages/yew/src/virtual_dom/vtag.rs index efd2d1f7e54..32b75f97323 100644 --- a/packages/yew/src/virtual_dom/vtag.rs +++ b/packages/yew/src/virtual_dom/vtag.rs @@ -594,14 +594,14 @@ impl VDiff for VTag { } } else { let el = self.create_element(parent); - super::insert_node(&el, parent, &Some(ancestor.first_node())); + super::insert_node(&el, parent, Some(&ancestor.first_node())); ancestor.detach(parent); (None, el) } } None => (None, { let el = self.create_element(parent); - super::insert_node(&el, parent, &next_sibling.get()); + super::insert_node(&el, parent, next_sibling.get().as_ref()); el }), }; diff --git a/packages/yew/src/virtual_dom/vtext.rs b/packages/yew/src/virtual_dom/vtext.rs index de65711cb53..a44ba7ac2a0 100644 --- a/packages/yew/src/virtual_dom/vtext.rs +++ b/packages/yew/src/virtual_dom/vtext.rs @@ -80,7 +80,7 @@ impl VDiff for VText { } let text_node = document().create_text_node(&self.text); - super::insert_node(&text_node, parent, &next_sibling.get()); + super::insert_node(&text_node, parent, next_sibling.get().as_ref()); self.reference = Some(text_node.clone()); NodeRef::new(text_node.into()) }