Add the ability to add child nodes conditionally in html!

[cecton]

Description

Add the ability to add child nodes conditionally in the html! macro.

    let boolean = true;
    html! { if boolean { html!() } };
    html! { if boolean { html!() } else { html!() } };
    html! { <div>if boolean { html!() }</div> };
    html! { <div>if boolean { html!() } else { html!() }</div> };

    let option = Some("text");
    html! { if let Some(text) = option { html!( {text} ) } };
    html! { if let Some(text) = option { html!( {text} ) } else { html!() } };
    html! { <div>if let Some(text) = option { html!( {text} ) }</div> };
    html! { <div>if let Some(text) = option { html!( {text} ) } else { html!() }</div> };

Alternative (not in the current commit):

    let boolean = true;
    html! { if boolean { html!() } };
    html! { if boolean { html!() } else { html!() } };
    html! { <div>{if boolean { html!() }}</div> };
    html! { <div>{if boolean { html!() } else { html!() }}</div> };

    let option = Some("text");
    html! { if let Some(text) = option { html!( {text} ) } };
    html! { if let Some(text) = option { html!( {text} ) } else { html!() } };
    html! { <div>{if let Some(text) = option { html!( {text} ) }}</div> };
    html! { <div>{if let Some(text) = option { html!( {text} ) } else { html!() }}</div> };

When I first started to use yew I tried to do something like this:

html! {
    <div>
        { // I tried with and without the {
            if condition {
                html!(...)
            }
        }
    </div>
}

Then I switched to something like this:

html! {
    <div>
        {
            if condition {
                html!(...)
            } else {
                Default::default() // or html!()
            }
        }
    </div>
}

Question: why is for implemented inside the { }? (i.e. <div>{ for first.map(display) }</div>

Wait I think I know why... It would parse the closing tag as part of the expression, right?

But this would have been possible right? <div>for { first.map(display) }</div>

I'm asking that question because the inconsistency I'm introducing in this PR between the for syntax and the if syntax bothers me. I can revert to the alternative implementation without problem (I believe).

Checklist

• I have run cargo make pr-flow
• I have reviewed my own code
• I have added tests

[cecton]

I did it again, I AM SORRY!

• It started as a simple feature (html_if)
• Then I realized the new feature syntax was inconsistent with an existing feature (html_iterable): if ... { ... } vs {for ...} (notice the braces)
• So I checked if I could make a new feature for {...} to make it more consistent and with the intent to deprecate {for ...}
• Then I realized I don't need to add html_if to the HtmlRoot parser because the job is done by HtmlTree already
• Then it feels like a refactoring because HtmlTree can now handle if and for syntax, no need to use HtmlRoot anymore. On top of that, no need to have an impl ToNodeIterator for either of them because that was use only when in a html_block

I think it is great overall but let me know what you think before I tidy it up. It's okay to disagree and I won't be mad even if we would discard this PR. I think it is important to have good points on the table and debate.

My points:

• I started using yew a few weeks ago and the if pattern in the macro was one of the very first thing I stumbled upon and it was a pain point.
• In term of semantic I see no reason why we shouldn't allow keyword like that directly in the macro. In React there is a big reason: free strings can be inserted. Here with yew's macro this is forbidden, you need to wrap it around {"..."}. So there is no collision, we could even add more keywords.
• Note that I can make the if inside the block syntax ({if ... { ... }}), it works too. It has the advantage of not breaking the consistency with the existing {for ...} syntax. I like that too but I prefer to have those outside the block. My reason is: inside a block it should be normal code, that's how I expect things work and that is why I'm not a big fan of the existing {for ...} syntax.

[mkawalec]

I like the idea very much, thank you for this implementation :) Looks good to me, but I haven't touched the vdom generator myself, so I'll leave it to the people familiar with that code to decide.

[cecton]

I think I could also allow literals to be written directly in the macro like this:

html! {
    <label>"Hello"</label>
}

(Notice the absence of {} around "Hello")

[siku2]

Sorry it took so long to get to this. If this had been an issue I would've picked up the discussion much earlier but since you still had another PR open I wanted to get that one done first.

I think I could also allow literals to be written directly in the macro like this:

html! {
    <label>"Hello"</label>
}

(Notice the absence of {} around "Hello")

See #757. Personally, I wouldn't mind allowing string literals directly but if you want to push that, please do so over there.

---

Now let's get on to the PR. I absolutely agree with your points. Conditionals and loops are an essential feature for ergonomic code. I'm glad that you took the initiative to start the discussion on this (even though you didn't bother opening an issue again :P).

The reason we support this weird for keyword in the block is as a workaround for the lack of specialization. It isn't meant as a replacement for a real for loop (that's why there's no if equivalent). Because the crab hasn't blessed us with specialization yet, we can't just allow types that implements IntoIterator and always collecting the iterators into a vector isn't great either.
It probably would've been better if we'd called it iter instead because it's nothing like a for loop, it's just changes the type the block accepts.

Now, I like the syntax you suggested but I want to make it more flexible. If we're adding this dedicated syntax, it should be usable without having to use html! again:

html! {
    <div>
        if let Some(key) = &props.key {
            <span>{ "KEY: " }</span>
            <p>{ key }</p>
        }
    </div>
}

Of course it should still support Rust code:

html! {
    <div>
        if self.request.has_finished() {
            let resp = self.request.read_response();
            let status = resp.status();
            html! { <span>{ status }</span> }
        }
    </div>
}

Luckily for us, this is fairly easy to accomplish because the html! macro already does this somewhat. At the root of the html! you can either use a code block (html! { "le block" }) or an HTML tree (html! { <a></a> }). So basically we just have to treat the block like an implicit html! invocation if xyz html! { // user code here } and we're there.

What's great about this is that it also removes the need for html_nested! in a lot of cases:

html! {
    <OnlyAcceptsUser>
        // works without the need for html_nested! because the type of the component is preserved
        for user in &props.users {
            <User key=user.id user=user />
        }
    </OnlyAcceptsUser>
}

I also want to make sure that we support all these expressions:

• if (also support else and else if)
• for
• switch - very useful for component state machines (ex. fetch request state: loading, ok, error)

The only one you didn't already add is the switch expression but I think that's the most exciting one!
I don't think the while expression makes sense but feel free to convince me otherwise.

TL;DR I think we're 90% there with the concept, I just want to make sure we push it to its full potential.

[cecton]

Sorry it took so long to get to this. If this had been an issue I would've picked up the discussion much earlier but since you still had another PR open I wanted to get that one done first.

No problem.

(even though you didn't bother opening an issue again :P)

Wait... I think I understand now. We see PR and issues differently. For you an issue is not a PR (very much like GitLab) and you like to discuss the specs on an issue (not a PR). While for me, PR and issue are actually the same, I just like to experiment a bit and give a concrete example to a long explanation of what I have in mind (example here where my idea turns out differently when I've put it in practice).

But ok. Next time I will create an issue first. Sorry for that 🙏

See #757. Personally, I wouldn't mind allowing string literals directly but if you want to push that, please do so over there.

Sure. I haven't started the code really. This is just a draft of what it could be and I didn't do the literals. I will make another PR for the literals and link it to that specific issue.

Now, I like the syntax you suggested but I want to make it more flexible. If we're adding this dedicated syntax, it should be usable without having to use html! again:

html! {
    <div>
        if let Some(key) = &props.key {
            <span>{ "KEY: " }</span>
            <p>{ key }</p>
        }
    </div>
}

1. I guess I can do that but it will be more complicated than it currently is.

At the moment I'm using the "if-expression" parser documented here which is very handy. To achieve what you ask, I will need to split this out by parsing the token "if" separately, then the expression, then a block, then else, then a block again. It won't be as trivial as it is right now unless I'm missing something.

In other words: I think it is simpler, it has less code and less maintenance.

2. Now that you have that in mind, I want to go back to what I said here:

inside a block it should be normal code, that's how I expect things work and that is why I'm not a big fan of the existing {for ...} syntax.

Following the same reasoning I must say I'm not a big fan of the syntax you are proposing here because it makes the {...} not a normal block but a block-ish thingy which feels a little bit magic to me to use because it can operates as a block of code or as a nested html!.

But again I don't have a strong opinion about this. Just letting you know I personally find it not intuitive. Maybe other people had a different experience when they used yew the first time.

I think in React this syntax makes more sense because you don't prefix the JSX with a macro. JSX just "appears" there and we're kinda use to it. In rust we do expect macros to start with "some_name!". Macros can have blocks of code ($some_code:block) or expressions ($some_expr:expr) this is why the following syntax makes sense to me:

html! {
    <div class=any_expression_i_can_think_of_which_evaluates_to_string>
        {
            // any valid Rust code
            let x = 42;
            any_normal_code_I_can_think_of();
            // must ends with html!() or a VNode (but not a literal or a String)
        }
    </div>
}

And I got really confused by this:

html! {
    <div>
        {"Arbitrary literal"}
        // I can't end a block by a literal but 
        // if I write a literal alone in a block it's ok
    </div>

I also want to make sure that we support all these expressions:

• if (also support else and else if)
• for
• switch - very useful for component state machines (ex. fetch request state: loading, ok, error)

Surely you mean "match" and not "switch"?

I agree it would be nice to have them all. Note that I believe the current implementation already supports the else if. (I'm not 100% sure, I think it does.)

I don't think the while expression makes sense but feel free to convince me otherwise.

It doesn't make sense to me either. I can't find a use case. But I do find a use case for the match/switch (for the router for example).

TL;DR I think we're 90% there with the concept, I just want to make sure we push it to its full potential.

I'm not completely there yet. But almost.

After #1626 I've been thinking a lot about these blocks (as I explained above already) and I did realize that this:

html! {
    <div>
        {"Some literal"}
    </div>
}

Could be coded as a normal case of Transformer literal -> VNvode instead of being a specific case in the macro. In other words: if we allow a default transformer from string literal to VNode, we could maybe remove the specialized code here.

I also see a lot of value about adding a Transformer String -> VNode as simple formatter could be written as this:

html! {
    <div class="greetings">
        {format!("Hello {}!", name)}
    </div>
}

(Maybe something like Transformer impl Display -> VNode would be better but I'm not sure if that's possible.)

Let me know your opinion and in any case I'm totally fine implementing what you asked. I just feel I still have this experience of "first time using yew" which can be valuable to evaluate what feels or feels not rust-intuitive. But maybe other people will have a different idea on this concern about blocks in html!().

[siku2]

In other words: I think it is simpler, it has less code and less maintenance.

But it's also much less useful. Currently this is just a way to avoid writing else { html!{} }.
And while that's certainly a step in the right direction it isn't big enough to warrant the introduction of new syntax for.

Surely you mean "match" and not "switch"?

Yes, of course, sorry! I had the displeasure of working entirely with C last week.

Following the same reasoning I must say I'm not a big fan of the syntax you are proposing here because it makes the {...} not a normal block but a block-ish thingy which feels a little bit magic to me to use because it can operates as a block of code or as a nested html!.

I can see where you're coming from but the html! macro already works like that, it's not like we would be treading new ground here.
Also, we want to discourage the user from writing Rust code within the html! macro. Not because there's anything wrong with it per se, but because it messes with Clippy and disables Rustfmt entirely. Currently, the more code outside of a macro the better.

Instead of thinking about it like we're somehow allowing both, think about it like we're only allowing an html tree.
We can still use Rust expressions inside of a nested block anyway:

html! {
  if test {
    { rust_expression() }
  }
}

but now we just remove the requirement for the double braces {{}} for convenience. That's exactly the reason why the html! macro behaves differently at the root, because it already has the {} block as part of the macro invocation.

So here's a quick rundown of the benefits of allowing an html tree inside of the block:

• makes html_nested! redundant in many cases. html_nested! is a confusing and ugly hack.
• practically no boilerplate code
• faster compilation because we don't have nested macros
• no need to place surrounding fragments in case we're dealing with multiple elements
• encourages users to place logic outside of the macro but is still flexible enough to allow it where it makes sense
• from what I've heard many new users are confused by the nested macro, it feels more natural to

compared to the downsides:

• more macro code
• normal blocks don't behave the same way as blocks following if, for, or match. This isn't as bad as it sounds though because Rust doesn't exclusively use blocks for statements either.

If you can think of another way to achieve the same flexibility, I'm all ears. The only thing I care about is that it doesn't require a nested html! invocation.

---

Could be coded as a normal case of Transformer literal -> VNvode instead of being a specific case in the macro. In other words: if we allow a default transformer from string literal to VNode, we could maybe remove the specialized code here.

Just to get you on the same page here. We're very likely going to remove the transformer logic entirely because it's a confusing mess. I'm not perfectly happy with how {} blocks are handled but in my opinion the solution isn't to make things more complicated. At the end of the day we don't want to work against Rust. We should try to keep things as Rusty as possible
When specialization lands we can think more about these things again, but for now Rust really pushes us towards explicit solutions (for good reasons too!) and all this hackery turns into a mess sooner or later.

I would love to discuss this more if you want, feel free to open a new issue to discuss this.

I just feel I still have this experience of "first time using yew" which can be valuable to evaluate what feels or feels not rust-intuitive. But maybe other people will have a different idea on this concern about blocks in html!().

Definitely appreciate that. Btw, in my experience other users are much more likely to chime in on issues rather than PRs :P

[cecton]

Also, we want to discourage the user from writing Rust code within the html! macro. Not because there's anything wrong with it per se, but because it messes with Clippy and disables Rustfmt entirely. Currently, the more code outside of a macro the better.

A valid point 🤔 I also have this issue on yewprint. I was thinking about writing a rustfmt expressely for formatting the yew's html! content. I will do as you say. I'm not 100% convinced but I think this was a very good argument.

but now we just remove the requirement for the double braces {{}} for convenience. That's exactly the reason why the html! macro behaves differently at the root, because it already has the {} block as part of the macro invocation.

tbh I do see this as an anomaly. Those brackets are for delimiting the macro. They can even be () instead of {}.

makes html_nested! redundant in many cases. html_nested! is a confusing and ugly hack.

I've never used it either. I think I didn't understand it. So I agree it's an upgrade to remove it.

normal blocks don't behave the same way as blocks following if, for, or match. This isn't as bad as it sounds though because Rust doesn't exclusively use blocks for statements either.

Rust does use {} exclusively as block for if, for and in match's arms.

Definitely appreciate that. Btw, in my experience other users are much more likely to chime in on issues rather than PRs :P

I guess I'm not normal....

[mc1098]

I like this change especially after playing around with it :D
I agree with the discussions that it does make {for ...} syntax feel awkward (but that's for another issue/PR).

I also want to make sure that we support all these expressions:

• if (also support else and else if)
• for
• switch - very useful for component state machines (ex. fetch request state: loading, ok, error)

Did you mean that before this can be merged all these have to be supported? or just that in the future they should be?
I think this PR will cover the if/else/else if/ if let/ part quite well, I would prefer another issue/PR for match (or switch for the C using Ferris traitors :P)

[mc1098]

Should be expecting the condition here.
The {} is being parsed because it is of type syn::Expr and then you run out of tokens which trips your first input.is_empty() check.
I think most users would realise what the error was about, but I still don't think it's correct.

[voidpumpkin]

@hamza1311 Could you add a short paragraph about this change somewhere under https://yew.rs/next/concepts/html ?

[github-actions]

Visit the preview URL for this PR (updated for commit f0e6091):

https://yew-rs--pr1609-html-if-macro-rrd1p0yv.web.app

_{(expires Mon, 29 Nov 2021 16:01:00 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

[voidpumpkin]

LGTM

[mc1098]

#1609 (comment)
I think this comment still applies but I'll admit it is being very nit-picky 😈

If you are curious Rust will spit out:

error: missing condition for `if` expression