Migrate to mini-css-extract-plugin from extract-text-webpack-plugin #1062
Conversation
|
Sorry this pr includes too many features and also some quite opinionated ones Please ping me for a discussion so we can find out which features we are going to merge :) |
|
@jantimon What about migrating to Can you point out the opinionated changes. So that I could revert them. I made few changes that are commonly found in many open source libs. |
|
In general it's best to keep to one type of change per PR. Looking at the changes here, I see linting fixes, gitignore changes, test cleanups, dependency bumps, deleting dist folders and the switch to mini-css-extract-plugin. Each of those should likely be a separate PR - which then makes it simpler to review/discuss :-) |
BREAKING: `contenthash` in now `templatehash`. Since webpack 4.3 `contenthash` is used by webpack. to avoid conflicts, it is renamed to `templatehash`
| { test: /\.png$/, loader: 'file-loader' } | ||
| ] | ||
| }, | ||
| plugins: [ | ||
| new HtmlWebpackPlugin({ | ||
| template: 'template.html' | ||
| }), | ||
| new ExtractTextPlugin('styles.css') | ||
| new MiniCssExtractPlugin({ filename: 'style.css' }) |
There was a problem hiding this comment.
Could you change style.css back to styles.css (here and elsewhere) to reduce the size of the diff? :-)
There was a problem hiding this comment.
yes, too many diff. I missed that. Updated now
Checkout #1064
| @@ -279,10 +279,10 @@ class HtmlWebpackPlugin { | |||
| return self.options.showErrors ? prettyError(err, compiler.context).toHtml() : 'ERROR'; | |||
| }) | |||
| .then(html => { | |||
| // Allow to use [contenthash] as placeholder for the html-webpack-plugin name | |||
| // Allow to use [templatehash] as placeholder for the html-webpack-plugin name | |||
There was a problem hiding this comment.
This change seems unrelated?
There was a problem hiding this comment.
Since mini-css-extract-plugin requires webpack>4.4. which use contenthash internally. that conflicts with what we use. Hence I renamed that.
contenthash was introduced in webpack 4.3
There was a problem hiding this comment.
Ah makes sense. I guess this change fixes #1033.
There was a problem hiding this comment.
Yes. Just noticed there is also another PR #1034 does the same.
There was a problem hiding this comment.
Actually it's fine for me - maybe we could also replace contenthash to templatehash before we compile it.
So users will be able to use contenthash and templatehash
There was a problem hiding this comment.
Cool. So that no breaking change will be introduced. 👍
|
What about updating webpack in peerDependency to |
|
Thanks alot 👍 |
Migrated from
ETWPtomini-css-extract-plugin. Webpack discourages the use of ETWP since webpack 4.0Since
mini-css-extract-pluginrequires webpack>4.4, hence webpack is updated.Which inturn causes conflicts with
contenthashso to fix tests. I renamed it totemplatehash.. I can't think of more suitable name than this.Breaking:
contenthashis renamed totemplatehash