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

Added support for minification using html-minifier #20

Closed
wants to merge 4 commits into from
Closed

Added support for minification using html-minifier #20

wants to merge 4 commits into from

Conversation

sebastienbarre
Copy link
Contributor

I discovered html-webpack-plugin today, great tool, thanks.

This PR adds a minify option (true or an options object), which enables the minification of all resulting HTML files, using kangax/html-minifier. This comes handy when you are working on your own (unminified) template file, but want the smallest final output for performance.

Please note that html-minifier is very conservative by default, as most of its options are set to false.
I suggest using these for example:

  var HtmlWebpackPlugin = require('html-webpack-plugin');
  var html_webpack_options = {
    template: 'src/assets/index.template.html'
  };
  if (production) {
    html_webpack_options.minify = {
      removeComments: true,
      collapseWhitespace: true,
      conservativeCollapse: true,
      minifyJS: true,
      minifyCSS: true
    };
  }
  config.plugins.push(new HtmlWebpackPlugin(html_webpack_options));

Hope this helps.

@sebastienbarre
Copy link
Contributor Author

Howdy. Anything else you would like me to add or fix there? Thanks.

@ampedandwired
Copy link
Collaborator

This looks pretty good. Is it perhaps possible to add a test or two and rebase?

@sebastienbarre
Copy link
Contributor Author

@ampedandwired I rebased, but I can't get your tests to run. It assumes jshint and jasmine-node are globally installed, which was easy enough to do, but is still failing:

> jshint -c .jshintrc *.js spec && jasmine-node --captureExceptions spec

Exception loading: /Volumes/Users/Shared/Src/react/html-webpack-plugin/spec/HtmlWebpackPluginSpec.js
{ [Error: Cannot find module 'tapable'] code: 'MODULE_NOT_FOUND' }
Error: Cannot find module 'tapable'

I have webpack and tapable globally installed too.

@jantimon
Copy link
Owner

jantimon commented May 7, 2015

@sebastienbarre I created a new feature branch for this pr:
https://github.com/ampedandwired/html-webpack-plugin/tree/feature/htmlmin

As soon as we got tests it should be ready to merge

To run the tests just use npm test.

@sebastienbarre
Copy link
Contributor Author

@jantimon Thanks. That's what I did, npm test. There are missing devDependencies, but I installed them globally, as mentioned above. It is still failing to run on my Mac as is though.

@SimenB
Copy link
Contributor

SimenB commented May 8, 2015

@sebastienbarre I'm on a Mac, and running npm i && npm t works fine.

$ npm -g ls --depth 0
/Users/simen/.nvm/versions/node/v0.12.2/lib
├── [email protected] -> /Users/simen/Development/csslint-stylish
├── [email protected] -> /Users/simen/Development/grunt-contrib-csslint
├── [email protected] -> /Users/simen/Development/gulp-amdcheck
├── [email protected] -> /Users/simen/Development/gulp-csslint
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]

@sebastienbarre
Copy link
Contributor Author

Thanks guys. I deleted my node_modules directory, re-installed, and tests are now passing. Maybe a glitch during a transition to iojs. I'll work on adding a test.

@SimenB
Copy link
Contributor

SimenB commented May 9, 2015

@sebastienbarre You should look at how html-loader does it, it picks up whether to minify from the environment if it's not explicit in its options. Not sure if there's a difference between a loader and a plugin though.
https://github.com/webpack/html-loader/blob/master/index.js#L59

(This as well as my own #32 would be obsolete if #10 were implemented, as we could just pass it through html-loader.)

@jantimon jantimon mentioned this pull request May 12, 2015
@jantimon jantimon closed this May 12, 2015
@lock
Copy link

lock bot commented May 31, 2018

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants