Skip to content

Fix small errors in webpack.prod.conf #897

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

Merged
merged 2 commits into from
Sep 30, 2017
Merged

Fix small errors in webpack.prod.conf #897

merged 2 commits into from
Sep 30, 2017

Conversation

robwierzbowski
Copy link
Contributor

I found a two small errors in webpack.prod.conf. One is an unused parameter, deleted in the commit below.

The second issue I need a little input on. Here you're requiring a package that's not installed in the project by default. BUT the option that requires it is turned off by default.

Do you think it's better to install the package, so users can turn the gzip option on and have everything work right away, or leave the package out of package.json, for quicker download? I think the first option is better, esp. for testing and maintenance of the project.

Let me know what you think and I'll resolve this second issue and ping for code review.

@robwierzbowski robwierzbowski changed the base branch from develop to master September 3, 2017 17:44
@LinusBorg LinusBorg changed the base branch from master to develop September 30, 2017 11:45
@LinusBorg
Copy link
Contributor

I think we will leave the gzip option as it is. /config/index.js has comments that tell users to install that package before setting that option to true, so I think this is covered well enough.

We usually like to keep the package.json as light as possible, it's already heavy enough.

@LinusBorg LinusBorg merged commit 8150b73 into vuejs-templates:develop Sep 30, 2017
frandiox pushed a commit to OnsenUI/vue-cordova-webpack that referenced this pull request Oct 13, 2017
* Update vue-loader link (vuejs-templates#886)

* Remove unused count variable in webpack.prod.conf.js
c0defre4k added a commit to neonblack-at/webpack that referenced this pull request Oct 30, 2017
* vuejs-templates/master: (29 commits)
  Fix missing dependency bumps (vuejs-templates#987)
  Bumping Vue+VueRouter versions, some minor fixes. (vuejs-templates#986)
  stick to ES5
  v1.1.1
  downgrade airbnb config to ensure compaibility with eslint.
  Typo fixed: 'use stxrict' -> 'use strict' (vuejs-templates#955)
  We now have port-detection, so README should reflect that.
  Revision of wording in the README docs (vuejs-templates#958)
  add version tag to config/index.js
  add version tag to config/index.js
  Reference dev script in start script instead of copy pasting (vuejs-templates#894)
  Reference dev script in start script instead of copy pasting (vuejs-templates#894)
  Put hotMiddleware before proxyMiddleware in dev server (vuejs-templates#837)
  fix component filename
  Rename Hello component to HelloWorld (vuejs-templates#944)
  bump more deps
  Add .vscode to gitignore (vuejs-templates#845)
  update URL of ESLint to https (vuejs-templates#927)
  Fix small errors in webpack.prod.conf (vuejs-templates#897)
  Standardize base webpack module variable name (vuejs-templates#912)
  ...

# Conflicts:
#	template/build/dev-server.js
#	template/build/webpack.prod.conf.js
#	template/package.json
#	template/src/components/HelloWorld.vue
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.

3 participants