cssinjs / jss

JSS is an authoring tool for CSS which uses JavaScript as a host language.
https://cssinjs.org
MIT License
7.08k stars 400 forks source link

Make sure styled-system is supported #974

Open kof opened 5 years ago

kof commented 5 years ago

Styled-System by @jxnblk https://github.com/jxnblk/styled-system seems like a very solid approach for building design systems. I think we should look into it and see how it can be supported.

Todo

jxnblk commented 5 years ago

Haven't really tested it recently, but styled-system should "just work" with styled-jss. Definitely let me know if you run into any bugs though

kof commented 5 years ago

I wonder if it also can work with react-jss and the future version of it based on hooks (prototype https://github.com/cssinjs/react-jss-hook)

HenriBeck commented 5 years ago

It should already work with react-jss

eddort commented 5 years ago

this only works with react-jss @ alpha-10 but if you want to use something like <Box p = {[1, 1/2, 1/4]}> </ Box> you will notice a bug #1044

kof commented 5 years ago

Putting high prio on that bug.

HenriBeck commented 5 years ago

I think the issue is that styled-system is accessing the theme via props and not from our themed styles function.

EDIT: The reason why this is not a problem for styled-components is that they generate an entirely new stylesheet when the props change and not just update the existing rules.

kevinSuttle commented 5 years ago

Would love to see this also.

kevinSuttle commented 5 years ago

What is the relationship between https://material-ui.com/system/basics/ and JSS here?

kof commented 5 years ago

MUI style system is an independent effort by @oliviertassinari inspired by styled-system. It is a separate package https://github.com/mui-org/material-ui/tree/next/packages/material-ui-system and seems to have no direct dependency on JSS, but can be used together.

I am also porting (right now) an SC-like, similar to styled-jss into the react-jss package to support styled-system and SC-pattern users.

kevinSuttle commented 5 years ago

See also: https://github.com/emotion-js/emotion/tree/master/packages/primitives-core

kof commented 5 years ago

@kevinSuttle what specific use case from primitives-core are you interested in?

kevinSuttle commented 5 years ago

Nothing in particular. It's just another approach—one that isn't included in the Prior Art section here: https://material-ui.com/system/basics/#prior-art

kof commented 5 years ago

Turns out styled system doesn't depend on SC pattern at all, those are just functions. I was confused by the fact they expose propTypes, so I thought they must be rendered as components, but actually those propTypes need to be merged into user component manually if you want the warnings.

The only thing that is actually special about styled system is the data structure which is used to return the styles:

[
  [
    {
      "paddingTop": "32px"
    },
    {
      "paddingBottom": "32px"
    },
    [
      {
        "paddingLeft": "16px"
      },
      {
        "@media screen and (min-width: 40em)": {
          "paddingLeft": "32px"
        }
      }
    ],
    [
      {
        "paddingRight": "16px"
      },
      {
        "@media screen and (min-width: 40em)": {
          "paddingRight": "32px"
        }
      }
    ]
  ],
  []
]
space({theme, px: [3,4], py: 4})

Also there is a hard dependency on props.theme. And an optional one on fn.propTypes.

kof commented 5 years ago

Turns out v3 of styled-system just works without any extra code, the only thing one needs to do is to set injectTheme to true:

    const styles = {
      root: compose(
        space,
        color,
        fontSize,
        width,
        fontWeight,
        lineHeight
      )
    }

    const MyComponent = ({classes}) => <div className={classes.root} />

    const MyStyledComponent = withStyles(styles, {injectTheme: true})(MyComponent)

    const renderer = TestRenderer.create(
      <ThemeProvider theme={theme}>
        <MyStyledComponent
          px={[3, 4]}
          py={[1, 2]}
          color="white"
          bg="blue"
          fontSize={[4, 5, 6]}
          fontWeight="bold"
        />
      </ThemeProvider>
    )
jxnblk commented 5 years ago

@kof Heads up: I've started a PR here: https://github.com/styled-system/styled-system/pull/473 and it's now published as styled-system@4.2.0-0 if you'd like to test it out

kevinSuttle commented 5 years ago

How does that affect performance, especially with regards to https://styled-system.com/babel-plugin?

kof commented 5 years ago

@jxnblk nice, gonna try it out in a bit

jxnblk commented 5 years ago

@kevinSuttle the babel plugin package is experimental and does not use the same code base as Styled System core

kof commented 5 years ago

@jxnblk I confirm styled-system@4.2.0-0 works with my tests