-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Reconcile difference between dynamic tags, literal tags, and manual creation #1269
Comments
Per the discussion we had on #1344 , I would question why we need dynamic strings as tag values at all. For HTML, MathML, and SVG elements, these should be easy to represent as some built-in Yew enum which could be represented as a In the case of custom elements, maybe that could be declared via some configuration, possibly in Cargo.toml or just in Yew config, stored as a type inside |
|
After doing some more research, a couple of modifications to my stance:
One more observation- although SVG has some elements that are defined as having capital letters in them such as |
Using an enum which contains all possible tag names isn't a terrible idea. We can use it in conjunction with a enum TagName {
Div,
// ...
Custom(String),
} Using this dynamic tag names would no longer accept a
In most cases this doesn't apply anyway. The most common case is creating a tag name through the macro where the names are verified at compile-time. The only case where we need to check tag names at runtime is for dynamic tag names (be it through the macro or using
No, it sure is a nice side-effect but the primary reason for them is to create dynamic tag names. There are cases where using a To summarise: I'm not against the idea of using an enum for tag names. In fact I've thought about it a lot myself. However, I think we need to make a strong case as to why it's beneficial in the long run.
That's exactly what the standard dictates:
Custom element names aren't case-insensitive in the same way. They're not allowed to use
Although tag names are case-insensitive the standard sometimes defines a canonical representation. This isn't really relevant to us because the representation doesn't matter for the lookup. |
Been thinking about this some more...
I'm not sure whether this is possible. It seems like macros are not able to parse arbitrary UTF8 sequences from files... which some custom tag names have (within restriction). That said, the capitalization issues are probably at least something we can fix.
This isn't quite true, dynamic tags have to begin with a non-hyphen. There is some question though about whether we could expand the definition of a dashed name to be more inclusive, although I suspect it would only be by a little, such as having a hyphen at the end perhaps.
I assume by this it was meant that in the normal case, one would not have to use this macro? I suppose we could have the HTML tag macro accept any of
Assuming this works, would the |
@philip-peterson Just a quick clarification:
I was talking about how it's currently implemented, not how it should be. You can see the runtime check that is inserted here: yew/yew-macro/src/html_tree/html_tag/mod.rs Lines 110 to 124 in 151b5b7
You're right, I mentioned that literal tags need to consist of valid rust identifiers. If you try to use invalid characters then compilation fails even before the macro comes into play so there's really nothing we can do about this.
The Here's a few benefits of doing it this way:
|
I personally think that dynamic tags aren’t necessary and are just unnecessarily increase binary sizes.
IMO they’re a “solution in search of a problem.”
They make Yew unsafe because the statics of a program don’t line up with the dynamics (by their nature) which makes it easy for mistakes to creep in (this isn’t really idiomatic Rust which I think is fundamentally about being safe – safe in the sense of having aligning statics and dynamics).
It would be nice to see some examples of why dynamic tags are needed and use cases in which a problem can be better solved by using a dynamically computed tag name than using the features Rust already provides.
Please let me know if I’ve misunderstood anything.
… On 7 Jul 2020, at 12:31, Simon ***@***.***> wrote:
@philip-peterson <https://github.com/philip-peterson> Just a quick clarification:
Dynamic tags can use any ASCII character at any position.
This isn't quite true, dynamic tags have to begin with a non-hyphen. There is some question though about whether we could expand the definition of a dashed name to be more inclusive, although I suspect it would only be by a little, such as having a hyphen at the end perhaps.
I was talking about how it's currently implemented, not how it should be. You can see the runtime check that is inserted here: https://github.com/yewstack/yew/blob/151b5b7db954e6d3f5f6ad5be0c0991131adde26/yew-macro/src/html_tree/html_tag/mod.rs#L110-L124 <https://github.com/yewstack/yew/blob/151b5b7db954e6d3f5f6ad5be0c0991131adde26/yew-macro/src/html_tree/html_tag/mod.rs#L110-L124>
All three methods should have exactly the same rules
I'm not sure whether this is possible. It seems like macros are not able to parse arbitrary UTF8 sequences from files... which some custom tag names have (within restriction). That said, the capitalization issues are probably at least something we can fix.
You're right, I mentioned that literal tags need to consist of valid rust identifiers. If you try to use invalid characters then compilation fails even before the macro comes into play so there's really nothing we can do about this.
In these rare cases dynamic tag names should come in handy.
We can use it in conjunction with a tag_name! macro that returns the appropriate variant from a string. Custom element names could be stored in a special variant
I assume by this it was meant that in the normal case, one would not have to use this macro?
I suppose we could have the HTML tag macro accept any of div, span, table, etc. which are actually Rust symbols imported from a prelude somewhere, corresponding to say yew::tags::{div, span, table}. And then, if someone wanted to specify a custom tag, they could import yew::tags::{Custom} and do
<Custom("my-custom-tag")>{ "foo" }</Custom("my-custom-tag")>
Assuming this works, would the tag_name! macro still be needed?
The tag_name! macro I have in mind actually works to enhance the setup you mentioned.
Let's look at the syn <https://docs.rs/syn/1.0.33/syn/> crate. Syn has a Token! <https://docs.rs/syn/1.0.33/syn/macro.Token.html> macro which is very similar to what I have in mind.
There is a module syn::token <https://docs.rs/syn/1.0.33/syn/token> that contains various Rust tokens and keyword (very similar to your tags module). Instead of accessing the members of said module directly syn recommends you use the Token macro.
Token![static] then resolves to syn::token::Static <https://docs.rs/syn/1.0.33/syn/token/struct.Static.html>.
Here's a few benefits of doing it this way:
No need to bring the tags into scope or memorise the struct name. We can just use the tags exactly like we would write them in HTML.
We can easily detect custom element names and verify their validity at compile time (Of course there's still the problem with unicode characters but this is easily solved by allowing literal strings in the macro).
Using the macro we can provide more detailed feedback to the developer.
We can integrate this macro directly into the html! macro. html! { <div/> } -> VTag::new(tag_name!(div)) -> VTag::new(yew::tags::Div).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#1269 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKFSTPMZELK5ZF6NKG7PG63R2MBQFANCNFSM4NNFF4SQ>.
|
@teymour-aldridge Just to clarify: I don't particularly care whether Yew supports dynamic tag names or not - I'm playing devil's advocate here. This discussion also isn't only about dynamic tag names. The same thing is possible with Here's an example of something that dynamic tags make possible: This is what it looks like using dynamic tag names: // ensure that 1 <= level <= 6
let h_tag = format!("h{}", level);
<@{h_tag}>{ contents }</@> And here's the same without: match level {
1 => html! { <h1>{ contents }</h1> },
2 => html! { <h2>{ contents }</h2> },
3 => html! { <h3>{ contents }</h3> },
4 => html! { <h4>{ contents }</h4> },
5 => html! { <h5>{ contents }</h5> },
6 => html! { <h6>{ contents }</h6> },
_ => panic!("invalid header level"),
} This isn't terribly bad but I'm doing it a lot of favours such as not using any attributes. Just imagine what it would look like if we wanted to add an Let's try to abstract this a bit more. The underlying problem here is that we're unable to propagate tag names. There's no way for our hypothetical component to accept All of this could be solved if we adopted a solution where each tag name is assigned a type. It would give us the ability to transparently pass tag names to components with type checking. use yew::tags::{H1, H2, H3, H4, H5, H6};
// this is just a marker trait to ensure that `Heading` is only used with heading tags.
trait HTag {}
impl HTag for H1 {}
impl HTag for H2 {}
impl HTag for H3 {}
impl HTag for H4 {}
impl HTag for H5 {}
impl HTag for H6 {}
struct Props<T> {
tag: T,
text: String,
}
struct Heading<H: HTag> {
props: Props<H>,
}
impl<H: HTag> Component for Heading<H> {
type Props = Props<H>;
fn view(&self) -> Html {
let Props { tag, text } = &self.props;
html! {
<@{tag}>{ text }</@>
}
}
} Note that in this example the syntax for dynamic tag names no longer accepts strings, only members of the |
Thanks for this explanation – it’s really helpful!
I also now see why dynamic tags can be useful.
… On 7 Jul 2020, at 13:31, Simon ***@***.***> wrote:
@teymour-aldridge <https://github.com/teymour-aldridge> Just to clarify: I don't particularly care whether Yew supports dynamic tag names or not - I'm playing the devil's advocate here. This discussion also isn't only about dynamic tag names. The same thing is possible with VTag::new but with even less restrictions.
Here's an example of something that dynamic tags make possible:
Let's say we have a typography component for headings which accepts the level (h1 - h6) as a prop.
This is what it looks like using dynamic tag names:
// ensure that 1 <= level <= 6
let h_tag = format!("h{}", level);
<@{h_tag}>{ contents }</@>
And here's the same without:
match level {
1 => html! { <h1>{ contents }</h1> },
2 => html! { <h2>{ contents }</h2> },
3 => html! { <h3>{ contents }</h3> },
4 => html! { <h4>{ contents }</h4> },
5 => html! { <h5>{ contents }</h5> },
6 => html! { <h6>{ contents }</h6> },
_ => panic!("invalid header level"),
}
This isn't terribly bad but I'm doing it a lot of favours such as not using any attributes. Just imagine what it would look like if we wanted to add an onclick listener to this.
Dynamic tags are only really useful for higher order components. It's important that we support this because it makes it a lot easier to create component libraries.
Let's try to abstract this problem a bit more. The underlying problem here is that we're unable to propagate tag names. There's no way for our hypothetical component to accept h1 - h6 as a value without going using an opaque representation like a string or a number representing the level.
In the macro tag names work like types but outside of it they don't exist; they're just strings. In a way, dynamic tag names are a patch solution for the inability to pass tag names around transparently.
All of this could be solved if we adopted a solution where each tag name is assigned a type. It would give us the ability to transparently pass tag names to components with type checking.
Here's what the same component might look like with statically typed tag names:
use yew::tags::{H1, H2, H3, H4, H5, H6};
// this is just a marker trait to ensure that `Heading` is only used with heading tags.
trait HTag {}
impl HTag for H1 {}
impl HTag for H2 {}
impl HTag for H3 {}
impl HTag for H4 {}
impl HTag for H5 {}
impl HTag for H6 {}
struct Props<T> {
tag: T,
text: String,
}
struct Heading<H: HTag> {
props: Props<H>,
}
impl<H: HTag> Component for Heading<H> {
type Props = Props<H>;
fn view(&self) -> Html {
let Props { tag, text } = &self.props;
html! {
<@{tag}>{ text }</@>
}
}
}
Note that in this example the syntax for dynamic tag names no longer accepts strings, only members from the yew::tags. How this is to be achieved is still up for discussion but either way, this accomplishes type safety and doesn't depend on any runtime checks.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#1269 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKFSTPKPGEO4JDMAANZ2MBDR2MIRRANCNFSM4NNFF4SQ>.
|
I agree with what you say @siku2 , having the macro would be useful for construction using For the example provided with the dynamic tag name, would that value passed into |
This might be a recent regression in browsers but it seems that this no longer holds true. |
I have a somewhat different view on treating browser-behavior: We shouldn't try to get in the way, except for I also don't think a enumeration of "allowable" tag names will do much good, if it's for more than debug-assertions and linting. It will eventually get out of date, besides the number of svg elements already being rather large. This list doesn't have to leak into the final binary. For capitalization, I'm not even sure about the rules the browsers currently implement. For example, an inline Now, the special casing for |
Problem
With the introduction of dynamic tags in #1266 there are now three different ways of constructing HTML tags:
html!
macro (<span></span>
)<@{"span"}></@>
)VTag
instances manually (VTag::new("span")
)All three of these have different restrictions with the third option (manually constructing) having virtually none at all.
Dynamic tags try to have the same restrictions as literal ones using runtime checks but there are still a few differences:
<sPAN></sPAN>
and<@{"sPAN"}></@>
produce equivalent HTML, Yew currently stores the former as "sPAN" internally and the latter as "span".-
) anywhere in the middle. Dynamic tags can use any ASCII character at any position.<br/>
but dynamic tags can do<@{"br"}></@>
. I think this one is actually a good thing because it gives dynamic tags more flexibility without any downside.There's another issue that currently plagues both macro methods. They only allow tag names containing characters in the ASCII range. This is too broad for normal tag names which only allow for alphanumerics[1] but it's also too narrow for custom elements which support a big range of characters outside of the ASCII range[2].
Somewhat related to this issue is that Yew currently preserves the casing for tag names. This can easily lead to bugs like this:
Proposal
All three methods should have exactly the same rules. For the literal case these rules should be enforced during compilation while the other two need runtime checks.
Valid Tag Names
The
VTag
constructor ensures the given tag name is valid.It might be a good idea to use a wrapper type
TagName
internally. This type could be treated just like a normal string but it can only contain valid tag names. It would also make it easier to ignore casing when comparing names.Void Element
Void elements must not have any children. To guard against this we store a
allow_children
flag inVTag
which is set in the constructor by comparing the tag name against the list of void elements.The
add_child
method is modified to check this flag first.Special Attributes
The attribute
value
needs to be handled differently depending on the tag name.This can be done by adding a check to the
set_value
method which forwards the value to the attributes instead if the tag isn'tinput
ortextarea
.In all these cases the literal tags in macros should already perform the checks during compilation. To avoid the performance penalty there should be "private" methods which avoid the checks entirely which can then be used by the macro.
There should also be equivalent methods that return a
Result
instead of panicking.This gives us a structure like this:
The text was updated successfully, but these errors were encountered: