fix: mode behaviour (#1824)

anshumanv · web-flow · commit 9e9c70bc1f30 · 2020-09-24T17:31:33.000+05:30
* fix: mode behaviour

* fix: mode behaviour
diff --git a/packages/webpack-cli/__tests__/resolveMode.test.js b/packages/webpack-cli/__tests__/resolveMode.test.js
@@ -2,41 +2,81 @@ const resolveMode = require('../lib/groups/resolveMode');
 
 describe('resolveMode', function () {
     it('should handle the mode option [production]', () => {
-        const result = resolveMode({
-            mode: 'production',
-        });
+        const result = resolveMode(
+            {
+                mode: 'production',
+            },
+            {},
+        );
         // ensure no other properties are added
         expect(result.options).toMatchObject({ mode: 'production' });
         expect(result.options.mode).toEqual('production');
     });
 
     it('should handle the mode option [development]', () => {
-        const result = resolveMode({
-            mode: 'development',
-        });
+        const result = resolveMode(
+            {
+                mode: 'development',
+            },
+            {},
+        );
 
         // ensure no other properties are added
         expect(result.options).toMatchObject({ mode: 'development' });
         expect(result.options.mode).toEqual('development');
     });
 
     it('should handle the mode option [none]', () => {
-        const result = resolveMode({
-            mode: 'none',
-        });
+        const result = resolveMode(
+            {
+                mode: 'none',
+            },
+            {},
+        );
 
         // ensure no other properties are added
         expect(result.options).toMatchObject({ mode: 'none' });
         expect(result.options.mode).toEqual('none');
     });
 
-    it('should prefer supplied move over NODE_ENV', () => {
+    it('should prefer supplied move flag over NODE_ENV', () => {
         process.env.NODE_ENV = 'production';
-        const result = resolveMode({
-            mode: 'development',
-        });
+        const result = resolveMode(
+            {
+                mode: 'development',
+            },
+            {},
+        );
 
         // ensure no other properties are added
         expect(result.options).toMatchObject({ mode: 'development' });
     });
+
+    it('should prefer supplied move flag over mode from config', () => {
+        const result = resolveMode(
+            {
+                mode: 'development',
+            },
+            { mode: 'production' },
+        );
+
+        // ensure no other properties are added
+        expect(result.options).toMatchObject({ mode: 'development' });
+    });
+
+    it('should prefer mode form config over NODE_ENV', () => {
+        process.env.NODE_ENV = 'development';
+        const result = resolveMode({}, { mode: 'production' });
+
+        // ensure no other properties are added
+        expect(result.options).toMatchObject({ mode: 'production' });
+    });
+
+    it('should prefer mode form flag over NODE_ENV and config', () => {
+        process.env.NODE_ENV = 'development';
+        const result = resolveMode({ mode: 'none' }, { mode: 'production' });
+
+        // ensure no other properties are added
+        expect(result.options).toMatchObject({ mode: 'none' });
+    });
 });
diff --git a/packages/webpack-cli/lib/groups/resolveMode.js b/packages/webpack-cli/lib/groups/resolveMode.js
@@ -1,26 +1,49 @@
 const PRODUCTION = 'production';
 const DEVELOPMENT = 'development';
 
-const resolveMode = (args) => {
+/*
+Mode priority:
+    - Mode flag
+    - Mode from config
+    - Mode form NODE_ENV
+*/
+
+/**
+ *
+ * @param {string} mode - mode flag value
+ * @param {Object} configObject - contains relevant loaded config
+ */
+const assignMode = (mode, configObject) => {
     const {
         env: { NODE_ENV },
     } = process;
-    const { mode } = args;
-
+    const { mode: configMode } = configObject;
     let finalMode;
-    /**
-     * It determines the mode to pass to webpack compiler
-     * @returns {string} The mode
-     */
-    if (!mode && NODE_ENV && (NODE_ENV === PRODUCTION || NODE_ENV === DEVELOPMENT)) {
+    if (mode) {
+        finalMode = mode;
+    } else if (configMode) {
+        finalMode = configMode;
+    } else if (NODE_ENV && (NODE_ENV === PRODUCTION || NODE_ENV === DEVELOPMENT)) {
         finalMode = NODE_ENV;
     } else {
-        finalMode = mode || PRODUCTION;
+        finalMode = PRODUCTION;
     }
+    return { mode: finalMode };
+};
+
+/**
+ *
+ * @param {Object} args - parsedArgs from the CLI
+ * @param {Object | Array} configOptions - Contains loaded config or array of configs
+ */
+const resolveMode = (args, configOptions) => {
+    const { mode } = args;
+    let resolvedMode;
+    if (Array.isArray(configOptions)) {
+        resolvedMode = configOptions.map((configObject) => assignMode(mode, configObject));
+    } else resolvedMode = assignMode(mode, configOptions);
 
-    return {
-        options: { mode: finalMode },
-    };
+    return { options: resolvedMode };
 };
 
 module.exports = resolveMode;
diff --git a/packages/webpack-cli/lib/webpack-cli.js b/packages/webpack-cli/lib/webpack-cli.js
@@ -77,8 +77,8 @@ class WebpackCLI extends GroupHelper {
         this._mergeOptionsToOutputConfiguration(resolvedConfig.outputOptions);
     }
 
-    async _baseResolver(cb, parsedArgs) {
-        const resolvedConfig = cb(parsedArgs);
+    async _baseResolver(cb, parsedArgs, configOptions) {
+        const resolvedConfig = cb(parsedArgs, configOptions);
         this._mergeOptionsToConfiguration(resolvedConfig.options);
         this._mergeOptionsToOutputConfiguration(resolvedConfig.outputOptions);
     }
@@ -143,6 +143,21 @@ class WebpackCLI extends GroupHelper {
      * @returns {void}
      */
     _mergeOptionsToConfiguration(options, strategy) {
+        /**
+         * options where they differ per config use this method to apply relevant option to relevant config
+         * eg mode flag applies per config
+         */
+        if (Array.isArray(options) && Array.isArray(this.compilerConfiguration)) {
+            this.compilerConfiguration = options.map((option, index) => {
+                const compilerConfig = this.compilerConfiguration[index];
+                if (strategy) {
+                    return webpackMerge.strategy(strategy)(compilerConfig, option);
+                }
+                return webpackMerge(compilerConfig, option);
+            });
+            return;
+        }
+
         /**
          * options is an array (multiple configuration) so we create a new
          * configuration where each element is individually merged
@@ -229,9 +244,9 @@ class WebpackCLI extends GroupHelper {
      */
     async runOptionGroups(parsedArgs) {
         await Promise.resolve()
-            .then(() => this._baseResolver(resolveMode, parsedArgs))
             .then(() => this._handleDefaultEntry())
             .then(() => this._handleConfig(parsedArgs))
+            .then(() => this._baseResolver(resolveMode, parsedArgs, this.compilerConfiguration))
             .then(() => this._handleGroupHelper(this.outputGroup))
             .then(() => this._handleCoreFlags())
             .then(() => this._handleGroupHelper(this.basicGroup))
diff --git a/test/mode/mode-single-arg/mode-single-arg.test.js b/test/mode/mode-single-arg/mode-single-arg.test.js
@@ -26,6 +26,12 @@ describe('mode flags', () => {
         expect(stdout).toContain(`mode: 'none'`);
     });
 
+    it('should pick mode form NODE_ENV', () => {
+        const { stderr, stdout } = run(__dirname, [], false, [], { NODE_ENV: 'development' });
+        expect(stderr).toBeFalsy();
+        expect(stdout).toContain(`mode: 'development'`);
+    });
+
     it('should throw error when --mode=abcd is passed', () => {
         const { stderr } = run(__dirname, ['--mode', 'abcd']);
         expect(stderr).toContain('configuration.mode should be one of these');
diff --git a/test/mode/mode-with-config/mode-with-config.test.js b/test/mode/mode-with-config/mode-with-config.test.js
@@ -92,10 +92,53 @@ describe('mode flags with config', () => {
         });
     });
 
-    it('should use mode from config over flags', () => {
+    it('should use mode flag over config', () => {
         const { stdout, stderr, exitCode } = run(__dirname, ['--mode', 'production', '-c', 'webpack.config2.js']);
         expect(stderr).toBeFalsy();
         expect(exitCode).toEqual(0);
+        expect(stdout).toContain(`mode: 'production'`);
+    });
+
+    it('should use mode from flag over NODE_ENV', () => {
+        const { stdout, stderr, exitCode } = run(__dirname, ['--mode', 'none', '-c', 'webpack.config2.js'], false, [], {
+            NODE_ENV: 'production',
+        });
+        expect(stderr).toBeFalsy();
+        expect(exitCode).toEqual(0);
+        expect(stdout).toContain(`mode: 'none'`);
+    });
+
+    it('should use mode from config over NODE_ENV', () => {
+        const { stdout, stderr, exitCode } = run(__dirname, ['-c', 'webpack.config2.js']);
+        expect(stderr).toBeFalsy();
+        expect(exitCode).toEqual(0);
+        expect(stdout).toContain(`mode: 'development'`);
+    });
+
+    it('should use mode from config when multiple config are supplied', () => {
+        const { stdout, stderr } = run(__dirname, ['-c', 'webpack.config3.js', '-c', 'webpack.config2.js']);
+        expect(stderr).toBeFalsy();
+        expect(stdout).toContain(`mode: 'development'`);
+        expect(stdout.match(new RegExp("mode: 'development'", 'g')).length).toEqual(1);
+    });
+
+    it('mode flag should apply to all configs', () => {
+        const { stdout, stderr, exitCode } = run(__dirname, ['--mode', 'none', '-c', './webpack.config3.js', '-c', './webpack.config2.js']);
+        expect(stderr).toBeFalsy();
+        expect(exitCode).toEqual(0);
+        expect(stdout).toContain(`mode: 'none'`);
+        expect(stdout.match(new RegExp("mode: 'none'", 'g')).length).toEqual(2);
+    });
+
+    it('only config where mode is absent pick up from NODE_ENV', () => {
+        const { stdout, stderr, exitCode } = run(__dirname, ['-c', './webpack.config3.js', '-c', './webpack.config2.js'], false, [], {
+            NODE_ENV: 'production',
+        });
+        expect(stderr).toBeFalsy();
+        expect(exitCode).toEqual(0);
+        expect(stdout).toContain(`mode: 'production'`);
         expect(stdout).toContain(`mode: 'development'`);
+        expect(stdout.match(new RegExp("mode: 'production'", 'g')).length).toEqual(1);
+        expect(stdout.match(new RegExp("mode: 'development'", 'g')).length).toEqual(1);
     });
 });
diff --git a/test/mode/mode-with-config/webpack.config3.js b/test/mode/mode-with-config/webpack.config3.js
@@ -0,0 +1,6 @@
+// eslint-disable-next-line node/no-unpublished-require
+const WebpackCLITestPlugin = require('../../utils/webpack-cli-test-plugin');
+
+module.exports = {
+    plugins: [new WebpackCLITestPlugin()],
+};
diff --git a/test/utils/test-utils.js b/test/utils/test-utils.js
@@ -19,7 +19,7 @@ const isWebpack5 = version.startsWith('5');
  * @param {Boolean} setOutput Boolean that decides if a default output path will be set or not
  * @returns {Object} The webpack output or Promise when nodeOptions are present
  */
-function run(testCase, args = [], setOutput = true, nodeArgs = []) {
+function run(testCase, args = [], setOutput = true, nodeArgs = [], env) {
     const cwd = path.resolve(testCase);
 
     const outputPath = path.resolve(testCase, 'bin');
@@ -29,6 +29,7 @@ function run(testCase, args = [], setOutput = true, nodeArgs = []) {
         cwd,
         reject: false,
         nodeOptions: nodeArgs,
+        env,
         stdio: ENABLE_LOG_COMPILATION ? 'inherit' : 'pipe',
     });
 
