Make Html (VNode) cheap to clone (#3431)

* Make VNode cheap to clone * Faster clone for list and portal * Fixes hopefully good * clippy * more fixes hopefully good * rustfmt * More fixes * more fixes... * more fixes * Update element-fail.stderr * Macro fixes... * CLEANUP * Benchmark with divan * WIP workflow * WIP * WIP * WIP * WIP * WIP * WIP * WIP * WIP * WIP * WIP * Use the 2 workflows approach, will fix after merge if not working * CLEANUP * can i push that change here pretty please * Trigger CI
yewstack · Oct 27, 2023 · 7f45af3 · 7f45af3
1 parent aa63234
commit 7f45af3
Show file tree

Hide file tree

Showing 25 changed files with 211 additions and 124 deletions.
diff --git a/examples/futures/src/markdown.rs b/examples/futures/src/markdown.rs
@@ -1,5 +1,7 @@
 /// Original author of this code is [Nathan Ringo](https://github.com/remexre)
 /// Source: https://github.com/acmumn/mentoring/blob/master/web-client/src/view/markdown.rs
+use std::rc::Rc;
+
 use pulldown_cmark::{Alignment, CodeBlockKind, Event, Options, Parser, Tag};
 use yew::virtual_dom::{VNode, VTag, VText};
 use yew::{html, Classes, Html};
@@ -56,16 +58,22 @@ pub fn render_markdown(src: &str) -> Html {
                     if let Some(top_children) = top.children_mut() {
                         for r in top_children.to_vlist_mut().iter_mut() {
                             if let VNode::VTag(ref mut vtag) = r {
-                                if let Some(vtag_children) = vtag.children_mut() {
+                                if let Some(vtag_children) = Rc::make_mut(vtag).children_mut() {
                                     for (i, c) in
                                         vtag_children.to_vlist_mut().iter_mut().enumerate()
                                     {
                                         if let VNode::VTag(ref mut vtag) = c {
                                             match aligns[i] {
                                                 Alignment::None => {}
-                                                Alignment::Left => add_class(vtag, "text-left"),
-                                                Alignment::Center => add_class(vtag, "text-center"),
-                                                Alignment::Right => add_class(vtag, "text-right"),
+                                                Alignment::Left => {
+                                                    add_class(Rc::make_mut(vtag), "text-left")
+                                                }
+                                                Alignment::Center => {
+                                                    add_class(Rc::make_mut(vtag), "text-center")
+                                                }
+                                                Alignment::Right => {
+                                                    add_class(Rc::make_mut(vtag), "text-right")
+                                                }
                                             }
                                         }
                                     }
@@ -79,7 +87,7 @@ pub fn render_markdown(src: &str) -> Html {
                             if let VNode::VTag(ref mut vtag) = c {
                                 // TODO
                                 //                            vtag.tag = "th".into();
-                                vtag.add_attribute("scope", "col");
+                                Rc::make_mut(vtag).add_attribute("scope", "col");
                             }
                         }
                     }
@@ -99,7 +107,7 @@ pub fn render_markdown(src: &str) -> Html {
     }
 
     if elems.len() == 1 {
-        VNode::VTag(Box::new(elems.pop().unwrap()))
+        VNode::VTag(Rc::new(elems.pop().unwrap()))
     } else {
         html! {
             <div>{ for elems.into_iter() }</div>

diff --git a/packages/yew-macro/src/html_tree/html_element.rs b/packages/yew-macro/src/html_tree/html_element.rs
@@ -359,7 +359,7 @@ impl ToTokens for HtmlElement {
                         quote! {
                             ::std::convert::Into::<::yew::virtual_dom::VNode>::into(
                                 ::yew::virtual_dom::VTag::__new_other(
-                                    ::std::borrow::Cow::<'static, ::std::primitive::str>::Borrowed(#name),
+                                    ::yew::virtual_dom::AttrValue::Static(#name),
                                     #node_ref,
                                     #key,
                                     #attributes,
@@ -416,7 +416,7 @@ impl ToTokens for HtmlElement {
                     // (note the extra braces). Hence the need for the `allow`.
                     // Anyways to remove the braces?
                     let mut #vtag_name = ::std::convert::Into::<
-                        ::std::borrow::Cow::<'static, ::std::primitive::str>
+                        ::yew::virtual_dom::AttrValue
                     >::into(#expr);
                     ::std::debug_assert!(
                         #vtag_name.is_ascii(),

diff --git a/packages/yew-macro/src/html_tree/html_list.rs b/packages/yew-macro/src/html_tree/html_list.rs
@@ -78,9 +78,9 @@ impl ToTokens for HtmlList {
         };
 
         tokens.extend(quote_spanned! {spanned.span()=>
-            ::yew::virtual_dom::VNode::VList(
+            ::yew::virtual_dom::VNode::VList(::std::rc::Rc::new(
                 ::yew::virtual_dom::VList::with_children(#children, #key)
-            )
+            ))
         });
     }
 }

diff --git a/packages/yew-macro/src/html_tree/mod.rs b/packages/yew-macro/src/html_tree/mod.rs
@@ -123,7 +123,7 @@ impl ToTokens for HtmlTree {
         lint::lint_all(self);
         match self {
             HtmlTree::Empty => tokens.extend(quote! {
-                ::yew::virtual_dom::VNode::VList(::yew::virtual_dom::VList::new())
+                <::yew::virtual_dom::VNode as ::std::default::Default>::default()
             }),
             HtmlTree::Component(comp) => comp.to_tokens(tokens),
             HtmlTree::Element(tag) => tag.to_tokens(tokens),
@@ -375,9 +375,9 @@ impl ToTokens for HtmlRootBraced {
 
         tokens.extend(quote_spanned! {brace.span.span()=>
             {
-                ::yew::virtual_dom::VNode::VList(
+                ::yew::virtual_dom::VNode::VList(::std::rc::Rc::new(
                     ::yew::virtual_dom::VList::with_children(#children, ::std::option::Option::None)
-                )
+                ))
             }
         });
     }

diff --git a/packages/yew-macro/tests/html_macro/component-any-children-pass.rs b/packages/yew-macro/tests/html_macro/component-any-children-pass.rs
@@ -83,12 +83,12 @@ impl ::std::convert::From<::yew::virtual_dom::VChild<AltChild>> for ChildrenVari
 impl ::std::convert::Into<::yew::virtual_dom::VNode> for ChildrenVariants {
     fn into(self) -> ::yew::virtual_dom::VNode {
         match self {
-            Self::Child(comp) => ::yew::virtual_dom::VNode::VComp(::std::convert::Into::<
+            Self::Child(comp) => ::yew::virtual_dom::VNode::VComp(::std::rc::Rc::new(::std::convert::Into::<
                 ::yew::virtual_dom::VComp,
-            >::into(comp)),
-            Self::AltChild(comp) => ::yew::virtual_dom::VNode::VComp(::std::convert::Into::<
+            >::into(comp))),
+            Self::AltChild(comp) => ::yew::virtual_dom::VNode::VComp(::std::rc::Rc::new(::std::convert::Into::<
                 ::yew::virtual_dom::VComp,
-            >::into(comp)),
+            >::into(comp))),
         }
     }
 }

diff --git a/packages/yew-macro/tests/html_macro/component-pass.rs b/packages/yew-macro/tests/html_macro/component-pass.rs
@@ -82,12 +82,12 @@ impl ::std::convert::From<::yew::virtual_dom::VChild<AltChild>> for ChildrenVari
 impl ::std::convert::Into<::yew::virtual_dom::VNode> for ChildrenVariants {
     fn into(self) -> ::yew::virtual_dom::VNode {
         match self {
-            Self::Child(comp) => ::yew::virtual_dom::VNode::VComp(::std::convert::Into::<
+            Self::Child(comp) => ::yew::virtual_dom::VNode::VComp(::std::rc::Rc::new(::std::convert::Into::<
                 ::yew::virtual_dom::VComp,
-            >::into(comp)),
-            Self::AltChild(comp) => ::yew::virtual_dom::VNode::VComp(::std::convert::Into::<
+            >::into(comp))),
+            Self::AltChild(comp) => ::yew::virtual_dom::VNode::VComp(::std::rc::Rc::new(::std::convert::Into::<
                 ::yew::virtual_dom::VComp,
-            >::into(comp)),
+            >::into(comp))),
         }
     }
 }

diff --git a/packages/yew-macro/tests/html_macro/element-fail.stderr b/packages/yew-macro/tests/html_macro/element-fail.stderr
@@ -703,20 +703,15 @@ error[E0277]: the trait bound `(): IntoPropValue<yew::NodeRef>` is not satisfied
            and $N others
    = note: this error originates in the macro `html` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error[E0277]: the trait bound `Cow<'static, str>: From<{integer}>` is not satisfied
+error[E0277]: the trait bound `implicit_clone::unsync::IString: From<{integer}>` is not satisfied
   --> tests/html_macro/element-fail.rs:77:15
    |
 77 |     html! { <@{55}></@> };
-   |               ^^^^ the trait `From<{integer}>` is not implemented for `Cow<'static, str>`
+   |               ^^^^ the trait `From<{integer}>` is not implemented for `implicit_clone::unsync::IString`
    |
    = help: the following other types implement trait `From<T>`:
-             <Cow<'a, CStr> as From<&'a CStr>>
-             <Cow<'a, CStr> as From<&'a CString>>
-             <Cow<'a, CStr> as From<CString>>
-             <Cow<'a, OsStr> as From<&'a OsStr>>
-             <Cow<'a, OsStr> as From<&'a OsString>>
-             <Cow<'a, OsStr> as From<OsString>>
-             <Cow<'a, Path> as From<&'a Path>>
-             <Cow<'a, Path> as From<&'a PathBuf>>
-           and $N others
-   = note: required because of the requirements on the impl of `Into<Cow<'static, str>>` for `{integer}`
+             <implicit_clone::unsync::IString as From<&'static str>>
+             <implicit_clone::unsync::IString as From<Cow<'static, str>>>
+             <implicit_clone::unsync::IString as From<Rc<str>>>
+             <implicit_clone::unsync::IString as From<String>>
+   = note: required because of the requirements on the impl of `Into<implicit_clone::unsync::IString>` for `{integer}`
diff --git a/packages/yew/src/dom_bundle/blist.rs b/packages/yew/src/dom_bundle/blist.rs
@@ -4,13 +4,13 @@ use std::cmp::Ordering;
 use std::collections::HashSet;
 use std::hash::Hash;
 use std::ops::Deref;
-use std::rc::Rc;
 
 use web_sys::Element;
 
 use super::{test_log, BNode, BSubtree, DomSlot};
 use crate::dom_bundle::{Reconcilable, ReconcileTarget};
 use crate::html::AnyScope;
+use crate::utils::RcExt;
 use crate::virtual_dom::{Key, VList, VNode, VText};
 
 /// This struct represents a mounted [VList]
@@ -30,10 +30,8 @@ impl VList {
 
         let children = self
             .children
-            .map(Rc::try_unwrap)
-            .unwrap_or_else(|| Ok(Vec::new()))
-            // Rc::unwrap_or_clone is not stable yet.
-            .unwrap_or_else(|m| m.to_vec());
+            .map(RcExt::unwrap_or_clone)
+            .unwrap_or_default();
 
         (self.key, fully_keyed, children)
     }

diff --git a/packages/yew/src/dom_bundle/bnode.rs b/packages/yew/src/dom_bundle/bnode.rs
@@ -7,6 +7,7 @@ use web_sys::{Element, Node};
 use super::{BComp, BList, BPortal, BRaw, BSubtree, BSuspense, BTag, BText, DomSlot};
 use crate::dom_bundle::{Reconcilable, ReconcileTarget};
 use crate::html::AnyScope;
+use crate::utils::RcExt;
 use crate::virtual_dom::{Key, VNode};
 
 /// The bundle implementation to [VNode].
@@ -95,31 +96,36 @@ impl Reconcilable for VNode {
     ) -> (DomSlot, Self::Bundle) {
         match self {
             VNode::VTag(vtag) => {
-                let (node_ref, tag) = vtag.attach(root, parent_scope, parent, slot);
+                let (node_ref, tag) =
+                    RcExt::unwrap_or_clone(vtag).attach(root, parent_scope, parent, slot);
                 (node_ref, tag.into())
             }
             VNode::VText(vtext) => {
                 let (node_ref, text) = vtext.attach(root, parent_scope, parent, slot);
                 (node_ref, text.into())
             }
             VNode::VComp(vcomp) => {
-                let (node_ref, comp) = vcomp.attach(root, parent_scope, parent, slot);
+                let (node_ref, comp) =
+                    RcExt::unwrap_or_clone(vcomp).attach(root, parent_scope, parent, slot);
                 (node_ref, comp.into())
             }
             VNode::VList(vlist) => {
-                let (node_ref, list) = vlist.attach(root, parent_scope, parent, slot);
+                let (node_ref, list) =
+                    RcExt::unwrap_or_clone(vlist).attach(root, parent_scope, parent, slot);
                 (node_ref, list.into())
             }
             VNode::VRef(node) => {
                 slot.insert(parent, &node);
                 (DomSlot::at(node.clone()), BNode::Ref(node))
             }
             VNode::VPortal(vportal) => {
-                let (node_ref, portal) = vportal.attach(root, parent_scope, parent, slot);
+                let (node_ref, portal) =
+                    RcExt::unwrap_or_clone(vportal).attach(root, parent_scope, parent, slot);
                 (node_ref, portal.into())
             }
             VNode::VSuspense(vsuspsense) => {
-                let (node_ref, suspsense) = vsuspsense.attach(root, parent_scope, parent, slot);
+                let (node_ref, suspsense) =
+                    RcExt::unwrap_or_clone(vsuspsense).attach(root, parent_scope, parent, slot);
                 (node_ref, suspsense.into())
             }
             VNode::VRaw(vraw) => {
@@ -149,20 +155,46 @@ impl Reconcilable for VNode {
         bundle: &mut BNode,
     ) -> DomSlot {
         match self {
-            VNode::VTag(vtag) => vtag.reconcile_node(root, parent_scope, parent, slot, bundle),
+            VNode::VTag(vtag) => RcExt::unwrap_or_clone(vtag).reconcile_node(
+                root,
+                parent_scope,
+                parent,
+                slot,
+                bundle,
+            ),
             VNode::VText(vtext) => vtext.reconcile_node(root, parent_scope, parent, slot, bundle),
-            VNode::VComp(vcomp) => vcomp.reconcile_node(root, parent_scope, parent, slot, bundle),
-            VNode::VList(vlist) => vlist.reconcile_node(root, parent_scope, parent, slot, bundle),
+            VNode::VComp(vcomp) => RcExt::unwrap_or_clone(vcomp).reconcile_node(
+                root,
+                parent_scope,
+                parent,
+                slot,
+                bundle,
+            ),
+            VNode::VList(vlist) => RcExt::unwrap_or_clone(vlist).reconcile_node(
+                root,
+                parent_scope,
+                parent,
+                slot,
+                bundle,
+            ),
             VNode::VRef(node) => match bundle {
                 BNode::Ref(ref n) if &node == n => DomSlot::at(node),
                 _ => VNode::VRef(node).replace(root, parent_scope, parent, slot, bundle),
             },
-            VNode::VPortal(vportal) => {
-                vportal.reconcile_node(root, parent_scope, parent, slot, bundle)
-            }
-            VNode::VSuspense(vsuspsense) => {
-                vsuspsense.reconcile_node(root, parent_scope, parent, slot, bundle)
-            }
+            VNode::VPortal(vportal) => RcExt::unwrap_or_clone(vportal).reconcile_node(
+                root,
+                parent_scope,
+                parent,
+                slot,
+                bundle,
+            ),
+            VNode::VSuspense(vsuspsense) => RcExt::unwrap_or_clone(vsuspsense).reconcile_node(
+                root,
+                parent_scope,
+                parent,
+                slot,
+                bundle,
+            ),
             VNode::VRaw(vraw) => vraw.reconcile_node(root, parent_scope, parent, slot, bundle),
         }
     }
@@ -246,10 +278,16 @@ mod feat_hydration {
             fragment: &mut Fragment,
         ) -> Self::Bundle {
             match self {
-                VNode::VTag(vtag) => vtag.hydrate(root, parent_scope, parent, fragment).into(),
+                VNode::VTag(vtag) => RcExt::unwrap_or_clone(vtag)
+                    .hydrate(root, parent_scope, parent, fragment)
+                    .into(),
                 VNode::VText(vtext) => vtext.hydrate(root, parent_scope, parent, fragment).into(),
-                VNode::VComp(vcomp) => vcomp.hydrate(root, parent_scope, parent, fragment).into(),
-                VNode::VList(vlist) => vlist.hydrate(root, parent_scope, parent, fragment).into(),
+                VNode::VComp(vcomp) => RcExt::unwrap_or_clone(vcomp)
+                    .hydrate(root, parent_scope, parent, fragment)
+                    .into(),
+                VNode::VList(vlist) => RcExt::unwrap_or_clone(vlist)
+                    .hydrate(root, parent_scope, parent, fragment)
+                    .into(),
                 // You cannot hydrate a VRef.
                 VNode::VRef(_) => {
                     panic!(
@@ -264,7 +302,7 @@ mod feat_hydration {
                          use_effect."
                     )
                 }
-                VNode::VSuspense(vsuspense) => vsuspense
+                VNode::VSuspense(vsuspense) => RcExt::unwrap_or_clone(vsuspense)
                     .hydrate(root, parent_scope, parent, fragment)
                     .into(),
                 VNode::VRaw(vraw) => vraw.hydrate(root, parent_scope, parent, fragment).into(),

diff --git a/packages/yew/src/dom_bundle/bportal.rs b/packages/yew/src/dom_bundle/bportal.rs
@@ -123,6 +123,8 @@ impl BPortal {
 mod layout_tests {
     extern crate self as yew;
 
+    use std::rc::Rc;
+
     use gloo::utils::document;
     use wasm_bindgen_test::{wasm_bindgen_test as test, wasm_bindgen_test_configure};
     use web_sys::HtmlInputElement;
@@ -151,10 +153,10 @@ mod layout_tests {
                 <div>
                     {VNode::VRef(first_target.clone().into())}
                     {VNode::VRef(second_target.clone().into())}
-                    {VNode::VPortal(VPortal::new(
+                    {VNode::VPortal(Rc::new(VPortal::new(
                         html! { {"PORTAL"} },
                         first_target.clone(),
-                    ))}
+                    )))}
                     {"AFTER"}
                 </div>
             },
@@ -166,10 +168,10 @@ mod layout_tests {
                 <div>
                     {VNode::VRef(first_target.clone().into())}
                     {VNode::VRef(second_target.clone().into())}
-                    {VNode::VPortal(VPortal::new(
+                    {VNode::VPortal(Rc::new(VPortal::new(
                         html! { {"PORTAL"} },
                         second_target.clone(),
-                    ))}
+                    )))}
                     {"AFTER"}
                 </div>
             },
@@ -181,10 +183,10 @@ mod layout_tests {
                 <div>
                     {VNode::VRef(first_target.clone().into())}
                     {VNode::VRef(second_target.clone().into())}
-                    {VNode::VPortal(VPortal::new(
+                    {VNode::VPortal(Rc::new(VPortal::new(
                         html! { <> {"PORTAL"} <b /> </> },
                         second_target.clone(),
-                    ))}
+                    )))}
                     {"AFTER"}
                 </div>
             },
@@ -207,11 +209,11 @@ mod layout_tests {
             node: html! {
                 <div>
                     {VNode::VRef(target_with_child.clone().into())}
-                    {VNode::VPortal(VPortal::new_before(
+                    {VNode::VPortal(Rc::new(VPortal::new_before(
                         html! { {"PORTAL"} },
                         target_with_child.clone(),
                         Some(target_child.clone().into()),
-                    ))}
+                    )))}
                 </div>
             },
             expected: "<div><i>PORTAL<s></s></i></div>",