Ignore Apache SSI directives by default #39
Conversation
dist/htmlminifier.js
Outdated
| @@ -26083,7 +26083,10 @@ function processOptions(values) { | |||
| canCollapseWhitespace: canCollapseWhitespace, | |||
| canTrimWhitespace: canTrimWhitespace, | |||
| html5: true, | |||
| ignoreCustomComments: [/^!/], | |||
| ignoreCustomComments: [ | |||
There was a problem hiding this comment.
Please revert the changes on the dist files. These are built on release.
|
Hi @textbook, thanks for your PR. I guess this is a SemVer-minor update then as this should not break any setups. |
It seems likely that anyone including these directives wants to retain them in the output, and changing the default rather than requiring explicit configuration support folks using e.g. Create React App.
|
Updated; sorry, as the tests built the 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:
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 My vote would be for minor, on that basis, but I'd be open to hearing opinions from others. |
Only if no other comments with |
|
The changes were released with html-minifier-terser v5.1.0. |
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!