Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

Styled Components? #4

Closed
kukagg opened this issue Apr 2, 2017 · 20 comments
Closed

Styled Components? #4

kukagg opened this issue Apr 2, 2017 · 20 comments
Assignees

Comments

@kukagg
Copy link

kukagg commented Apr 2, 2017

Hey, Lee!

What are your thoughts on incorporating styled-components instead of SCSS?

@leebenson
Copy link
Owner

leebenson commented Apr 2, 2017

I'm personally find the syntax a little clunky. It feels like a clever abstraction to a problem that needn't exist.

Having separate template functions to represent underlying tags could also get very messy, very quickly - const Wrapped = styled.h1(...) belies the fact that you're generating a h1 under the hood, when you later only see <Wrapped> in your code. If you mean h1, why not just use h1?

Example: The first snippet given on the style-components site is:

import React from 'react';

import styled from 'styled-components';

// Create a <Title> react component that renders an <h1> which is
// centered, palevioletred and sized at 1.5em
const Title = styled.h1`
  font-size: 1.5em;
  text-align: center;
  color: palevioletred;
`;

// Create a <Wrapper> react component that renders a <section> with
// some padding and a papayawhip background
const Wrapper = styled.section`
  padding: 4em;
  background: papayawhip;
`;

<Wrapper>
  <Title>Hello World, this is my first styled component!</Title>
</Wrapper>

The 'ReactQL way' could be achieved as follows:

style.css

.papaya {
  padding: 4em;
  background: papayawhip;
  & h1 {
    font-size: 1.5em;
    text-align: center;
    color: palevioletred;
  }
}

app.js

import css from './style.css';

const Component = (
  <section className={css.papaya}>
    <h1>Hello World, this is my first styled component!</h1>
  </section>
)

There are a few advantages to this approach, IMO:

  1. CSS and UI is separate. style.css is re-usable across any number of components.
  2. The 'papaya' class name is hashed automatically, so there's zero bleed-out to other components
  3. The original React chain and HTML tags remain exactly the same. You're not artificially wrapping stuff or adding abstractions over 'plain' tags just to achieve a style - it conforms to your original markup, which itself should have semantic meaning.
  4. You get the full power of http://cssnext.io/ via PostCSS - nesting, variables, auto browser prefixes, etc. This is further extensible through Webpack, if required.
  5. There's an optimised pipeline that already works on the client & server. Styles get extracted out into a separate style.css that is crunched/minified in production. In dev, sourcemaps are available and CSS is co-located next to the components.
  6. In my experience, it's much easier to edit CSS 'holistically' when you have the full context of related styles at-a-glance in a single file, than try to follow an abstracted HOC chain to figure out how one tag might relate to another.
  7. There's already a light-weight abstraction in place for styling React without dropping down to template strings- className. IMO, there's zero benefit having styles live alongside JS directly beyond linking via class names.
  8. It's potentially easier for teams where designers work side-by-side with front-end developers, but not necessarily on JS directly. There's a slightly reduced API surface area having JS + CSS communicate via classnames rather than embedded JS.

Obviously, YMMV. Much of this is based on personal preference.

To be clear, SASS is included as an option for legacy code that requires it, but the recommended approach is to write 'plain' next-gen CSS a la http://cssnext.io/. If you have a .s(c|a)ss file, it will pass it through node-sass first, but it'll still go through the PostCSS hop to inherit the other benefits like autoprefixing. So SASS is completely optional.

@kukagg
Copy link
Author

kukagg commented Apr 2, 2017

Concise as always. Thank you!

@leebenson
Copy link
Owner

My pleasure @kuka

@j-jackson
Copy link

j-jackson commented Apr 10, 2017

@leebenson I agree with your points above, but would you mind explaining how this approach would work in more dynamic scenarios?

  1. Lets say you have 30 possible sections/components that all have their own './style.css' that a user can select from, but they are only using 5 of them. How would you prevent bundling the other 25 unused components css?

  2. How do you handle dynamic styles and theming? i.e a button component has a default background of red, a possible theme which would make it blue or some other value the user has saved?

I have this working with styled-components now, but as you pointed out, it does get rather messy and harder to work with so I would prefer to stick with PostCSS & cssnext if possible.

@leebenson
Copy link
Owner

@j-jackson - thanks for sharing that use case. I see three possible situations here:

  1. You have pre-defined styles that your users can select from. These are styles that are known ahead of time, that make up your 'global' style sheet. If a user selects, say, the 'blue' theme, you would simply write some logic that determines the theme, and apply it to the style -- something like this:

styles.css

.theme {
  /* main settings that apply to ALL themes */
  font-weight: bold;
  font-size: 1.2rem;

  /* ONLY the 'blue' theme */
  &.blue {
    color: blue;
  }

  /* ONLY the 'blue' theme */
  &.red {
    color: red;
  }
}

components.js

// Lib for dynamically combining classnames
import cn from 'classnames';

// global styles -- pre-defined ahead of time
import css from './styles.css';

// assume that `user.settings.theme` contains the string 'blue' or 'red'.
// This might be gleaned from a DB that stores the user settings, or some
// other source 

const themeClass = cn(css.theme, css[user.settings.theme]);

// Create a Button component with the user's chosen theme
const Button = () => (
  <button className={themeClass}>Styled</button>
)

In this scenario, you already know the user's theme and can set the appropriate classes just by selecting from that existing sheet.

This might be a good choice if you have a limited selection of styles that can easily be described by simple strings.


  1. The user's settings are completely dynamic. In this scenario, it probably doesn't make sense to use CSS at all, since compiling a single styles.css file might have literally millions of possible combinations to accommodate all users.

Instead, we can use the native style= attribute available on standard React component to feed in a style:

// Same <Button> component, but this time it takes the `theme` as a prop
// which is basically a plain JS object of CSS properties
const Button = ({ theme }) => (
  <button style={theme}>Styled</button>
);

// This is the 'shape' we're expecting -- for example
Button.propTypes = {
  theme: PropTypes.shape({
    fontSize: PropTypes.string.required,
    fontWeight: PropTypes.string.required,
  }).isRequired,
};

// Assume `user.theme` is a plain JS object that contains CSS.  We might get
// this from our NoSQL DB or from other async source.  We can now use the
// button component like this...

<Button theme={user.theme} />

  1. Styles are known ahead of time, BUT, you don't want them all to appear in a global styles.css since users are then downloading a bunch of styles that might not apply to them.

Instead, you want to download each .css file dynamically, along with your code-split components that are loaded on-the-fly as needed.

This one is trickier because importing a secondary .css file from within the code-split component MIGHT mean that the .css file code path is scanned in Webpack and added to the main styles.css anyway... or it could mean that it won't get loaded until the component itself is loaded. I haven't tested this use-case, so I'm not sure -- I need to play with it. IMO, the ideal scenario would be that secondary .css files can be loaded on the fly, in the same way as JS. I'm not sure if the css loaders are that smart-- will test.

In any case, this option generally might be overkill. By the time PostCSS has stripped back white spaces, combined tags and generally optimised the css, and gzip has done its thing, including even a few hundred theme classes that may or may not be used is generally only introducing a few KB of overhead.

It's often quicker to load the whole lot in one go (and one HTTP request) than to worry about saving a few bytes with a second HTTP request, as-needed. Obviously YMMV - especially if you're doing lots of fancy things in the CSS like loading extra fonts or included embedded images - but 70-80% of the time in my own projects, fiddling with CSS turns out to be over-engineering/premature optimisation.

In any case, I'd like to test this scenario because it'd be useful to know the code path Webpack + the css loaders take in the scenario of async component loading.

Will let you know what I find.

@leebenson
Copy link
Owner

Re-opening to track point 3 above

@leebenson leebenson reopened this Apr 11, 2017
@j-jackson
Copy link

j-jackson commented Apr 11, 2017

@leebenson thanks for your detailed reply, I have learnt a lot from reading the issues!

I started reading up on css-modules today, and I think a combination of 2 & 3 looks like a good solution to my particular use case, as it would allow for saved user styles via the style= attribute and hopefully dynamically importing the components css.

I'm fairly new to webpack, but I will continue with this tomorrow and let you know if I find anything useful.

@leebenson leebenson self-assigned this Apr 11, 2017
@arvigeus
Copy link

I use styletron as CSS-in-JS solution and like it so far. Syntax is not that clunky. Who said designers have to work with css? Just extract given styled component in its own file and let the designers modify it without bothering with rest of the code logic. It's not that different as having them to learn LESS or SASS for example.
CssNext is not CSS4: CSS4 is far from final draft and lots of things could change. It's not a future proof solution. I personally doubt it will change that much, but if you want to play safe, stick to writing vanilla CSS3 for now.

But like I suggested in #20, CSS-in-JS is an "extra" feature and should not be part of this kit by default. Unless you go the (NextJs)[https://zeit.co/blog/next] route and ditch CSS entirely. CssNext is a good enough solution with vars and other improvements. I like it more than using LESS/SASS abstractions.

@leebenson
Copy link
Owner

leebenson commented Apr 12, 2017

I use styletron as CSS-in-JS solution and like it so far. Syntax is not that clunky. Who said designers have to work with css?

The nice thing about styletron or similar is that enabling them doesn't require webpack changes. A user of this starter kit can yarn add it and start using it, without it being 'supported' in core.

I personally am not a fan of HOC for style. Writing CSS strings in JS doesn't feel natural to me, and in my professional life, the assets I'm delivered from a front-end team are usually plain CSS that I'd then need to wrestle into JS.

I prefer keeping stylesheets separate, but for those that don't, styletron are easy drop-ins.

CssNext is not CSS4: CSS4 is far from final draft and lots of things could change. It's not a future proof solution. I personally doubt it will change that much, but if you want to play safe, stick to writing vanilla CSS3 for now.

This is probably stating the obvious, but for the avoidance of doubt, CSSNext compiles down to plain CSS, so there's no concern of compatibility issues. Even if the CSS4 draft changes, it won't affect any prior compiled styles. Nested declarations are unnested, variables are hard-coded, etc - you're not relying on capabilities of the browser to parse this-- the browser sees 'plain', minified CSS.

But like I suggested in #20, CSS-in-JS is an "extra" feature and should not be part of this kit by default. Unless you go the (NextJs)[https://zeit.co/blog/next] route and ditch CSS entirely. CssNext is a good enough solution with vars and other improvements. I like it more than using LESS/SASS abstractions.

Per #10, I think CSS and third-party (LESS/SASS) support can be relegated to an option at installation time. If you're using styletron or similar, you can simply drop them in at your convenience.

The CSS compilation pipeline is one of the tricker aspects of setting Webpack up correctly. Getting import statements to play nicely across LESS and SASS means adding third-party libs like resolve-url-loader, and configuring things in the right order. It took me a while to figure that stuff out and I've not seen it done properly anywhere, so I think this is still a win for developers and is something I'd like to see offered during installation of the kit.

The point of LESS/SASS is that many third-party frameworks use them, so developers sometimes don't get the luxury of starting from scratch with their own tooling. If you want to edit themes in Semantic UI, for example, you're stuck with LESS. I want to make it simple to play nicely with common themes/frameworks.

To that end, I should note that SASS/LESS integration currently works alright, but isn't completely finished. There are differences in the way imports are handled, especially when pulling from node_modules. Those fixes are on my to-do after playing about with code-splitting styles.

@arvigeus
Copy link

arvigeus commented Apr 12, 2017

CssNext is not CSS4: CSS4 is far from final draft and lots of things could change. It's not a future proof solution. I personally doubt it will change that much, but if you want to play safe, stick to writing vanilla CSS3 for now.

This is probably stating the obvious, but for the avoidance of doubt, CSSNext compiles down to plain CSS, so there's no concern of compatibility issues. Even if the CSS4 draft changes, it won't affect any prior compiled styles. Nested declarations are unnested, variables are hard-coded, etc - you're not relying on capabilities of the browser to parse this-- the browser sees 'plain', minified CSS.

This is a lib issue. The browser will always get easy to digest css, but if (theoretically) CSS4 changes specs for var(--name) CssNext would probably drop the old syntax in future release and you'll have to either upgrade your codebase, or just stuck with older CssNext version. PostCss could mitigate things with a separate plugin allowing old and new syntax, but this is a bad solution (especially for a lib trying to abstract such issues away from user).

Per #10, I think CSS and third-party (LESS/SASS) support can be relegated to an option at installation time. If you're using styletron or similar, you can simply drop them in at your convenience.

Settings are evil. There is nothing wrong for having them, but each option add another use case to support.

Unfortunately no solution can completely replace CSS and things would remain "foggy" for a long time.

@leebenson
Copy link
Owner

leebenson commented Apr 12, 2017

This is a lib issue. The browser will always get easy to digest css, but if (theoretically) CSS4 changes specs for var(--name) CssNext would probably drop the old syntax in future release and you'll have to either upgrade your codebase, or just stuck with older CssNext version. PostCss could mitigate things with a separate plugin allowing old and new syntax, but this is a bad solution (especially for a lib trying to abstract such issues away from user).

This is unlikely to happen. First, features such as var() are already making their way into browsers, solidifying the likelihood of this being a firm spec. Even if the the syntax changes, it's likely CSSNext would continue to transpile var() into hard-coded variables long into the future, so the 'old' syntax and the new would result in exactly the same CSS. The same could be set of nested variables, etc.

I find nesting incredibly useful to organise styles, and IMO that was the biggest selling point of SASS and other CSS 'abstractions' to begin with. It's likely nesting will be supported natively in future browsers, but for now, CSSNext takes care of transpilation down to unnested classes, so this is unlikely to be a major stickler to maintainability.

Settings are evil. There is nothing wrong for having them, but each option add another use case to support.

What's the alternative? The point of this starter kit is to give developers a leg-up on the complexity of configuring common use cases of building React + GraphQL front-ends. Cutting this back to a 'base' starter kit that disavows CSS and other options contradicts the aim of trying to make complex configurations simpler.

This isn't so much a settings, as it is a template. If a user chooses css support, then the starter kit is decorated with CSS support by injecting webpack config. Without it, it'd simply cut those pieces out.

I'm trying to walk the fine balance of providing a useful starter kit where most of the obvious plumbing is offered done-for-you, but without so much bloat that it becomes a behemoth to manage. Everyone has different opinions on what makes an 'ideal' stack. I'd like to make it easy to skip the tedious set-up with complex config, but also make it simple to drop it when it's not needed.

I'm open to ideas for how to play that better.

@kukagg
Copy link
Author

kukagg commented Apr 12, 2017

Initially this ticket arose from the interest of why there were made certain choices like CSSNext. I’m of opinion that current state of this starter-kit does a great job at giving the initial setup. It saves a decent amount of time with the least amount of friction so those who are not that familiar with React + GraphQL can get up to speed quickly. That’s the whole point.

As for CSS implementation, replacing it with any other can be done relatively quickly.

The other point is that this repo could gain from a few branches with different implementations of styles and anything else for reference similar to what react-starter-kit has. That way main branch could be kept as simple as possible when other branches could provide a decent point for some variation in this stack. Everyone can put their effort into that without disturbing the main “KISS” branch.

@leebenson
Copy link
Owner

@kuka - I like the idea of branches. That would make it super clean.

@arvigeus - what do you think to that?

@arvigeus
Copy link

Maintaining multiple branches could be a lots of work, but with enough maintainers it's doable.

@leebenson
Copy link
Owner

I dropped a comment here that follows on from the idea of having different branches, BTW.

@leebenson
Copy link
Owner

Moving to reactql/kit#5

@leebenson
Copy link
Owner

Just an FYI for those following this thread - I've updated this issue with an experiment on how Webpack handles CSS on dynamically loaded components.

@leebenson
Copy link
Owner

I know this is an old issue so may no longer be important to the OP, but v3 of the kit introduces Styled Components. I don’t consider my original observations to be valid any longer— I’ve used SC very successfully in production so it’s now part of the kit.

@togume
Copy link

togume commented May 19, 2019

@leebenson - I don't see SC mentioned anymore on the latest version. Has this changed since your last comment above?

@leebenson
Copy link
Owner

yep, I switched to Emotion. Similar premise, but with a better API IMO.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants