-
Notifications
You must be signed in to change notification settings - Fork 20
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
More hygenic version of styled_components
#48
Conversation
Macros still need test cases for |
Thank you for the PR. However, I don't think Consider the following two designs: use yew::prelude::*;
use stylist::yew::styled_component;
#[styled_component(Comp)]
pub fn comp() -> Html {
html! {<div class={css!("color: red;")}></div>}
} use yew::prelude::*;
use stylist::yew::use_stylist;
#[function_component(Comp)]
pub fn comp() -> Html {
use_stylist!(css);
html! {<div class={css!("color: red;")}></div>}
} In my opinion, Despite I understand what the code above does, when seeing the code above, my mind tells me that it's invoking a function call taking a variable Even the following does not mess with my mind as much. use yew::prelude::*;
use stylist::yew::managed;
#[function_component(Comp)]
pub fn comp() -> Html {
#[managed]
use stylist::css;
html! {<div class={css!("color: red;")}></div>}
} This also applies to other crates use a syntax like this to declare stuff which I wish to avoid to introduce a similar usage. Why This is an ergonomic design that adds 0 lines of code over standard All other ways I can think of introduce at least 1 line of code for each component which is an unnecessary overhead to pay compare to this design. In addition, leaking This is true. But I am happy to overlook this issue at the moment this because it's very unlikely to collide this variable name unless the user is deliberately doing so. Additionally, this can be solved with
This is by design. Users should use APIs in I don't think use of Emotion also make a similar distinction on
A similar effect can be achieved by
The current procedural macro accepts any valid function and passes it to the upstream In addition, a In addition, if users feel |
My biggest concern: it starts getting unergonomic once another library adopts a similar design and wants to replace
Alternate syntax
Reading code written with the current
Why? Both In fact, I think that using the "global" versions of the macros in any component makes that component less usable than it could be and can cause bugs downstream, but that's for another discussion. To close, I don't think saying "Add |
You can combine the usage with the standard function component attribute (and other #[styled_component_base]
#[function_component(Comp)]
pub fn comp() -> Html {
html! {<div class={css!("color: red;")}></div>}
}
During the initial design, I have considered to introduce an equivalent I don't think there's ultimately a difference of introducing a custom syntax inside of a macro and and adding an variable as the same is achievable with replacing all However, I think if the variable leaves the macro's control, then it's bad. fn func() {
declare_a!(); // bad, and shouldn't be possible?
println!("{}", a);
} #[declare_a] // Should be fine as custom syntax is allowed in this context.
fn func() {
println!("{}", a);
}
I see this question as an opportunity cost. |
I forgot that Will file an issue to improve this. |
I didn't know this could work. I'd actually prefer this way over the current implementation and repurpose this PR towards that. To make sure, |
Could you please close this PR and open a separate one if
This is possible with any function but I am not sure about the evaluation order of multiple attributes. #[styled_component_base]
#[function_component(Comp)]
pub fn comp() -> Html {
html! {<div class={css!("color: red;")}></div>}
} |
The current version of
styled_components
leaks the identifier__stylist_style_manager__
. This can't be worked around without ditching the design and switching to a proc macro declaring other proc_macros, since__css_yew_impl
has to have that identifier in scope.In any case, I don't see a reason to not use
function_component
directly and have a hook-like macro that introduces the scoped versions. That's whatuse_stylist!(css);
does: get the manager withuse_context
, then introducemacro_rules
that are ready to use at the call-site, that refer to that manager (and the unmodifedstylist-macros
).There are a few hacks along the way that can be improved in the future, if the rust compiler follows accordingly.
($dol:tt)
to expand to a literal dollar sign in a proc macro comes from this issue but when RFC 3086 gets merged, this gets simplified to just using$$
.stylist
, if it was defined instylist
, it needs a#[macro_export]
slapped onto it, which also makes it visible from the crate root. Trying to hide it there in the docs withdoc(hidden)
also hides the re-export. If it's okay with you, I'd accept the re-export at the crate root, maybe with a textual warning in the docs?There's also goodies to get
use_stylist!(css as stylist_css)
or similar.function_component
macro to mirror