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
fix(postcss-merge-rules): prevent breaking rule merges #1072
Conversation
Move all checks together.
Do not merge if the pseudo-selector is not in the list of well-known pseudo-selectors and does not start with a vendor prefix. We let through vendor prefixes as they are handled elsewhere in the code. Fix #999
29f9de5
to
bd8c6f1
Compare
Codecov Report
@@ Coverage Diff @@
## master #1072 +/- ##
=======================================
Coverage 96.37% 96.38%
=======================================
Files 115 115
Lines 3588 3594 +6
Branches 1058 1060 +2
=======================================
+ Hits 3458 3464 +6
Misses 121 121
Partials 9 9
Continue to review full report at Codecov.
|
I think the pseudo-selector list https://github.com/cssnano/cssnano/blob/master/packages/postcss-merge-rules/src/lib/ensureCompatibility.js is incomplete, which causes deoptimizations after this pull request. For example, the list is missing
becomes
That's because before it sees |
This PR fixes two related issues.
First commit, fixes: #730 by not merging the
:host
selector with others.I have rebased #731 as it seems to me it fixes a legitimate problem (angular/angular-cli#18672).
Although this comment #876 (comment) says the PR is already done, the test fails on master.
There's a comment #730 (comment) mentioning the fix is not safe if the browser does not support
:host
but it seems the current situation is what would caus the bug.As #999 states, merging unknown and known selectors causes the browser to ignore the whole rule, so not merging should be the safe path.
Second commit fixes #999. If the pseudo-selector is unknown and does not start with a vendor prefix, do not merge.