Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Html (VNode) cheap to clone #3431

Merged
merged 31 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions examples/futures/src/markdown.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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")
}
}
}
}
Expand All @@ -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");
}
}
}
Expand All @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions packages/yew-macro/src/html_tree/html_element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
4 changes: 2 additions & 2 deletions packages/yew-macro/src/html_tree/html_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
))
});
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/yew-macro/src/html_tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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)
)
))
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))),
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions packages/yew-macro/tests/html_macro/component-pass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))),
}
}
}
Expand Down
19 changes: 7 additions & 12 deletions packages/yew-macro/tests/html_macro/element-fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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}`
8 changes: 3 additions & 5 deletions packages/yew/src/dom_bundle/blist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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)
}
Expand Down
74 changes: 56 additions & 18 deletions packages/yew/src/dom_bundle/bnode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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),
}
}
Expand Down Expand Up @@ -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!(
Expand All @@ -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(),
Expand Down
18 changes: 10 additions & 8 deletions packages/yew/src/dom_bundle/bportal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>
},
Expand All @@ -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>
},
Expand All @@ -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>
},
Expand All @@ -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>",
Expand Down
Loading
Loading