FormidableLabs / radium

A toolchain for React component styling.
http://formidable.com/open-source/radium/
MIT License
7.39k stars 308 forks source link

Generated CSS Ordering #790

Open alexlande opened 8 years ago

alexlande commented 8 years ago

The Problem

Right now, classes generated by addCSS aren't guaranteed to be unique (we attempt to deduplicate them), which means that media query behavior doesn't always work as expected. If we use a previously generated media query class which is defined prior to a new base class, the base class will be rendered after the media query class in our stylesheet and will break. For a much better description of this problem, see #593.

@tptee did a lot of work on resolving this one in #635, and found that it was a tough problem. In #788, he says:

After trying a mess of stuff to get #635 resolved, I think the simplest solution is to just order the media queries. Not only does it simplify the deduplication story, it also restores the media query behavior that designers understand and depend on. Yes, it means that rules inside media queries aren't necessarily precedence-order. I think it's worth it.

In the same issue, @rofrischmann says:

You might also want to check out how I handled specificity, order and naming issues within Fela as media query order is not as trivial as is might sound.

This issue will prevent us from using addCSS for the rest of our styles, so we need to get it sorted out.

What Do We Do About It?

Good question. Sorting media queries as @tptee mentioned doesn't sound terrible to me, although it could get complicated when dealing with more complex query statements. (i.e. it's easy enough to order min-width media queries, but what about max-width, min-width and max-width, or any of the other types).

Another thing that I've been thinking about is generating functional CSS-style individual classes per style rule, rather than shared classes.

Instead of this:

.r-1 {
  background: red;
  padding: 10px;
}

.r-2 {
  background: blue;
  padding: 10px;
}

we would have this:

.r-1 { background: red }
.r-2 { padding: 10px }
.r-3 { background: blue }

That way, deduplication of classes in the generated style tag is extremely simple. (See #635 for details on how complex that can get otherwise). This does not help us with ordering of media queries, but it should reduce stylesheet bloat at least.

ianobermiller commented 8 years ago

"individual classes per style rule" trades stylesheet bloat for CSS selector and DOM bloat, I'm not sure which is worse. Aphrodite has taken the opposite approach, where it doesn't even try to deduplicate (we could maybe do the same based on the component name).

alexlande commented 8 years ago

Maybe worth benchmarking. I would expect the resulting DOM bloat to gzip extremely well (and be smaller than inline styles even if it didn't), and would expect the resulting style tag to be much smaller than it would be with style objects as classes.

Aphrodite has taken the opposite approach, where it doesn't even try to deduplicate (we could maybe do the same based on the component name).

I thought Aphrodite hashed style object contents for this purpose.

ianobermiller commented 8 years ago

I guess it would deduplicate if you had the same key and the same contents, so you're right.

robinweser commented 8 years ago

As far as I know, @necolas used to generate single rule classes for Twitter and it turned out to be quite effective. If I am not wrong he's also using this approach with react-native-web. Might be worth checking that out.

necolas commented 8 years ago

I think you're going to have a hard time doing this, because radium decided to support CSS media queries and pseudo-classes. I should also mention that react-native-web no longer uses the "atomic CSS" approach for 2 main reasons:

  1. You can still generate a lot of CSS in large apps. For example, twitter.com would produce >100KB of CSS. I wasn't particularly excited about creating >100KB strings in the browser, or working around it and perhaps re-rendering stylesheets more frequently.
  2. Vjeux did some investigation at Facebook to determine the relative costs of inline styles vs generating and updating CSS (e.g., in response to lazy loading bundles). Overall, there can be some pretty high costs working with stylesheets, and difficulty predicting when you're going to incur those costs in an app.

"individual classes per style rule" trades stylesheet bloat for CSS selector and DOM bloat, I'm not sure which is worse.

Is there any evidence that adding more classes to the DOM "bloats" it (presumably this is only a concern for SSR too)? Last time I ran an experiment on a large app, the per-page difference was negligible (~1KB) compared to the savings in CSS.

Another data point from a much smaller example, the generated HTML for the "Game2048" example in react-native-web:

Inline styles:

Input: 18.35 KB
Output: 18.30 KB
Gzip: 1.27 KB

All styles removed:

Input: 2.31 KB
Output: 2.29 KB
Gzip: 643 bytes
quinn commented 6 years ago

is there a way to add stuff directly to StyleKeeper? My current workaround for this is to generate all of the media queries I know i'm going to need for a grid by generating all of the components ahead of time in the correct order (for me, this is starting with the smallest breakpoints first and iterating up). Is there there a way i can call addCSS directly instead?

quinn commented 6 years ago

@ianobermiller @alexlande any ideas how i could prefill using the StyleKeeper?