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

[WIP] Preprocess CSS #25

Closed
wants to merge 8 commits into from
Closed

[WIP] Preprocess CSS #25

wants to merge 8 commits into from

Conversation

kitten
Copy link
Member

@kitten kitten commented Jan 26, 2017

Edit: Closed since the Stylis one will likely work out better

  • Minimal CSS parser that outputs an AST
  • Un-nesting transformer that flattens the AST
  • Determine the format of the result we want to generate for styled-components to consume
  • Add preprocessTemplateLiterals visitor (Very easy; can be seen in the postcss-preprocessing branch)
  • Determine how preprocessed CSS in interpolations should work
  • Write ludicrously high amount of unit tests

@kitten
Copy link
Member Author

kitten commented Jan 26, 2017

So for now I completed a minimal CSS parser and have a transformer that flattens it. To continue I'll need to figure out what shape the result should take on for styled-components to consume. Also I haven't quite figured out how the keyframes and css helpers should behave. Especially the css one might be tricky, since that will be another interpolation that might have to be combined with the higher level one.

It's still missing a lot of testing since this is pretty much just some experimental stuff right now.

Edit: An alternative approach, that is even more naïve and might or might not work, can be found in the postcss-preprocessing branch. It essentially just does the preprocessing by filling in some placeholders, like babel-plugin-css-to-js does.

@mxstbr
Copy link
Member

mxstbr commented Jan 26, 2017

I reckon the shape needs to be some sort of object that can be parsed with very little effort. Possibly we could even put in just the AST? Would that be too big possibly?

@kitten
Copy link
Member Author

kitten commented Jan 26, 2017

@mxstbr I do believe that is too big. I think we want to get as close to a normal string as possible, while being able to replace & in the selector values that contain it. Since we are already able to parse template strings, an object of the usual structure might be suitable?

Something like this:

{
  '&': [[ 'color: ', ';' ], props => props.color ],
  '&:hover': [[ 'color: green;' ]]
}

@kitten
Copy link
Member Author

kitten commented Jan 26, 2017

... Actually we can interleave this and diverge from what template strings need to do:

{
  '&': [ 'color: ', props => props.color, ';' ],
  '&:hover': [ 'color: green;' ]
}

Since we don't need to differentiate between interpolations and CSS strings anymore, this should be fine, no?

@mxstbr
Copy link
Member

mxstbr commented Jan 27, 2017

Yeah that sounds reasonable! @geelen what do you think?

@kitten
Copy link
Member Author

kitten commented Jan 27, 2017

@mxstbr Just noticed a small problem with the selector un-nesting.

& {
  &:hover {
  }
}

The nested rule here is gonna be kept as &:hover.

& {
  > div {
  }
}

While the nested rule here is going to be & > div... So if this selector is a function, like:

`& {
  ${x => x ? '&:hover' : '> div'} {
  }
}`

Then we don't know whether to prepend the ancestor's selector or not :(

@kitten
Copy link
Member Author

kitten commented Jan 27, 2017

It turns out that stylis.js is extremely forgiving with placeholders and really forgiving. So I might replace this with some stylis magic! ✨ We can still keep the AST parser around for when we decide to actually do static/dynamic css splitting, but for now it's totes not needed 😄

Edit: DEMO! http://www.webpackbin.com/41LPGnEDz

Edit 2: Actually, I just discovered that & PLACEHOLDER_2 is not really the most clever thing to come out of the placeholder transpilation, but I think we can solve that at runtime.

@kitten
Copy link
Member Author

kitten commented Jan 27, 2017

Another thing 😭 inline placeholders that are placed some lines before { w/o any other properties between the two, are recognised as blocks of their own 👎 Maybe I was a bit fast to try to solve this with stylis.

Edit: I can solve this by appending each placeholder with a semicolon ;, except the ones that are for selectors (which are identified as interpolations that have a curly brace { after them on the same line)

@kitten
Copy link
Member Author

kitten commented Jan 28, 2017

Closing this PR in favour of #26. That one will probably be closer to how v2 already processes its CSS, and it will add less surface area for bugs.

@kitten kitten closed this Jan 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants