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

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

Open
wants to merge 49 commits into
base: develop
Choose a base branch
from

Conversation

noamkfir
Copy link

This PR includes the following:

  • webpack updated to 4.6.0
  • vue-loader updated to 15.0.3
  • many other packages updated to their most recent versions, including: karma, mocha, sinon-chai, babel, various eslint and webpack plugins and loaders, and others
  • extract-text-webpack-plugin replaced with mini-css-extract-plugin

When upgrading vue-loader, I followed the migration guide. The notable exception is that I did not add CSS modules because that would require a bit of a rewrite of how the webpack rules are generated in utils, and CSS modules aren't supported yet by the template anyway, as far as I can tell.

Vue is kind of new for me, so I'm not sure how to verify that everything works correctly. Things seem to be working well enough for me :). The options I use are: standalone, with vue-router, eslint, and unit tests, the Standard esline preset, karma and mocha, Nightwatch, and npm. I've run all the npm scripts and they all seem to be doing their jobs.

#1280
#1367

@ChadTaljaardt
Copy link

Thanks for taking the time to do this.

@noamkfir
Copy link
Author

I'm trying to get the build to pass. I noticed that the chromedriver being used for the e2e tests is 2.35, which is apparently installed by the image in /usr/bin/chromedriver. It should be using 2.38, which is specified in package.json and located in node_modules.

The Dockerfile used for the build tries to install the latest chromedriver, which should be 2.38, but either the script isn't working properly, some cache (maybe CircleCI's?) is returning the older version, or the chromedriver is preinstalled and the script is failing to overwrite it.

I'm trying to configure a build that is identical to the current build, except that it does not try to install the global chromedriver, by commenting out just the chromedriver installation command. Turns out it's pretty complicated to get that set up and running. I'm working on it, but if anybody knows a faster way to get CircleCI to use a slightly different Dockerfile, I would be obliged :).

@noamkfir noamkfir mentioned this pull request Apr 28, 2018
@Garito
Copy link

Garito commented Jul 1, 2018

Hi!
I should be agree with @LinusBorg about the point of this template
But!
That doesn't mean that this ends here
As I see it you have a good amount of knowledge stacked since this project starts
Why don't use it to start small plugins that solves the same features but with the new cli?
I bet you that you will come up with ideas as soon as you start to check what has been made so far
I would like to thank you, too, for your work till now (I've started and not gone back till cli3 with the pwa which uses this one as base)

@LinusBorg
Copy link
Contributor

@Garito every single feature of this template is already included in vue-cli 3.

And if you do a quick search on npmjs.com, you will find that there are already dozens of plugins available for it, including things like vuetify, i18n, Apollo graphql etc. pp.

@Garito
Copy link

Garito commented Jul 1, 2018

I know but there's always something to do
I'm trying not to be negative only while expressing my agreement with the idea of the obsolescence of this template
Isn't easy to accept sometimes (we have jumped from project to project several times)
But always that I've changed my path because of this I carry new skills which help me start over
I just can thank you

@mikeerickson
Copy link

As much as I applaud and like projects such as the Vue CLI, there is always room for options. I my case, we have taken the approach of iterating much of our UI forms from PHP to more API based and all new UI work is JS (we have used Angular, React and Vue)

@LinusBorg LinusBorg mentioned this pull request Jul 6, 2018
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.

@HenryWong-fe
Copy link

@LinusBorg I know that vue-cli has a lot of configuration items, but how can vue-cli ensure that my project public files can exist every time I create them? I need to use some configuration of the meta files in the template to decide whether to use some plugins and file

@@ -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.

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.

]
],
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.

👍

// 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.

@yuicer
Copy link

yuicer commented Aug 27, 2018

well....will it be merged soon? or it still has some problems to do?
thanks to all of you ;)

},
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.

@ruchernchong
Copy link

Hi @LinusBorg , what is the status of this? It does looks good to merge now.

@mjdietzx
Copy link

Any update here, would really like to updgrade to webpack4! Thanks for this PR!

@PanJiaChen
Copy link
Contributor

I think it is easier and safer to migrate directly to vue-cli@3 than to upgrade from this template to webpack4 now.

@ruchernchong
Copy link

Old projects might still need this

@mjdietzx
Copy link

@PanJiaChen i've been migrating my project to vue-cli@3 and it's amazing! so i actually don't need this anymore

@miaulightouch
Copy link

miaulightouch commented Oct 29, 2018

some of deps still using beta package, I would take time for updating these packages.

and I would say in some case, like desktop app development, this template is still a good start.

@LeFnord
Copy link

LeFnord commented Mar 3, 2019

is where any progress on it, like to have mörged it, thanks

@ruchernchong
Copy link

ruchernchong commented Mar 3, 2019

This PR is great, but Webpack 5 is imminent and I am not sure if this will come effective by then.

Also, vue-cli is now v3 and this should be deprecated by now.

marienfressinaud added a commit to lessy-community/lessy that referenced this pull request May 5, 2019
I spent a bit more time to update to Webpack 4. I hope it's finished
now...

References:

- vuejs-templates/webpack#1280
- vuejs-templates/webpack#1369
marienfressinaud added a commit to lessy-community/lessy that referenced this pull request May 5, 2019
I spent a bit more time to update to Webpack 4. I hope it's finished
now...

References:

- vuejs-templates/webpack#1280
- vuejs-templates/webpack#1369
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.