Skip to content

webpack 4, vue-loader 15, et al #1369

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

Open
wants to merge 49 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
e98bcc0
Updated webpack template to use webpack 4 and latest versions of many…
noamkfir Apr 27, 2018
6bba5e4
Restored chunking in webpack 4 based on config for webpack 3.
noamkfir Apr 27, 2018
709b528
Added [name] to chunk filename for readability.
noamkfir Apr 27, 2018
1c7856a
Fixed incorrect path regex.
noamkfir Apr 27, 2018
00aae83
Added runtime chunk (replaces manifest chunk).
noamkfir Apr 27, 2018
edd85ff
Merged some recommended chunking settings based on webpack 4 defaults.
noamkfir Apr 27, 2018
27481de
Removed "enforce" property. Doesn't seem to do anything useful.
noamkfir Apr 27, 2018
f6f9cd9
Moved uglifyjs plugin to new optimization.minimizer config property.
noamkfir Apr 27, 2018
c699f00
Updated vue-loader to 15. Also refactored generateLoaders for clarity.
noamkfir Apr 27, 2018
8d638f0
Updated nightwatch to 1.0.3 beta. Attempt to fix failing build.
noamkfir Apr 28, 2018
3ebf10b
Testing a docker image that doesn't install the chromedriver globally.
noamkfir Apr 28, 2018
00d5aa3
Another try.
noamkfir Apr 28, 2018
b2c2785
And another one.
noamkfir Apr 28, 2018
2056ba6
Last try before we die.
noamkfir Apr 28, 2018
0783d37
Attempt to build using a custom CircleCI docker image.
noamkfir Apr 29, 2018
3fc4965
Adding artifacts to failing builds.
noamkfir May 1, 2018
e6cc3f2
Created logs folder, set nightwatch to store webdriver logs there, an…
noamkfir May 2, 2018
63f95b3
Removed chromedriver start_process instruction.
noamkfir May 2, 2018
df5dadf
Added server info to the chromedriver logging preferences.
noamkfir May 2, 2018
d731f25
Fixed log path for test-full-karma-airbnb in circleci config.yml.
noamkfir May 2, 2018
597f74a
Removed 'server' logging unsupported by chromedriver.
noamkfir May 2, 2018
cee65fd
Testing debian 9 (stretch) image.
noamkfir May 2, 2018
6162c84
Testing node 6.14.1 on debian 9 (stretch) browsers image.
noamkfir May 2, 2018
cd325ef
Trying node 6.14.1 with debian 9 (stretch) and browsers, but no drivers.
noamkfir May 2, 2018
c503037
Added headless arg for chrome browser.
noamkfir May 2, 2018
7ed7f28
Back to circleci 6.12.3-browsers image.
noamkfir May 2, 2018
6d13e1e
Trying circleci/node:6.14.1-browsers again.
noamkfir May 2, 2018
2d13661
Restoring generated image with LATEST chrome instead of CircleCI's v64.
noamkfir May 2, 2018
0db5de0
Unified webpack output filename templates for js and css files, based…
noamkfir May 19, 2018
052a757
Removed obsolete reference to CommonsChunkPlugin in comment.
noamkfir May 19, 2018
b7a10c1
Optimize both statically and dynamically imported chunks per webpack …
noamkfir May 19, 2018
aac5cc0
Force creation of vendor chunk and restore default webpack 4 prioriti…
noamkfir May 19, 2018
3aa7005
Changed vendor chunk test to match *any* node_modules files, not just…
noamkfir May 19, 2018
882d316
Removed explicit "app" chunk in favor of default webpack configuratio…
noamkfir May 19, 2018
7fe7b3c
Added NamedChunksPlugin before HashedModuleIdsPlugin to stabilize chu…
noamkfir May 19, 2018
3604040
Cleaned up nightwatch.conf.js a bit.
noamkfir May 23, 2018
ad1999d
Added debugTests constant (defaulting to false) to nightwatch.conf.js.
noamkfir May 23, 2018
31844ea
Update to babel 7
miaulightouch Jun 9, 2018
9bdb716
Update deps
miaulightouch Jun 9, 2018
0e7e90c
Upgrade to vue-cli 3
miaulightouch Jun 9, 2018
922b68c
Update docs [ci skip]
miaulightouch Jun 9, 2018
f43a379
Fix typo [ci skip]
miaulightouch Jun 10, 2018
26ea1fd
Fix @babel/register lost
miaulightouch Jun 10, 2018
13904d2
Merge pull request #1 from miaulightouch/pr-to-babel
noamkfir Jun 10, 2018
3b24582
Merge branch 'develop' into webpack-4
noamkfir Jun 10, 2018
82b49a3
Fix @babel/runtime missing issue
miaulightouch Jun 19, 2018
baead7a
Merge pull request #2 from miaulightouch/pr-to-babel
noamkfir Jun 30, 2018
7717e47
Removed redundant concatenateModules setting in production webpack co…
noamkfir Jul 19, 2018
5b90c53
Removed redundant namedModules setting in webpack dev config.
noamkfir Jul 19, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
version: 2
vm_settings: &vm_settings
docker:
- image: circleci/node:6.12.3-browsers
# - image: circleci/node:6.14.1-browsers
- image: boundlesscode/node:6.14.1-stretch-browsers-sans-drivers

jobs:
install_template_deps:
Expand Down Expand Up @@ -86,6 +87,10 @@ jobs:
name: Test build
command: npm run build
when: always
- store_artifacts:
path: ~/project/webpack-template/test-full/logs
- store_artifacts:
path: ~/project/webpack-template/test-full/tests_output

scenario_full-karma-airbnb:
<<: *vm_settings
Expand Down Expand Up @@ -119,6 +124,10 @@ jobs:
name: Test build
command: npm run build
when: always
- store_artifacts:
path: ~/project/webpack-template/test-full-karma-airbnb/logs
- store_artifacts:
path: ~/project/webpack-template/test-full-karma-airbnb/tests_output


workflows:
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 18 additions & 12 deletions template/build/utils.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict'
const path = require('path')
const config = require('../config')
const ExtractTextPlugin = require('extract-text-webpack-plugin')
const MiniCssExtractPlugin = require('mini-css-extract-plugin')
const packageConfig = require('../package.json')

exports.assetsPath = function (_path) {
Expand Down Expand Up @@ -31,7 +31,22 @@ exports.cssLoaders = function (options) {

// generate loader string to be used with extract text plugin
function generateLoaders (loader, loaderOptions) {
const loaders = options.usePostCSS ? [cssLoader, postcssLoader] : [cssLoader]
const loaders = []

// Extract CSS when that option is specified
// (which is the case during production build)
if (options.extract) {
loaders.push(MiniCssExtractPlugin.loader)
}
else {
loaders.push('vue-style-loader')
}

loaders.push(cssLoader)

if (options.usePostCSS) {
loaders.push(postcssLoader)
}

if (loader) {
loaders.push({
Expand All @@ -42,16 +57,7 @@ exports.cssLoaders = function (options) {
})
}

// Extract CSS when that option is specified
// (which is the case during production build)
if (options.extract) {
return ExtractTextPlugin.extract({
use: loaders,
fallback: 'vue-style-loader'
})
} else {
return ['vue-style-loader'].concat(loaders)
}
return loaders
}

// https://vue-loader.vuejs.org/en/configurations/extract-css.html
Expand Down
12 changes: 1 addition & 11 deletions template/build/vue-loader.conf.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,9 @@
'use strict'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Does this file still makes sense? vue-loader config was weird enough to justify its own file, but since now most of its configuration is wiped, should we consider inlining it to webpack.base.conf.js?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gkatsanos, they seem to work well on my machine... The CSS file in dist is minified and when I click a selector, Chrome opens the .vue file and highlights the proper line.

Can you point me to something specific so I can verify?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@afontcu, I think it should stay, but I'm not really sure.

Vue is very new for me, so I don't know how every setting affects the build or the app. I only made changes I felt certain were equivalent to the previous behavior or completely redundant, mainly by following Vue's migration guide, reading docs, and trial and error.

For example, I got rid of the loaders property because the new vue-loader explicitly deprecates it in favor of the the webpack loaders configuration. And I renamed the transformToRequire property to transformAssetUrls, based on the guide.

My hunch is that it should probably stay for the sake of clarity, even though the usage has changed a bit.

Having said that, I haven't checked if the cacheBusting property is used anywhere, and I'm not sure if we should add other properties that are relevant for the new vue-loader.

const utils = require('./utils')
const config = require('../config')
const isProduction = process.env.NODE_ENV === 'production'
const sourceMapEnabled = isProduction
? config.build.productionSourceMap
: config.dev.cssSourceMap

module.exports = {
loaders: utils.cssLoaders({
sourceMap: sourceMapEnabled,
extract: isProduction
}),
cssSourceMap: sourceMapEnabled,
cacheBusting: config.dev.cacheBusting,
transformToRequire: {
transformAssetUrls: {
video: ['src', 'poster'],
source: 'src',
img: 'src',
Expand Down
4 changes: 4 additions & 0 deletions template/build/webpack.base.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const path = require('path')
const utils = require('./utils')
const config = require('../config')
const vueLoaderConfig = require('./vue-loader.conf')
const { VueLoaderPlugin } = require('vue-loader')

function resolve (dir) {
return path.join(__dirname, '..', dir)
Expand Down Expand Up @@ -31,6 +32,9 @@ module.exports = {
? config.build.assetsPublicPath
: config.dev.assetsPublicPath
},
plugins: [
new VueLoaderPlugin(),
],
resolve: {
extensions: ['.js', '.vue', '.json'],
alias: {
Expand Down
9 changes: 6 additions & 3 deletions template/build/webpack.dev.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const HOST = process.env.HOST
const PORT = process.env.PORT && Number(process.env.PORT)

const devWebpackConfig = merge(baseWebpackConfig, {
mode: 'development',
module: {
rules: utils.styleLoaders({ sourceMap: config.dev.cssSourceMap, usePostCSS: true })
},
Expand Down Expand Up @@ -49,8 +50,6 @@ const devWebpackConfig = merge(baseWebpackConfig, {
'process.env': require('../config/dev.env')
}),
new webpack.HotModuleReplacementPlugin(),
new webpack.NamedModulesPlugin(), // HMR shows correct file names in console on update.
new webpack.NoEmitOnErrorsPlugin(),
// https://github.com/ampedandwired/html-webpack-plugin
new HtmlWebpackPlugin({
filename: 'index.html',
Expand All @@ -65,7 +64,11 @@ const devWebpackConfig = merge(baseWebpackConfig, {
ignore: ['.*']
}
])
]
],
optimization: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessary. In the mode: 'development' is the default configuration.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right about namedModules. I'm removing it.

However, the docs for noEmitOnErrors seem to indicate that it does not have a default value, so I'm leaving it for now.

namedModules: true,
noEmitOnErrors: true,
},
})

module.exports = new Promise((resolve, reject) => {
Expand Down
84 changes: 32 additions & 52 deletions template/build/webpack.prod.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const merge = require('webpack-merge')
const baseWebpackConfig = require('./webpack.base.conf')
const CopyWebpackPlugin = require('copy-webpack-plugin')
const HtmlWebpackPlugin = require('html-webpack-plugin')
const ExtractTextPlugin = require('extract-text-webpack-plugin')
const MiniCssExtractPlugin = require('mini-css-extract-plugin')
const OptimizeCSSPlugin = require('optimize-css-assets-webpack-plugin')
const UglifyJsPlugin = require('uglifyjs-webpack-plugin')

Expand All @@ -16,6 +16,7 @@ const env = {{#if_or unit e2e}}process.env.NODE_ENV === 'testing'
: {{/if_or}}require('../config/prod.env')

const webpackConfig = merge(baseWebpackConfig, {
mode: 'production',
module: {
rules: utils.styleLoaders({
sourceMap: config.build.productionSourceMap,
Expand All @@ -27,30 +28,15 @@ const webpackConfig = merge(baseWebpackConfig, {
output: {
path: config.build.assetsRoot,
filename: utils.assetsPath('js/[name].[chunkhash].js'),
chunkFilename: utils.assetsPath('js/[id].[chunkhash].js')
},
plugins: [
// http://vuejs.github.io/vue-loader/en/workflow/production.html
new webpack.DefinePlugin({
'process.env': env
}),
new UglifyJsPlugin({
uglifyOptions: {
compress: {
warnings: false
}
},
sourceMap: config.build.productionSourceMap,
parallel: true
}),
// extract css into its own file
new ExtractTextPlugin({
filename: utils.assetsPath('css/[name].[contenthash].css'),
// Setting the following option to `false` will not extract CSS from codesplit chunks.
// Their CSS will instead be inserted dynamically with style-loader when the codesplit chunk has been loaded by webpack.
// It's currently set to `true` because we are seeing that sourcemaps are included in the codesplit bundle as well when it's `false`,
// increasing file size: https://github.com/vuejs-templates/webpack/issues/1110
allChunks: true,
new MiniCssExtractPlugin({
filename: utils.assetsPath('css/[name].[chunkhash].css'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}),
// Compress extracted CSS. We are using this plugin so that possible
// duplicated CSS from different components can be deduped.
Expand All @@ -75,43 +61,12 @@ const webpackConfig = merge(baseWebpackConfig, {
// more options:
// https://github.com/kangax/html-minifier#options-quick-reference
},
// necessary to consistently work with multiple chunks via CommonsChunkPlugin
// necessary to consistently work with multiple chunks
chunksSortMode: 'dependency'
}),
// keep module.id stable when vendor modules does not change
new webpack.NamedChunksPlugin(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

@noamkfir noamkfir Jul 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PanJiaChen Thanks for the thorough review!

I did a lot of experimentation before I added the NamedChunksPlugin because the HashedModuleIdsPlugin did not guarantee consistent hashing, especially of the vendor module.

I found that almost every change I made to any import statement in my code could cause the vendor module hash to change. The NamedChunksPlugin uses the path instead of the arbitrary module id to calculate the hash, and this proved to be far more consistent.

It's definitely possible I missed something, or that the behavior improved since webpack 4.6 (it's now 4.16), but I hesitate to reopen this right now as changing this or upgrading to 4.16 might have further consequences. In any case, after this PR is approved, doing that kind of change is a much smaller and far more focused change.

new webpack.HashedModuleIdsPlugin(),
// enable scope hoisting
new webpack.optimize.ModuleConcatenationPlugin(),
// split vendor js into its own file
new webpack.optimize.CommonsChunkPlugin({
name: 'vendor',
minChunks (module) {
// any required modules inside node_modules are extracted to vendor
return (
module.resource &&
/\.js$/.test(module.resource) &&
module.resource.indexOf(
path.join(__dirname, '../node_modules')
) === 0
)
}
}),
// extract webpack runtime and module manifest to its own file in order to
// prevent vendor hash from being updated whenever app bundle is updated
new webpack.optimize.CommonsChunkPlugin({
name: 'manifest',
minChunks: Infinity
}),
// This instance extracts shared chunks from code splitted chunks and bundles them
// in a separate chunk, similar to the vendor chunk
// see: https://webpack.js.org/plugins/commons-chunk-plugin/#extra-async-commons-chunk
new webpack.optimize.CommonsChunkPlugin({
name: 'app',
async: 'vendor-async',
children: true,
minChunks: 3
}),

// copy custom static assets
new CopyWebpackPlugin([
{
Expand All @@ -120,7 +75,32 @@ const webpackConfig = merge(baseWebpackConfig, {
ignore: ['.*']
}
])
]
],
optimization: {
concatenateModules: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In mode: 'production', it default enable.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

splitChunks: {
chunks: 'all',
cacheGroups: {
vendor: {
name: 'vendor',
test: /[\\/]node_modules[\\/]/,
enforce: true,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find any documentation on this enforce option?
https://webpack.js.org/plugins/split-chunks-plugin/#splitchunks-cachegroups

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zevdg Quoting @thatisuday (webpack/webpack#6647 (comment)):

enforce will force modules to split whether or not condition mentioned in splitChunks suitable to modules to split.

If I recall correctly, I added it to ensure consistent hashing, but I really don't remember the details and, in hindsight, that probably doesn't make much sense :(. I do remember that it took me way too long to solve whatever the problem was because, as you said, there was no documentation for enforce so I didn't know the option existed until I came across it by accident on some unrelated article.

The SplitChunksPlugin code basically uses it to arbitrarily set other parameters to default values that guarantee that the chunk will be created.

},
},
},
runtimeChunk: 'single',
minimizer: [
new UglifyJsPlugin({

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed since you are already using mode: production?

According to the docs, they have removed UglifyJsPlugin.

uglifyOptions: {
compress: {
warnings: false
}
},
sourceMap: config.build.productionSourceMap,
parallel: true
}),
],
},
})

if (config.build.productionGzip) {
Expand Down
1 change: 1 addition & 0 deletions template/build/webpack.test.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const merge = require('webpack-merge')
const baseWebpackConfig = require('./webpack.base.conf')

const webpackConfig = merge(baseWebpackConfig, {
mode: 'development',
// use inline sourcemap for karma-sourcemap-loader
module: {
rules: utils.styleLoaders()
Expand Down
2 changes: 2 additions & 0 deletions template/logs/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
*
!.gitignore
Loading