Responsive variant order is applied incorrectly
Bug report
Describe the bug
I have a Stack component that is using the responsive variant syntax to define an @initial value, a @md, and a @lg value for its gap variant. At the @lg breakpoint, the @md value is being applied instead.
It looks like it is being caused by the fact that a parent Stack component is using the same breakpoint -> value variant. In my example, I have a parent Stack that has { '@initial': 8, '@lg': 20 } for its gap variant, and a descendent Stack with { '@initial': 8, '@md': 10, '@lg': 20 } for its gap variant. The @lg values seems to be clashing. You can verify this by changing the parent Stack's @lg value to something other than 20, or by removing it completely, and the variants are applied in the correct order.
I imagine what is happening is that the @md class is generated and injected after the @lg class, and the cascade does the rest.
To Reproduce
Expected behavior
Responsive variants are applied in the correct order.
Screenshots
Here's the variants being incorrectly applied:
System information
- Version of Stitches: 1.2.6
@hwhmeikle Hey ๐
Styles are generally injected by the order of rendering (with a few exceptions that weโve introduced recently) so for this case, when multiple variant values might show up on different breakpoints, our recommended approach is to make use of ranged media queries so that the different breakpoints arenโt fighting with each others based on the order that they are injected in.
Please see this modified codesandbox and let me know if it matches the output you would expect.
Iโve modified stitches.config.ts to include a range version of the breakpoints and then used it in App.tsx on line 26
Hey, thanks for the quick reply!
That definitely seems to solve this scenario. This might be a contrived example, but what would you suggest if we were skipping a breakpoint e.g. '@initial': 8, '@rmd': 15, '@xl:': 30}? Because @rmd only ranges between md/lg, at the lg breakpoint it reverts to what was defined at @initial. I'm leaning towards '@initial': 8, '@rmd': 15, '@rlg': 15, '@xl:': 30}.
Perhaps that's just part of the discipline of using ranged media queries inside your responsive variants, you have to ensure you're defining a value at every breakpoint between your base/highest variant.
Yes, what you suggested is probably how I would do it.
We're also thinking about ordering the responsive variants styles by the order of breakpoints in the media object so that your original code would work without defining ranged media queries since @lg would always be injected after @md no matter what but we still haven't fully decided on that yet.
Hey all, jumping into this conversation.
While I understand what @hadihallak is saying/suggesting I think this is such a weird way to solve the issue :(. in @hwhmeikle example is pretty simple and shows the use case, but you can perfectly have a huge page with many Stack components rendered in many different places with many different variants.
So a random <Stack gap={{ "@initial": 8, "@lg": 20 }} > somewhere in the page could affect badly another random <Stack gap={{ "@initial": 8, "@md": 10, "@lg": 20 }}> somewhere else in the page ๐ข๐.
I also understand the suggested solution of having range medias to target specific medias, but that is not really the solution because the "bad" css selector is already injected in the DOM in the order it was injected, so another <Stack component could be instantiated elsewhere with the "wrong" responsive variants too.
Basically what I'm saying is that an instance of Stack component must not care about what other Stack instances might have done with their styles.
To finish up on the range medias approach: yes, you could use range medias everywhere, and that would solve the problem, but that is not how you usually write css/styles. It'll be pretty annoying having to specify the styles for all medias all the time (because you can't really use normal medias, ex: (min-width: 740px) because then you might be the one injecting the "bad" css selector to the DOM)
Ok, so, what could we do?
I haven't looked at the internals of stitches, but perhaps, instead of creating css classNames for each component + variant + value combination, we could try to create unique css classNames for the whole variant per se.
So for example, say this is the prop gap={{ "@initial": 8, "@md": 10, "@lg": 20 }},
it currently generates 3 classNames:
c-dhzjXW-ctMUrj-gap-8 // with its media query selector
c-dhzjXW-cJJBop-gap-10 // with its media query selector
c-dhzjXW-deging-gap-20 // with its media query selector
We could generate a unique className for the group, example c-dhzjXW-ctMUrj-gap-cccppp that has the 3 mediaqueries inside.
Then some sort of memoization strategy is applied and if any other Stack instance uses the same exact props then it reuses that className, otherwise generate a new className for the new group, and so on and so on.
Performance? This is maybe? less performant than what stitches has right now, but again, a component instance styles shouldn't care what other component instance styles have; nor we should be using range media queries
@hadihallak I have to agree with @csantos1113 that defining ranged media queries everywhere isn't the best developer experience. I like your solution around ordering the responsive variants by how they're defined in the media object.
@csantos1113 @hwhmeikle
Basically what I'm saying is that an instance of Stack component must not care about what other Stack instances might have done with their styles.
I 100% agree with that statement โ Rendering order should never produce different visual results. The solution I suggested is in no way the final form and the recent release to stitches mainly focus on fixing and improving on some of the most pressing injection order issues.
we've also experimented with the idea of generating unique hashes but as you mentioned, this is likely to cause performance implications so we're not set on that yet.
All in all, I do think that this is a bug, and the recommended approach for now is to use ranged media queries as a workaround until we provide a fix for it which is likely land once we figure out React 18 and concurrency support. Until then, we're gonna keep this issue open.
I've hit the same issue when developing a grid-system. The problem occurs when a combination of media/variant is used a second time โ as the combo was found before, its rendering is market as cached, and therefore it isn't rendered again. Not a problem generally, but a problem if there are other media selectors on the same component which would match as well and weren't rendered before: then, the last found is rendered last, and gains specificity.
Solutions
No automatic solution is possible, as this is how CSS works. If you are doing mobile-first, this might be a pain and seems possible to fix, but you could as well be using other kinds of media queries or even top-to-bottom media selectors, and therefore Stitches cannot assume it.
1) Configuration:
I would suggest stitches renders media styles in split up style tags โ following whatever order is found on the createStitches.media.
We should advise users that numeric keys could break this, for the well know order of numeric props issues on JavaScript.
2) Workaround:
Avoid using variants, and use local variables instead:
// From
const Component = styled('div', { variants: { color: { blue: { color: 'blue' }, red: { color: 'red' } } } })
<Component color={ { '@bp1': 'blue', '@bp2': 'red' } } />
// To
const Component = styled('div', { $$color: 'initial', color: '$$color' })
<Component css={ { '@bp1': { $$color: 'blue' }, '@bp2': { color: "red' } } } />
3) Dynamic styled-components:
Create a wrapper component that generates one new styled-component per media/variant combos found. This is probably not performant, but could serve as a workaround.
Here is a codesanbox example for 2) and 3).
stitches folks, are there any updates on this?
maintainers are so quiet lately. Started to worry about this lib not longer being maintained? ๐
@csantos-nydig not currently this is going to be fixed but not as a minor update as it requires some decent re-work of the internals and rigorous testing before this is fixed. so, rest assured that Stitches is maintained and we're working on some exciting things that we hope to share very soon
Hey @hadihallak,
Is there a status update? Unfortunately, we are now running into exactly the same problem.
Thanks!