astroturfcss / astroturf

Better Styling through Compiling: CSS-in-JS for those that want it all.
https://astroturfcss.github.io/astroturf/
MIT License
2.28k stars 60 forks source link

API musings #320

Open jquense opened 5 years ago

jquense commented 5 years ago

Since we've got a lot of new-ish api surface area i thought it would be helpful to maybe talk through a more unified vision and approach. Below are a few API options I think are feasible technically but obviously have some tradeoffs

Unified style to props direction

with the following constraints

decss style

Pros

Cons

const Button = styled.button`
  display: inline-block;

  &.busy {
    background-image: url(...);
  }

  &.variant-primary {
    color: white;
    background-color: blue;
  }
  &.variant-secondary {
    color: black;
    background-color: palegray;
  }
`;

css prop

function Button({ variant, ...props }) {
  return (
    <button
      variant={variant}
      css={css`
        display: inline-block;

        &.variant-primary {
          color: white;
          background-color: blue;
        }
        &.variant-secondary {
          color: black;
          background-color: palegray;
        }
      `}
      {...props}
    />
  );
}

decss with custom props

Pros

Cons

const Button = styled.button`
  display: ${p => p.display || 'inline-block'};

  &.busy {
    background-image: url(...);
  }

  &.variant-primary {
    color: white;
    background-color: blue;
  }

  &.variant-secondary {
    color: black;
    background-color: palegray;
  }
`;

css prop

same as above

styled components/emotion

Pros

Cons

const Button = styled.button`
  display: inline-block;

  /* this would use css props, but doesn't technically need to. hard to optimize tho*/
  background-image: ${p => p.busy && 'url(...)'};

  ${p =>
    p.primary &&
    css`
      color: white;
      background-color: blue;
    `}

  ${p =>
    p.secondary &&
    css`
      color: black;
      background-color: palegray;
    `}
`;

css prop

function Button({ variant, ...props }) {
  return (
    <button
      css={css`
        display: inline-block;

        ${variant === 'primary' &&
          css`
            color: white;
            background-color: blue;
          `}

        ${variant === 'secondary' &&
          css`
            color: black;
            background-color: palegray;
          `}
      `}
      {...props}
    />
  );
}
jquense commented 5 years ago

css @ai @taion @sloria @itajaja interested in what ya'll think is a good direction or what you're currently happy or unhappy with, pain points etc.

ai commented 5 years ago

I prefer decss because of the better readability

taion commented 5 years ago

In terms of "bad typescript support for css prop" as a "con" – do any of these cases handle this well?

Also, which of these gets least-mangled by Prettier? :p

ai commented 5 years ago

In this case “decss with custom props” will be my favourite option

jquense commented 5 years ago

In terms of "bad typescript support for css prop" as a "con" – do any of these cases handle this well?

The fully dynamic one does, since it avoids needing to separately pass in props, you can rely on the scope and compile to something like:

css={_helper(styles, [variant === 'primary', variant === 'secondary'])

I can't think of a good way to do that.

I do think the custom properties is a good pairing to the decss api, and i really don't want to get rid of it, it's ended up being a really solid convention. I do wonder if it's worth supporting both or if that's too confusing? I'm also not a huge fan of increasing the runtime for folks not using a feature, but I guess they could specifically opt in/out

One thing I never liked about the old 0.4.0 component API was the asymmetry between the way css worked. In the context of a styled tag it "returned" a class but as a stand alone usage it returns the whole style module. Now that we have interpolations we could let css always be a single class but i think that's probably not a good idea.

itajaja commented 5 years ago

just quickly skimmed the examples, I think having the css tags inside css tag impacts readability in the latter proposal

jquense commented 5 years ago

for the css prop, you can use a template literal or plain string if you want without the css tag. HOWEVER, the ${variant && css} for defining classes requires some sort of sigil, otherwise we can't differentiate it from basic value interpolation. e.g. this is more css vs this is a literal string from props like color.

tbf I agree with you on readability, tho it seems to be what all the other libraries do, in their case tho it's required for runtime functionality.

jquense commented 4 years ago

ok here is my suggestion i think for an overall API

stylesheets

Move the css to return single classes and add stylesheet or similar tag to do the classic "css module in a JS file" giving us something like

const alignLeft = css`
  text-align: left;
`

const utils = stylesheet`
  .text-upper {
     text-transform: uppercase;
  }

 .text-lower {
     text-transform: lowercase;
  }
`

Styled components

Leave this API as is mostly, marketing this approach as a higher level version of the css prop. This API would include the decss convention, since it's very well suited for it, and nicely solves the props forwarding issue SC and Emotion suffer from.

CSS prop

This is the flagship API, and should be generally preferred to the Styled api for non-trivial cases since its lightweight, less runtime code, and can fully express the styled API. We still need a prop-based mapping technique tho and so i suggest this API:

function Button({ variant, ...props }) {
  return (
    <button
      css={css`
        display: inline-block;

        ${variant === 'primary' &&
          css`
            color: white;
            background-color: blue;
          `}
      `}
      {...props}
    />
  );
}

It's easy to type in flow/typescript, unambiguous enough to differentiate from normal dynamic (css custom prop) interpolations, and avoids the prop forwarding issue. We may decide to also support the decss API here, but that's awkward, and I think that it's probably not adding much ergonomically in this particular case, but may be free to add so IDK.

Tooling going forward

We will probably not be able to rely on css-loader's built in css-modules support forever. Mostly due to css-module infra being dead and basically unmaintained. I'm working on a prototype approach using @tivac's modular-css which gives us maintained foundation as well as more features (such as correct selector interpolation from other files). I'm also working on a way forward for css modules in webpack that doesn't rely on css-loader, by extracting our the css-module stuff on top of css-loader (https://github.com/jquense/css-module-loader) this may be the "official" recommendation for using css-modules in webpack in the future.

If that all comes to be, it does give use the opportunity to start doing css parsing in the astroturf pipeline which would open opportunities to have nicer more focused APIs. In particular, if we could parse the styles we could differentiate between property value interpolation, selector composition, and value insertion. This would mean better errors, as well as simplifications like not requiring the css tag in the class/props interpolation example above

We also need a recipe for publishing astroturf using packages as libraries, and ideally have them still be easily extendable. A nice benefit of using modular-css here is there is already a lot of good non-webpack tools, e.g for rollup or building clis

mikestopcontinues commented 4 years ago

I very much like adding a bit of runtime dynamism too astroturf. It's my preferred lib, but having just tried a project with styled-components, I feel I'm in the no-man's-land of css-in-js requirements. Having been looking around (again) recently, I've been wondering:

What if astroturf filtered out dynamic styles and simply pipe them into the style prop of the underlying component? After all, style is the recommended way of handling dynamic styles. And the cool thing about this is that it would mean astroturf gets to avoid runtime code when using a virtual dom library.

This could also open up theming via context, and support most syntax pretty gracefully.

Given:

import {solid} from './borderStyles.js';

const Button = styled.button`
  display: inline-block;
  background-image: ${p => p.busy && 'url(...)'};
  ${({theme}) => css`color: ${theme.primaryColor}`}
  border-style: ${solid};
`;

And, to support theming:

import ThemeContext from 'astroturf/theme-context';

function App() {
  return (
    <ThemeContext.Provider theme={{primaryColor: '#fc9'}}>
      <Button />
    </ThemeContext.Provider>
  );
}

We could reasonably expect something like:

import ThemeContext from 'astroturf/theme-context';
import {solid} from './borderStyles.js';

function Button(props) {
  return (
    <ThemeContext.Consumer>
      {(theme) => (
        <button {...props} className="button" style={{
          backgroundColor: (p => p.busy && 'url(...)')({...props, theme}),
          ...(({theme}) => css`color: ${theme.primaryColor}`)({...props, theme}),
          borderStyle: solid,
        }} />
      )}
    </ThemeContext.Consumer>
  );
}

The mechanism would require some string parsing to support all of the above configurations, but it doesn't seem like a terrible compile-time price to pay, unless I'm way off in this. And if I am way off, I'd love to know how. I haven't dug into the code, so I'm only looking at this as a consumer.

Jayphen commented 4 years ago

This is my solution for dynamic styling — just using CSS. This is nothing groundbreaking, perhaps there's some reason this isn't the recommended way?

const styles = css`
  .button {
    color: red;
    &[data-bigly] {
      font-size: 3em;
    }
  }
`

const IndexPage = () => (
  <Layout>
    <Button>I am small</Button>
    <Button large>I am large</Button>
    <Button
      css={css`
        color: pink;
      `}
    >
      I am whatever I wanna be
    </Button>
  </Layout>
)

function Button({ className, large, children }) {
  return (
    <button
      className={classnames(styles.button, className)} /* Merge base styles with styles applied via css prop */
      data-bigly={large} /* apply a data attribute based on props for dynamic styling */
    >
      {children}
    </button>
  )
}
jquense commented 4 years ago

@Jayphen that's effectively like making another class isn't it? except you run into the same potential issues as normal css, e.g. namespace conflicts. using a .bigly class you'd at least get a hashed name

Jayphen commented 4 years ago

@jquense What do you mean by namespace conflicts?

If styles.button returns Button__generated_style_whatever as the className string, the above CSS would compile to

.Button__generated_style_whatever {
  color: red
}
.Button__generated_style_whatever[data-bigly] {
  font-size: 3em;
}

So there are no namespace conflicts there, since the data attribute selector is scoped by the dynamic className.

This also works, but the ergonomics aren't great:

const styles = css`
  .button {
    color: red;
    &.conditionalStyle {
      font-size: 3em;
    }
  }
`;

const IndexPage = () => (
  <Layout>
    <Button>I am small</Button>
    <Button large>I am large</Button>
    <Button
      css={css`
        color: pink;
      `}
    >
      I am whatever I wanna be
    </Button>
  </Layout>
);

function Button({ className, large, children }) {
  return (
    <button
      className={classnames(
        styles.button,
        className,
        { [styles.conditionalStyle]: large } /** the conditional className cannot have dashes. [styles['conditional-style']] does not work */
      )}
    >
      {children}
    </button>
  );
}
Jayphen commented 4 years ago

Here's another example, illustrating how data attributes offer a lot in terms of dynamic styling

const styles = css`
  .notice {
    color: blue;
    &[data-alert] {
      color: red;
    }
    &[data-alert~='normal'] {
      font-size: 1.5em;
    }
    &[data-alert~='urgent'] {
      font-size: 2em;
    }
  }
`;
function Notice({ className, alert = false, children }) {
  return (
    <div
      className={classnames(styles.button, className)}
      data-alert={alert}
      /** This is a contrived and probably silly example, but you can
       * conditionally style this alert based on the `alert` prop.
       *
       * If the prop is not passed, it will be blue.
       * If it is passed as truthy, it will be red
       * if it is passed as 'normal', it will be red and 1.5em
       * if it is passed as 'urgent', it will be red and 2em
       * */
    >
      {children}
    </div>
  );
}
tivac commented 4 years ago

@Jayphen we've been using a similar approach (albeit w/ svelte & @modular-css/svelte) using data-* attributes for minor modifications of the base-level classes. If it's a big change we create another class for it, but for things like color/background/opacity swaps it's super-useful vs doing conditional logic in the class attribute itself or using something like classnames().

mikestopcontinues commented 4 years ago

@Jayphen I use classes for as much as possible, but you just can't do dynamic styles with them. See the ThemeProvider example I raised earlier.

One middling solution is to leverage css custom-props, but browser support is still not great, and there's a lot of overhead dealing with them because you must always refer to them by their string name, since you can't currently use imported vars in astroturf blocks. Personally, I'd much rather be able to write my dynamic and static styles in the same place.

lunelson commented 4 years ago

I really agree with having the css function export a single classname, since it would enable dead-code elimination by finding unused variables.

One enhancement I'd love to see though is allowing modifier or coincident classes to be addressed as properties of that variable: this could follow the css-modules style of syntax, where coincident classes are pulled out by name...

const mystyle, {success, warning} = css`
  color: black;
  &.success {
    background: green;
  }
  &.warning {
    background: yellow;
  }
`;

...or it could do something more like the nyancss convention, where an interpolated modifier class is created, which keeps the specificity of the selector low:

const mystyle, {mystyleSuccess, mystyleWarning} = css`
  color: black;
  &-success {
    background: green;
  }
  &-warning {
    background: yellow;
  }
`;
taion commented 4 years ago

BTW, note that many of these are now available in the v1 pre-releases.