-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Remove extra braces in html_nested macro #2169
Remove extra braces in html_nested macro #2169
Conversation
@hamza1311 sorry for the repeated effort in #2167 , I initially marked the issue #2157 as |
# test target can be optionally specified like `cargo make test html_macro`, | ||
args = ["test", "${@}"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this useful in development
@@ -120,8 +120,8 @@ impl ToTokens for HtmlComponent { | |||
|
|||
tokens.extend(quote_spanned! {ty.span()=> | |||
{ | |||
#[allow(clippy::unit_arg)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously the now __yew_props
was inlined in the new
call. When there are no properties (e.g. html_nested!{<MyComponent/>}
), #build_props
would return MyComponent::Properties::builder().build()
which is the unit type, triggering the unit arg warning.
Extracting a variable solves that.
Can you also remove the |
// can test "unused braces" warning inside the macro | ||
// /~https://github.com/yewstack/yew/issues/2157 | ||
fn make_my_component()-> ::yew::virtual_dom::VChild<MyComponent>{ | ||
::yew::html_nested!{<MyComponent/>} | ||
} | ||
|
||
// can test "unused braces" warning inside the macro | ||
// /~https://github.com/yewstack/yew/issues/2157 | ||
fn make_my_component_html()-> ::yew::Html{ | ||
::yew::html!{<MyComponent/>} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test for both html
and html_nested
@@ -168,7 +168,7 @@ impl ToTokens for HtmlRootVNode { | |||
fn to_tokens(&self, tokens: &mut TokenStream) { | |||
let new_tokens = self.0.to_token_stream(); | |||
tokens.extend(quote! {{ | |||
#[allow(clippy::useless_conversion, unused_braces)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part is for the html!
macro, it's no longer needed.
@@ -346,7 +346,6 @@ impl ToTokens for HtmlElement { | |||
} | |||
} | |||
TagName::Expr(name) => { | |||
#[allow(unused_braces)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small correction in the html!
marco. This is most likely misplaced. It does nothing here. No wonder, cuz the previous paragraphs repeated this pattern, this must have been copy-pasted here.
#[allow(unused_braces)] | ||
// e.g. html!{<@{"div"}/>} will set `#expr` to `{"div"}` | ||
// (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> | ||
>::into(#expr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where the misplaced #[allow]
really belongs. Maybe the allow
here is also liftable? I'm however too inexperienced with proc macro to figure it out. So I just left some comments instead
@hamza1311 done now :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems more than fine to me 😏
* removed unused braces from html_nested marco * allow specifying test name * also fix for html! macro * also fix for html! macro * remove misplaced #[allow(unused_braces)] (cherry picked from commit 4c3d693)
Description
Refactored the
html_nested
macro to avoid extra bracesFixes #2157
Checklist
cargo make pr-flow