yew-macro: Allow omitting redundant property names

[Follpvosten]

Description

I'm submitting a feature request:

Going along with #745, this would be a very nice usability improvement (as also mentioned in #756).

For example, this:

let placeholder = if something { "Something" } else { "Something else" };
let maxlength = if blah { 7 } else { 10 };
html! {
    <input type="text" /* ... */ placeholder=placeholder maxlength=maxlength />
}

Could be written like this:

let placeholder = if something { "Something" } else { "Something else" };
let maxlength = if blah { 7 } else { 10 };
html! {
    <input type="text" /* ... */ placeholder maxlength />
}

This is getting rid of a lot of redundance in the long run by allowing the developer to omit the =property when a variable with the property's name is in scope.

[jplatte]

This conflicts with attributes that don't need a value like hidden. I don't know how yew currently handles those, but personally I don't like the syntax you propose for implicitly using a variable as the value for some HTML attribute, because of this conflict.

What about instead of

<input type="text" /* ... */ placeholder maxlength />

this could be expressed as

<input type="text" /* ... */ placeholder= maxlength= />

in the style of Python 3.8's = in f-strings? This might look a little alien on first sight, but is not ambiguous.

[Follpvosten]

@jplatte Yes, after thinking about it for a while, this makes sense. #745 is the one that matches the usual HTML API, but you're right, it only makes sense for booleans to do it the exact same way.

I like the x => x= idea.

[samuelvanderwaal]

I'm pretty junior but would be willing to take a stab at this if someone is available to review it and point me in the right direction.

Looks like the HtmlProp parsing is defined here: https://github.com/yewstack/yew/blob/master/crates/macro/src/html_tree/html_prop.rs#L23. Is this going to be the right place to implement this feature?

[jstarry]

@samuelvanderwaal yup, that's the place!

[samuelvanderwaal]

Ok, I have taken a look at this and have some ideas. The most straight-forward approach seems to be to copy the input ParseStream and turn the parsed Expr from a result into an Option, something like this:

impl Parse for HtmlProp {
    fn parse(input: ParseStream) -> ParseResult<Self> {
        let fork_input = input.clone();
        let label = input.parse::<HtmlPropLabel>()?;
        input.parse::<Token![=]>()?;
        let value_opt = input.parse::<Expr>().ok();
        let value: Expr = match value_opt {
            Some(expr) => expr,
            None => fork_input.parse::<Expr>()?,
        };
        // backwards compat
        let _ = input.parse::<Token![,]>();
        Ok(HtmlProp { label, value })
    }
}

I am not sure if using clone() is expensive in this context but if so, another approach is to use lookahead and/or possibly cursor to try to determine if we get an expression or not after the = token.

Let me know if I'm going in the right direction here or if I need to rethink my approach.

[jstarry]

I am not sure if using clone() is expensive in this context

clone() is fine. Performance is not a huge concern in the macro code, worst it can do is slow down builds slightly.

I think we have everything we need from cloning label though. Check out HtmlDashedName. We could implement TryInto<Expr> for HtmlDashedName and create an Expr from the Ident that looks like this:

            Path(
                ExprPath {
                    attrs: [],
                    qself: None,
                    path: Path {
                        leading_colon: None,
                        segments: [
                            PathSegment {
                                ident: <IDENT HERE>,
                                arguments: None,
                            },
                        ],
                    },
                },
            )

[samuelvanderwaal]

This is great, thanks! I should be able to work on it tomorrow after the day job, or this weekend.

[jplatte]

create an Expr from the Ident that looks like this: [...]

One could also use syn::parse_quote(#ident) instead.

[jstarry]

One could also use syn::parse_quote(#ident) instead.

Great suggestion, wasn't aware of that macro!

[samuelvanderwaal]

Using syn::parse_quote() is nice but using it I'm not sure that we need TryInto instead of just Into. If we're using the name field of HtmlDashedName it will always be a valid Ident so the macro should never panic. So it seems like this should work:

// html_dashed_name.rs

. . .

#[derive(Clone, PartialEq)]
pub struct HtmlDashedName {
    pub name: Ident,
    pub extended: Vec<(Token![-], Ident)>,
}

. . .

impl Into<Expr> for HtmlDashedName {
    fn into(self) -> Expr {
        parse_quote!(self.name)
    }
}

// html_prop.rs

. . .

impl Parse for HtmlProp {
    fn parse(input: ParseStream) -> ParseResult<Self> {
        let label = input.parse::<HtmlPropLabel>()?;
        input.parse::<Token![=]>()?;
        let value: Expr = match input.parse::<Expr>().ok() {
            Some(expr) => expr,
            None => label.clone().into(),
        };
        // backwards compat
        let _ = input.parse::<Token![,]>();
        Ok(HtmlProp { label, value })
    }
}

. . .

This compiles but I haven't set up any specific tests for it yet.

[jplatte]

That doesn't do what you want. It has to be

let name = & self.name;
parse_quote!(#name)

Otherwise you're converting every HtmlDashedName to literally the expression self.name.

[jplatte]

But you're right about the conversion being infallible, so it makes sense to impl From or Into rather than one of the Trys.

[samuelvanderwaal]

That doesn't do what you want. It has to be

let name = & self.name;
parse_quote!(#name)

Otherwise you're converting every HtmlDashedName to literally the expression self.name.

Ah, after reading more about the quote! macro, using the #var syntax makes sense. Thanks!

[samuelvanderwaal]

@jstarry No rush, but when you get a chance, let me know if you like this solution and how you'd like me to approach testing it before putting in a PR.

[jstarry]

@samuelvanderwaal I prefer to iterate on PRs and I think this is ready to start work!

I'm admittedly a bit unsure of the syntax still. Maybe we could add a feature flag for it for now? We could pass the feature through yew to yew-macro. Something like "unstable/prop-shorthand"? @jplatte @Follpvosten would that work for you?

And I would want this syntax sugar to work for both "tags" and "components".

[Follpvosten]

@jstarry I'm open to a feature flag; right now, this doesn't apply to me as much as it used to, mostly because I've started to like using semantically sensible names for my callbacks (like onclick=do_something), but it'll still be useful for other stuff.

[jplatte]

@jstarry im happy to try this out by exchanging the crates.io dependency by a git dependency and/or add a cargo feature.

[jstarry]

Cool, thanks. Let's go with a feature then for now

[intendednull]

I like this! Would be very nice to have.

Imo it makes most sense to follow struct shorthand:

 <input /* ... */ placeholder maxlength />

I'm not really a fan of implicit values like hidden. Implementing them seems a bit unnecessary, just to avoid writing hidden=true (which reads better anyways).

[mc1098]

This was fixed by #1970 🎉
I think this can be closed :)