yewstack / yew-autoprops

Apache License 2.0
8 stars 1 forks source link

Merge port of yew-autoprops in Yew to yew-autoprops #10

Closed cecton closed 10 months ago

cecton commented 11 months ago

Fixes #8 Fixes #7 Fixes #9 Fixes #3 Related to https://github.com/yewstack/yew/pull/3505

cecton commented 11 months ago

cc @kirillsemyonkin

kirillsemyonkin commented 11 months ago

image

Seems like its generating an unexpected comma somewhere (be it struct or fn) when there are no props but there are generic types

kirillsemyonkin commented 11 months ago

Repro:

#[autoprops]
#[function_component]
pub fn Test<T>() -> Html {
    html! {}
}
cecton commented 11 months ago

@kirillsemyonkin fixed

kirillsemyonkin commented 11 months ago

I think to trigger CI you may wanted to rerun all jobs, sometimes I see that button in the actions section image

cecton commented 11 months ago

No I really wanted to trigger the CI with a different commit hash ^_^

cecton commented 11 months ago

TIL that you need to add the rust version in swatinem/cache key for it to work properly :woman_facepalming: I have like 10 repos that are not doing that huh

kirillsemyonkin commented 11 months ago

image

Here is another one

cecton commented 11 months ago

Fixed

kirillsemyonkin commented 11 months ago
  1. Autoprops does not require -> Html on the fn, nor HtmlResult works (seems like that is replaced by Html) - following does not work:
#[autoprops]
#[function_component]
pub fn Test() -> HtmlResult {
    Ok(html! {})
}
  1. Defaults for fn generic type params are ignored by Rust, here are three cases (doesnt work, good; works, good; doesnt work, oops):

image image image

cecton commented 11 months ago

@kirillsemyonkin fixed

cecton commented 11 months ago

@hamza1311 @kirillsemyonkin ok I think this is ready now

kirillsemyonkin commented 11 months ago

What is the current status of this?

cecton commented 11 months ago

@yewstack/yew can I have a final review on this?

Madoshakalaka commented 10 months ago

@cecton hi you are my angel, thanks for the work

kirillsemyonkin commented 10 months ago

@yewstack/yew can I have a final review on this?

Can we skip such a review since it is more-or-less separate from Yew and proceed to merge? We can have extra merges later, independent of Yew's slow cycle

kirillsemyonkin commented 10 months ago

Ideally anyone's work on a stylist PR for #[styled_component] -> #[styled] (for #[styled] #[autoprops] #[component] progress) should happen somewhere around now as well

cecton commented 10 months ago

Can we skip such a review since it is more-or-less separate from Yew and proceed to merge? We can have extra merges later, independent of Yew's slow cycle

Yeah I think the other maintainers are a bit overbooked at the moment so let's merge it as is. Worst case scenario we open issues or fix things later on.