Skip to content

Initial CSS support#1636

Merged
vjeux merged 1 commit intoprettier:masterfrom
vjeux:css
May 24, 2017
Merged

Initial CSS support#1636
vjeux merged 1 commit intoprettier:masterfrom
vjeux:css

Conversation

@vjeux
Copy link
Copy Markdown
Contributor

@vjeux vjeux commented May 20, 2017

I wanted to see how hard it would be to add support for CSS inside of prettier. Turns out, it's not that hard. I spent a few hours printing post-css values and getting all the stylefmt unit tests to not throw.

@azz
Copy link
Copy Markdown
Member

azz commented May 20, 2017

Pretty-print the planet!

@vjeux
Copy link
Copy Markdown
Contributor Author

vjeux commented May 20, 2017

I migrated over to https://github.com/shellscape/postcss-values-parser (from https://github.com/TrySound/postcss-value-parser ) and had to send a PR to fix nested function parsing ( shellscape/postcss-values-parser#27 ).

Except for media queries (I need to start using https://github.com/dryoma/postcss-media-query-parser ) and crazy sass functions, the output of all the stylefmt tests look good at first glance. This is extremely promising :)

The big thing that's going to block real adoption is to port the comment handling logic we have in JS to CSS. Right now most of the CSS parsers just drop all the comments, which is not acceptable!

I did a quick look at all the stylefmt outstanding bugs and most of them are already fixed in this PR. It's great to be able to use a solid foundation!

@vjeux
Copy link
Copy Markdown
Contributor Author

vjeux commented May 20, 2017

I now prefix all the nodes and added media query parsing

@mathieudutour
Copy link
Copy Markdown

@vjeux
Copy link
Copy Markdown
Contributor Author

vjeux commented May 20, 2017

@mathieudutour yeah, need to add an indentation in this case, so it's obvious it belongs to the same rule. Note that this is valid css :)

@lydell
Copy link
Copy Markdown
Member

lydell commented May 21, 2017

Would it make sense to have a separate printer.js file for every non-JS-y language?

@vjeux
Copy link
Copy Markdown
Contributor Author

vjeux commented May 21, 2017

Yes, most likely. Also there's a lot of cleanup to do, the printer file could be split into more meaningful pieces.

@andreypopp
Copy link
Copy Markdown

Pinging @lahmatiy with his csstree/csstree parser which might do better work at handling comments.

@vjeux
Copy link
Copy Markdown
Contributor Author

vjeux commented May 24, 2017

The ast of csstree seems to be much cleaner than postcss.

It doesn't seem to be outputting any comment: https://astexplorer.net/#/gist/244e2fb4da940df52bf0f4b94277db44/f9011cd86adfea73546ab352ae6b74b403cef20d

I don't see it supporting less and sass?

@ai
Copy link
Copy Markdown

ai commented May 24, 2017

It has fork API to create Sass and Less parsers, but right not csstree parses only valid CSS (I am not sure about parsing CSS Modules or PreCSS extensions).

BTW, right now we are discussing of merging Sass, PostCSS and csstree parses together. But it could takes a months, so you can start from PostCSS.

@vjeux vjeux force-pushed the css branch 3 times, most recently from f75d3fe to 93829c5 Compare May 24, 2017 17:05
I wanted to see how hard it would be to add support for CSS inside of prettier. Turns out, it's not that hard. I spent a few hours printing post-css values and getting all the stylefmt unit tests to not throw.
@vjeux vjeux merged commit 78ba808 into prettier:master May 24, 2017
@vjeux
Copy link
Copy Markdown
Contributor Author

vjeux commented May 24, 2017

I'm merging it as is as I want to spend the day refactoring the codebase a bit.

@quantuminformation
Copy link
Copy Markdown

Nice work!

@prettier prettier locked as resolved and limited conversation to collaborators Dec 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants