Add for-loops to `html!` #3498

schvv31n · 2023-10-29T00:10:44Z

Description

Adds the following syntax:

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

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

I have reviewed my own code
I have added tests

futursolo · 2023-10-29T02:06:31Z

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 · 2023-10-29T10:25:24Z

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 · 2023-10-29T10:25:28Z

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 · 2023-10-29T10:30:08Z

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 · 2023-10-29T10:31:00Z

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.

schvv31n · 2023-10-29T11:22:18Z

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

futursolo · 2023-10-29T11:59:23Z

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:

As mentioned in Parse blocks in html! as Rust block expressions #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.
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 Make key fall back to id for VTags #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 · 2023-10-29T12:09:01Z

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 · 2023-10-29T12:12:47Z

I also agree, however it's not possible to impl Into 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 · 2023-10-29T12:16:22Z

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.

schvv31n · 2023-10-29T12:35:12Z

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 · 2023-10-29T12:48:28Z

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.

…f a for loop, added a check for duplicate keys, added keyed state compile-time inference

schvv31n · 2023-10-29T15:00:26Z

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.

schvv31n · 2023-10-30T10:25:17Z

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

…de per iteration

added for loops to html macro

b725e3e

schvv31n added 3 commits October 29, 2023 09:55

disallowed divrging expressions in for loops

217636f

updated the docs

910c7f0

Merge branch 'master' into add-html-for

7950dc1

updated the docs

353259e

schvv31n added 4 commits October 29, 2023 10:33

updated the docs

088a723

fixed tests

1246087

fix docs

ff39cef

fix docs

eb02648

delegated divergence prohibition to rustc by using for_each instead o…

31c6aa9

…f a for loop, added a check for duplicate keys, added keyed state compile-time inference

schvv31n added 3 commits October 29, 2023 15:09

fixed formatting

1b1d814

fixed a bug, optimised fragments' creation

9a57be0

fixed tests

d0247bb

schvv31n added 2 commits December 4, 2023 15:16

removed an allocation optimisation for for-nodes generating only 1 no…

b3927ba

…de per iteration

fixed formatting

a9a1c49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add for-loops to `html!` #3498

Add for-loops to `html!` #3498

schvv31n commented Oct 29, 2023

futursolo commented Oct 29, 2023

github-actions bot commented Oct 29, 2023 •

edited

github-actions bot commented Oct 29, 2023 •

edited

github-actions bot commented Oct 29, 2023 •

edited

github-actions bot commented Oct 29, 2023 •

edited

schvv31n commented Oct 29, 2023

futursolo commented Oct 29, 2023 •

edited

ranile commented Oct 29, 2023

futursolo commented Oct 29, 2023

futursolo commented Oct 29, 2023

schvv31n commented Oct 29, 2023 •

edited

futursolo commented Oct 29, 2023 •

edited

schvv31n commented Oct 29, 2023 •

edited

schvv31n commented Oct 30, 2023

Add for-loops to html! #3498

Are you sure you want to change the base?

Add for-loops to html! #3498

Conversation

schvv31n commented Oct 29, 2023

Description

Checklist

futursolo commented Oct 29, 2023

github-actions bot commented Oct 29, 2023 • edited

Benchmark - core

Yew Master

Pull Request

github-actions bot commented Oct 29, 2023 • edited

github-actions bot commented Oct 29, 2023 • edited

Benchmark - SSR

Yew Master

Pull Request

github-actions bot commented Oct 29, 2023 • edited

Size Comparison

schvv31n commented Oct 29, 2023

futursolo commented Oct 29, 2023 • edited

ranile commented Oct 29, 2023

futursolo commented Oct 29, 2023

futursolo commented Oct 29, 2023

schvv31n commented Oct 29, 2023 • edited

futursolo commented Oct 29, 2023 • edited

schvv31n commented Oct 29, 2023 • edited

schvv31n commented Oct 30, 2023

Add for-loops to `html!` #3498

Add for-loops to `html!` #3498

github-actions bot commented Oct 29, 2023 •

edited

github-actions bot commented Oct 29, 2023 •

edited

github-actions bot commented Oct 29, 2023 •

edited

github-actions bot commented Oct 29, 2023 •

edited

futursolo commented Oct 29, 2023 •

edited

schvv31n commented Oct 29, 2023 •

edited

futursolo commented Oct 29, 2023 •

edited

schvv31n commented Oct 29, 2023 •

edited