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
feat(postcss-colormin): switch to colord and solve multiple issues #1107
Conversation
9e38c9e
to
95af87b
Compare
Codecov Report
@@ Coverage Diff @@
## master #1107 +/- ##
==========================================
- Coverage 96.37% 96.37% -0.01%
==========================================
Files 115 115
Lines 3589 3582 -7
Branches 1059 1054 -5
==========================================
- Hits 3459 3452 -7
Misses 121 121
Partials 9 9
Continue to review full report at Codecov.
|
95af87b
to
9d669d7
Compare
* ensure that the replacement color values are shorter than the original. Previously, the plugin returned the converted value without checking whether it was shorter than the original, we now ensure we always return the shortest color value * rework the colomin cache so use only one cache as in #771 (without using Ramda as in that PR). The cache key is now just the declaration value. Before there were two caches, one with a CSS `property|value` key and one that cached the single colors. * change the underlying library to [colord](https://github.com/omgovich/colord). Using `colord` comes with some trade-offs. On the plus side, it automatically rounds up up, which fixes an [open issue](#819) without additional work, reduces transitive dependencies from 5 to 0 (although I think the previous 4 are all maintained by the same people), is smaller and claims to be faster (haven't checked but I suspect css-value-parser is more the bottle neck than color conversion here). On the minus side, the `colord` parser is very lenient so it accepts invalid CSS colors (rgb with mixed percentages and numbers) which cssnano did not convert before, so I've added some checks to skip these values. Overall there are less lines of code. * get rid of the JSON file with precomputed CSS color names. I guess the idea was that the JSON file only contained color names that are shorter than the hex, but the color conversion libraries include all color names anyway to be able to parse, so in fact we ship the color names twice. I don't think the extra code for generating the file, plus loading the JSON is worth it to avoid a an extra length check. fix(postcss-colormin): do not replace original value with longer one fix #1042 feat(postcss-colormin): switch to colord * round color values fix #819 * reduce dependencies perf(postcss-colormin): improve caching Fix #771 refactor(postcss-colormin): drop the generate script
9d669d7
to
1d36ad1
Compare
Great job! |
Performance might have improved as well. I've run a quick test with benchmark.js and I get
|
Hey guys! Thanks for choosing colord as a color manipulation library. If you'll face any issues, contact me here or via issues in the colord's repo. |
BTW guys, are 4 and 8 digit color formats are supported by the cssnano? |
|
It tries to convert it to a shorthand hex only if alpha is 1
|
By using 4 digit hexadecimal Here is an example: |
The only thing that bothers me is the browser support. As far as I know, |
We can implement this as feature and check support using |
If you don't mind I'll create a pull request and ask for your help with configuring the |
Created #1109. Take a look please when you'll have time. |
property|value
key and one that cached the single colors.Using
colord
comes with some trade-offs. On the plus side, it automatically rounds up up, which fixes an open issue without additional work, reduces transitive dependencies from 5 to 0 (although I think the previous 4 are all maintained by the same people), is smaller and claims to be faster (haven't checked but I suspect css-value-parser is more the bottle neck than color conversion here).On the minus side, the
colord
parser is very lenient so it accepts invalid CSS colors (rgb with mixed percentages and numbers) which cssnano did not convert before, so I've added some checks to skip these values. Overall there are less lines of code.fix(postcss-colormin): do not replace original value if shorter than conversion
fix #1042
feat(postcss-colormin): change to colord
perf(postcss-colormin): improve caching
Fix #771
refactor(postcss-colormin): drop the generate script