Skip to content

Usage of DefinePlugin should follow Webpack's recommendation #3579

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

Closed
LinusBorg opened this issue Mar 5, 2019 · 0 comments
Closed

Usage of DefinePlugin should follow Webpack's recommendation #3579

LinusBorg opened this issue Mar 5, 2019 · 0 comments

Comments

@LinusBorg
Copy link
Member

LinusBorg commented Mar 5, 2019

What problem does this feature solve?

In the webpack docs for its DefinePlugin, this warning can be found:

When defining values for process prefer 'process.env.NODE_ENV': JSON.stringify('production') over process: { env: { NODE_ENV: JSON.stringify('production') } }. Using the latter will overwrite the process object which can break compatibility with some modules that expect other values on the process object to be defined.

However, we currently do pass the env variables that we want to inject into our app code as such a nested object, see:

https://github.com/vuejs/vue-cli/blob/dev/packages/%40vue/cli-service/lib/util/resolveClientEnv.js#L19-L21

What does the proposed API look like?

Implementation should follow the recommendation from webpack docs and construct the object that we pass to the DefinePlugin accordingly, or we should switch to using the EnvironmentPlugin.

The tricky part about the second option is that it only accepts an array of env variable names to pick from process.env, but we have to inject BASE_URL from options.publicPath as this value isn't avaliable on process.env. We can use an object notation to provide "defaults", which should do the job just fine.

I prefer the second option as it would slim simplify the implementation quite a bit. We should keep the DefinePlugin in the code either way as som people may have chainWebpack configs that tap into its options.

I'll send a PR if we want to do this.

haoqunjiang added a commit to haoqunjiang/vue-cli that referenced this issue Apr 9, 2019
haoqunjiang added a commit to haoqunjiang/vue-cli that referenced this issue Apr 9, 2019
haoqunjiang added a commit to haoqunjiang/vue-cli that referenced this issue Apr 9, 2019
BREAKING CHANGE:
This change breaks use cases where users have tapped the `define`
plugin options in `chainWebpack`

fixes vuejs#3579
haoqunjiang added a commit that referenced this issue Apr 22, 2019
BREAKING CHANGE:
This change breaks use cases where users have tapped the `define`
plugin options in `chainWebpack`

fixes #3579
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants