stitches: Incorrect style order
Bug report
Description
Styles are applied in incorrect order depending on how styled component are composed.
const StyledBase - styled(...)
const Foo = styled(StyledBase, {...}) // works correctly
const Base = (props) => <StyledBase {...props} />;
const Bar = styled(Base, {...}) // StyledBase styles take precedence
Things get even more unpredictable when using Foo & Bar on the same view. Depending on the order of Foo & Bar the styles might or might not apply correctly, see reproduction below.
To Reproduce
- Open the following sandbox: https://codesandbox.io/s/nifty-blackburn-m792w?file=/src/App.js
- Observe behavior by uncommenting commented out variations one by one. Make sure to refresh the built-in csb browser each time.
Expected behavior
When using styled(Foo, styleObject), rules defined in styleObject should override Foo’s existing styles.
System information
- OS: MacOS
- Browser: Chrome, Safari, Firefox
- Version of Stitches: 0.2+
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 20
- Comments: 33 (18 by maintainers)
This is being worked on.
This is now fixed in a release candidate under the version 1.2.6-0 under a canary tag. Should be pretty stable if you’d like to upgrade now but expect a final release sometime next week once we finish migrating our products to use the updated version.
Please, don’t hesitate in providing us with any feedback regarding this release. very much appreciated 🙏
Hey @jonathantneal,
Thank you for the clarification. Without knowing much about Stitches internals, I think the assumption that this would work was because a lot of us are coming from styled-components/emotion where it consistently worked that way (and in Stitches < 0.2).
To expand on the use case above, one very common scenario is where we extend UI library components with more app-specific styles:
Where
appButtonStylesoften contains app state/behavior-specific variants, e.g.isOpen,isActive, etc. Not being able to use variants here basically makes one of the coolest features unavailable.With all that being said, is there at all a world where this can be supported, even if it’s in future versions?
I am suffering from what I think is the same bug in v1.2.8.
Edit: I experimented a bit, and the CodeSandbox seems to reproduce this with a basic style composition. This seems beyond the immediate situation in this ticket, but I’m wondering if I’m missing someting (?).
Reproduction: https://codesandbox.io/s/determined-almeida-wbizik?file=/src/App.js
I’m not sure if this is 100% reproducible (perhaps it’s flaky?), so a screenshot is included below. I am on Linux Firefox 99.0.1.
Thank you so much for addressing this. Deleting all the
!importants is going to feel good!This is now fixed in #875 and is pending release so i’m gonna keep the issue open until we release it publicly
I noticed what I think is the same issue with core: https://codesandbox.io/s/priceless-chatterjee-cpqij
I expected that if the
classNamepassed tobase({ className })is a stitches css type, Stitches wouldObject.assignthe two style objects to create a new class.Fela have a dedicated section on this issue explaining why you have to pass your "rule"s through a util to ensure specificity: https://fela.js.org/docs/latest/advanced/combined-rules#problem-order-matters
Seems to be the same issue?
I raised this previously with @peduarte and he had an API concern regarding the proposed solution for ppl who want to compose multiple class names in the component but I’d personally be okay with always doing the following in that case:
It ensures as much as possible is statically composed at least, since the proposed solution means styles wouldn’t be known until runtime I assume? I’m stumped for other ways this could be solved tho 😬
@jonathantneal If we ordered the CSS the way it was originally created, wouldn’t it still break in cases like this? https://codesandbox.io/s/wonderful-sutherland-jtebo
@frgarciames it’s a known bug and @hadihallak is working on it as we speak 👀
I was pointed here from discord, and just for posterity’s sake I’ll copy over some of my message and the example case I put together. Hopefully more examples are helpful!
I believe there was a change in 0.2.0 with the shift to CSSOM that caused some changes in the ordering of styles.
The general problem is when you have a composition chain of components like
StitchesTop > CustomReact > StitchesLow. WhenStitchesTopandStitchesLowshare a variant (let’s imagine it’scolor), andStitchesLowhas a default variant forcolor, the default variant can take priority over the variant given toStitchesTopdepending on the rendering order of components.As far as I can tell this is because
coloris passed toStitchesTop, translated to a classname when passed toCustomReact, which then passes it down toStitchesLowas a classname, but no variant. So it uses the default variant forcolor. Then it’s just a battle between those two variant classnames based on what order the css was inserted.I made a demo of this here: https://codesandbox.io/s/jqbgk?file=/src/App.js. If you uncomment the first
Boxyou’ll see that the last line renders correctly as red.Hi @StephenHaney,
Thanks so much for keeping this on the radar.
Your example is exactly what I would put together. This has proven the most useful in cases where we extend a component with more domain-specific variants like in my example above
Published under
1.2.6🎉Oh right, sorry i missed that. Will modify it on Monday so that the same ordering logic run against forward ref components.
@frgarciames Taking a second look here, @hadihallak is correct. This isnt related. Can you open up a new issue about this so we dont add noise to this one? And we can discuss further. Alternatively, join our Discord and post a message in #help and I’ll start a thread there.
Hi everyone, we’re still evaluating this issue — but it may take a rather fundamental change in how Stitches interfaces with CSS classes to make this possible. It likely won’t be a quick fix. So please keep using workarounds for now and we’ll keep you updated.
@tigranpetrossian and anyone that mentioned expecting this to work because you come from SC/Emotion, can you put together a quick example of exactly what you were doing?
Would it look like this?
Result from SC and the above code when rendering
Button2We have similar use-cases and had to come up with weird workarounds in so many places
I wouldn’t say it’s #wontfix.
Yes, it would be difficult for Stitches to reach into
Buttonand know ofStyledButton.However, we might benefit from a new rendering order algorithm that ensures the order you expect. It seems very reasonable.
I would say, we need more time to think about this issue.
Hi @tigranpetrossian,
In the code example,
Bardoes not extendStyledBase, leaving the injection order up to runtime conditions, whenBarrenders another styled component that happens to beStyledBase.We can resolve this by having
BarextendStyledBase.Fixed example: https://codesandbox.io/s/recursing-grothendieck-phjij?file=/src/App.js
@hadihallak will this new behavior be available to
@stitches/core? https://codesandbox.io/s/x-e6yy7?file=/src/App.js Maybe acxfunction like https://emotion.sh/docs/@emotion/css#cxWhat about this ? We created a react components library with stitches and our typography (and every component) component has default styles and then, when we try to override these styles with styled function it does not override. This is the example code: https://codesandbox.io/s/stitches-override-example-4yqmo?file=/src/App.js:344-346 Is this behaviour right ? @peduarte
Yes, I think ordering styles based on creation should solve this. I’d be happy to test it out if it’s necessary.
That is fully supported. You should be able to do this.
I was commenting on reaching inside a functional component, which I would not like to do since that would run components out of order (running the component for side effects, then attaching the class names, versus what we do which is pass the class name into the component).
We have been working out another ordering issue, which may happen to do what this issue expects. My first response was connecting these issues, when I should have distinguished these. Let me phrase this another way with some code from the reproduction:
Oh yea! I think we can order style injection based on creation. Would this ultimately render what you expect?
I originally thought this issue might be related to our “order by creation” issue, but then I saw, based on the reproduction, that it was more a side-effect.
Perhaps we should reopen this. I bet the team would be in favor! I feel I was a little stuffy in my last response, anyway (sorry)!
I’d like your thoughts, but the new presumption would be that we want the CSS to be ordered the way it was originally created.