stitches icon indicating copy to clipboard operation
stitches copied to clipboard

Should newest styles got override by older style?

Open yosafatkesuma opened this issue 3 years ago • 15 comments

Bug report

Describe the bug

when i try to override older style with newest style, i notice that newest style got override by older style while inspect it

To Reproduce

Codesandbox repro: https://codesandbox.io/s/bug-stitches-022-bw9jr?file=/src/App.tsx

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Defined a theme
import { createCss, StitchesCss } from "@stitches/react";
export type { StitchesVariants } from "@stitches/react";

const stitches = createCss({});

export type CSS = StitchesCss<typeof stitches>;

export const { styled, global, css, getCssString, theme } = stitches;
  1. Create styled component
import { styled } from "./styles";

const StyledText = styled("span", {
  fontSize: 20
});

export default StyledText;
  1. create new css rule
const testStyle = css({ fontSize: 50 });
  1. Insert new css rule to the component
<Text className={testStyle()}>Test</Text>

Expected behavior

Newest styles override older style

System information

  • OS: [Windows]
  • Browser [chrome: 91]
  • Version of Stitches: [0.2.2]

yosafatkesuma avatar Jun 17 '21 02:06 yosafatkesuma

Hey!

Thanks for raising this. It's a good question!

At this current stage, Stitches React Components and Stitches Core Component were not meant be used together.

This is why Stitches React Components offer a css prop.

<Text css={{ fontSize: 20 }}>Test</Text>

You could still abstract your styles into an object:

const styles = {
  text: { fontSize: 20 }
}
<Text css={styles.text}>Test</Text>

The css prop was designed to ensure its styles are always added later in the style sheet, thus having higher specificity.

PS: you may want to consider using variants instead.


At this stage, we're not planning on spending time on this, as we have other higher priorities issues. But let me know your thoughts!

peduarte avatar Jun 17 '21 08:06 peduarte

Since a recent update to 0.2.2 from 0.1.9, we've noticed the same.

In a brief chat about this issue on Discord, I could only think of one reason why one would use the CSS API (not the prop) and that's so that classNames can be applied conditionally in combination with nicer APIs like the one from the classnames package. That being said, I think the variants and compoundVariants APIs do just that.

It sure would be a pain to go back now and migrate it all. The only feedback I could give regarding this is, perhaps a clearer statement on the documentation page? Because, it does require a mindset switch and getting used to after using stitches for a bit. If we could point people to the docs where this is stated it would make life much easier 😁

Darko avatar Jun 28 '21 12:06 Darko

Same thing happened to me upgrading from 0.1.9 to 0.2.2.

I have a Flex which has a variant wrap that defaults to flexWrap: noWrap (similar to this).

const Container = styled(Flex, {
   ...
   flexWrap: "wrap"
   ...
})

<Container /> --> noWrap
<Container wrap="wrap" /> --> wrap

When I declare a Container on top of Flex, stitches decided to prioritize noWrap (default variant) rather than wrap (custom styles) and I have to fix all from <Container /> to <Container wrap="wrap" /> in order to override default variants (same with other variants like align, justify, etc), which is a little annoying since I have to migrate (I initially thought SSR broke at first) and my purpose of using a custom component is to abstract all of the stylings out of my React components.

huypham50 avatar Jul 06 '21 05:07 huypham50

@phamstack interesting. Can you provide an example? Thanks!

peduarte avatar Jul 07 '21 06:07 peduarte

@phamstack interesting. Can you provide an example? Thanks!

@peduarte https://codesandbox.io/s/broken-fire-wf0t6?file=/src/App.js

huypham50 avatar Jul 07 '21 14:07 huypham50

@phamstack this is because base styles can't override variants.

If you compose a component that sets default variants, and you wanna override that default style, you need to essentially override the variant. This is by design.

peduarte avatar Jul 12 '21 07:07 peduarte

@peduarte so essentially you're saying packages/third party ui component written with stitches may only be used on project written in stitches right? because telling projects using class names to just have some of their css as an inline css makes no sense. stitches components HAVE to be able get override their styles by classNames to be useable on all stacks.

giladv avatar Sep 25 '21 06:09 giladv

@giladv I didnt say any of those things. The examples provided are all using Stitches.

Is the problem youre describing theoretical, or do you have a real-world reproduction to share?

peduarte avatar Sep 27 '21 11:09 peduarte

@peduarte first of all my surprised reaction might have came a bit aggressive on a second read - that was not my intention.

so just to clarify i'm using a ui library written as a stiches styled component, lets say:

export const Text = styled("span", {
  fontSize: 20
});

now if i'm trying to override the font size while using this component like so:

const testStyle = css({ fontSize: 50 });

<Text className={testStyle()}>Test</Text>

the original style with the fontSize: 20 overrides my fontSize 50. is this an expected behavior? or a bug in the ui library i'm using?

giladv avatar Sep 27 '21 11:09 giladv

@giladv no worries, thanks for that

At this current stage, this is by design. Stitches Components offers 1st-class APIs for overriding styles. Variants and css prop.

I'm open to re-consider this if it's something people really really want, but first I'd like to promote the API's above.

I am not convinced the pattern in question is valid. I appreciate that it's a simple example, my initial reaction would be first to question the project structure and understand why this is happening.

peduarte avatar Sep 27 '21 12:09 peduarte

i'll explain why this is a big issue IMO: there's a very big chunk of projects using plain old css and classes which will have to avoid using open source/third party components built with stitches. because having to use inline css prop for a single css aspect that can't be overridden with a className is a no go.

the most "outer" className should always win.

giladv avatar Sep 27 '21 12:09 giladv

But if it's a third-party CSS file, you should have control over where you inject it within the head, no? If you ensure that that's injected at the end, or after Stitches' CSS in the case of SSR, it should be fine.

I get the issue theoretically, but not convinced it's a problem in the real world.

peduarte avatar Sep 27 '21 12:09 peduarte

To be clear: the current decision to not allow stitches css instances to override styled is open for discussion

peduarte avatar Sep 27 '21 12:09 peduarte

This is actually related to https://github.com/modulz/stitches/issues/671

We're gonna do some experiments and see what we can do to clear this confusion.

peduarte avatar Sep 27 '21 13:09 peduarte

Hey @peduarte - if I may offer a real-world use case where I'm running into this issue (expanding on my comment in this discussion).

We have a React component library currently built with styled-components. We are looking at rebuilding it using Stitches, and one of the big asks that we've had from our users is being able to re-use the styles from our library, but in plain HTML + JS projects. Currently we don't have anything like that available, and I was hoping that Stitches would help us solve that problem (while also improving DX for the React components themselves).

As I noted in the linked discussion above, I think I may have a working solution for the default styles - however I'm running into this specificity issue when trying to override some styles on the consuming app side. And that would be a real use case of where our users wouldn't necessarily be using @stitches/react in their projects, but would rely on @stitches/core instead.

Would love some insight if I'm completely off-base here, and if there's a better way to tackle this problem. Thanks!

EDIT: OK so turns out I was approaching this wrong. No need for @stitches/core in my above use case after all. Currently exploring the following:

Example Button component from React lib:

// ./Button.js
import { styled } from './stitches-config';

export const styles = {
  // style defs
};

export default const Button = styled('button', styles);

To get the styles in plain HTML + JS app:

import { Button, css } from 'my-component-library';

const btn = document.getElementById('my-button');
btn.classList.add(css(Button.styles)());

And to override the default Button styles:

btn.classList.add(css({ ...Button.styles, padding: '$2' })());

This seems to work as expected, both in a React app, as well as a plain JS one.

mryechkin avatar Nov 17 '21 05:11 mryechkin