fix: 🐛 do not apply own defaults while setting mode (#1565)

anshumanv · web-flow · commit 4ca25bc01d8e · 2020-05-23T14:25:39.000+03:00
diff --git a/packages/webpack-cli/__tests__/ZeroConfigGroup.test.js b/packages/webpack-cli/__tests__/ZeroConfigGroup.test.js
@@ -1,7 +1,7 @@
 const ZeroConfigGroup = require('../lib/groups/ZeroConfigGroup');
 
-describe('GroupHelper', function () {
-    it('should  load the dev zero config', () => {
+describe('ZeroConfigGroup', function () {
+    it('should load the dev zero config', () => {
         const group = new ZeroConfigGroup([
             {
                 dev: true,
@@ -27,8 +27,9 @@ describe('GroupHelper', function () {
                 mode: 'production',
             },
         ]);
-
         const result = group.run();
+        // ensure no other properties are added
+        expect(result.options).toMatchObject({ mode: 'production' });
         expect(result.options.mode).toEqual('production');
     });
 
@@ -40,6 +41,8 @@ describe('GroupHelper', function () {
         ]);
 
         const result = group.run();
+        // ensure no other properties are added
+        expect(result.options).toMatchObject({ mode: 'development' });
         expect(result.options.mode).toEqual('development');
     });
 
@@ -51,6 +54,8 @@ describe('GroupHelper', function () {
         ]);
 
         const result = group.run();
+        // ensure no other properties are added
+        expect(result.options).toMatchObject({ mode: 'none' });
         expect(result.options.mode).toEqual('none');
     });
 });
diff --git a/packages/webpack-cli/lib/groups/ZeroConfigGroup.js b/packages/webpack-cli/lib/groups/ZeroConfigGroup.js
@@ -16,7 +16,7 @@ class ZeroConfigGroup extends GroupHelper {
      * It determines the mode to pass to webpack compiler
      * @returns {string} The mode
      */
-    getEnvFromOptionsAndMode() {
+    resolveMode() {
         if (process.env.NODE_ENV && (process.env.NODE_ENV === PRODUCTION || process.env.NODE_ENV === DEVELOPMENT)) {
             return process.env.NODE_ENV;
         } else {
@@ -44,28 +44,8 @@ class ZeroConfigGroup extends GroupHelper {
         }
     }
 
-    resolveZeroConfig() {
-        const defaultConfigType = this.getEnvFromOptionsAndMode();
-        let defaultConfig;
-        if (defaultConfigType === PRODUCTION) {
-            defaultConfig = require('../utils/production-config')();
-        } else if (defaultConfigType === DEVELOPMENT) {
-            defaultConfig = require('../utils/development-config')();
-        } else {
-            defaultConfig = require('../utils/none-config')();
-        }
-
-        const isEntryObject = defaultConfig.entry && defaultConfig.entry instanceof Object;
-        const isOutputDefined = defaultConfig.output && defaultConfig.output.filename;
-        const isConflictingOutput = isEntryObject && isOutputDefined && defaultConfig.output.filename === 'main.js';
-        if (isConflictingOutput) {
-            defaultConfig.output.filename = '[name].main.js';
-        }
-        this.opts.options = defaultConfig;
-    }
-
     run() {
-        this.resolveZeroConfig();
+        this.opts.options.mode = this.resolveMode();
         return this.opts;
     }
 }
diff --git a/packages/webpack-cli/lib/utils/development-config.js b/packages/webpack-cli/lib/utils/development-config.js
diff --git a/packages/webpack-cli/lib/utils/none-config.js b/packages/webpack-cli/lib/utils/none-config.js
diff --git a/packages/webpack-cli/lib/utils/production-config.js b/packages/webpack-cli/lib/utils/production-config.js
diff --git a/scripts/prepareSuite.js b/scripts/prepareSuite.js
@@ -33,21 +33,19 @@ function extractFolder(folderToRead, folders = []) {
     return folders;
 }
 
-{
-    Promise.all(
-        collectTestingFoldersWithPackage().map(async (folder) => {
-            return execa('yarn', {
+(async () => {
+    try {
+        const folders = collectTestingFoldersWithPackage();
+        for (const folder of folders) {
+            await execa('yarn', {
                 cwd: folder,
                 stdio: 'inherit',
             });
-        }),
-    )
-        .then(() => {
-            console.log(chalk.inverse.green(' Successfully prepared the test suite '));
-        })
-        .catch((e) => {
-            console.error(chalk.inverse.red(' Unable to prepare the test suite '));
-            console.error(e.stack);
-            process.exitCode = 1;
-        });
-}
+        }
+        console.log(chalk.inverse.green(' Successfully prepared the test suite '));
+    } catch (e) {
+        console.error(chalk.inverse.red(' Unable to prepare the test suite '));
+        console.error(e.stack);
+        process.exitCode = 1;
+    }
+})();
diff --git a/test/mode/mode-with-config/.gitignore b/test/mode/mode-with-config/.gitignore
@@ -0,0 +1,3 @@
+yarn.lock
+package-lock.json
+node_modules/
diff --git a/test/mode/mode-with-config/index.js b/test/mode/mode-with-config/index.js
@@ -0,0 +1,10 @@
+require("react")
+console.log("Ichigo")
+if (process.env.NODE_ENV === "production") {
+    console.log("production mode")
+} else if (process.env.NODE_ENV === "development") {
+    console.log(console.log("development mode"))
+} else {
+    console.log(console.log("none mode"))
+}
+
diff --git a/test/mode/mode-with-config/mode-with-config.test.js b/test/mode/mode-with-config/mode-with-config.test.js
@@ -0,0 +1,94 @@
+'use strict';
+const { stat, readFile } = require('fs');
+const { resolve } = require('path');
+// eslint-disable-next-line node/no-unpublished-require
+const { run } = require('../../utils/test-utils');
+
+describe('mode flags with config', () => {
+    it('should run in production mode when --mode=production is passed', (done) => {
+        const { stderr, stdout } = run(__dirname, ['--mode', 'production', '--config', './webpack.config.js']);
+        expect(stderr).toBeFalsy();
+        expect(stdout).toBeTruthy();
+
+        // Should generate the appropriate files
+        stat(resolve(__dirname, './bin/main.js.OTHER.LICENSE.txt'), (err, stats) => {
+            expect(err).toBe(null);
+            expect(stats.isFile()).toBe(true);
+        });
+
+        stat(resolve(__dirname, './bin/main.js'), (err, stats) => {
+            expect(err).toBe(null);
+            expect(stats.isFile()).toBe(true);
+        });
+
+        // Should not generate source maps when not specified
+        stat(resolve(__dirname, './bin/main.js.map'), (err) => {
+            expect(err).toBeTruthy();
+        });
+
+        // Correct mode should be propagated to the compiler
+        readFile(resolve(__dirname, './bin/main.js'), 'utf-8', (err, data) => {
+            expect(err).toBe(null);
+            expect(data).toContain('"production mode"');
+            done();
+        });
+    });
+
+    it('should run in development mode when --mode=development is passed', (done) => {
+        const { stderr, stdout } = run(__dirname, ['--mode', 'development', '--config', './webpack.config.js']);
+        expect(stderr).toBeFalsy();
+        expect(stdout).toBeTruthy();
+
+        // Should generate the appropriate files
+        stat(resolve(__dirname, './bin/main.js.OTHER.LICENSE.txt'), (err, stats) => {
+            expect(err).toBe(null);
+            expect(stats.isFile()).toBe(true);
+        });
+
+        stat(resolve(__dirname, './bin/main.js'), (err, stats) => {
+            expect(err).toBe(null);
+            expect(stats.isFile()).toBe(true);
+        });
+
+        // Should not generate source maps when not specified
+        stat(resolve(__dirname, './bin/main.js.map'), (err) => {
+            expect(err).toBeTruthy();
+        });
+
+        // Correct mode should be propagated to the compiler
+        readFile(resolve(__dirname, './bin/main.js'), 'utf-8', (err, data) => {
+            expect(err).toBe(null);
+            expect(data).toContain('development mode');
+            done();
+        });
+    });
+
+    it('should run in none mode when --mode=none is passed', (done) => {
+        const { stderr, stdout } = run(__dirname, ['--mode', 'none', '--config', './webpack.config.js']);
+        expect(stderr).toBeFalsy();
+        expect(stdout).toBeTruthy();
+
+        // Should generate the appropriate files
+        stat(resolve(__dirname, './bin/main.js.OTHER.LICENSE.txt'), (err, stats) => {
+            expect(err).toBe(null);
+            expect(stats.isFile()).toBe(true);
+        });
+
+        stat(resolve(__dirname, './bin/main.js'), (err, stats) => {
+            expect(err).toBe(null);
+            expect(stats.isFile()).toBe(true);
+        });
+
+        // Should not generate source maps when not specified
+        stat(resolve(__dirname, './bin/main.js.map'), (err) => {
+            expect(err).toBeTruthy();
+        });
+
+        // Correct mode should be propagated to the compiler
+        readFile(resolve(__dirname, './bin/main.js'), 'utf-8', (err, data) => {
+            expect(err).toBe(null);
+            expect(data).toContain('none mode');
+            done();
+        });
+    });
+});
diff --git a/test/mode/mode-with-config/package.json b/test/mode/mode-with-config/package.json
@@ -0,0 +1,6 @@
+{
+  "dependencies": {
+    "terser-webpack-plugin": "^3.0.1",
+    "react": "^16.13.0"
+  }
+}
diff --git a/test/mode/mode-with-config/webpack.config.js b/test/mode/mode-with-config/webpack.config.js
@@ -0,0 +1,26 @@
+const path = require('path');
+const dirname = __dirname;
+const TerserPlugin = require('terser-webpack-plugin');
+
+module.exports = () => {
+    const config = {
+        entry: './index.js',
+        output: {
+            path: path.join(dirname, 'dist'),
+            filename: '[name].js',
+        },
+        optimization: {
+            minimizer: [
+                new TerserPlugin({
+                    sourceMap: false,
+                    extractComments: {
+                        filename: (fileData) => {
+                            return `${fileData.filename}.OTHER.LICENSE.txt${fileData.query}`;
+                        },
+                    },
+                }),
+            ],
+        },
+    };
+    return config;
+};

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+yarn.lock`
	`2`	`+package-lock.json`
	`3`	`+node_modules/`