terser / html-minifier-terser

actively maintained fork of html-minifier - minify HTML, CSS and JS code using terser - supports ES6 code
https://terser.org/html-minifier-terser
MIT License
385 stars 32 forks source link

[Bug]: Minifying CSS deletes blocks of code #180

Open chrisstaite-menlo opened 3 months ago

chrisstaite-menlo commented 3 months ago

What happened?

When running the minifier with the following input:

<style type="text/css">
{% ignore1 %}
a {
    {% ignore2 %}
}
{% ignore3 %}
</style>

And settings:

{
   ignoreCustomFragments: [
      /\{%[\s\S]*?%\}/
   ],
   collapseWhitespace: true,
   conservativeCollapse: true,
   minifyCSS: {
      level: 0
   },
}

The output is:

<style type="text/css"> {% ignore1 %} a{ {% ignore2 %} }</style>

Where it should be:

<style type="text/css"> {% ignore1 %} a{ {% ignore2 %} } {% ignore3 %} </style>

Full test program:

const { minify } = require('html-minifier-terser');

const html = `<style type="text/css">
{% ignore1 %}
a {
    {% ignore2 %}
}
{% ignore3 %}
</style>`;

const output = minify(html, {
  ignoreCustomFragments: [/\{%[\s\S]*?%\}/],
  collapseWhitespace: true,
  conservativeCollapse: true,
  minifyCSS: {
    level: 0,
  },
}).then(console.info);

Version

7.2.0

What browsers are you seeing the problem on?

No response

Link to reproduce

https://stackblitz.com/edit/node-hnsagk?file=index.js

Relevant log output

No response

Willing to submit a PR?

None

Nantris commented 3 months ago

Yes indeed, I'm seeing issues too since yarn upgrade which only affected our sub-dependencies. I'm guessing this might be a cssnano issue? We use Emotion for CSS in JS and I'm guessing they're not playing nice.

I'm not certain our issues are identical, but I suspect they come from the same source since disabling minifyCSS resolves it.

Nantris commented 2 weeks ago

Regardless of the validity of their example, this is a real issue - or at least was when I commented.

DanielRuf commented 2 weeks ago

Yes indeed, I'm seeing issues too since yarn upgrade which only affected our sub-dependencies.

Can you clarify? Because I've checked and the problem is not new.

DanielRuf commented 2 weeks ago

I think that we should refactor the css logic, because there are more problems which arise when custom fragments are used:

<style type="text/css">
{% ignore1 %}
a {
    {% ignore2 %}
}
.a {
    {% ignore2 %}
}
{% ignore3 %}
</style>
<style type="text/css"> {% ignore1 %} a{} {% ignore2 %} .a{ {% ignore2 %} }</style>

And with the patch from the PR:

<style type="text/css"> {% ignore1 %} a{} {% ignore2 %} .a{ {% ignore2 %} } {% ignore3 %} </style>
Nantris commented 2 weeks ago

My original issue seems to be resolved now, so whatever broke it seems to have been fixed since then. It was definitely new around the time I posted though. I see the repro hasn't changed, so it could have been caused by yet another subdependency of this one if I had to guess.