asyncLiz / minify-html-literals

Minify HTML template literal strings
MIT License
68 stars 13 forks source link

parts are concattenated #26

Closed bennypowers closed 3 years ago

bennypowers commented 4 years ago

Thanks for publishing this awesome package.


Consider this repl:

https://repl.it/@bennypowers/minify-html-literals-css-shadow-parts-concat-bug#index.js

input:

html`<h1> hi </h1>`;
css`
#shadow::part(a b c) {
  color: red;
}

CSS-unminified output:

html`<h1>hi</h1>`;
css`
#shadow::part(a b c) {
  color: red;
}
`

Actual

CSS-minified output:

html`<h1>hi</h1>`;
css`#shadow::part(abc){color:red}`

Expected

CSS-minified output:

html`<h1>hi</h1>`;
css`#shadow::part(a b c){color:red}`

this package should maintain the part selector a b c.

I'm pretty sure this is an upstream issue with CleanCSS

bennypowers commented 4 years ago

workaround:

  1. remove postcss-clean from postcss.config.js
  2. use minifyLitHtml rollup plugin like so : minifyLitHtml({ options: { shouldMinifyCSS() { return false; } } })
  3. use rollup-plugin-styles like so: styles({ mode: 'emit', minimize: true })
asyncLiz commented 3 years ago

I added a patch to restore space-separated pseudo class selectors while waiting on https://github.com/jakubpawlowicz/clean-css/issues/996

bennypowers commented 3 years ago

see also https://github.com/leodido/postcss-clean/issues/39

asyncLiz commented 3 years ago

see also leodido/postcss-clean#39

Got it! Added my findings to that bug. Unfortunately I think for other libs like postcss-clean you'd need to completely disable tidySelectors unless a manual patch is run like what I added.

The core issue is that the check for pseudo classes occurs on the whole selector value. This only works if the pseudo class is at the beginning of the selector (and they're often not).

:not(.complex .selector) { /* works in clean-css, first character is ":" */ }

foo:not(.complex .selector) { /* doesn't pass, spaces are removed */ }

I unfortunately don't have much time to contribute to clean-css, and the activity on that reopen is pretty fresh (a few days ago). So if you want to take that and help fix the bug I'm sure they'd appreciate it!