Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow literals without braces in html! #3440

Conversation

its-the-shrimp
Copy link
Contributor

Description

Allows for all kinds of literals to be passed to the html! macro without enclosing them in braces as would be required for any other Rust expression, thus, for example, making the following valid:

html!{<span>"Yew" 0.21</span>}

The docs are updated accordingly.

Checklist

  • I have reviewed my own code
  • I have added tests

@github-actions
Copy link

github-actions bot commented Oct 3, 2023

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

https://yew-rs--pr3440-allow-literal-withou-5evvdh9m.web.app

(expires Tue, 10 Oct 2023 19:13:40 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

github-actions bot commented Oct 3, 2023

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 102.943 102.943 0 0.000%
boids 175.671 175.671 0 0.000%
communication_child_to_parent 95.304 95.304 0 0.000%
communication_grandchild_with_grandparent 109.033 109.033 0 0.000%
communication_grandparent_to_grandchild 105.715 105.715 0 0.000%
communication_parent_to_child 92.782 92.782 0 0.000%
contexts 113.449 113.449 0 0.000%
counter 89.198 89.198 0 0.000%
counter_functional 89.935 89.935 0 0.000%
dyn_create_destroy_apps 92.316 92.316 0 0.000%
file_upload 103.513 103.513 0 0.000%
function_memory_game 174.629 174.629 0 0.000%
function_router 353.593 353.593 0 0.000%
function_todomvc 163.456 163.456 0 0.000%
futures 227.446 227.446 0 0.000%
game_of_life 112.217 112.217 0 0.000%
immutable 188.784 188.784 0 0.000%
inner_html 85.980 85.980 0 0.000%
js_callback 113.463 113.463 0 0.000%
keyed_list 201.208 201.208 0 0.000%
mount_point 89.189 89.189 0 0.000%
nested_list 115.753 115.753 0 0.000%
node_refs 96.291 96.291 0 0.000%
password_strength 1721.272 1721.272 0 0.000%
portals 98.363 98.363 0 0.000%
router 319.605 319.605 0 0.000%
simple_ssr 144.247 144.247 0 0.000%
ssr_router 391.382 391.382 0 0.000%
suspense 119.068 119.068 0 0.000%
timer 91.797 91.797 0 0.000%
timer_functional 100.472 100.472 0 0.000%
todomvc 143.688 143.688 0 0.000%
two_apps 89.896 89.896 0 0.000%
web_worker_fib 138.886 138.886 0 0.000%
web_worker_prime 190.422 190.422 0 0.000%
webgl 88.553 88.553 0 0.000%

✅ None of the examples has changed their size significantly.

@github-actions
Copy link

github-actions bot commented Oct 3, 2023

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 360.045 360.609 360.155 0.165
Hello World 10 670.269 684.615 679.201 4.316
Function Router 10 2104.477 2144.185 2130.432 11.339
Concurrent Task 10 1006.545 1008.004 1007.361 0.500
Many Providers 10 1641.043 1771.096 1680.434 38.044

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 360.056 360.665 360.263 0.187
Hello World 10 672.067 680.028 676.560 2.545
Function Router 10 2125.527 2152.265 2136.932 8.353
Concurrent Task 10 1006.824 1008.029 1007.474 0.371
Many Providers 10 1653.317 1701.984 1667.160 13.836

@ranile
Copy link
Member

ranile commented Oct 4, 2023

I remember it being a deliberate decision to require braces around the literals. I, personally, am not in favor of this change either.

Why do you think it would be better to allow literals like that? I think if we can't allow

<span>yew 0.21</span>

then there isn't really a point in removing the braces (we can't allow that because it's a parsing nightmare). The braces indicate any rust expression, which includes literals. Also, "Yew" 0.21 not only looks weird, compared to "Yew 0.21" but also isn't even valid Rust (literally or expression).

There's also the question of what to do with white spaces. HTML treats them as one whitespace, Rust ignores them. How should multiple literals be joined? Should they just be concatenated? Should they be different text nodes?

@its-the-shrimp
Copy link
Contributor Author

I've read the prior discussions on this proposal, and the main argument against it was that it makes it unclear whether the literals will be parsed as HTML or as Rust code, and that is a valid concern, however I believe that, in this case, being idiomatic about the syntax only hurts the developer experience. If we really wanted to be idiomatic, we'd have to remove if and {for x} expressions, since the first is "ambiguous" (for someone who sees Yew for the first time) and the latter is wrapped in braces but is not valid Rust syntax. html! is not Rust, it's not HTML, it's yew::html!, and there's documentation for it, so I believe we should focus on improving the experience for developers, not for those who see the framework for the first time and haven't checked any guides, examples, or the docs.

Regarding handling of whitespace, even though html! doesn't claim to replicate Rust syntax, it is lexed by Rust, which is whitespace-insensitive, I think we should respect that.

Besides, the PR only makes braces optional, if you want your literals separated from your HTML, you can always do that.

@futursolo
Copy link
Member

I agree with the point raised by @hamza1311 and this has been discussed numerous times.
The concerns raised by the original issue still stands today and hasn't changed.

Also, "Yew" 0.21 not only looks weird, compared to "Yew 0.21" but also isn't even valid Rust (literally or expression).

There is also the question of whether "Yew" 0.21 should be interpreted as Yew 0.21 or "Yew" 0.21 when passed to create a Text node. When you write <div>"Yew" 0.21</div> in html, it is created as <div>"Yew" 0.21</div>.

Regarding handling of whitespace, even though html! doesn't claim to replicate Rust syntax, it is lexed by Rust, which is whitespace-insensitive, I think we should respect that.

However, the common writing style is space aware.

If we are not aware of the spaces, then we have to make a decision of whether we want to insert spaces for users.
We could not make a choice of between of having space or not having spaces between literals and represents both
"Yew" 0.21 and 20"km/h" nicely.

Copy link
Member

@cecton cecton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also the question of whether "Yew" 0.21 should be interpreted as Yew 0.21 or "Yew" 0.21 when passed to create a Text node

you mean using the debug formatting? "Yew" 0.21

I think it's more intuitive to use the ToString implementation. That's already how it is today for the things wrapped in {} no?

We could not make a choice of between of having space or not having spaces between literals and represents both
"Yew" 0.21 and 20"km/h" nicely.

Anecdote: well actually xD you don't write 20km/h but 20 km/h because it's the International System of Units https://grammarhow.com/do-you-put-a-space-between-number-and-unit/

More seriously: I think a single breakable space for separation is the most intuitive solution. It's like when writing spaces between arguments in bash, you do expect a single space to come out, not all the extra space you added in your terminal.

$ echo      woow ok     Hello
woow ok Hello

All in all I think allowing only literals to be written in the macro is safe and actually consistent with how attributes work (which also allow literals). ✔️

@futursolo
Copy link
Member

futursolo commented Oct 5, 2023

you mean using the debug formatting? "Yew" 0.21

I meant to say that if the content is not inside a brace, it should be rendered as if the content are written between 2 html tags.

I think it's more intuitive to use the ToString implementation. That's already how it is today for the things wrapped in {} no?

This is because {expr} represents a Rust expression, and it actually uses the ToHtml trait to decide how expressions are converted to Html.

Anecdote: well actually xD you don't write 20km/h but 20 km/h because it's the International System of Units

TIL, Interesting. Maybe I should switch to the canonical way in the future.

More seriously: I think a single breakable space for separation is the most intuitive solution. It's like when writing spaces between arguments in bash, you do expect a single space to come out, not all the extra space you added in your terminal.

Terminals actually are aware of whether there are spaces around tokens.
The distinction here is being made of 0 or 1 space and not 1 or more spaces.

 ➜  ~ echo 20"px"
20px
 ➜  ~ echo 20 "px"
20 px

We do not have the spacing information with the current procedural macro.

There are cases where things are legitimately broken if spaces are present / not present between tokens when they should not or should be when it comes to html contents, in which I have come across in another project. So I do not wish Yew to pertain a similar kind of issue with html! when I already had to accept that it wouldn't be fixed anytime soon.

See: futursolo/stylist-rs#104

@cecton
Copy link
Member

cecton commented Oct 5, 2023

There are cases where things are legitimately broken if spaces are present / not present between tokens when they should not or should be when it comes to html contents, in which I have come across in another project. So I do not wish Yew to pertain a similar kind of issue with html! when I already had to accept that it wouldn't be fixed anytime soon.

See: futursolo/stylist-rs#104

I understand better where you're coming from. I'm not sure if the html! macro can be compared to the css! one tbh... The issues you explain are real in CSS but in HTML the behavior of spacing is very different:

  1. One or more spaces are translated as one space: https://jsbin.com/talusevuzu/edit?html,output
  2. It's only really used for putting arbitrary text in arbitrary node. There isn't any syntax quirk here like in CSS.

We do not have the spacing information with the current procedural macro.

I don't think we need to. The spacing behavior in Rust parser is actually similar to the HTML. One or more space is basically the same as a single space.

This case you mention is very specific to bash:

 ➜  ~ echo 20"px"
20px
 ➜  ~ echo 20 "px"
20 px

Because "px" is first evaluated as string then put in place into the rest of the expression. This should not exist in html! macro. Actually I don't know any modern programing language that has this feature but it's a very confusing one so it's best not to have it.

What I'm trying to say is that maybe we don't need more than that. It has to be close to HTML and that's it.

This is because {expr} represents a Rust expression, and it actually uses the ToHtml trait to decide how expressions are converted to Html.

I do think it makes sense to wrap expressions into {}. The parser of syn will force us to do so (we had that exact problem back in the day when attributes where allowing expressions without the {} such as <input stuff=Animal::Cat />... the tldr was that, if we want to be able to do <input ...props />, we need to force expressions to be wrapped in {}).

But just like the parser for attributes, literals were not the issue. Strings, numbers and booleans can be used with syn without the {} just fine. <input foo=42 bar=true baz={ah_thats_an_expression()} />

@ranile
Copy link
Member

ranile commented Oct 5, 2023

The thing is, we don't even get one space. The Rust lexer ignores the spaces and will give the literals to us as if they were written without any spaces

@futursolo
Copy link
Member

futursolo commented Oct 5, 2023

I'm not sure if the html! macro can be compared to the css!

CSS can be inlined in the <style></style> tag. So issues applies to css! ultimately applies to html!.
(Unless it's OK to tell everyone to use stylist for CSS inlining.)

20"px" ... This should not exist in html! macro.

I agree, kind of.

I feel if I write <div>20"px"</div>, then I should get 20"px" in my browser window as if I am writing HTML.
(How React-JSX works.)

I do think it makes sense to wrap expressions into {}.

If the ultimate goal is to pursue text content / literals rendered without braces,
for content between html tags, we have to choose between:

  1. content inside of {} (or another kind of identifier) is evaluated as expression, otherwise HTML text nodes.
  2. everything is evaluated as an expression.

The ideal result for these solutions are:

  1. <div>The latest version of Yew is {ver}.</div>
  2. <div>"The latest version of Yew is " ver "."</div>
    (Let's assume we don't have spaces between tokens for now. Or we have to use format!)

By the end of the day, I would definitely prefer 1 over 2.

Solution 1 is partially achievable once proc_macro_span is stabilised.
(Given the content can be tokenised as valid Rust tokens.)

Pursuing 1 with "" formatted with Display as a temporary solution is also not ideal, because this results in <div>"The latest version of Yew is "{ver}"."</div>.

If in the future we wish to make <div>The latest version of Yew is {ver}.</div> possible,
it will be very difficult for users to remove quotation marks from all of their html! as it will be rendered as-is.

@cecton
Copy link
Member

cecton commented Oct 5, 2023

CSS can be inlined in the <style></style> tag. So issues applies to css! ultimately applies to html!.

Good point

I feel if I write <div>20"px"</div>, then I should get 20"px" in my browser window as if I am writing HTML.

I would definitely prefer a compile error if no space.

By the end of the day, I would definitely prefer 1 over 2.

Same 👍

If in the future we wish to make <div>The latest version of Yew is {ver}.</div> possible,
it will be very difficult for users to remove quotation marks from all of their html! as it will be rendered as-is.

Good point... though I feel comfortable committing into the solution 2. But your example clearly shows that not forcing the space could be really useful. So I'm in doubt.

@its-the-shrimp
Copy link
Contributor Author

To be honest, the arguments here were so compelling I'm no longer in favor of my own PR, feel free to dismiss it.

@cecton
Copy link
Member

cecton commented Oct 7, 2023

By the end of the day, I would definitely prefer 1 over 2.

Same 👍

I misread. I'm definitely in favor of 2 instead of 1. Solution one is risky, it will brings tons of issues with the parser I can smell that.

To be honest, the arguments here were so compelling I'm no longer in favor of my own PR, feel free to dismiss it.

coward lol

tbh I don't care that much but I do recognize it would be useful and safe to allow literals. But again I don't really care, the {} does not bother me at all.

@cecton cecton closed this Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants