`class=("foo-bar", bool)` tuple syntax doesn't work on components
tqwewe opened this issue ยท 12 comments
Describe the bug
The usual class:foo-bar=bool
syntax works on components in 0.7 beta, however the more advanced syntax for classes wrapped in quotes does not work. class=("foo-bar", bool)
Leptos Dependencies
Please copy and paste the Leptos dependencies and features from your Cargo.toml
.
For example:
leptos = { version = "=0.7.0-rc1", features = ["csr"] }
To Reproduce
Use the class tuple syntax on a component and see an error due to the syntax not being supported.
- If the component has no
class
prop, the error is:
191 | | <NumberWithTrend class=("foo-bar", bool) ...
| | -^^^^^ method cannot be called on `NumberWithTrendPropsBuilder` due to unsatisfied trait bounds
| |_____________________________________________________|
|
|
::: src/components/number_with_trend.rs:19:1
|
19 | #[component]
| ------------ method `class` not found for this struct because it doesn't satisfy `_: AsRef<Element>` or `_: ElementExt`
- If the component has a
class
prop, the error is:
error[E0308]: mismatched types
--> src/components/number_with_trend.rs:44:27
|
44 | class=("foo-bar", bool)
| ^^^^^^^^^^^^^ expected `IconPropsBuilder_Error_Repeated_field_class`, found `(&str, Signal<bool>)`
|
= note: expected enum `IconPropsBuilder_Error_Repeated_field_class`
found tuple `(&'static str, Signal<bool>)`
Expected behavior
As with the class:foo=bool
syntax being supported on components, class=("foo", bool)
should also be valid.
I'm not entirely sure how to handle situations where the component itself has a class
prop, since that will conflict. Possible breaking change solution is to move the syntax to be class:"foo"=bool
. Or if its even possible, it would be cool if class:foo-bar=bool
just worked with no problems.
There was a PR to fix this, and we're currently on rc1, beta is a few versions old. Can you update and retry?
Sorry when I said beta I meant rc1
. As shown in the leptos version I'm using, its set to rc1 which this issue was still occuring on. Is there a link to the PR for me to check it out?
Interesting, I found the PR I was referring to, which is for a slightly different syntax: #3152
So, the one commit in this PR so far should support class:foo-bar=true
on components.
In fact, that sort of name is valid anywhere in the view macro now. That was the originally use case for the tuple syntax, but changes to rstml
enabled it, I think. Still, people will always come up with crazier and crazier Tailwind class names, so there's always a place for the string literal syntax.
The tuple syntax can be supported by using a spread instead:
#[component]
pub fn App() -> impl IntoView {
let attrs = view! { <{..} class=("foo-bar", true)/> };
view! {
<SomeDiv {..attrs}/>
}
}
#[component]
pub fn SomeDiv() -> impl IntoView {
view! { <div/> }
}
I do not think there is a good way to support the tuple syntax for classes being added directly to components, but the spread above works fine.
Interesting, I guess thats a workaround for now. Thankfully it seems like <SomeDiv {..} class=("foo-bar", true)/>
directly also works. Happy to close this issue given there's a workaround.
This has probably been asked before, but is class:"w-[10px]"=|| true
syntax not possible with proc macros? I would imagine it could be done, but maybe there's something I'm missing which made leptos have to use the tuple class=(..., ...)
syntax? As a user who hasn't read the book, but knows of class:w=[10px]=...
, the first thing I'd try would be wrapping "w-[10px]"
in quotes there
Is there anything wrong with this snippet?
view! {
<Icon
attr:class=class
icon=icon
{..}
class=("opacity-0", move || !trend_visible.get())
class=("opacity-100", move || trend_visible.get())
class=("transition-opacity", move || !trend_visible.get())
/>
}
error[E0797]: base expression required after `..`
--> src/components/number_with_trend.rs:94:20
|
94 | {..}
| ^
help: add a base expression here
|
94 | {../* expr */}
| ++++++++++
error[E0308]: mismatched types
--> src/components/number_with_trend.rs:96:23
|
96 | class=("opacity-100", move || trend_visible.get())
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `IconPropsBuilder_Error_Repeated_field_class`, found `(&str, {closure@number_with_trend.rs:96:39})`
|
= note: expected enum `IconPropsBuilder_Error_Repeated_field_class`
found tuple `(&'static str, {closure@src/components/number_with_trend.rs:96:39: 96:46})`
Besides the {..}
syntax having issues, it also is interpreting class
as a prop.
Adding attr:
prefix to the classes errors with: This element already has a attr:class attribute
Interesting, I guess thats a workaround for now. Thankfully it seems like
<SomeDiv {..} class=("foo-bar", true)/>
directly also works. Happy to close this issue given there's a workaround.
Good point. I can confirm that this also works.
This has probably been asked before, but is
class:"w-[10px]"=|| true
syntax not possible with proc macros? I would imagine it could be done, but maybe there's something I'm missing which made leptos have to use the tupleclass=(..., ...)
syntax?
As a matter of fact, this has never been asked before!
I am sure that it is possible with proc macros. However, whether it's possible with the particular library we use for parsing the view
macro (which is called rstml
) is another question. I don't think it would be possible as currently implemented: attribute names are defined as something like a syn::Punctuated<syn::Ident, P>
(this is probably not the exact type, but you get the idea I hope), i.e., a list of Rust idents punctuated by something else (say, a :
or -
, or mixture of both) Adding in primitives (like "w-[10px]"
) makes that parsing API more complicated.
Historical syn-rsx
(the original parsing crate, whose maintainer unfortunately died) tried to preserve as close to a valid XML syntax as possible, so wasn't open to things like class:"w-[10px]"
as attribute names. I'm not sure about the position of rstml
on this topic. But that's the origin of the tuple syntax.
Today, the tuple syntax is also convenient because we actually do implement IntoClass
or whatever for (&'static str, C)
... So the class=("foo-bar", baz)
syntax actually expands to .class(("foo-bar", baz))
which is nice for several reasons (discoverability, docs, feels good to my brain, etc.)
As a user who hasn't read the book, but knows of
class:w=[10px]=...
, the first thing I'd try would be wrapping"w-[10px]"
in quotes there
I'm sympathetic to the argument "I think X is more intuitive than Y," but the tuple syntax is described both in the book and also in the docs for the view
macro. "I haven't read the docs or the book but this would be my guess" is reasonable, but there's only so much we can do with API design IMO, sometimes reading docs is helpful.
(Oh, and I don't see anything wrong with the second <Icon/>
snippet, and wasn't able to reproduce an issue with it -- Could you share a more complete repro if it's still causing an error? Otherwise I think we can close this.)
Thanks for the detailed response! I wasn't aware of rstml, and it being used for leptos.
I understand the reasoning behind the tuple syntax, and it definitely makes sense as you described it. I guess the only two arguments for class:"..."` syntax support are:
- Might be a guessable syntax by new leptos users who know about class:foo
- Would probably solve the workaround mentioned earlier with
<MyComponent {..} class=("foo-bar", bool) />
, since it would disambiguate from aclass
prop letting it be<MyComponent class:"foo-bar"=bool />
Perhaps its something that can be looked into in the future if others might come across this issue.
I'll try and share a repro repo with the issue and reply back with a link ๐๐ฟ
Here's a reproduction repo:
https://github.com/tqwewe/leptos_attr_issue
Initially I couldn't even reproduce it myself, but that's because I was doing icon="..."
. If I instead replace the string literal with a variable, it triggers the issue. icon=icon
.
The repro uses 0.7.0-rc1
, but I also tested using git main branch of leptos which still has the issue.