Incorrect style order

[tigranpetrossian]

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

1. Open the following sandbox:
   https://codesandbox.io/s/nifty-blackburn-m792w?file=/src/App.js
2. 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+

[peduarte]

Thanks for that!

[jonathantneal]

Hi @tigranpetrossian,

In the code example, Bar does not extend StyledBase, leaving the injection order up to runtime conditions, when Bar renders another styled component that happens to be StyledBase.

We can resolve this by having Bar extend StyledBase.

const Bar = styled(Base, StyledBase, {...})

Fixed example:
https://codesandbox.io/s/recursing-grothendieck-phjij?file=/src/App.js

[tigranpetrossian]

Hi @jonathantneal,

Thanks so much for the clarification. At the time of opening the issue I didn't even know styled(Component1, Component2, {...}) API existed.

However, in few use cases where I've stumbled upon this StyledBase might not be available (not exported), e.g.

// Button.js

const StyledButton = styled('button', { ...buttonStyles });

export const Button = React.forwardRef((props, ref) => {
  // additional button logic here
  return <StyledButton {...props} ref={ref} />
})

Consuming:

// App.js
import Button from `@ui/Button`

// No access to StyledButton here, since it's an unexported implementation detail of the Button
const MySpecialAppButton = styled(Button, { ...specialAppButtonStyles })

Just curious whether this is a #wontfix, or your solution is a temporary workaround.

[jonathantneal]

I wouldn’t say it’s #wontfix.

Yes, it would be difficult for Stitches to reach into Button and know of StyledButton.

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.

[stefanivic]

Same issue :( We've. used styled on functional components quite liberally coming from emotion. It worked well until the upgrade from v0.1.9 to v1, but finding a workaround for all the places it broke is unfortunately isn't possible, had to roll back.

[Darko]

We have similar use-cases and had to come up with weird workarounds in so many places

[emilmikaelnilsson]

Same. Had to roll back the upgrade as the specificity of styles flipped in a lot of components causing a lot of visual regressions.

[peduarte]

This is being worked on.

[jonathantneal]

Hey, it seems like there was some kind of misunderstanding. I let y’all know a ways back that this issue is not with Stitches.

I’m here to say now that the issue we are resolving will not actually resolve this.

Here’s a distilled version of the original issue:

const StyledBase = styled('button', {
  display: 'inline-flex',
  color: 'inherit'
})

const Base = (props) => {
  return React.createElement(StyledBase, props)
}

const Test = styled(Base, {
  color: 'red'
})

How would Test know to reach into the return value of Base to extend StyledBase? I’m sorry to disappoint you all, but that’s not how JavaScript works. My mistake was not calling that out. I was hopeful another ordering issue would resolve this.

[tigranpetrossian]

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:

// App.js
import Button from `@ui/Button`

// Button is a wrapper around StyledButton with extra logic, however StyledButton isn't exported.
const MyAppButton = styled(Button, { ...appButtonStyles })

Where appButtonStyles often 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?

[jonathantneal]

one very common scenario is where we extend UI library components with more app-specific styles

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:

/* StyledBase renders "button". */
const StyledBase = styled("button", {
  display: "inline-flex",
  color: "inherit"
});

/* Base does not extend StyledBase. Base renders StyledBase. */
const Base = (props) => {
  return <StyledBase {...props} />;
};

/* Button1 extends StyledBase. Button1 styles will overwrite StyledBase. */
const Button1 = styled(StyledBase, {
  color: "red"
});

/* Button2 renders Base. It does not extend Base or StyledBase. Its styles are not necessary before/after StyledBase. */
const Button2 = styled(Base, {
  color: "red"
});

With all that being said, is there at all a world where this can be supported, even if it’s in future versions?

Oh yea! I think we can order style injection based on creation. Would this ultimately render what you expect?

/* StyledBase is created first. StyledBase styles will be ordered first. StyledBase renders "button". */
const StyledBase = styled("button", {
  display: "inline-flex",
  color: "inherit"
});

/* Base renders StyledBase. Base does not extend StyledBase. */
const Base = (props) => {
  return <StyledBase {...props} />;
};

/* Button1 is created second. Button1 extends StyledBase. Button1 styles will overwrite StyledBase. */
const Button1 = styled(StyledBase, {
  color: "red"
});

/* Button2 is created third. Button2 renders Base. Button2 styles will overwrite Button1 and StyledBase. */
const Button2 = styled(Base, {
  color: "red"
});

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.

[tigranpetrossian]

Yes, I think ordering styles based on creation should solve this. I'd be happy to test it out if it's necessary.

[jjenzz]

I noticed what I think is the same issue with core: https://codesandbox.io/s/priceless-chatterjee-cpqij

I expected that if the className passed to base({ className }) is a stitches css type, Stitches would Object.assign the 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:

// compose multiple upfront
const text = css(fontSize, color, {});

function Text(props) {
  return (
    <span 
      {...props} 
      className={text({ className: props.className )} 
    />
  );
}

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

[StephenHaney]

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?

// Using Styled Components
const Button = styled.button`padding: 10px; color: red;`;

const MyButton = (props) => <Button className={props.className}>{props.children}</Button>;

const Button2 = styled(MyButton)`color: green;`;

Result from SC and the above code when rendering Button2

[tigranpetrossian]

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

[TWaltze]

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. When StitchesTop and StitchesLow share a variant (let's imagine it's color), and StitchesLow has a default variant for color, the default variant can take priority over the variant given to StitchesTop depending on the rendering order of components.

As far as I can tell this is because color is passed to StitchesTop, translated to a classname when passed to CustomReact, which then passes it down to StitchesLow as a classname, but no variant. So it uses the default variant for color. 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 Box you'll see that the last line renders correctly as red.

[mikeytown19]

Looking at the docs, in this first example
https://stitches.dev/docs/composing-components

const BaseButton = styled('button', {
// I added these styles for demonstration. 
  backgroundColor: 'black',
  borderRadius: '10px',
  color: 'white',
});

const CheckoutButton = styled(BaseButton, {
  borderRadius: 0,
  backgroundColor: 'hotpink',
  color: 'white',
  '&:hover': {
    backgroundColor: 'deeppink',
  },
});

The CheckoutButton wouldn't be hotpink, it would inherit the styles from BaseButton (if it had some styles). I'm trying to figure out some kind of solution for this. I started switching my works whole UI-library to stitches (because I freaking love it so far) and this issue is creating some hiccups for me.

I tried creating different Variants and DefaultVairants to see if there was some workaround, but this seems wack and I'm sure it's not a good way to handle this.
https://stackblitz.com/edit/nextjs-ac5bme?file=pages%2Findex.js

[peduarte]

@mikeytown19 I updated your example here: https://codesandbox.io/s/adoring-perlman-1dvnh?file=/src/App.tsx:956-966

It's looking good to me. What am I missing?

[frgarciames]

What 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

[peduarte]

@frgarciames it's a known bug and @hadihallak is working on it as we speak 👀

[hadihallak]

Hey @frgarciames I just had a quick look at your sandbox and I think the behavior you're seeing is correct and isn't related to the bug reported in this issue.

You seem to have a `kind="primary" variant that's giving the H2 the red color from the css custom property and variants win over base styles even when extending so you're gonna have to not trigger the default variant or override the styles in the wrapping component.

Screenshot of the variant I'm talking about:

t

[frgarciames]

Hi @hadihallak, so the only way to have a component with default variants and override some properties is adding !important to that property ?
Or the correct way it should be to not have default variants and apply variants when use it ?

Edit: But adding css property to the component it overrides that property. Shouldn't be the same behaviour with styled function ?

[peduarte]

@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.

[rafgraph]

Hi @peduarte, I think you mentioned the team is working on the Stitches style injection issues. Just wanted to share a few thoughts with regards to StyleX's atomic strategy, which I really like.

cc: @hadihallak

• I think the idea of using atomic css classes paired with javascript that's responsible for ensuring no element receives conflicting atomic classes makes a lot of sense.
• This can work with both runtime style injection (no tooling!) and build time style extraction (better performance!). To start, there could only be runtime injection so no need to create tooling.
• This can work with the styled() function to create components (which I generally prefer over the className prop).
• Using atomic class names as unique keys to represent specific styles allows for multi level styled() composition with proper style overrides, even when there are non-Styled components between two styled() calls.
  • General narrative: each <Styled> extracts any atomic classes (unique keys) it receives as part of its className prop; it combines the received atomic classes with the styles passed into the styled() function (when the component was created); it passes down a set of atomic classes that have no style conflicts and are ready to be rendered in the DOM or passed into another <Styled>. It doesn't matter if the resulting class names are passed to a non-Styled component that itself renders another <Styled> so long as the atomic class names are passed through.
  • Note that each <Styled> is autonomous, it receives styles when the component is created using the styled() function, and it receives atomic class names (unique keys) at runtime (passed down from a parent <Styled>), it then ensures that the atomic classes it passes down have no conflicting styles and are ready to render in the DOM (or be passed to another <Styled>).

Here's an example of multi-level styled() composition, adapted from #671 (comment)
The original example showed it was impossible to have Button2 styles overwrite StyledBase, but that is now possible with this approach.

/* StyledBase renders "button". */
const StyledBase = styled("button", {
  display: "inline-flex",
  color: "inherit"
});

/* Base renders StyledBase and passes through the className prop */
const Base = (props) => {
  return <StyledBase {...props} />;
};

/* Button1 renders StyledBase. Button1 styles will overwrite StyledBase. */
const Button1 = styled(StyledBase, {
  color: "red"
});

/* Button2 renders Base.
Button2 styles are passed to Base in the form of atomic classes as part of the className prop.
Base passes the atomic classes through to StyledBase which combines them with its own styles.
Button2 styles will overwrite StyledBase. */
const Button2 = styled(Base, {
  color: "red"
});

[hadihallak]

This is now fixed in #875 and is pending release so i'm gonna keep the issue open until we release it publicly

[tigranpetrossian]

Thank you so much for addressing this. Deleting all the !importants is going to feel good!

[hadihallak]

@tigranpetrossian Thanks a lot for your patience and the simple reproduction of the issue🙏

[hadihallak]

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 🙏

[tigranpetrossian]

Just got around testing this and looks like it works when wrapping regular functional components with styled, but not forwardRef components. For the latter, base styles still take priority, except for when both regular and forwardRef components are rendered together.

Here's a sandbox with reproduction
You'll need to reload the built-in browser after commenting/uncommenting as HMR doesn't seem to reflect correctly.

[hadihallak]

Oh right, sorry i missed that.
Will modify it on Monday so that the same ordering logic run against forward ref components.

[hadihallak]

Published under 1.2.6 🎉

[mrmntte]

@hadihallak will this new behavior be available to @stitches/core?
https://codesandbox.io/s/x-e6yy7?file=/src/App.js
Maybe a cx function like https://emotion.sh/docs/@emotion/css#cx

[azuline]

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.