Skip to content
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

fix: respect --color/--no-color arguments #2042

Merged
merged 6 commits into from Nov 9, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -10,7 +10,6 @@ Object {
"serveIndex": true,
},
"webpackArgs": Object {
"color": true,
"env": Object {
"WEBPACK_SERVE": true,
},
Expand Down Expand Up @@ -41,7 +40,6 @@ Object {
"serveIndex": true,
},
"webpackArgs": Object {
"color": true,
"env": Object {
"WEBPACK_SERVE": true,
},
Expand Down
6 changes: 6 additions & 0 deletions packages/webpack-cli/lib/bootstrap.js
Expand Up @@ -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';

Expand All @@ -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') {
snitin315 marked this conversation as resolved.
Show resolved Hide resolved
coloretteOptions.enabled = Boolean(parsedArgs.opts.color);
}
snitin315 marked this conversation as resolved.
Show resolved Hide resolved

if (parsedArgs.unknownArgs.length > 0) {
entry = [];

Expand Down
1 change: 0 additions & 1 deletion packages/webpack-cli/lib/utils/cli-flags.js
Expand Up @@ -92,7 +92,6 @@ const core = [
usage: '--color',
type: Boolean,
negative: true,
defaultValue: true,
description: 'Enable/Disable colors on console',
},
{
Expand Down
20 changes: 17 additions & 3 deletions packages/webpack-cli/lib/webpack-cli.js
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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') {
snitin315 marked this conversation as resolved.
Show resolved Hide resolved
colors = args.color;
}
// From stats
else if (typeof stats.colors !== 'undefined') {
colors = stats.colors;
}
// Default
else {
colors = coloretteOptions.enabled;
}

stats.colors = colors;

return stats;
};
Expand Down
55 changes: 54 additions & 1 deletion test/colors/colors.test.js
Expand Up @@ -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);
});
snitin315 marked this conversation as resolved.
Show resolved Hide resolved

it('should work with the "stats" option from the configuration', () => {
const { stderr, stdout, exitCode } = run(__dirname, ['--config=stats-string.webpack.config.js']);

Expand Down Expand Up @@ -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`);
}
});
});
12 changes: 12 additions & 0 deletions 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',
},
];
1 change: 1 addition & 0 deletions test/colors/src/first.js
@@ -0,0 +1 @@
console.log('first');
1 change: 1 addition & 0 deletions test/colors/src/second.js
@@ -0,0 +1 @@
console.log('second');
Expand Up @@ -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();
});
Expand Down
49 changes: 45 additions & 4 deletions test/json/json.test.js
Expand Up @@ -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) => {
Expand All @@ -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();
});
});
21 changes: 20 additions & 1 deletion 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');
Expand Down