explodingcamera / esm

Monorepo with most of my JavaScript/Typscript Packages
MIT License
7 stars 0 forks source link

Bug in literals minifier #1

Closed kaifaty closed 1 year ago

kaifaty commented 1 year ago

I've receive an error splitHTMLByPlaceholder() must return same number of strings as template parts when i try to minify component with styles like so:

:host {
      ${LitLink.cssKey('color')}: ${this.cssVar('color')};
      display: inline-block;
      box-sizing: border-box;
      position: relative;
 }

The problem is in CleanCSS, it return warnings with wrong styles. I've solved it. On warnings now just removing spaces and \r\n:

async minifyCSS(css, options = {}) {
    const adjustedOptions = adjustMinifyCSSOptions(options)
    const output = await new CleanCSS({
      ...adjustedOptions,
      returnPromise: true,
    }).minify(css)

    if (output.errors?.length) throw new Error(output.errors.join('\n\n'))
    if (output.warnings.length) {
      return fixCleanCssTidySelectors(css, css.replace(/(\n)|(\r)|(  )/g, ''))
    }
    return fixCleanCssTidySelectors(css, output.styles)
  }
explodingcamera commented 1 year ago

Thanks for the bug report! Should be fixed in version 0.2.1.

explodingcamera commented 1 year ago

@kaifaty be sure to either include the LICENSE.md in your project or use literals-minifier as a dependency

kaifaty commented 1 year ago

Ok, it was temp solution. ps.

return css.replace(/(\n)|(\r)/g, "");

You don't replace spaces

explodingcamera commented 1 year ago

@kaifaty I'm not replacing spaces since that might break some css things like the content: "some text" attribute, code with template strings as properties should however now just use clean-css to minify, the replacement is just a fallback now in case other warnings appear.

kaifaty commented 1 year ago

Regexp has 2 repeated spaces. So content: "some text" will safe, but yeah, in some cases will be errors.