Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Babel transform #59

Closed
mxstbr opened this issue Oct 13, 2016 · 67 comments

Comments

@mxstbr
Copy link
Member

commented Oct 13, 2016

We need a way (probably babel transform?) to extract and precompile the static styles of our components!

Extracting static styles is actually not what we want, we want to pre-parse the CSS in a build step but leave it in the JS: See this comment

Has anybody done this before? I know I haven't…

@tizmagik

This comment has been minimized.

Copy link
Contributor

commented Oct 13, 2016

Take a look at CSJS, similar template strings construct and has a pipeline to statically extract styles

@mxstbr

This comment has been minimized.

Copy link
Member Author

commented Oct 13, 2016

Good tip, this doesn't look too hard! https://github.com/rtsao/csjs-extractify/blob/master/index.js (browserify though…)

@oliviertassinari

This comment has been minimized.

Copy link

commented Oct 13, 2016

I'm curious about the motivation behind. Shouldn't we aim for the opposite?

What I mean is, injecting as small bits of CSS as possible in the document in order to:

  • reduce network usage when inlining CSS for SSR
  • reduce bootstrap JS execution
  • reduce cache invalidation with code splitting
  • reduce bootstrap browser CSSDOM processing

Does it scale up?

@geelen

This comment has been minimized.

Copy link
Member

commented Oct 13, 2016

@oliviertassinari the reason is that once certain styles are defined they can never change. Some component X will always have some chunk of styles applied. So we can basically parse over the files and extract that into a static mapping, which is effectively all a CSS file is anyway :)

If you're doing SSR, you might prefer not to do this, but if you have a fairly common set of components across the entire site it could be better to extract all those into a static, separately-cached CSS file. But not everyone is doing SSR.

As for bootstrap JS & CSSDOM parsing, serving raw CSS is always going to be faster than serving JS, executing it and dynamically injecting styles. Browsers have been optimising their CSS parsing since... CSS became a thing. So I'd like to make use of as much of that as is possible.

@ndelangen

This comment has been minimized.

Copy link

commented Oct 13, 2016

With the content of the stylesheet classes changing between renders (because they are based on props) how is this going to work?

Whatever CSS is extracted, is only for the initial state? Looking at how it works it seems each different render wich results in a different style output will also result in a new classname..
I don't see an easy way of pre-generating all possible classes.

take this style:

styled.button`
  background: ${(props) => props.theme.main || 'palevioletred'};
  border: ${(props) => (props.width || 2) + 'px'} solid ${(props) => props.theme.main || 'palevioletred'};
`;

Potentially props.width could be any Integer, resulting in infinite classes to generate?

As @tizmagik pointed out, CSJS seems rather similar and also support interpolation inside the template strings, so might deal with the same issue?

I'm trying to understand how this might work, would love to contribute in some way.

@DrewML

This comment has been minimized.

Copy link

commented Oct 13, 2016

This is probably possible to do with the static portions of styles, but, as mentioned by @ndelangen, I'm not sure there is a way to handle anything that uses ${} inside the template.

I'm wondering if Babel may not be the best tool for the job either way? Few problems I can see with that:

  1. Babel transforms run per-file. So, to share style information (so you can get 1 final stylesheet across all JS files), you'd need to write to a tmp file along the way. This would probably become a major problem when you introduce incremental compilation (Babel's watch mode, for example).
  2. Where would the file be written? Since you're not running inside a module bundler, you'd likely have to have your transform accept config in .babelrc, and then just dump to some file the user then includes in their bundler.

My suspicion is that something like this might be much better suited for Webpack + Browserify plugins. I believe you'd be able to solve both 1 and 2 above.

@geelen

This comment has been minimized.

Copy link
Member

commented Oct 13, 2016

I think extracting just the static portion of styles is enough to start with. @DrewML you might be right we might need something more build-tool-specific like a Webpack/Browserify plugin to actually do something with the output css, but as a first step I'd like to transform a single file and extract the css, and so babel seems like the obvious choice, right?

@geelen geelen referenced this issue Oct 13, 2016
@DrewML

This comment has been minimized.

Copy link

commented Oct 13, 2016

as a first step I'd like to transform a single file and extract the css, and so babel seems like the obvious choice, right?

Yep, definitely. I went to go mock up an example for y'all before I left my comment, but then I realized I had been using JSCodeShift instead of Babel 😛 - whoops!. Having said that, definitely possible to update the template literal to only have the dynamic bits.

Knowing nothing about this project (besides that it looks awesome), what would be your suggested approach, while extracting the static styles, to get a dynamic classname onto that component. Need something once those styles are ripped out to associate them with the component.

@SeeThruHead

This comment has been minimized.

Copy link

commented Oct 14, 2016

I've been thinking about content addressable css rules which could be useful here. Each rule that is identified as being static could be output to a stylesheet with a classname that directly maps to the rule contents itself, then you apply that classname to the component. This is nice because you could write the same style rules all over the place and they would only ever end up in the extracted css once. Implementation wise you could build up a { classname: ruleContent } hash and just set every rule, then parse the object into actual css using one of the many tools for that on npm.

@geelen

This comment has been minimized.

Copy link
Member

commented Oct 14, 2016

Yep these are the lines that currently do that logic:

const hash = hashStr(flatCSS)
if (!inserted[hash]) {
const selector = nameGenerator(hash)
inserted[hash] = selector
const root = parse(`.${selector} { ${flatCSS} }`)
postcssNested(root)
autoprefix(root)

Then we'd just need an API to get that on to the class at runtime, but that should be easy enough.

@geelen

This comment has been minimized.

Copy link
Member

commented Oct 14, 2016

@SeeThruHead yeah I've wanted to do that!! But I decided we had to strictly preserve the ordering of definitions. If you have two conflicting rules on an element then using content-addressable css means the one defined later would apply. So you'd need to resolve all the conflicts per component, then only inject the CSS classnames that "win". So I gave up 😄

This was actually issue #1 on this repo, and we've gone through a few iterations to make sure that the CSS you get is in the same order as the Styled Components you write. But if we could resolve things ahead of time it would make the output pretty amazing. And the extracted CSS would be super minimal.

If you're up for trying, let me know! I can throw you all the edge-cases that stumped me 😜

@DrewML

This comment has been minimized.

Copy link

commented Oct 14, 2016

Here is a not-so-pretty example of a Babel plugin that gets the static pieces (it's not removing them from the source for this example). Comes fully-loaded with a million edge-cases, but it gives you a starting point. Open console to see output.

http://astexplorer.net/#/Zlh1EM2eE7/1

@geelen

This comment has been minimized.

Copy link
Member

commented Oct 14, 2016

😍 this is an excellent start!

I think it's safe to assume that if a line has a ${} on it it can be treated as dynamic, yeah.

@tizmagik

This comment has been minimized.

Copy link
Contributor

commented Oct 14, 2016

Then would you strip out all the "static" pieces and leave the dynamic lines in for runtime evaluation? This looks really promising...

@ndelangen

This comment has been minimized.

Copy link

commented Oct 14, 2016

Potentially if proptypes for the prop contains a list of potential values for the prop, we could use it to pre-generate these dynamic classes as well?

Starting with just extracting static styles seems like a good idea that can be expanded on later.

@mxstbr

This comment has been minimized.

Copy link
Member Author

commented Oct 14, 2016

Then would you strip out all the "static" pieces and leave the dynamic lines in for runtime evaluation? This looks really promising...

Yes exactly.

@DrewML that's an amazing start, wow!

@mxstbr mxstbr referenced this issue Oct 14, 2016
@chrislloyd

This comment has been minimized.

Copy link

commented Oct 14, 2016

This might be slightly off topic, but extracting static CSS is going to result in a FOUC when server rendering styled-components. The browser will apply static CSS immediately, then apply dynamic CSS once JS has evaluated. That kind of defeats the purpose of doing it in the first place.

I worry that mixing both dynamic and static CSS in the same declaration might be more confusing than useful. Is there another approach that makes it obvious to developers when they can write static vs. dynamic styles?

Historical note, learning about this FOUC was why I've somewhat abandoned css-loader#287.

@wmertens

This comment has been minimized.

Copy link
Contributor

commented Oct 14, 2016

FOUC could be handled by making the resulting React component have state, and only render itself once the CSS is in the DOM?

@geelen

This comment has been minimized.

Copy link
Member

commented Oct 15, 2016

@ndelangen yeah! I imagine a lot of function interpolations will be of the form props => props.val, props => props.flag && 'val' or props => props.flag ? 'value1' : 'value2'. Sounds pretty achievable to be able to take expressions that have no side-effects and pre-calculate the potential set of values that will be in the output.

@chrislloyd I don't think that FOUC is real. If you're doing SSR for the markup, then extract the "rendered" CSS from rendering the components alongside the <link href="extracted.css">. If, like in react-snapshot, you're pre-rendering using JSDOM or Phantom or something, when you take a snapshot of the DOM you'll get the <style> tags from rendering inline. Then again, if you're doing either of these, you might not even need to use extracted.css. In any case, I don't think it's so much of a potential problem.

@mxstbr mxstbr modified the milestone: Backlog Oct 18, 2016
@pheuter

This comment has been minimized.

Copy link

commented Oct 18, 2016

A benefit I can see of extracting static styles is to prevent flickering when doing server-side rendering. Right now, you would see very basic DOM elements render with minimal, if any styling, and most of it would flicker in when the js bundle loads and inserts the <style> classes.

If we could inline static styles as the components' style prop, then the differential between the initial render from the server and the one driven by the client when it loads would be smaller and potentially reduce the flicker substantially.

Edit: Sorry, just noticed the comment above. That makes sense as well, inlining the actual CSS in the <style>.

@chrislloyd

This comment has been minimized.

Copy link

commented Oct 18, 2016

@geelen does that mean styled components will only support jsdom/phantom style SSR?

@mxstbr

This comment has been minimized.

Copy link
Member Author

commented Oct 18, 2016

@geelen does that mean styled components will only support jsdom/phantom style SSR?

No, not at all. What makes you think that? We're going to extract a static .css file, that has nothing to do with jsdom or phantomjs!

If we could inline static styles as the components' style prop, then the differential between the initial render from the server and the one driven by the client when it loads would be smaller and potentially reduce the flicker substantially.

This is an interesting proposal that I didn't realize was a possibility. The one issue I see is that more complicated styles (e.g. media queries) won't be there for the initial render, but this might be an interim solution until we've built the static style extracting/without a build process… Hmm.

@hikkyu

This comment has been minimized.

Copy link

commented Oct 18, 2016

Helmet doing a nice thing with server usage:
https://github.com/nfl/react-helmet#server-usage

Maybe it's possible to doing something like that

@geelen

This comment has been minimized.

Copy link
Member

commented Jan 25, 2017

@philpl awesome, thanks for kicking this off!

@jacobp100 @wmertens are both right re: interpolations. At the moment we YOLO the entire result so there's absolutely zero restrictions on what someone can output. I wanted to keep it flexible to see what came up as a valid usecase. It's one of the benefits of using strings over objects, you can combine and insert chunks of text much more easily than merging a data structure.

But I agree that it'd be great to strip the CSS back to the simplest possible representation that requires the least amount of run-time overhead. And the only real barrier is restricting what interpolations are treated as valid. Here's what I think we should support:

styled.div`
  ${ ruleSets }
  property: ${ value };
  other-prop: rgba(${ partialValue }, 0.5);
  ${ selector } {
    ...
  }
`
  • ruleSets – generated by calling css. Can have multiple rules, nesting, etc etc. But can only appear where a property: value; or a selector { } would be valid.
  • value – any valid CSS value i.e. no ; or {} allowed except in quoted url(" ... ") expressions
  • partialValue – same rules as value.
  • selector – any valid selector. &s and ,s should be handled as if they were written literally, so nesting can't be known until this gets evaluated.

All of these interpolations can be functions that return values based off props or theme so there would need to be some runtime validation.

Have I missed anything?

@kitten

This comment has been minimized.

Copy link
Member

commented Jan 26, 2017

@geelen I think that sums up all the current possible (and valid) use cases quite nicely. It obviously just comes down to injecting the interpolations at the right place in the AST, since most of the restrictions (interpolations can't contain ;, {, or }, except values which can contain these in quotes) can only be applied at runtime.

@wmertens

This comment has been minimized.

Copy link
Contributor

commented Jan 26, 2017

@mxstbr

This comment has been minimized.

Copy link
Member Author

commented Jan 26, 2017

Right now, styled-components is 87K minimized in my app

Sidenote: That will be half that size already with v2 since we have the new tiny parser. Please try out the v2 branch!

@jacobp100

This comment has been minimized.

Copy link
Contributor

commented Jan 26, 2017

I think it will be difficult to parse mixins as well as selectors. This is ambiguous, but is probably common.

styled`
  ${maybeMixinMaybeSelector}

  ${selector} {

  }
`
@ndelangen

This comment has been minimized.

Copy link

commented Jan 26, 2017

Yeah I've used the above syntax for a grid component. https://gist.github.com/ndelangen/da41797e9e506d6d6f1aca182d516281

@kitten

This comment has been minimized.

Copy link
Member

commented Jan 26, 2017

@jacobp100 @ndelangen Like @geelen said, there are zero restrictions right now as to what can be injected using an interpolation, but a restriction that is becoming more likely is, that entire rulesets have to be wrapped using the css helper. This way during the parsing process, it's possible to put different restrictions on css interpolations, than on normal ones.

I haven't figured most of this stuff out yet though, so if you want to chime in, feel free to check out this WIP PR 😄 styled-components/babel-plugin-styled-components#25

@jacobp100

This comment has been minimized.

Copy link
Contributor

commented Jan 27, 2017

I was looking at this in my project. There’s a few differences to styled-components, but otherwise it’s the same. Firstly, we’re doing Native (you can’t do interpolations for web). Secondly, we split components into those that use CSS custom properties (Dynamic Component), and those that don’t (Static Component). You can ignore the second part for styled-components, but if you’re reading my code, be aware of that!

The gist of what happens is

  1. Iterate through template strings
  2. Convert ${interpolations} into CSS comments, where ${interpolation1} becomes /*substitution-1*/
    • Keep a map of comment to original interpolation value, because we drop them in later
  3. Run through postcss
  4. Convert all comment nodes to mixins using the interpolation from the comment to interpolation map
  5. Convert all comments within decl.values as interpolated values using the interpolation from the comment to interpolation map

So,

styled.div`
  ${mixinInterpolation}
  color: ${valueInterpolation};
`

Becomes

styled.div`
  /*substitution-1*/
  color: /*substitution-2*/;
`

Where you have a substitution map,

{
  '/*substitution-1*/': mixinInterpolation,
  '/*substitution-2*/': valueInterpolation,
}

Then your AST looks like,

{
  type: 'root',
  nodes: [{
    /* This will become a mixin */
    type: 'comment',
    name: 'substitution-1',
  }, {
    /* This has an interpolated value */
    type: 'decl',
    prop: 'color',
    value: '/*substitution-2*/',
  }],
}

Then you use your AST and substitution map for the following.

Mixins

Youtake fragments of non-mixin code, which we pre-parse in babel, and combine it with the interpolated mixins. We have to put the mixins through a run-time helper that parses them on the client. But we have at least parsed some of the CSS.

// in
styled.div`
  background-color: blue;
  ${mixin}
  color: red;
`
// out
composeFragments([
  { backgroundColor: 'blue' },
  parseMixin(mixin),
  { color: 'red' },
]);

You could also assume that the mixins are pre-parsed, as was suggested here. Then you just remove the run-time helper.

Values

Because this is Native, we also try and do optimisations on the interpolated values, but fall back to css-to-react-native when we can’t optimise it. This won’t be too relevant for styled-components on the web, but will be for styled-components/native.

// in
styled.View`
  padding-top: ${large};
  color: ${primary};
  margin: ${shortHandValues};
`;
// out
const styles = StyleSheet.create({
  0: Object.assign(
    /* The bits we could parse */
    {
      paddingTop: Number(large),
      color: String(primary).trim(),
    },
    /* The bits we could not */
    cssToReactNative([
      ['margin', shortHandValues],
    ])
  ),
});

styledHelper([
  styles[0],
]);

However

It looks like identifying all comment nodes as mixins is fundamentally incorrect, as they could be selectors.

My intent was that this should always be applied, but looking at the selector issue, it looks like it will still have to be an optional optimisation. Without the optimisation turned on, you’ll just have to ignore any component with interpolated values.

I hope this somewhat helps. It’s pretty complicated, so it's hard to get it all in one post. Let me know if you need more information.

@kitten

This comment has been minimized.

Copy link
Member

commented Jan 27, 2017

@jacobp100 Thanks that's really helpful. I've tried out using substitutions on a separate branch, although I haven't done so using comments yet, and I'm not sure whether it's the correct road to go down to. There's a lot of things we can do in the future, but I do believe all of that stuff could become easier by having a custom, naïve AST, if that makes sense?

By doing that the substitutions become quite trivial, but I'm running into other issues, like selector concatenation. If you don't mind, I'd very much appreciate it, if you could take the time to look through the ongoing PR over at the babel plugin 😄

@jacobp100

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2017

So previously I said that a mixin was ambiguous from a selector because

styled`
  ${maybeMixinMaybeSelector}

  ${selector} {
    …
  }
`

But what if,

styled`
  ${clearlyAMixin}

  .${clearlyASelector} {
    /* If you missed it, the selector has a . before */
  }
`

This is not just great for pre-parsing, it's a lot more declarative for the user.

It is a breaking change, however.

@kitten

This comment has been minimized.

Copy link
Member

commented Mar 9, 2017

@jacobp100 you can have a look at my branch over at the babel plugin repo. The parser now just uses stylis and is already done. However, I haven't found the time yet to actually add support for preparsed css to styled-components :(

@jacobp100

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2017

Am I right in thinking all stylis is now doing in the browser is scoping the styles? If so, I think you might almost be there.

I think it should be possible to scope keyframe constructors. We'd just need to create a code path in styled-components that doesn't require stylis for preprocessed components. Maybe something like this?

@thysultan

This comment has been minimized.

Copy link

commented Mar 9, 2017

@jacobp100 global css, flat css, nested css, namespace scoping(including keyframes when enabled) & vendor-prefixing. @philpl are you trying to do something like this dynamic styles mode, pre-parse + collect & remove the dynamic parts.

@jacobp100

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2017

@thysultan This is after the babel plugin. The aim is to use stylis as a pre-compilation step rather than on the client for file size and performance optimisations.

You can see philpl's work here. If you look at test/fixtures/15-preprocess-keyframes/after.js, it looks like we're really close to the above goal for keyframes!

@kitten

This comment has been minimized.

Copy link
Member

commented Mar 12, 2017

@jacobp100 sorry, it's getting hard for me to sift through the github notifications. I consider the preprocessor as done for a first version. Of course there needs to be more testing involved, but I haven't completed the part of this work on styled-components itself. I really need to get onto that ;)

@thysultan this is only for preprocessing it, like Jacob said. However, it should make splitting static and dynamic styles really easy, in the future.

@kitten kitten closed this Mar 12, 2017
@kitten kitten reopened this Mar 12, 2017
@jacobp100

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2017

No worries! Let me know if I can help in any way!

@kitten

This comment has been minimized.

Copy link
Member

commented Mar 13, 2017

@jacobi pinged you on gitter. Since I haven't done anything on implementing the preprocessed css in a while, we could definitely divvy it up, if you want

@kitten

This comment has been minimized.

Copy link
Member

commented Mar 14, 2017

I've started from scratch on the 'v2-preprocessed' branch. It looks like I'm actually going to be done with it soon. Maybe today even? Who knows ;)

@kitten

This comment has been minimized.

Copy link
Member

commented Mar 25, 2017

Released experimental CSS preprocessing with babel-plugin-styled-componenrs v1.1.0 and styled-components v2.0.0-9

@garmjs

This comment has been minimized.

Copy link

commented Mar 26, 2017

@philpl how can we try it now

@kitten

This comment has been minimized.

Copy link
Member

commented Mar 26, 2017

@garmjs Get the latest styled-components v2 prerelease and use the babel plugin with the preprocess option turned on ;) https://github.com/styled-components/babel-plugin-styled-components

@wmertens

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2017

@philpl I have styled-components 2.0.0-10 and the plugin 1.1.1, turning on preprocess complains that it can't find the no-parser export…

@wmertens

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2017

Oops, PEBCAK. Disregard, I had 2.0.0-4, was reading the output of yarn wrongly.

@WilliamDugdale

This comment has been minimized.

Copy link

commented Jun 8, 2017

Hi all,
I'm grappling with SSR and subsequent client-side connecting, and was wondering if someone here could help me - I suspect I'm missing some very important and obvious point.
State-wise, all is well, but using style-components seems to mean that SSR HTML is immediately being updated solely because the client side 'processor' is generating a different sc-tag from the server side on component render. Am I trying to use the wrong tool for the job, or am I missing an important point?
Thanks in advance - I'm keen to get this up and running; component-scoped style seems like a very sensible idea to me!
Will

@kitten

This comment has been minimized.

Copy link
Member

commented Jun 14, 2017

@WilliamDugdale please create a new issue and we'll help you. This issue is unrelated :)

@styled-components styled-components locked and limited conversation to collaborators Jun 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
You can’t perform that action at this time.