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

Safe set of minification defaults #43

Merged
merged 1 commit into from
Feb 15, 2016

Conversation

chrisnicola
Copy link
Contributor

I've reviewed a few sources and tried to put a safe set of minification defaults
that will not break common use cases such as some javascript and CSS use cases.
I've used a subset of the "safe" options from here.

Options not used are:

  • Remove redundant attributes (can break styles/JS)
  • Remove empty attributes (can break styles/JS)
  • Remove empty elements (if the empty tag is indeed unintended it should be removed from the original)
  • Minify URLs (seems like it should be at the users discretion to rewrite absolute URLs)

This addresses a number of issues: #40, #38.

I've reviewed a few sources and tried to put a safe set of minification defaults 
that will not break common use cases such as some javascript and CSS use cases.
 I've used a subset of the "safe" options [from here](https://kangax.github.io/html-minifier/).

 Options not used are:

- Remove redundant attributes (can break styles/JS)
- Remove empty attributes (can break styles/JS)
- Remove empty elements (if the empty tag is indeed unintended it should be removed from the original)
- Minify URLs (seems like it should be at the users discretion to rewrite absolute URLs)

This addresses a number of issues: webpack-contrib#40, webpack-contrib#38.
@jantimon
Copy link

Any update on this?

@chrisnicola
Copy link
Contributor Author

Ok this is getting silly, can we get some response on this? The loader needs to do the right thing by default, or people are going to consistently have problems getting started with Webpack. To make this worse since people generally only minify in production they won't even notice the problem until it reaches production. This should be being taken far more seriously than it appears to be.

@jhnns
Copy link
Member

jhnns commented Feb 14, 2016

Thx for investing time on this 👍.

This looks good to me, I'll try to merge and publish this asap. Just waiting for write access 😁

jhnns added a commit that referenced this pull request Feb 15, 2016
Safe set of minification defaults
@jhnns jhnns merged commit 2abb0d7 into webpack-contrib:master Feb 15, 2016
@jhnns
Copy link
Member

jhnns commented Feb 15, 2016

Has been published as 0.4.1 0.4.2

@mlegenhausen
Copy link
Contributor

Does this solve webpack/webpack#752?

@jhnns
Copy link
Member

jhnns commented Feb 15, 2016

Yep, I've added tests for it: 1bd81c9

@chrisnicola chrisnicola deleted the patch-1 branch February 16, 2016 00:24
@chrisnicola
Copy link
Contributor Author

Awesome thanks @jhnns!

@kevinrenskers
Copy link

This change actually broke my layout since I depended on all whitespace being removed. Kind of strange that this pretty big change was released as a patch release.

Anyway, any way to restore the old behavior and remove conservativeCollapse?

@jhnns
Copy link
Member

jhnns commented Jul 1, 2016

I'm sorry. You're right, this was a breaking change.

You should get back the hold behavior with this config:

{ test: /\.html$/, loader: "html?" + JSON.stringify({ minimize: true, conservativeCollapse: false }) }

@kevinrenskers
Copy link

Yep, just figured that out as well :) Thanks!

@jantimon
Copy link

jantimon commented Jul 1, 2016

cleaner: { test: /\.html$/, loader: "html", query: { minimize: true, conservativeCollapse: false } }

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

Successfully merging this pull request may close these issues.

5 participants