Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(compiler): foundation css prefix #1288

Closed
wants to merge 1 commit into from

Conversation

ematipico
Copy link
Contributor

Summary

This PR adds a new feature to the Rome compiler. Now that our CSS parser is gaining traction, we can start applying transformations to them during the compilation phase.

I wanted to set up the foundations of a sort of autoprefixer. I didn't call it that way for obvious reasons. So for now, I called it css-prefix but feel free to suggest a better name. Maybe it should not be called css-prefix if we think that its purpose is broader.

I created a index.ts where we could put all the CSS transformations and each transformation will be its own file.

I also fixed a small issue inside the CSS format.

Test Plan

I set up a new test fixtures which is better compared to what we had.

@ematipico ematipico requested a review from sebmck January 8, 2021 09:46
@ematipico ematipico force-pushed the feature/autoprefixer branch 2 times, most recently from 32f91c4 to c30df91 Compare January 8, 2021 12:49
@ematipico ematipico force-pushed the feature/autoprefixer branch from c30df91 to f633a66 Compare January 8, 2021 13:52
@jer3m01
Copy link
Contributor

jer3m01 commented Jan 8, 2021

Just me being picky but wouldn't css-prefixer be better, following parser. Doesn't matter much and the more I think about it the less sense the difference makes.

Also I know this is just a foundation but there are a few other prefixes we could add (-moz-, don't know if we want -ms- as it's on its final version and most MS products are dropping support for it soon).
There are also value prefixes i.e. display: -webkit-box.

Personally I'd use something different than just if statements in a single code block to do all the prefixes as it would get tedious if there are a lot.
Don't have any idea on what the implementation would look like though.

And the ability to use something like browserlist could be considered (but than we would need it make a custom implementation or port it).

Anyway, looks great!

@ematipico
Copy link
Contributor Author

@jer3m01 thank you very much for your feedback, it's really appreciated!

I initially implemented the logic of the transition by taking what autoprefixer does. Although I totally overlooked the browser part! I noticed that the playground has set the value "browser" to"last 4 versions" that's why I worked only on two prefixes.

Totally missed that one and your comment raised it, thank you. We should plan this more thoroughly I guess, as the part of the target browser should come from the configuration.

I think this prefixer should be more than just transformations, and it should be implemented more like the linter is, with some caching under hood, maybe.

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.

2 participants