search

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
376 stars 30 forks source link

Ignore Apache SSI directives by default #39

Closed textbook closed 4 years ago

textbook commented 4 years ago

It seems likely that anyone including these directives wants to retain them in the output, and changing the default rather than requiring explicit configuration supports folks using e.g. Create React App.

Also I should note that the extensive test coverage and well-configured automation made this codebase an absolute pleasure to work with, so thanks for your efforts on that front!

DanielRuf commented 4 years ago

Hi @textbook,

thanks for your PR. Please revert the dist changes.

I guess this is a SemVer-minor update then as this should not break any setups. But I'm not sure as we change defaults which is an API change and so it is a breaking change because the default output changes because of this, which would require a SemVer-major release.

textbook commented 4 years ago

Updated; sorry, as the tests built the dist/ and it wasn't .gitignored I assume I should include it.

In terms of semver, that's a tricky one. I wouldn't necessarily consider changing a default an API change (unlike e.g. renaming or removing the configuration option). Let's think about the groups of users:

Config \ Directives Yes No
Explicit No change No change
Default Output changes No change

My guess would be that the group that has directives and is using the default configuration will be pretty small, maybe even none. Only people who were relying on this tool to strip out those directives but didn't explicitly set ignoreCustomComments to [] or false would see any changes to their output, and that change only alters any behaviour if, despite wanting SSI to definitely not happen, it was enabled on the server.

My vote would be for minor, on that basis, but I'd be open to hearing opinions from others.

DanielRuf commented 4 years ago

My guess would be that the group that has directives and is using the default configuration will be pretty small, maybe even none. Only people who were relying on this tool to strip out those directives but didn't explicitly set ignoreCustomComments to [] or false would see any changes to their output, and that change only alters any behaviour if, despite wanting SSI to definitely not happen, it was enabled on the server.

Only if no other comments with # are there ;-) I thin a minor release should be fine then.

DanielRuf commented 4 years ago

The changes were released with html-minifier-terser v5.1.0.