replace NODE_ENV with mode #49
Conversation
Codecov Report
@@ Coverage Diff @@
## master #49 +/- ##
==========================================
+ Coverage 98.78% 98.83% +0.05%
==========================================
Files 2 2
Lines 82 86 +4
==========================================
+ Hits 81 85 +4
Misses 1 1
Continue to review full report at Codecov.
|
test/core.js
Outdated
| test.serial('with NODE_ENV=production', async t => { | ||
| process.env.NODE_ENV = 'production'; | ||
|
|
||
| test.serial('with mode=production', async t => { |
src/index.js
Outdated
| @@ -34,7 +34,7 @@ export default class DynamicCdnWebpackPlugin { | |||
|
|
|||
| apply(compiler) { | |||
| if (!this.disable) { | |||
| this.execute(compiler, {env: this.env}); | |||
| this.execute(compiler, {env: this.env || compiler.options.mode || 'production'}); | |||
There was a problem hiding this comment.
Here how compiler.options.mode I believe should map:
none:developmentundefined:production
production and development stays the same
There was a problem hiding this comment.
probably moving the logic into its own function will make it nicer?
|
Updated the README and moved the mapping logic to a function. |
readme.md
Outdated
| @@ -158,7 +158,7 @@ Usefull when working offline, will fallback to webpack normal behaviour | |||
| #### options.env | |||
|
|
|||
| Type: `string`<br> | |||
| Default: `process.env.NODE_ENV || 'development'`<br> | |||
| Default: `mode` used by webpack<br> | |||
There was a problem hiding this comment.
Any ideas on the wording here? I really couldn't think of much.
There was a problem hiding this comment.
This is how we've done it: https://github.com/danethurber/webpack-manifest-plugin#optionspublicpath
With the pseudo code at the beginning: https://github.com/danethurber/webpack-manifest-plugin#api
|
I believe we are good to go! :D |
Towards closing #40.
The breaking change is that I'm defaulting to
productionwhen an environment is not specified. Since webpack >= 4 does the same whenmodeis omitted, I believe we can do the same thing.I don't think there is any webpack <= 3 code left to change. README will need to be updated - I think a separate PR will be needed since there'll be other changes to be added in the README once #40 is done.