Skip to content
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

CSS minification combines vendor-specific selectors creating invalid rules #1776

Closed
adamwathan opened this issue Nov 15, 2021 · 5 comments
Closed

Comments

@adamwathan
Copy link

If you feed something like this into esbuild's CSS minification:

.form-input:-moz-placeholder-shown {
  color: black;
}
.form-input:-ms-input-placeholder {
  color: black;
}
.form-input:placeholder-shown {
  color: black;
}

...you get this output:

.form-input:-moz-placeholder-shown,
.form-input:-ms-input-placeholder,
.form-input:placeholder-shown {
  color: black;
}

This unfortunately breaks the rule completely because Chrome has no idea what -ms-input-placeholder is for example and marks the entire rule as invalid and doesn't apply it.

In this example, the selectors should not be combined since they can't be combined safely and the output should remain as:

.form-input:-moz-placeholder-shown {
  color: black;
}
.form-input:-ms-input-placeholder {
  color: black;
}
.form-input:placeholder-shown {
  color: black;
}

Twitter thread where this surfaced just in case helpful: https://twitter.com/mmmykolas/status/1460249898499166211

Thanks as always for your amazing work on this project! 🙌

@evanw
Copy link
Owner

evanw commented Nov 15, 2021

Additional context:

  • This behavior was introduced recently to fix this esbuild issue: Improve minify css result. #1755.

  • I found this thread on how to fix this: fix(postcss-merge-rules): prevent breaking rule merges cssnano/cssnano#1072. The approach seems to be to completely disable this optimization and then only re-enable it in cases where you are 100% sure it won't cause any issues, either because all selectors involved have been supported in CSS from the very beginning or in at least the browsers listed in esbuild's target setting (which would require a dense feature compatibility matrix for CSS, which I don't have at the moment).

@adamwathan
Copy link
Author

Thanks Evan! Here's another related discussion where this is starting to show up more: tailwindlabs/tailwindcss#6096

@hyrious
Copy link

hyrious commented Nov 16, 2021

Merging rules is difficult, as well as DCE (removing unnecessary rules that are covered by other rules below) because of keeping specificity. I've looked for a super CSS minification tool before but each of them have some drawback. We can just turn off this optimization for now, until a better algorithm be found.

@evanw evanw closed this as completed in 6e724a1 Nov 16, 2021
@adamwathan
Copy link
Author

Thank you @evanw!

@Neophen
Copy link

Neophen commented Nov 16, 2021

Thank you!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants