futursolo / stylist-rs

A CSS-in-Rust styling solution for WebAssembly Applications
https://crates.io/crates/stylist
MIT License
366 stars 22 forks source link

global_style! maybe busted? #102

Closed simbleau closed 1 year ago

simbleau commented 1 year ago

This bug will be a can of worms, sorry to warn you.

I have a SPA application, which looks like this:

struct Theme1{
    color: "black"
}

struct Theme2{
    color: "blue"
}

#[hook]
fn use_theme() {
   let theme = use_context::<Theme>();
   use_global_style!("html,body { color: ${theme.color} }");
}

#[function_component]
pub fn Root -> Html {
    html!{
        <BrowserRouter>
            <Context theme={Theme1}>
                <App />
            </Context>
        </BrowserRouter>
    }
}

#[function_component]
pub fn Slide1 -> Html {
    use_theme(); // Both slides use the global CSS
    let navigator = use_navigator();
    let onclick = Callback::from(|_| navigator.push('/slide2');
    html!{<>
        <h1>{"This is black"}</h1>
        <button {onclick}>{"Go to slide 2"}</button>
    </>)
}

#[function_component]
pub fn Slide2 -> Html {
    theme.set(Theme2);
    use_global_css(); // Both slides use the global CSS
    html!(<h1>{"This should be blue, but it's black :("}</h1>)
}

Please excuse my brevity, since I don't have the time to create a proper concrete example (holidays).

But, if you change global_style! to <Global css={...} /> it works as expected.

WorldSEnder commented 1 year ago
   global_style!("html,body { color: ${theme.color} }");

So the reason I believe is that this usage does not properly unmount the global style when it should be no longer used. Work-around: To follow later, ping me if I don't update this in the next week. Happy holidays.

simbleau commented 1 year ago
   global_style!("html,body { color: ${theme.color} }");

So the reason I believe is that this usage does not properly unmount the global style when it should be no longer used. Work-around: To follow later, ping me if I don't update this in the next week. Happy holidays.

That would explain it pretty well I think. If you go to /slide2 and hit refresh, the style is correct (only mounts the correct one), but if you go to /slide1 and press the button (which navigates to /slide2), the first style doesn't get unmounted.

My workaround is to use the <Global css={} /> syntax provided. I'd consider explaining how different the two are, in documentation.

This would make global_style! a common pitfall for use with changeable themes (or light/dark mode).

WorldSEnder commented 1 year ago

We previously considered removing the *_style! macros altogether, I don't think they are of much use to end-users. Style mounting should happen implicitly via use_*_style() or css! where you provide a style source, not a Style. I think at least adjusting documentation makes a lot of sense.

simbleau commented 1 year ago

Opinion: I'd probably consent with removing use_*_style() calls. They haven't done much for me, at least. I think this was the first I attempted it. I've only ever used css! and styled_component, really. (I also struggle to understand the need for runtime styling, could probably be a feature)

futursolo commented 1 year ago

I do not think there is a use_global_style hook, only global_style! macro.

APIs under stylist::yew provides Yew integration where as stylist::global_style is application agnostic. This means that APIs under stylist::yew is mounting / unmounting aware and APIs outside of this module isn't.

<Global /> calls GlobalStyle::unregister() automatically upon unmounting which unmounts the style. global_style! is not aware of the Yew lifecycle events, so it is expected that it will not unmount.

Could you please try calling GlobalStyle::unregister (GlobalStyle is returned by global_style!) in an effect cleanup function?

simbleau commented 1 year ago

I do not think there is a use_global_style hook, only global_style! macro.

APIs under stylist::yew provides Yew integration where as stylist::global_style is application agnostic. This means that APIs under stylist::yew is mounting / unmounting aware and APIs outside of this module isn't.

<Global /> calls GlobalStyle::unregister() automatically upon unmounting which unmounts the style. global_style! is not aware of the Yew lifecycle events, so it is expected that it will not unmount.

Could you please try calling GlobalStyle::unregister (GlobalStyle is returned by global_style!) in an effect cleanup function?

Ah... Yes this all makes sense.