Enable minification by default when 'mode' is production #1048
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously minification was disabled by default. Now, if `minify`
is `undefined` and `mode` is `'production'`, then it is enabled using
the following options:
```
{
collapseWhitespace: true,
removeComments: true,
removeRedundantAttributes: true,
removeScriptTypeAttributes: true,
removeStyleLinkTypeAttributes: true,
useShortDoctype: true
}
```
These options were based on the settings used by create-react-app,
Neutrino and vue-cli, and are hopefully fairly conservative. See:
#1036 (comment)
https://github.com/kangax/html-minifier#options-quick-reference
These same defaults can enabled regardless of `mode`, by setting
`minify` to `true` (which previously passed an empty object to
html-minifier, meaning most minification features were disabled).
Similarly, minification can be disabled even in production, by setting
`minify` to `false`.
This change has no effect on users who pass an object to `minify`.
Fixes #1036.
jantimon
requested changes
Sep 16, 2018
index.js
Outdated
| @@ -99,6 +99,19 @@ class HtmlWebpackPlugin { | |||
| this.options.filename = path.relative(compiler.options.output.path, filename); | |||
| } | |||
|
|
|||
| const minify = this.options.minify; | |||
| if (minify === true || (minify === undefined && compiler.options.mode === 'production')) { | |||
There was a problem hiding this comment.
Could you please add the following code to make it easier to understand?
// Check if webpack is running in production mode
// @see https://github.com/webpack/webpack/blob/3366421f1784c449f415cda5930a8e445086f688/lib/WebpackOptionsDefaulter.js#L12-L14
const isProductionLikeMode = compiler.options.mode === 'production' || !compiler.options.mode;
typings.d.ts
Outdated
| @@ -53,7 +53,7 @@ interface HtmlWebpackPluginOptions { | |||
| * HTML Minification options | |||
| * @https://github.com/kangax/html-minifier#options-quick-reference | |||
| */ | |||
| minify: boolean | {}, | |||
| minify: undefined | boolean | {}, | |||
There was a problem hiding this comment.
All properties are optional so we don't need to add the undefined here :)
There was a problem hiding this comment.
Without this I got a type error:
index.js:39:11 - error TS2322: Type '{ template: string; templateContent: false; templateParameters: (compilation: any, assets: { publ...' is not assignable to type 'HtmlWebpackPluginOptions'.
Types of property 'minify' are incompatible.
Type 'undefined' is not assignable to type 'boolean | {}'.
39 const defaultOptions = {
~~~~~~~~~~~~~~
I can try changing it to optional instead?
There was a problem hiding this comment.
Oh sorry you are absolutely right - usually all options are non optional after the constructor.
But as we don't know the compiler state at this time your solution is absolutely correct.
There was a problem hiding this comment.
Would you prefer optional or a union with undefined?
|
One of the Travis jobs failed due to an infra error - could you restart it? (Travis only permits those with commit access to restart jobs unfortunately) |
jantimon
approved these changes
Sep 17, 2018
|
Thank you so much 👍 |
|
Thank you for the review/merge :-) |
This was referenced Sep 17, 2018
edmorley
added a commit
to neutrinojs/neutrino
that referenced
this pull request
Sep 20, 2018
The next release of `html-webpack-plugin` will enable minification by default when `mode` is production, using `minify` options that are based on those used by popular projects: jantimon/html-webpack-plugin#1048 Those changes haven't yet been released, so in the meantime we can emulate them via preset options, so that we can test out the new defaults sooner (ie: in the upcoming alpha/beta), rather than making the breaking change just before Neutrino 9 final. Once a new version of `html-webpack-plugin` is released, we can stop overriding `minify` entirely.
edmorley
added a commit
to neutrinojs/neutrino
that referenced
this pull request
Sep 28, 2018
The new version includes jantimon/html-webpack-plugin#1048, which means we can remove the temporary additions made in #1121.
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously minification was disabled by default. Now, if
minifyisundefinedandmodeis'production', then it is enabled using the following options:These options were based on the settings used by create-react-app, Neutrino and vue-cli, and are hopefully fairly conservative. See:
#1036 (comment)
https://github.com/kangax/html-minifier#options-quick-reference
These same defaults can enabled regardless of
mode, by settingminifytotrue(which previously passed an empty object to html-minifier, meaning most minification features were disabled). Similarly, minification can be disabled even in production, by settingminifytofalse.This change has no effect on users who pass an object to
minify.Fixes #1036.