Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

customAttrCollapse doesn't respect conservativeCollapse #50

Open
xReeQz opened this issue Oct 28, 2020 · 4 comments
Open

customAttrCollapse doesn't respect conservativeCollapse #50

xReeQz opened this issue Oct 28, 2020 · 4 comments

Comments

@xReeQz
Copy link

xReeQz commented Oct 28, 2020

In our angular.js based app we have attributes with long expressions which we sometimes spread over multiple lines like this:

repeat="role in modal.roles |
        filter: {displayName: $select.search}
        track by role.id"

Earlier we used minimize to minify HTML and that stuff would be collapsed correctly into:

repeat="role in modal.roles | filter: {displayName: $select.search} track by role.id"

Now after switching to webpack + html-loader we noticed that such attributes are not processed at all by default. customAttrCollapse + conservativeCollapse looked like exactly what we need, however it turned out that they don't work together. Is there a way to work around that?

@DanielRuf
Copy link
Contributor

Hi @xReeQz,

can you provide a full example code for the html-minifier-terser so we have a testcase and the options that you use?

https://github.com/terser/html-minifier-terser/blob/v5.1.1/src/htmlminifier.js#L327

@xReeQz
Copy link
Author

xReeQz commented Nov 16, 2020

@DanielRuf here's an HTML sample:

<ui-select-choices 
    repeat="status in segment.statuses |
            filter: {displayName: $select.search}
            track by status.id"
>
  <span class="tu-match" 
        ng-bind-html="status.displayName | 
                      highlight: $select.search"
  ></span>
</ui-select-choices>

The options I use are:

{
  caseSensitive: true,
  collapseWhitespace: true,
  conservativeCollapse: true,
  keepClosingSlash: true,
  minifyCSS: true,
  minifyJS: true,
  removeComments: true,
  removeRedundantAttributes: true,
  removeScriptTypeAttributes: true,
  removeStyleLinkTypeAttributes: true,
  customAttrCollapse: /(ng-class|messages|repeat)/
}

@ybaws
Copy link
Contributor

ybaws commented Dec 2, 2020

We have a similar issue in an Angular project, where multiple consecutive spaces get removed from our customAttrCollapse attributes/directives despite having conservativeCollapse: true in our options.

i.e.

alias="
  foo           as bar,
  bar           as foo,
  reallyLongFoo as bar
"

becomes

alias="fooas bar, baras foo, reallyLongFoo as bar"

changing https://github.com/terser/html-minifier-terser/blob/v5.1.1/src/htmlminifier.js#L328 to be

attrValue = trimWhitespace(attrValue.replace(/\n+|\r+/g, '').replace(/\s{2,}/g, options.conservativeCollapse ? ' ' : ''));

fixes our issue, but I'm not sure of the reasoning behind the \s{2,} part of the regex in the first place as to whether this is a reasonable fix or not?

@DanielRuf
Copy link
Contributor

Can you test if this would result in a green CI build? You can provide a PR and we can see then if there are any negative side-effects or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants