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
chore: update css-declaration-sorter to v6.0.3 #1071
Conversation
Seems like some prefixed properties like |
Codecov Report
@@ Coverage Diff @@
## master #1071 +/- ##
=======================================
Coverage 96.37% 96.37%
=======================================
Files 115 115
Lines 3589 3589
Branches 1059 1059
=======================================
Hits 3459 3459
Misses 121 121
Partials 9 9 Continue to review full report at Codecov.
|
I think it would be a good idea to add some tests to https://github.com/cssnano/cssnano/blob/master/packages/cssnano-preset-default/src/__tests__/css-declaration-sorter.js, for example to check that integrating the new |
Agree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add test for all
?
Rebased and added four tests, one test has weird output not sure what to do with it, maybe something to put into
|
Yep, weird output, maybe problem due order of plugin? |
@alexander-akait I did try removing css-declaration-sorter but the test fails with the same output, maybe it's a bug in the merge-longhand logic? Also the framework integration tests keep failing now, even though I did run |
let's skip it and open an issue for this |
@alexander-akait alright, it's odd though if I run the same input in the cssnano playground it doesn't have this behaviour. |
Need fix CI |
Probably need to regenerate the integration tests
|
Yes as I said before:
So even after running this command there are no changes, any help appreciated. |
/cc @ludofischer Can you look? Maybe we have problems with tests... |
I rebased, and ran |
But did you commit the modified CSS fixtures? They're in |
The CSS fixtures stay the same, so nothing to commit. Just to be 200% sure I removed all files in the directories you mention manually before running the build:integration script. |
Do the tests pass locally? Because in the test logs the result does not match the fixture. |
I've opened #1071 to add a commit with the fixtures fixed. What I think happened is that the tests testthe code in |
Alright let me know where to 'continue' this PR, I've cherry picked 4ec0f00. Can also close this one. Thanks for the help! |
Update so Node.js 10 is supported in combination with PostCSS 8.
#1065 (comment)