Skip to content

Commit

Permalink
Fixed NodeRef not being implicitly cloned with components (#2775)
Browse files Browse the repository at this point in the history
* Use IntoPropValue for node refs in html component

* Add NodeRef ImplicitClone test for html element

* Change node_refs example to use ImplicitClone

* Reuse key and ref attribute wrapping
  • Loading branch information
wdcocq committed Jul 7, 2022
1 parent 2e4a919 commit 5570710
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 73 deletions.
4 changes: 2 additions & 2 deletions examples/node_refs/src/main.rs
Expand Up @@ -79,7 +79,7 @@ impl Component for App {
<label>{ "Email" }</label>
<input
type="text"
ref={self.refs[0].clone()}
ref={&self.refs[0]}
class="input-element"
onmouseover={ctx.link().callback(|_| Msg::HoverIndex(0))}
placeholder="abcd@xyz.com"
Expand All @@ -89,7 +89,7 @@ impl Component for App {
<div class="input-container">
<label>{ "Password" }</label>
<InputComponent
ref={self.refs[1].clone()}
ref={&self.refs[1]}
on_hover={ctx.link().callback(|_| Msg::HoverIndex(1))}
placeholder="password"
/>
Expand Down
37 changes: 11 additions & 26 deletions packages/yew-macro/src/html_tree/html_component.rs
Expand Up @@ -106,32 +106,17 @@ impl ToTokens for HtmlComponent {
Some(quote! { ::yew::html::ChildrenRenderer::new(#children) })
};
let build_props = props.build_properties_tokens(&props_ty, children_renderer);

let special_props = props.special();
let node_ref = if let Some(node_ref) = &special_props.node_ref {
let value = &node_ref.value;
quote! { #value }
} else {
quote! { <::yew::html::NodeRef as ::std::default::Default>::default() }
};

let key = if let Some(key) = &special_props.key {
let value = &key.value;
quote_spanned! {value.span().resolved_at(Span::call_site())=>
#[allow(clippy::useless_conversion)]
Some(::std::convert::Into::<::yew::virtual_dom::Key>::into(#value))
}
} else {
quote! { ::std::option::Option::None }
};
let use_close_tag = if let Some(close) = close {
let close_ty = &close.ty;
quote_spanned! {close_ty.span()=>
let _ = |_:#close_ty| {};
}
} else {
Default::default()
};
let node_ref = props.special().wrap_node_ref_attr();
let key = props.special().wrap_key_attr();
let use_close_tag = close
.as_ref()
.map(|close| {
let close_ty = &close.ty;
quote_spanned! {close_ty.span()=>
let _ = |_:#close_ty| {};
}
})
.unwrap_or_default();

tokens.extend(quote_spanned! {ty_span=>
{
Expand Down
40 changes: 6 additions & 34 deletions packages/yew-macro/src/html_tree/html_element.rs
Expand Up @@ -112,37 +112,17 @@ impl ToTokens for HtmlElement {
booleans,
value,
checked,
node_ref,
key,
listeners,
special,
} = &props;

// attributes with special treatment

let node_ref = node_ref
.as_ref()
.map(|attr| {
let value = &attr.value;
quote_spanned! {value.span().resolved_at(Span::call_site())=>
::yew::html::IntoPropValue::<::yew::html::NodeRef>
::into_prop_value(#value)
}
})
.unwrap_or(quote! { ::std::default::Default::default() });
let key = key
.as_ref()
.map(|attr| {
let value = attr.value.optimize_literals();
quote_spanned! {value.span().resolved_at(Span::call_site())=>
::std::option::Option::Some(
::std::convert::Into::<::yew::virtual_dom::Key>::into(#value)
)
}
})
.unwrap_or(quote! { ::std::option::Option::None });
let node_ref = special.wrap_node_ref_attr();
let key = special.wrap_key_attr();
let value = value
.as_ref()
.map(wrap_attr_prop)
.map(|prop| wrap_attr_value(prop.value.optimize_literals()))
.unwrap_or(quote! { ::std::option::Option::None });
let checked = checked
.as_ref()
Expand Down Expand Up @@ -262,14 +242,7 @@ impl ToTokens for HtmlElement {
.collect::<Vec<(LitStr, Value)>>();
try_into_static(&attrs).unwrap_or_else(|| {
let keys = attrs.iter().map(|(k, _)| quote! { #k });
let values = attrs.iter().map(|(_, v)| {
quote_spanned! {v.span()=>
::yew::html::IntoPropValue::<
::std::option::Option::<::yew::virtual_dom::AttrValue>
>
::into_prop_value(#v)
}
});
let values = attrs.iter().map(|(_, v)| wrap_attr_value(v));
quote! {
::yew::virtual_dom::Attributes::Dynamic{
keys: &[#(#keys),*],
Expand Down Expand Up @@ -473,8 +446,7 @@ impl ToTokens for HtmlElement {
}
}

fn wrap_attr_prop(prop: &Prop) -> TokenStream {
let value = prop.value.optimize_literals();
fn wrap_attr_value<T: ToTokens>(value: T) -> TokenStream {
quote_spanned! {value.span()=>
::yew::html::IntoPropValue::<
::std::option::Option<
Expand Down
9 changes: 3 additions & 6 deletions packages/yew-macro/src/props/element.rs
Expand Up @@ -26,8 +26,7 @@ pub struct ElementProps {
pub booleans: Vec<Prop>,
pub value: Option<Prop>,
pub checked: Option<Prop>,
pub node_ref: Option<Prop>,
pub key: Option<Prop>,
pub special: SpecialProps,
}

impl Parse for ElementProps {
Expand All @@ -48,8 +47,7 @@ impl Parse for ElementProps {
.map(|prop| ClassesForm::from_expr(prop.value));
let value = props.pop("value");
let checked = props.pop("checked");

let SpecialProps { node_ref, key } = props.special;
let special = props.special;

Ok(Self {
attributes: props.prop_list.into_vec(),
Expand All @@ -58,8 +56,7 @@ impl Parse for ElementProps {
checked,
booleans: booleans.into_vec(),
value,
node_ref,
key,
special,
})
}
}
Expand Down
32 changes: 31 additions & 1 deletion packages/yew-macro/src/props/prop.rs
Expand Up @@ -2,13 +2,16 @@ use std::cmp::Ordering;
use std::convert::TryFrom;
use std::ops::{Deref, DerefMut};

use proc_macro2::{Spacing, TokenTree};
use proc_macro2::{Spacing, Span, TokenStream, TokenTree};
use quote::{quote, quote_spanned};
use syn::parse::{Parse, ParseBuffer, ParseStream};
use syn::spanned::Spanned;
use syn::token::Brace;
use syn::{braced, Block, Expr, ExprBlock, ExprPath, ExprRange, Stmt, Token};

use super::CHILDREN_LABEL;
use crate::html_tree::HtmlDashedName;
use crate::stringify::Stringify;

pub struct Prop {
pub label: HtmlDashedName,
Expand Down Expand Up @@ -325,6 +328,33 @@ impl SpecialProps {
pub fn check_all(&self, f: impl FnMut(&Prop) -> syn::Result<()>) -> syn::Result<()> {
crate::join_errors(self.iter().map(f).filter_map(Result::err))
}

pub fn wrap_node_ref_attr(&self) -> TokenStream {
self.node_ref
.as_ref()
.map(|attr| {
let value = &attr.value;
quote_spanned! {value.span().resolved_at(Span::call_site())=>
::yew::html::IntoPropValue::<::yew::html::NodeRef>
::into_prop_value(#value)
}
})
.unwrap_or(quote! { ::std::default::Default::default() })
}

pub fn wrap_key_attr(&self) -> TokenStream {
self.key
.as_ref()
.map(|attr| {
let value = attr.value.optimize_literals();
quote_spanned! {value.span().resolved_at(Span::call_site())=>
::std::option::Option::Some(
::std::convert::Into::<::yew::virtual_dom::Key>::into(#value)
)
}
})
.unwrap_or(quote! { ::std::option::Option::None })
}
}

pub struct Props {
Expand Down
7 changes: 5 additions & 2 deletions packages/yew-macro/tests/html_macro/component-fail.stderr
Expand Up @@ -386,11 +386,14 @@ note: required by a bound in `ChildPropertiesBuilder::string`
| ------ required by a bound in this
= note: this error originates in the derive macro `Properties` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
error[E0277]: the trait bound `(): IntoPropValue<NodeRef>` is not satisfied
--> tests/html_macro/component-fail.rs:80:31
|
80 | html! { <Child int=1 ref={()} /> };
| ^^ expected struct `NodeRef`, found `()`
| ^^
| |
| the trait `IntoPropValue<NodeRef>` is not implemented for `()`
| required by a bound introduced by this call

error[E0277]: the trait bound `u32: IntoPropValue<i32>` is not satisfied
--> tests/html_macro/component-fail.rs:82:24
Expand Down
8 changes: 6 additions & 2 deletions packages/yew-macro/tests/html_macro/component-pass.rs
Expand Up @@ -55,6 +55,7 @@ impl ::yew::Component for Container {
fn create(_ctx: &::yew::Context<Self>) -> Self {
::std::unimplemented!()
}

fn view(&self, _ctx: &::yew::Context<Self>) -> ::yew::Html {
::std::unimplemented!()
}
Expand Down Expand Up @@ -116,6 +117,7 @@ impl ::yew::Component for Child {
fn create(_ctx: &::yew::Context<Self>) -> Self {
::std::unimplemented!()
}

fn view(&self, _ctx: &::yew::Context<Self>) -> ::yew::Html {
::std::unimplemented!()
}
Expand All @@ -129,6 +131,7 @@ impl ::yew::Component for AltChild {
fn create(_ctx: &::yew::Context<Self>) -> Self {
::std::unimplemented!()
}

fn view(&self, _ctx: &::yew::Context<Self>) -> ::yew::Html {
::std::unimplemented!()
}
Expand All @@ -151,14 +154,14 @@ impl ::yew::Component for ChildContainer {
fn create(_ctx: &::yew::Context<Self>) -> Self {
::std::unimplemented!()
}

fn view(&self, _ctx: &::yew::Context<Self>) -> ::yew::Html {
::std::unimplemented!()
}
}

mod scoped {
pub use super::Child;
pub use super::Container;
pub use super::{Child, Container};
}

fn compile_pass() {
Expand All @@ -181,6 +184,7 @@ fn compile_pass() {
<Child ref={::std::clone::Clone::clone(&node_ref)} int={2} ..::yew::props!(Child::Properties { int: 5 }) />
<Child int=3 ref={::std::clone::Clone::clone(&node_ref)} ..::yew::props!(Child::Properties { int: 5 }) />
<Child ref={::std::clone::Clone::clone(&node_ref)} ..::yew::props!(Child::Properties { int: 5 }) />
<Child ref={&node_ref} ..<<Child as ::yew::Component>::Properties as ::std::default::Default>::default() />
<Child ref={node_ref} ..<<Child as ::yew::Component>::Properties as ::std::default::Default>::default() />
</>
};
Expand Down
1 change: 1 addition & 0 deletions packages/yew-macro/tests/html_macro/html-element-pass.rs
Expand Up @@ -51,6 +51,7 @@ fn compile_pass() {
::yew::html! {
<div>
<div data-key="abc"></div>
<div ref={&parent_ref}></div>
<div ref={parent_ref} class="parent">
<span class="child" value="anything"></span>
<label for="first-name">{"First Name"}</label>
Expand Down

0 comments on commit 5570710

Please sign in to comment.