Closed WorldSEnder closed 2 years ago
Macros still need test cases for style!
and global_style!
and the impl is probably wrong, cause with_manager
doesn't yet exist for those.
Thank you for the PR.
However, I don't think use_stylist(css)
is a good replacement of styled_component
.
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, use_stylist!(css);
is counter intuitive.
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 css
instead of some kind of assignment.
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 #[styled_component(Comp)]
over other designs?
This is an ergonomic design that adds 0 lines of code over standard function_component
.
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, styled_component
is very iconic and will leave a deep first impression to users which is a deliberate choice I made when making this API.
leaking __stylist_style_manager__
is unhygienic.
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 Span::def_site()
once that becomes available in stable Rust.
style!
and global_style!
do not support with_manager
.
This is by design.
Users should use APIs in stylist::yew::*
over other APIs when using stylist with Yew.
I don't think use of style!
and global_style!
is justified when css!
and <Global />
is available and more ergonomic to use.
Emotion also make a similar distinction on @emotion/react
and @emotion/css
where one is context aware and the other isn't.
Allow the user to chose the name of the macro. Simple as use_stylist!(css as stylist_css) or similar.
A similar effect can be achieved by #[styled_component(Comp, rename_css = styled)]
.
(not implemented at the moment, as I think it's not critical to include in the first round.)
No need to track yew-upstream for changes to the function_component macro to mirror
The current procedural macro accepts any valid function and passes it to the upstream function_component
attribute.
I think it's unlikely that it's going to be changed to something other than a function.
In addition, a #[styled_component_base]
attribute can be added to provide a usage in combination with #[function_component(Comp)]
which means they can be made completely API neutral or use in combination with other libraries.
In addition, if users feel styled_component
is too magical (or unhygienic), they can opt for the hook based design mentioned in #23 which is not landed at the moment.
This is an ergonomic design that adds 0 lines of code over standard function_component.
My biggest concern: it starts getting unergonomic once another library adopts a similar design and wants to replace function_component
, too. The two results don't compose and the user will have to chose between one or the other. I posit we aren't large enough to argue that our replacement is the one true one. A local use_*
macro is perfectly composable with an arbitrary amount of libraries at hand.
In my opinion,
use_stylist!(css);
is counter intuitive.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
css
instead of some kind of assignment.
Alternate syntax use_stylist!(use css);
should be more intuitive in that regard.
A similar effect can be achieved by #[styled_component(Comp, rename_css = styled)].
Reading code written with the current styled_component
, which introduces an accessible name, doesn't make that clear. I have not seen other macros I use do that, and it's discouraged with macro_rules!
. Sorry that I can't find the relevant section from the rust guide that talks about this, but I don't think it's wise to introduce names the user never mentioned, no matter how big a warning is in the docs of styled_component
. When you only see the code in a diff of a larger project, it can get confusing.
style! and global_style! do not support with_manager. This is by design.
Why? Both Style
and GlobalStyle
have a new_with_manager
and should insert the style at the closest (shadow) DOM root. I don't see the design choice that they aren't context sensitive. style!
and global_style!
might as well be supported.
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 use_stylist!(use css);
to your function_components" is harder to teach than "Replace your function_components with styled_components".
My biggest concern: it starts getting unergonomic once another library adopts a similar design and wants to replace function_component, too.
You can combine the usage with the standard function component attribute (and other #[attrs]
) with a similar attribute #[styled_component_base]
(not implemented at this moment).
#[styled_component_base]
#[function_component(Comp)]
pub fn comp() -> Html {
html! {<div class={css!("color: red;")}></div>}
}
Reading code written with the current styled_component introduces an accessible name, at all. I have not seen other macros I use do that, and it's discouraged with macro_rules!. Sorry that I can't find the relevant section from the rust guide that talks about this, but I don't think it's wise to introduce names the user never mentioned, no matter how big a warning is in the docs of styled_component. When you only see the code in a diff of a larger project, it can get confusing.
During the initial design, I have considered to introduce an equivalent stylist::yew::css!
macro with the __stylist_style_manager__
named __this_macro_is_only_available_inside_of_a_styled_component__
. But ultimately deemed better to just include it for the end user so it's not possible to accidentally import the wrong one.
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 css!
with ::stylist::yew:: __css_yew_impl
manually.
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);
}
Why? Both Style and GlobalStyle have a new_with_manager and should insert the style at the closest (shadow) DOM root.
I see this question as an opportunity cost.
Why would user use style!
and global_style!
if css!
and <Global />
is both more powerful and simpler to use?
I really cannot think of any reason one would prefer style!
and global_style!
if using stylist with Yew
.
I forgot that Span::mixed_site()
will achieve the same effect of isolating the variable from call site.
Will file an issue to improve this.
You can combine the usage with the standard function component attribute (and other #[attrs]) with a similar attribute #[styled_component_base] (not implemented at this moment).
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, #[styled_component_base] fn ...
would work for any function, not just function_component
s, right? I can see the appeal of wanting to write helper functions to call from function components and use it there, too.
repurpose this PR towards that.
Could you please close this PR and open a separate one if use_
is dropped so the history is not as messy?
To make sure, #[styled_component_base] fn ... would work for any function
This is possible with any function but I am not sure about the evaluation order of multiple attributes. I think it evaluates from top to bottom which is why I wrote
#[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