kraftvaerk / generator-rammevaerk

Scaffold a web project in kraftvaerk style
MIT License
5 stars 4 forks source link

webpack 4 migration #35

Closed mi2oon closed 6 years ago

mi2oon commented 6 years ago

Hi there,

You've probably heard webpack 4 is out.. so lets update and get testing 💥 😸

Closes #30

kristoforsalmin commented 6 years ago

Hi @mi2oon, that's awesome! Cool that we keep the pace 👍 Man, I have to get back to rammevaerk, just a bit busy with all kinds of other stuff (you know what I'm talking about 😄).

4 version is just as fast as everyone claims it is 😄

Here's few things I noticed, mostly configuration-related stuff (yeah, sorry, I always try to make config as tiny as possible). Let's see if some of it really does make sense 😃

devtool

devtool: global.production ? 'source-maps' : 'cheap-module-eval-source-map'

I think we can remove this one, because when mode set to development we're getting eval maps out of box and no source maps when mode set to production.

babel-loader

{
    test: /\.js$/,
    loader: 'babel-loader',
    exclude: /node_modules/
}

Not so much relevant to webpack 4, but I think we can cache Babel's transformation to file system when we're in development mode.

{
    test: /\.js$/,
    exclude: /node_modules/,
    loader: 'babel-loader',
    options: {
        cacheDirectory: !global.production
    }
}

https://github.com/babel/babel-loader#babel-loader-is-slow

optimization.minimizer

new UglifyJsPlugin({
    sourceMap: true,
    extractComments: true
})

Maybe we should just get rid of this altogether because when mode set to production webpack does this automatically (without extractComments option though, but I'm not sure why we need it). Another thing is that we won't uglify anything in development, so faster builds 🚀 What do you think? 😄

optimization.splitChunks.cacheGroups

It seems I'm going "how I would've done it" kind of way here, so please take it with a grain of salt 😄 But! As we don't have any docs I read somewhere that webpack by default has two cache groups: default and vendor. Last one already includes regex (/node_modules/) and name (vendor) that actually describes what this group is all about (common is actually fine too). After my experiments I end up with this configuration:

vendor: {
    name: 'Test.vendor',
    chunks: 'all',
    filename: '[name].bundle.js'
}

I was just thinking that we may need common cache group for, let's say, shared code between multiple bundles (and, yeah, I saved one line of code 🥇, joking of course).

They also have priority option and it has a negative value (-10) for vendor group by default. I need to spend more time on figuring out how it works out for us in different cases (please share if you already experimented with it 😄). Maybe it just works the way it did before with CommonsChunkPlugin. It's actually just a side note for myself, so it doesn't affect merge or anything.

plugins

Lastly, I think we can just remove this:

new webpack.EnvironmentPlugin({
    NODE_ENV: 'development'
})

As webpack seems to do exactly the same depending on what mode property is set to.


I know I put a lot of stuff on the table, but I think at least it's worth discussing it and no matter what we decide in the end — we will know why it was coded like that and so on 😄

Looking forward to hear from you 👍

kristoforsalmin commented 6 years ago

Hi @mi2oon, just a quick summary after our talk about some of the stuff above:

devtool

Here to stay as we usually do need them in production. Right?

optimization.minimizer

Here to stay as we do need to extract comments to a separate file and this option is off by default.

optimization.splitChunks.cacheGroups

It only makes sense to rename from common to vendor, so common could be used for shared code between multiple entries (test: /node_modules/ is here to stay as it's more obvious). Is that right? 😄

kristoforsalmin commented 6 years ago

@mi2oon I can commit to the upstream, hehe 😄 Joking, just wanted to help you here, please take a look at my changes and let me know what you think!

kristoforsalmin commented 6 years ago

Great, let's merge it 😄