Add for-loops to `html!`

[its-the-shrimp]

Description

1. Adds the following syntax:

html! {
  for x in 0 .. 10 {
    <span>{x}</span>
  }
}

2. Disallows top-level {for x}, i.e. the following is no longer valid:

html! { for x }

To disambiguate it and the new for loops, it'll be required to wrap the expression in braces like so:

html! { {for x} }

Checklist

• [x] I have reviewed my own code
• [x] I have added tests

[futursolo]

Thanks for the pull request, however, I do not think an ordinary for-loop should be supported.

What would happen for the following expression?

html! {
  for x in 0..10 {
    <span>{break x}</span>
  }
}

[github-actions[bot]]

Benchmark - core

Yew Master

vnode           fastest       │ slowest       │ median        │ mean          │ samples │ iters
╰─ vnode_clone  2.929 ns      │ 5.538 ns      │ 2.936 ns      │ 3 ns          │ 100     │ 1000000000

Pull Request

vnode           fastest       │ slowest       │ median        │ mean          │ samples │ iters
╰─ vnode_clone  2.47 ns       │ 5.305 ns      │ 2.473 ns      │ 2.519 ns      │ 100     │ 1000000000

[github-actions[bot]]

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

https://yew-rs--pr3498-add-html-for-jj737upv.web.app

_{(expires Mon, 11 Dec 2023 15:29:52 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

[github-actions[bot]]

Benchmark - SSR

Yew Master

Benchmark	Round	Min (ms)	Max (ms)	Mean (ms)	Standard Deviation
Baseline	10	289.252	289.735	289.524	0.156
Hello World	10	485.611	488.605	486.709	0.999
Function Router	10	1623.898	1639.146	1629.502	5.340
Concurrent Task	10	1005.412	1008.659	1006.536	0.850
Many Providers	10	1137.893	1163.400	1150.507	8.630

Pull Request

Benchmark	Round	Min (ms)	Max (ms)	Mean (ms)	Standard Deviation
Baseline	10	308.931	309.936	309.327	0.341
Hello World	10	519.053	540.152	526.954	6.759
Function Router	10	1634.606	1659.121	1644.871	6.844
Concurrent Task	10	1005.404	1006.981	1006.500	0.507
Many Providers	10	1131.666	1218.810	1157.161	24.660

[github-actions[bot]]

Size Comparison

examples	master (KB)	pull request (KB)	diff (KB)	diff (%)
async_clock	100.229	100.229	0	0.000%
boids	173.670	173.693	+0.023	+0.013%
communication_child_to_parent	92.752	92.752	0	0.000%
communication_grandchild_with_grandparent	105.754	105.754	0	0.000%
communication_grandparent_to_grandchild	101.023	101.023	0	0.000%
communication_parent_to_child	89.076	89.076	0	0.000%
contexts	106.004	106.004	0	0.000%
counter	86.129	86.129	0	0.000%
counter_functional	86.516	86.535	+0.020	+0.023%
dyn_create_destroy_apps	88.986	89.026	+0.040	+0.045%
file_upload	99.996	99.996	0	0.000%
function_memory_game	172.398	172.398	0	0.000%
function_router	350.001	349.994	-0.007	-0.002%
function_todomvc	161.170	161.170	0	0.000%
futures	229.413	229.440	+0.027	+0.012%
game_of_life	110.176	110.176	0	0.000%
immutable	185.443	185.505	+0.062	+0.033%
inner_html	79.899	79.899	0	0.000%
js_callback	109.499	109.556	+0.057	+0.052%
keyed_list	199.527	199.571	+0.044	+0.022%
mount_point	82.791	82.791	0	0.000%
nested_list	113.886	113.886	0	0.000%
node_refs	90.325	90.325	0	0.000%
password_strength	1750.628	1750.628	0	0.000%
portals	93.464	93.500	+0.036	+0.039%
router	318.834	318.229	-0.604	-0.190%
simple_ssr	140.306	140.306	0	0.000%
ssr_router	387.204	387.197	-0.007	-0.002%
suspense	115.724	115.724	0	0.000%
timer	88.608	88.629	+0.021	+0.023%
timer_functional	97.979	97.961	-0.018	-0.018%
todomvc	141.339	141.339	0	0.000%
two_apps	85.830	85.830	0	0.000%
web_worker_fib	134.757	134.729	-0.027	-0.020%
web_worker_prime	184.968	185.004	+0.036	+0.020%
webgl	82.508	82.508	0	0.000%

✅ None of the examples has changed their size significantly.

[its-the-shrimp]

Fixed it, now an error is raised when trying to break out of a for loop or continue it

[futursolo]

If we block break and continue, then code like the following will no longer work, which should be allowed.

#[function_component]
fn Comp() -> Html {
    html! {
        for i in 0..5 {
            <div>
                {{
                    loop {
                        let a = rand_number();
                        if a % 2 == 0 {
                            break a;
                        }
                    }
                }}
            </div>
        }
    }
}

I am fine with asking users to write the following:

let children = (0..5).map(
    html! { <span key={x}>{x}</span> }
);

html! { for x }

There are 2 reasons:

1. As mentioned in #3466, we want to discourage users to write rust code in html! as much as possible. I think this also extends to complex html expressions.
2. Elements generated in the for-loop requires elements to be keyed, this further complicates the keying requirements. If you think the current keying requirement is peculiar (as mentioned in #3480). We should not add more rules to it.

The current key requirement can be summarised in 1 sentence:

Everything defined in html! is keyless-safe, with the exception of the top-most ancestor element if it is created from an iterator.

(I consider current {for expr} as a bug.)

[ranile]

I'm fine with supporting loops. This was discussed on discord as well.

I agree that breaks and continues can be problematic but we could disallow those. These loops can be a for-loop over an iterator that always yields a VNode. This allows us to also lint for presence of keys at compile time.

(I consider current {for expr} as a bug.)

I also agree, however it's not possible to impl Into<Html> for any Iterator type that can be collected into an Html, so it's kinda a necessary evil

[futursolo]

I also agree, however it's not possible to impl Into<Html> for any Iterator type that can be collected into an Html, so it's kinda a necessary evil

I am specifically referring to the {for expr} rendering html! key required. Each {...} should result in exactly 1 expression and not many.

{for expr} can be fixed by casting a VList at the top of the expression and not expanding it to the parent VList.

[futursolo]

This allows us to also lint for presence of keys at compile time.

html! {
    for i in fragments {
        {i.to_html()}
    }
}

I think it would be kind of difficult to lint this at compile time.

[its-the-shrimp]

we want to discourage users to write rust code in html! as much as possible. I think this also extends to complex html expressions

Which is why we can just not care about the peculiar case of nested loops shown above, it'll be rejected anyway.

Elements generated in the for-loop requires elements to be keyed

Not necessarily, unlike {for x}, the new for loops expand into a VList and are not inlined into the parent which discards the problem with keys.

Another argument that can be made in favour of the new for-loops are the possible optimisations and checks, of course one can always just

{for iter.into_iter().map(|x| html!{...})}

Yet, even ignoring how unreadable it can get, which is subjective in a way, calling html! inside html! makes the context unclear, I'll try to add more optimisations & checks to the new for-loops to demonstrate this

[futursolo]

Which is why we can just not care about the peculiar case of nested loops shown above, it'll be rejected anyway.

We need to ensure that our implementation is sound. If it is a valid Rust expression, we should accept it. If we wish to reject complex expressions, it needs to be rejected based on complexity and not a limitation introduced by blocking expressions.

Not necessarily, unlike {for x}, the new for loops expand into a VList and are not inlined into the parent which discards the problem with keys.

If you reconcile a for-generated VList of 4 items into 5 items, the renderer will not understand which one you are trying to remove. (That's why you need to key the iterator expression.)

The keying requirement always presents for VList items with a non-fixed length.

The syntax proposed in this pull request (for i in 0..10 { <span key={i}>{i}</span> }) is actually a syntax sugar for (0..10).map(|i| html! {<span key={i}>{i}</span>}). So everything that applies to an iterator also applies to for-loop.

You can read the React documentation, which explains the keying requirement for Lists in detail: https://react.dev/learn/rendering-lists

The bug for {for expr} is because it renders its adjacent elements to be non keyless-safe as it expands into a parent VList and not items yielded by expr needs keying.

Another argument that can be made in favour of the new for-loops are the possible optimisations and checks, of course one can always just

{for iter.into_iter().map(|x| html!{...})}

Optimisation is also possible for iterators-based implementations with helpers like size_hint(). I do not recommend users do this for readability, but that is also kinda a necessary evil. However, what is in our control is to simplify and ensure soundness of the html syntax, so we do become part of that evilness.

[its-the-shrimp]

Added a check to outlaw the following cases:

html!{
  for i in 0 .. 10 {
    <div key="duplicate" />
  }
}

Outlaws the most obvious cases of incorrect keying: when a key is a complex path (i.e. smth::VALUE) or a literal.

Also added an optimisation which allows for avoiding calling recheck_fully_keyed when creating a VList from a for-loop.

Regarding break & continue, I chose a dumb but durable approach of compiling the loop to a call to for_each so that any attempt to break the loop will raise a "break in a closure" error. Not the most straightforward approach but a bug-free one, an alternative would be to basically parse all the expressions by ourselves and run complex AST visitors on them.

[its-the-shrimp]

There's some problem with the benchmarks again, some file missing...