yew
yew copied to clipboard
yewstack
Struggling with ways to convert someting into `Html`.
Problem
Steps To Reproduce Steps to reproduce the behavior:
#[test]
fn test_to_html_to_children_comp_compiles() {
use crate::prelude::*;
struct Foo;
impl ToHtml for Foo {
fn to_html(&self) -> Html {
html!()
}
}
#[derive(PartialEq, Properties)]
pub struct FooProps {
pub children: Children,
}
#[function_component(Comp)]
fn comp(props: &FooProps) -> Html {
html!()
}
let _ = html!(<Comp>{Foo}</Comp>);
}
Expected behavior
In the past it was possible to implement From<Foo> for Html to use something in the html macro, like this:
struct Foo;
html! (
<SomeComponent>{ Foo }</SomeComponent>
)
Now there seem to be two new variants: ToHtml, and IntoPropValue<ChildrenRenderer<Vnode>>. However, it's not always clear when to use what and why sometimes ToHtml, doesn't work.
Environment:
- Yew version:
v0.21.0. - Rust version:
1.17.1
Questionnaire
- [x] I'm interested in fixing this myself but don't know where to start
- [ ] I would like to fix and I have a solution
- [ ] I don't have time to fix this right now, but maybe later
The absolute downside of moving away from From<T> for Html is that now I need to implement three traits for the same effect: From<T> for Html, IntoPropValue<ChildrenRenderer<VNode>>, and ToHtml. Depending on the context I use this.
I think either From<T> for Html or ToHtml would be ok. But the requirement for IntoPropValue<ChildrenRenderer<VNode>> is a bit much.
What prevents you from just using ToHtml? Html can be passed as children to a component now. I don't see a case where ChildrenRenderer is needed
I think an implementation:
impl<T: Into<Html>> ToHtml for T {}
could be helpful.
CC: @futursolo
In the past all children was the type Children. Or ChildrenWithProperties<T>.
All the examples I found in the docs still follow this pattern. If that has changed, the maybe the docs need some update. As well a the pattern of using ChildrenWithProperties. Or a specific type of children.
That was changed in the latest release 0.21 and docs were updated to reflect the change. Are there any docs you're seeing that have not been updated?
I was just looking for them. Could it be that the switch, defaulting to the 0.21 docs , was made a bit after the release? I might have been running into the old docs.
At least in the module yew::html::component::children I can find the old examples. As well as the Children type itself. Wouldn't it make sense to deprecate it, having a proper message.
Docs default to 0.21 when the release is made. You can see the updated children docs: https://yew.rs/docs/concepts/function-components/children
Any type, including Html, can be passed as children. This was also mentioned in the release blog
Docs default to 0.21 when the release is made.
I am pretty sure I saw 0.20 docs by default after the release was made.
Any type, including Html, can be passed as children. This was also mentioned in the release blog
However, there was nothing in the migration guide: https://yew.rs/docs/migration-guides/yew/from-0_20_0-to-0_21_0 … so there should at least be some "also see the blog post" link in there.
Docs default to 0.21 when the release is made.
I am pretty sure I saw 0.20 docs by default after the release was made.
Hmm, not sure why that is. Could be caching?
Also, the migration guide is for breaking changes. I'm not sure where exactly this kind of change would be mentioned. I agree that it be mentioned, just not sure where. Perhaps we could add a section for non breaking changes in the migration guide? Pinging @futursolo as they were the one who made this change.
I would really call that a breaking change though.
What no longer works with Html is a pattern like this:
{ for props.children.iter().map(|item| html!(
<div class="pf-v5-c-accordion__expandable-content-body">
{ item }
</div>
)) }
I don't think this is a breaking change as no existing code breaks. You're allowed to still use ChildrenRenderer if it's needed.
You can pass any type as children now. For your case, VList exists (in addition to children). html_nested! can be used to construct any variant of VNode without wrapping it in VNode
Right, but when I need to use Children, I also need to implement the IntoProp trait … so I am back to the problem above. All in all, not a step forward IMHO.
Besides, I does break current code, because Into<Html> no longer works.
Using VList for children I indeed can iterate over it. But I can no longer use the html macro. Because the html macro will generate all kind of nodes, not necessarily VList.
Yea, but assume I have something like this:
html!(
<Foo>
<Bar>
<Baz>
</Baz>
</Bar>
</Foo>
)
If I want Foo to be aware of the number of children, using VList, then I need to inject html_nested now?
hi, we run into a similar (the same?) issue, we often implemented Into<Html> (or Into<VNode>) on the properties struct, so that we can use a builder pattern for them and still use them in the html!{} macro. Since upgrading to 0.21 this does not work anymore.
I played around with implementing a blanket impl block for this, but ran into issues with conflicting implementations (either with the other ToHtml implementations or with the IntoPropValue implementations) so maybe someone who has a better understanding of how they play together can whip something up?
Hi,
the bug was closed, but i think there is still a valid concern here (Please tell if i should open (a) new bug(s).
We have an issue with From<...> for Html and the ToHtml trait that worked before and does not now.
A short example that worked in 0.20 and doesn't anymore:
use yew::prelude::*;
struct Print {
text: String,
}
impl Print {
fn new(text: String) -> Self {
Self { text }
}
}
impl From<Print> for Html {
fn from(val: Print) -> Self {
html! {<>{"Hello "}{val.text}</>}
}
}
#[function_component]
fn App() -> Html {
let text = Print::new("World".to_owned());
html! { <div>{text}</div> }
}
fn main() {
yew::Renderer::<App>::new().render();
}
Implementing ToHtml for that is often unpractical, since you have to copy/clone the properties to implement to_html even if you just want it to be able to convert it without copying/cloning anything
also maybe related is the following issue:
use yew::prelude::*;
fn print<T: ToHtml>(text: T) -> Html {
html! {<div>{"Hello "}{text}</div>}
}
#[function_component]
fn App() -> Html {
let vnode = html! {"test"};
print(vnode)
}
fn main() {
yew::Renderer::<App>::new().render();
}
here we get the error that VNode does not implement ToHtml which is also super unpractical since sometimes you want to have a function that wants to copy/clone and consume the parameter (depending on the context)
Implementing ToHtml for that is often unpractical, since you have to copy/clone the properties to implement to_html even if you just want it to be able to convert it without copying/cloning anything
There are 2 methods on ToHtml.
1 is to_html and the other one is into_html. into_html is provided with a blanket implementation of to_html. You can implement into_html as well if you think you are able to provide more efficient implementation for fn(Self) -> Html.
I think one of the issues with the new approach is, that one needs to implement many ways now. Depending on the context. Assuming the following for example:
Icon(Icon),
Text(String),
Custom(Html),
}
What I want is to just use:
// either using From, or some custom trait.
impl From<Label> for Html {
}
fn component() -> Html {
let label : Label;
html!(<div>{&label}</div>)
}
What I need to do is:
impl ToHtml for Label {
fn to_html() {}
fn into_html() {} // because I want to avoid cloning
}
// because some people might want to use `Children`.
impl IntoPropValue<ChildrenRenderer<VNode>> for Label {
}
fn component() -> Html {
let label : Label;
html!(<div>{label.clone()}</div>) // second clone, unless I implement into_html()
}
So it more work, to get to the same result.
Implementing ToHtml for that is often unpractical, since you have to copy/clone the properties to implement to_html even if you just want it to be able to convert it without copying/cloning anything
There are 2 methods on
ToHtml. 1 isto_htmland the other one isinto_html.into_htmlis provided with a blanket implementation ofto_html. You can implementinto_htmlas well if you think you are able to provide more efficient implementation forfn(Self) -> Html.
true but i have to implement a copying/cloning version now, where as before i did not (as @ctron correctly wrote)
wouldn't it be possible to split the trait into two? e.g. an ToHtml trait with to_html that copies/clones and an IntoHtml trait with into_html that consumes (or ideally just reuse Into<Html> for the second part) ? That way i could just implement the consuming part if i want to avoid it being copied
I'm sorry that i don't have a very deep understanding of the ToHtml/IntoPropValue/ChildRenderer traits to make a more concrete suggestion, but the current way seems a bit unwieldy to use for some use-cases.
(currently we work around the issue by requiring 'Clone' on the types we use that for and use
impl ToHtml for XXX {
fn to_html(&self) -> Html {
self.clone().into_html()
}
fn into_html(self) -> Html {
self.into()
}
}
where .into() leads back to our From/Into impls
@futursolo when you left in Children, was there any reason other than backwards compatibility? If not, we can deprecate that and point everyone to use Html or any of its variants as needed@futursolo when you left in Children, was there any reason other than backwards compatibility? If not, we can deprecate that and point everyone to use Html or any of its variants as needed
I'm starting to think ToHtml was a mistake. With #3431, the cost of cloning Html goes down greatly so perhaps we could switch to Into<Html>
wouldn't it be possible to split the trait into two? e.g. an
ToHtmltrait withto_htmlthat copies/clones and anIntoHtmltrait withinto_htmlthat consumes (or ideally just reuseInto<Html>for the second part) ? That way i could just implement the consuming part if i want to avoid it being copied
No, not on stable at least, as far as I know. That would need specialization so we can have a blanket impl of IntoHtml and be allowed to specialize it when possible
I have given a try at migrating to help understand the issue here and this is what I found:
-
&AttrValue cannot be converted automatically to html anymore:
error[E0277]: the trait bound `&implicit_clone::unsync::IString: IntoPropValue<ChildrenRenderer<VNode>>` is not satisfied --> yewprint-doc/src/text/example.rs:15:18 | 14 | <Text ellipsize={props.ellipsize}> | ---- required by a bound introduced by this call 15 | {&props.text} | ^^^^^^^^^^^ the trait `IntoPropValue<ChildrenRenderer<VNode>>` is not implemented for `&implicit_clone::unsync::IString` | note: required by a bound in `yewprint::TextPropsBuilder::children` --> /home/cecile/repos/yewprint/src/text.rs:4:28 | 4 | #[derive(Clone, PartialEq, Properties)] | ^^^^^^^^^^ required by this bound in `TextPropsBuilder::children` ... 9 | pub children: Children, | -------- required by a bound in this associated function = note: this error originates in the derive macro `Properties` (in Nightly builds, run with -Z macro-backtrace for more info) help: consider dereferencing here | 15 | {&*props.text} | + help: consider removing the leading `&`-reference | 15 - {&props.text} 15 + {props.text} | For more information about this error, try `rustc --explain E0277`. error: could not compile `yewprint-doc` (lib) due to previous errorI had to do the following change:
diff --git a/yewprint-doc/src/text/example.rs b/yewprint-doc/src/text/example.rs index ed83f1c8..eb2f7a67 100644 --- a/yewprint-doc/src/text/example.rs +++ b/yewprint-doc/src/text/example.rs @@ -12,7 +12,7 @@ pub fn example(props: &ExampleProps) -> Html { html! { <div style="width: 150px; height: 20px"> <Text ellipsize={props.ellipsize}> - {&props.text} + {&*props.text} </Text> </div> }I'm quite unhappy (but not too much) with this change because I think AttrValue should be handled in a very transparent way by Yew.
-
Things that implement ToString/Display cannot be converted to Html directly:
error[E0277]: the trait bound `sv::Value<Ratio>: ToHtml` is not satisfied --> xxxxxxxxxx | 78 | / html! { 79 | | <div class={classes!("value")}> 80 | | {setback} | | ^^^^^^^ the trait `ToHtml` is not implemented for `sv::Value<Ratio>` 81 | | </div> 82 | | }, | |_________________________- required by a bound introduced by this call | = help: the following other types implement trait `ToHtml`: bool char isize i8 i16 i32 i64 i128 and 26 others = note: required for `sv::Value<Ratio>` to implement `yew::html::IntoPropValue<VNode>`I had to do this change:
diff --git a/xxxxx b/xxxxxxxx index 9f7128b..14413d7 100644 --- a/xxxxxxx +++ b/xxxxxx @@ -77,7 +77,7 @@ impl Component for Config { }, html! { <div class={classes!("value")}> - {setback} + {setback.to_string()} </div> }, )I never had to implement anything on sv::Value (a struct of mine) itself, this code used to work in yew 0.20. My guess is that it was using the ToString/Display trait to convert my struct sv::Value to html.
Though I must say I'm not unhappy with this change because the conversion to string should be quite intentional. The user should realize that it is something that will require allocation.
-
I inspected the code around the conversion in Yew and it looks really wrong:
macro_rules! impl_to_html_via_display { ($from_ty: ty) => { impl ToHtml for $from_ty { #[inline(always)] fn to_html(&self) -> Html { Html::VText(VText::from(self)) } } // Mirror ToHtml to Children implementation. impl IntoPropValue<ChildrenRenderer<VNode>> for $from_ty { #[inline(always)] fn into_prop_value(self) -> ChildrenRenderer<VNode> { ChildrenRenderer::new(vec![VText::from(self).into()]) } } }; } // These are a selection of types implemented via display. impl_to_html_via_display!(bool); impl_to_html_via_display!(char); impl_to_html_via_display!(String); impl_to_html_via_display!(&str); impl_to_html_via_display!(Rc<str>); impl_to_html_via_display!(Rc<String>); impl_to_html_via_display!(Arc<str>); impl_to_html_via_display!(Arc<String>); impl_to_html_via_display!(AttrValue); // ...And in VText:
impl<T: ToString> From<T> for VText { fn from(value: T) -> Self { VText::new(value.to_string()) } }This means we are allocating a String to convert AttrValue to String and then to VText :woman_facepalming: that is not efficient.
@hamza1311 @futursolo if you don't mind I'm re-opening this ticket until OP and others are happy or accept the change of behavior. It looks like something needs to be done: either Yew needs to be updated OR our community see and understand why we change things.
(It's not for me, I am not personally much affected by the change of Yew 0.21. The change 1 is a minor annoyance for me, change 2 I think it's actually better to explicitly write down to_string (because it's more explicit) but I don't mind if ToString/Display works again. The change 3 is annoying for me BUT it is unrelated to this issue)
[edit: lots of formatting issue in the post haha xD]
I agree with @cecton here. We should do something to mitigate the issues here
We need some blanket implementations that are missing. I'm not sure if we can add them since specialization doesn't exist:
impl<T: Into<VNode>> ToHtml for T {}
Perhaps even removing ChildrenRenderer outright. I don't see where it's needed. Are there any use cases for it?
Or like this:
if !props.children.is_empty() { }
Should we add a method .is_empty() on VNode? And also an impl Iterator?
If the VNode is a VList it would be a proxy but if it's anything else it will return true (as not empty) and a single element if using the iterator?
(Also we can yield clones of Html with the iterator since VNode will be cheap to clone. But I guess it's best to include that in the #3431 PR)
Should we add a method .is_empty() on VNode?
I do not think we should be inspecting a VNode. I do not think delivers a very good schematics to users when it comes to some layouts.
e.g.:
For the following layout, should is_empty() return true or false?
html!{
<>
<></>
</>
}