Readme best practices: Where to put styles?

[mattkrick]

Something I'd like to see in the readme is where to put the styles. I see them above & below for the examples, but on a pretty large project (like Khan academy) where do you stick the styles?

• Above the component
  • Pro: Follows eslint's no-use-before-define.
  • Con: styles take main stage over component
• Below the component
  • Con: breaks eslint rule so you either have to make an exception in every file or disable it
• Declare styles above (let styles = {}) and redefine below the component
  • Pro: follows eslint rule
  • Con: ugly
• In another file
• Other?

[kentcdodds]

I think this is a good opportunity to talk about an ESLint plugin. I think @nmn started working on one and @lencioni showed interest as well.

Personally, here's how I've done my styles:

function MyComponent() {
  const {styles} = MyComponent
  // etc.
}

MyComponent.styles = StyleSheet.create({/* styles stuff */})

I like doing it this way because it makes the relationship more visually explicit. It also looks natural when combined with the propTypes and defaultProps properties.

[lencioni]

We have been using context to share "theme" information, like colors, fonts, breakpoints, and stuff like that. So, I recently wrote a HOC for adding styles to a component. Here's an example:

function MyComponent({ styles }) {
  return (
    <div className={styles.container}>
      Try to be a rainbow in someone's cloud.
    </div>
  );
}

export default withStyles(({ color, unit }) => ({
  container: {
    color: color.primary,
    marginBottom: 2 * unit,
  },
}))(MyComponent);

I hope to open source these things as well as publish a style guide like document on how we approach styling with Aphrodite, but there are some hoops to jump through first.

[mattkrick]

@lencioni that's interesting... so you have 1 HOC for every styled component? By chance does that HOC extend a component that already exposes your context.theme as a prop?

Also, is that example all the same file? I'm concerned that it'll start looking ugly after I withRouter(withStyles(withHotkey(reduxForm(.....

[lencioni]

We have 1 HOC for every styled component. It does not extend anything. Edit: I'm suddenly not sure I understand what you mean by "1 HOC for every styled component". We have 1 HOC called withStyles that we use everywhere.

We also have a singleton where we register themes (which are just bundles of variables and functions), and a provider component where we can tell subtrees to switch to a different theme.

<ThemeProvider theme="foo">
  ...
</ThemeProvider>

And yes, this is all the same file. If you have a bunch of HOCs that you want to use, you could certainly break it up by assigning to an interstitial variable instead of stacking them if you want.

Also, this HOC was written in a way that will play nicely as a decorator, if you want to use it as such.

@withStyles(({ color, unit }) => ({
  container: {
    color: color.primary,
    marginBottom: 2 * unit,
  },
}))
export default function MyComponent({ styles }) {
  return (
    <div className={styles.container}>
      Try to be a rainbow in someone's cloud.
    </div>
  );
}

[xymostech]

Most of the styles at Khan Academy are after the component. We don't have eslint's no-use-before-define rule on, so we don't encounter that problem, but I can see that might be a concern. I personally like this because the actual content of the styles doesn't really matter when you're reading the component, and hopefully the style names are descriptive enough to tell you what's going on without actually consulting the styles themselves.

In files with multiple components, I've seen two styles:

• Put all of the styles for the file into one large styles object at the bottom of the file (which sometimes ends up with long, odd names, like buttonInnerWrapper)
• Make a new style object after each component for styles used in that component.

In a couple of other places, we've also tried putting styles into component statics (like what @kentcdodds did but we use React.createClass) which you can see here: https://github.com/Khan/perseus/blob/master/src/widgets/radio/base-radio.jsx#L107

Overall though, I'm not sure I see why it's important to make this a best practice. I think it would be interesting to document the different styles somewhere so people can use what works for them (and presumably a given project should try to remain relatively consistent), but I don't think Aphrodite should be dictating which style people use. Maybe there are arguments for it though.

[kentcdodds]

a given project should try to remain relatively consistent

Yup, this is where an eslint plugin would come in handy. Allow people to configure where they want it to keep the project consistent.

I don't think Aphrodite should be dictating which style people use

I agree 👍

[mattkrick]

@xymostech agreed, no need to tell folks how to write their code, maybe instead of best practices just a few popular options. This is something that affects the broader inline-styles community, and since it's so young, I think it'd be beneficial to get a majority of folks writing styles in the same fashion.

[mattkrick]

@kentcdodds I think your option of using a static makes the most sense (for both functional and class components, as we saw with @xymostech's radio widget). It follows basic linting rules & expresses a coupling between styles & component.

Has anyone tried a separate file approach, like sticking styles.js next to your component? Any strong opinions either way? It seems pretty appealing to me as I could split out imports like typography, global static themes, icons, etc.

[kentcdodds]

I've shared styles by extracting them to a separate file. But haven't felt the need to pull them all out. I wouldn't really see much issue with it though.

[mattkrick]

@kentcdodds go on. can you give me a little detail about shared styles? you'd build certain objects representing CSS classes & import those from multiple components? That seems really intriguing, but for now, I've just been copy n pasting commonalities 🙈 .

[kentcdodds]

Haha, yeah so CSS in JS is just JavaScript so you share it the same way you share anything else. My biggest project using aphrodite is JavaScriptAir.com. Here's an example of shared-styles and a usage

Remember: It's just JavaScript. Don't treat it special or think of it differently. This is hard to do because we're just so used to styling with CSS. Takes a while to get used to CSS in JS and good patterns :)

[mattkrick]

@kentcdodds oh that's fantastic! I was thinking of sharing a POJOs & putting that into my StyleSheet.create, but you are sharing the the output of StyleSheet.create. :+1:

Maybe the result of this issue could be a nice recipes folder like https://github.com/avajs/ava#recipes?

Again, nothing to force the developers hand, but a few smart ideas that might push the community towards a little more structural homogeneity.

[lencioni]

If you are using Aphrodite with React, I think it is good to prefer composing components over sharing styles.

[kentcdodds]

Yeah, honestly it's probably better to just have POJOs 👍

[nmn]

@kentcdodds I have started work on the Eslint plugin. I've spent a long time just compiling rules for CSS properties. It's not nearly strict enough, but its something.

I'll just switch to writing the Eslint part and publish it this week. My weekend was kinda busy.

[ctrlplusb]

@nmn Any update on the eslint plugin?

No pressure, I understand full well given time to OSS can be a challenge. Just interested in hearing what sort of progress you were able to make. :)

[nmn]

I started working on flow types. I'll try and get the eslint plugin working first I guess. And the community can make the tests better over time.

[lencioni]

I've also recently published this CSS-in-JS style guide: https://github.com/airbnb/javascript/tree/master/css-in-javascript

[ctrlplusb]

@nmn I also played around with flow types. I struggled with dynamic props such as media queries though. Here is an example gist.

Did you hit this issue at all?