fix: resplect color/--no-color arguments (#2042)

packages/serve/__tests__/__snapshots__/parseArgs.test.ts.snap

@@ -10,7 +10,6 @@ Object {
     "serveIndex": true,
   },
   "webpackArgs": Object {
-    "color": true,
     "env": Object {
       "WEBPACK_SERVE": true,
     },
@@ -41,7 +40,6 @@ Object {
     "serveIndex": true,
   },
   "webpackArgs": Object {
-    "color": true,
     "env": Object {
       "WEBPACK_SERVE": true,
     },

packages/webpack-cli/lib/bootstrap.js

@@ -4,6 +4,7 @@ const logger = require('./utils/logger');
 const { isCommandUsed } = require('./utils/arg-utils');
 const argParser = require('./utils/arg-parser');
 const leven = require('leven');
+const { options: coloretteOptions } = require('colorette');
 
 process.title = 'webpack-cli';
 
@@ -23,6 +24,11 @@ const runCLI = async (cliArgs) => {
         // If the unknown arg starts with a '-', it will be considered an unknown flag rather than an entry
         let entry;
 
+        // enable/disable colors
+        if (typeof parsedArgs.opts.color !== 'undefined') {
+            coloretteOptions.enabled = Boolean(parsedArgs.opts.color);
+        }
+
         if (parsedArgs.unknownArgs.length > 0) {
             entry = [];
 

packages/webpack-cli/lib/utils/cli-flags.js

@@ -109,7 +109,6 @@ const core = [
         usage: '--color',
         type: Boolean,
         negative: true,
-        defaultValue: true,
         description: 'Enable/Disable colors on console',
     },
     {

packages/webpack-cli/lib/webpack-cli.js

@@ -47,9 +47,9 @@ class WebpackCLI {
     }
 
     async resolveArgs(args, configOptions = {}) {
-        // Since color flag has a default value, when there are no other args then exit
+        // when there are no args then exit
         // eslint-disable-next-line no-prototype-builtins
-        if (Object.keys(args).length === 1 && args.hasOwnProperty('color') && !process.env.NODE_ENV) return {};
+        if (Object.keys(args).length === 0 && !process.env.NODE_ENV) return {};
 
         const { outputPath, stats, json, mode, target, prefetch, hot, analyze } = args;
         const finalOptions = {
@@ -367,7 +367,21 @@ class WebpackCLI {
                     }
                 }
 
-                stats.colors = typeof stats.colors !== 'undefined' ? stats.colors : coloretteOptions.enabled;
+                let colors;
+                // From flags
+                if (typeof args.color !== 'undefined') {
+                    colors = args.color;
+                }
+                // From stats
+                else if (typeof stats.colors !== 'undefined') {
+                    colors = stats.colors;
+                }
+                // Default
+                else {
+                    colors = coloretteOptions.enabled;
+                }
+
+                stats.colors = colors;
 
                 return stats;
             };

test/colors/colors.test.js

@@ -43,6 +43,25 @@ describe('colorts', () => {
         expect(exitCode).toBe(0);
     });
 
+    it('should disable colored output with --no-color', () => {
+        const { stderr, stdout, exitCode } = run(__dirname, ['--stats=verbose', '--no-color']);
+
+        expect(stderr).toBeFalsy();
+        const output = isWebpack5 ? 'successfully' : 'main.js';
+        expect(stdout).not.toContain(`\u001b[1m\u001b[32m${output}\u001b[39m\u001b[22m`);
+        expect(stdout).toContain(output);
+        expect(exitCode).toBe(0);
+    });
+
+    it('should work with the "stats" option and --color flags', () => {
+        const { stderr, stdout, exitCode } = run(__dirname, ['--stats=verbose', '--color']);
+
+        expect(stderr).toBeFalsy();
+        const output = isWebpack5 ? 'successfully' : 'main.js';
+        expect(stdout).toContain(`\u001b[1m\u001b[32m${output}\u001b[39m\u001b[22m`);
+        expect(exitCode).toBe(0);
+    });
+
     it('should work with the "stats" option from the configuration', () => {
         const { stderr, stdout, exitCode } = run(__dirname, ['--config=stats-string.webpack.config.js']);
 
@@ -84,9 +103,43 @@ describe('colorts', () => {
 
         expect(stderr).toBeFalsy();
         const output = isWebpack5 ? 'successfully' : 'main.js';
-        console.log(stdout);
+
         expect(stdout).not.toContain(`\u001b[1m\u001b[32m${output}\u001b[39m\u001b[22m`);
         expect(stdout).toContain(output);
         expect(exitCode).toBe(0);
     });
+
+    it('should prioritize --color over colors in config', () => {
+        const { stderr, stdout, exitCode } = run(__dirname, ['--config=colors-false.webpack.config.js', '--color']);
+
+        expect(stderr).toBeFalsy();
+        const output = isWebpack5 ? 'successfully' : 'main.js';
+
+        expect(stdout).toContain(`\u001b[1m\u001b[32m${output}\u001b[39m\u001b[22m`);
+        expect(exitCode).toBe(0);
+    });
+
+    it('should prioratize --no-color over colors in config', () => {
+        const { stderr, stdout, exitCode } = run(__dirname, ['--config=colors-true.webpack.config.js', '--no-color']);
+
+        expect(stderr).toBeFalsy();
+        const output = isWebpack5 ? 'successfully' : 'main.js';
+
+        expect(stdout).not.toContain(`\u001b[1m\u001b[32m${output}\u001b[39m\u001b[22m`);
+        expect(stdout).toContain(output);
+        expect(exitCode).toBe(0);
+    });
+
+    it('should work in multicompiler mode', () => {
+        const { stderr, stdout, exitCode } = run(__dirname, ['--config=multiple-configs.js', '--color']);
+
+        expect(stderr).toBeFalsy();
+        expect(exitCode).toBe(0);
+
+        if (isWebpack5) {
+            expect(stdout).toContain(`\u001b[1mfirst-config`);
+            expect(stdout).toContain(`\u001b[1msecond-config`);
+            expect(stdout).toContain(`\u001b[1m\u001b[32msuccessfully\u001b[39m\u001b[22m`);
+        }
+    });
 });

test/colors/multiple-configs.js

@@ -0,0 +1,12 @@
+module.exports = [
+    {
+        name: 'first-config',
+        entry: './src/first.js',
+        stats: 'normal',
+    },
+    {
+        name: 'second-config',
+        entry: './src/second.js',
+        stats: 'normal',
+    },
+];

test/colors/src/first.js

@@ -0,0 +1 @@
+console.log('first');

test/colors/src/second.js

@@ -0,0 +1 @@
+console.log('second');

test/config/type/function-with-argv/function-with-argv.test.js

@@ -9,7 +9,7 @@ describe('function configuration', () => {
         expect(stderr).toBeFalsy();
         expect(stdout).toBeTruthy();
         expect(exitCode).toBe(0);
-        expect(stdout).toContain("argv: { color: true, mode: 'development' }");
+        expect(stdout).toContain("argv: { mode: 'development' }");
         // Should generate the appropriate files
         expect(existsSync(resolve(__dirname, './dist/dev.js'))).toBeTruthy();
     });

test/json/json.test.js

@@ -3,20 +3,20 @@ const { run } = require('../utils/test-utils');
 const { stat, readFile } = require('fs');
 const { resolve } = require('path');
 
+const successMessage = 'stats are successfully stored as json to stats.json';
+
 describe('json flag', () => {
     it('should return valid json', () => {
         const { stdout, exitCode } = run(__dirname, ['--json']);
-
         expect(() => JSON.parse(stdout)).not.toThrow();
         expect(exitCode).toBe(0);
-
         expect(JSON.parse(stdout)['hash']).toBeDefined();
     });
 
     it('should store json to a file', (done) => {
         const { stdout, exitCode } = run(__dirname, ['--json', 'stats.json']);
 
-        expect(stdout).toContain('stats are successfully stored as json to stats.json');
+        expect(stdout).toContain(successMessage);
         expect(exitCode).toBe(0);
 
         stat(resolve(__dirname, './stats.json'), (err, stats) => {
@@ -34,11 +34,52 @@ describe('json flag', () => {
         });
     });
 
+    it('should store json to a file and respect --color flag', (done) => {
+        const { stdout, exitCode } = run(__dirname, ['--json', 'stats.json', '--color']);
+
+        expect(stdout).toContain(`[webpack-cli] \u001b[32m${successMessage}`);
+        expect(exitCode).toBe(0);
+
+        stat(resolve(__dirname, './stats.json'), (err, stats) => {
+            expect(err).toBe(null);
+            expect(stats.isFile()).toBe(true);
+
+            readFile(resolve(__dirname, 'stats.json'), 'utf-8', (err, data) => {
+                expect(err).toBe(null);
+                expect(JSON.parse(data)['hash']).toBeTruthy();
+                expect(JSON.parse(data)['version']).toBeTruthy();
+                expect(JSON.parse(data)['time']).toBeTruthy();
+                expect(() => JSON.parse(data)).not.toThrow();
+                done();
+            });
+        });
+    });
+
+    it('should store json to a file and respect --no-color', (done) => {
+        const { stdout, exitCode } = run(__dirname, ['--json', 'stats.json', '--no-color']);
+
+        expect(stdout).not.toContain(`[webpack-cli] \u001b[32m${successMessage}`);
+        expect(stdout).toContain(`[webpack-cli] ${successMessage}`);
+        expect(exitCode).toBe(0);
+
+        stat(resolve(__dirname, './stats.json'), (err, stats) => {
+            expect(err).toBe(null);
+            expect(stats.isFile()).toBe(true);
+            readFile(resolve(__dirname, 'stats.json'), 'utf-8', (err, data) => {
+                expect(err).toBe(null);
+                expect(JSON.parse(data)['hash']).toBeTruthy();
+                expect(JSON.parse(data)['version']).toBeTruthy();
+                expect(JSON.parse(data)['time']).toBeTruthy();
+                expect(() => JSON.parse(data)).not.toThrow();
+                done();
+            });
+        });
+    });
+
     it('should return valid json with -j alias', () => {
         const { stdout, exitCode } = run(__dirname, ['-j']);
         expect(() => JSON.parse(stdout)).not.toThrow();
         expect(exitCode).toBe(0);
-
         expect(JSON.parse(stdout)['hash']).toBeDefined();
     });
 });

test/unknown/unknown.test.js

@@ -1,12 +1,31 @@
 const { run } = require('../utils/test-utils');
 
+const unknownError = 'Unknown argument: --unknown';
+
 describe('unknown behaviour', () => {
-    it('warns the user if an unknown flag is passed in', () => {
+    it('throws error if an unknown flag is passed in', () => {
         const { stderr, exitCode } = run(__dirname, ['--unknown']);
         expect(stderr).toBeTruthy();
         expect(stderr).toContain('Unknown argument: --unknown');
+        expect(stderr).toContain(unknownError);
+        expect(exitCode).toBe(2);
+    });
+
+    it('should throw error and respect --color flag', () => {
+        const { stderr, exitCode } = run(__dirname, ['--unknown', '--color']);
+        expect(stderr).toBeTruthy();
+        expect(stderr).toContain(`[webpack-cli] \u001b[31m${unknownError}`);
         expect(exitCode).toBe(2);
     });
+
+    it('throws error for unknow flag and respect --no-color', () => {
+        const { stderr, exitCode } = run(__dirname, ['--unknown', '--no-color']);
+        expect(stderr).toBeTruthy();
+        expect(stderr).not.toContain(`[webpack-cli] \u001b[31m${unknownError}`);
+        expect(stderr).toContain(unknownError);
+        expect(exitCode).toBe(2);
+    });
+
     it('suggests the closest match to an unknown flag', () => {
         const { stderr, stdout, exitCode } = run(__dirname, ['--entyr', './a.js']);
         expect(stderr).toContain('Unknown argument: --entyr');