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

Preprocess CSS using Stylis #26

Merged
merged 23 commits into from
Mar 21, 2017
Merged

Preprocess CSS using Stylis #26

merged 23 commits into from
Mar 21, 2017

Conversation

kitten
Copy link
Member

@kitten kitten commented Jan 28, 2017

  • Stylis preprocessor with placeholders and a temporary classname
  • Visitor that passes the template string arguments into the preprocessor
  • Determine the format of the result we want to generate for styled-components to consume
  • Write ludicrously high amount of unit tests
  • Write preprocessor that works for css and keyframes (They should actually "just work™")
  • Disable minification when preprocessing is on (Separate PR)
  • Write simplified preprocessor for keyframes helper

This adds the transpilation step that preprocesses CSS, which is then accepted in styled-components under a new entrypoint. An open PR for accepting the preprocessed CSS is here: styled-components/styled-components#585

@kitten kitten mentioned this pull request Jan 28, 2017
6 tasks
@mxstbr
Copy link
Member

mxstbr commented Jan 28, 2017

That looks amazing! 😍

One note: the identifier we pass through from the Babel plugin is the className that will later be assigned to the component, so we can even replace & with that!

@kitten kitten force-pushed the feature/stylis-preprocessing branch 2 times, most recently from e207cd1 to 98b0706 Compare January 28, 2017 22:41
@mxstbr
Copy link
Member

mxstbr commented Jan 29, 2017

Is this done?!

@kitten
Copy link
Member Author

kitten commented Jan 29, 2017

@mxstbr It's a bit rough around the edges :) The keyframes helper will need a simpler preprocessor, but that won't take long to write. Most of the development of this will need to be on styled-components itself now ;)

@kitten
Copy link
Member Author

kitten commented Jan 29, 2017

@mxstbr The last commit adds the missing stuff for the keyframes, so this is complete, if we go by the original to-do list 🎉

I'm treading carefully here, because I spotted another limitation: You won't be able to add new rulesets inside your styled template strings, since it won't be flattened...

That was something we spotted earlier, but we need to resolve it now, I guess. To solve this, all interpolation-rulesets need to be wrapped in css. (Again sth we already wanted)

If we want to resolve the css helpers at runtime, we'd need to gather the selector in which the css interpolation is in and append the new ruleset, where possible. This is not as trivial as I thought it'd be.

If we want to do this at compile-time, we would recursively go through all css interpolations and flatten them all into one styled tag, essentially resolving them. But we'd run into the problem that a css interpolation can be passed in as a variable... And my babel-plugin-fu is not as good as I wish it was, to resolve that ^^

@vdanchenkov
Copy link
Member

vdanchenkov commented Jan 30, 2017

Unfortunately, babel-plugin-fu have limits. It will not help to flatten css interpolation if it is imported from another file.

import {verticalCenter} from 'helpers'

const Component = styled.div`
  ${verticalCenter}
`

Also, I want to ask if you considered conditionally applied rulesets? Something like.

import {verticalCenter, horizontalCenter} from 'helpers'

const Component = styled.div`
  ${props => props.direction == 'v' ? verticalCenter : horizontalCenter }
`

@kitten
Copy link
Member Author

kitten commented Jan 30, 2017

@vdanchenkov Exactly these two things are the ones I'm concerned about.
I don't think we can do anything about them at compile-time.

That's where we gotta bring the magic to runtime ^^

@kitten kitten force-pushed the feature/stylis-preprocessing branch from 03a5cf1 to ed0e3bf Compare March 15, 2017 00:30
@kitten kitten changed the title [WIP] Preprocess CSS using Stylis Preprocess CSS using Stylis Mar 15, 2017
Copy link
Member

@geelen geelen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems great, I think it'd be a good idea to get it out behind a flag of some sort and get it running on real-world projects.

@kitten kitten force-pushed the feature/stylis-preprocessing branch from e84ad42 to a2e4554 Compare March 15, 2017 02:23
@kitten
Copy link
Member Author

kitten commented Mar 21, 2017

Since this is now behind a default-false flag and has an "experimental" note, I guess we can ship it :shipit:

@kitten kitten merged commit c7caa63 into master Mar 21, 2017
@wmertens
Copy link

@mxstbr I can't wait to test this out! npm publish?

@mxstbr mxstbr deleted the feature/stylis-preprocessing branch March 21, 2017 16:50
@mxstbr
Copy link
Member

mxstbr commented Mar 21, 2017

Test suite fails locally, so Ican't publish 😕

@kitten
Copy link
Member Author

kitten commented Mar 21, 2017

@mxstbr Oops! Seems like my stylis was out of date, when I updated the fixtures 😆 I updated it to 1.0.6 in the package.json

9fc0a6e

Edit: Oh god! Now there's sth wrong in the tests 💀
Edit: DOH! Babel and the yarn.lock file were out-of-date as well. It works now

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

5 participants