Skip to content

Commit

Permalink
fix: only set output path on passing flag (#1855)
Browse files Browse the repository at this point in the history
* fix: only set output path on passing flag
  • Loading branch information
anshumanv committed Oct 3, 2020
1 parent ef14a7c commit 2f36b9d
Show file tree
Hide file tree
Showing 16 changed files with 36 additions and 49 deletions.
2 changes: 1 addition & 1 deletion packages/webpack-cli/__tests__/arg-parser.test.js
Expand Up @@ -368,7 +368,7 @@ describe('arg-parser', () => {
expect(res.unknownArgs.length).toEqual(0);
expect(res.opts.entry).toEqual(['test.js']);
expect(res.opts.hot).toBeTruthy();
expect(res.opts.output).toEqual('./dist/');
expect(res.opts.outputPath).toEqual('./dist/');
expect(res.opts.stats).toEqual(true);
expect(warnMock.mock.calls.length).toEqual(0);
});
Expand Down
5 changes: 3 additions & 2 deletions packages/webpack-cli/__tests__/resolveOutput.test.js
@@ -1,10 +1,11 @@
const { resolve } = require('path');
const resolveOutput = require('../lib/groups/resolveOutput');

describe('OutputGroup', function () {
it('should handle the output option', () => {
const result = resolveOutput({
output: './bundle.js',
outputPath: './bundle',
});
expect(result.options.output.filename).toEqual('bundle.js');
expect(result.options.output.path).toEqual(resolve('bundle'));
});
});
8 changes: 3 additions & 5 deletions packages/webpack-cli/lib/groups/resolveOutput.js
Expand Up @@ -5,15 +5,13 @@ const path = require('path');
* @param {args} args - Parsed arguments passed to the CLI
*/
const resolveOutput = (args) => {
const { output } = args;
const { outputPath } = args;
const finalOptions = {
options: { output: {} },
outputOptions: {},
};
if (output) {
const { dir, base, ext } = path.parse(output);
finalOptions.options.output.path = ext.length === 0 ? path.resolve(dir, base) : path.resolve(dir);
if (ext.length > 0) finalOptions.options.output.filename = base;
if (outputPath) {
finalOptions.options.output.path = path.resolve(outputPath);
}
return finalOptions;
};
Expand Down
4 changes: 2 additions & 2 deletions packages/webpack-cli/lib/utils/cli-flags.js
Expand Up @@ -117,8 +117,8 @@ const core = [
description: 'Outputs list of supported flags',
},
{
name: 'output',
usage: '--output <path to output directory>',
name: 'output-path',
usage: '--output-path <path to output directory>',
alias: 'o',
type: String,
description: 'Output location of the file generated by webpack e.g. ./dist/',
Expand Down
18 changes: 5 additions & 13 deletions test/config/basic/basic-config.test.js
@@ -1,21 +1,13 @@
'use strict';
const { stat } = require('fs');
const { existsSync } = require('fs');
const { resolve } = require('path');
const { run } = require('../../utils/test-utils');

describe('basic config file', () => {
it('is able to understand and parse a very basic configuration file', (done) => {
const { stdout, stderr } = run(
__dirname,
['-c', resolve(__dirname, 'webpack.config.js'), '--output', './binary/a.bundle.js'],
false,
);
it('is able to understand and parse a very basic configuration file', () => {
const { stdout, stderr } = run(__dirname, ['-c', resolve(__dirname, 'webpack.config.js'), '--output-path', './binary'], false);
expect(stderr).toBeFalsy();
expect(stdout).not.toBe(undefined);
stat(resolve(__dirname, './binary/a.bundle.js'), (err, stats) => {
expect(err).toBe(null);
expect(stats.isFile()).toBe(true);
done();
});
expect(stdout).toBeTruthy();
expect(existsSync(resolve(__dirname, './binary/a.bundle.js'))).toBeTruthy();
});
});
6 changes: 3 additions & 3 deletions test/defaults/output-defaults.test.js
Expand Up @@ -5,7 +5,7 @@ const { run } = require('../utils/test-utils');

describe('output flag defaults', () => {
it('should create default file for a given directory', (done) => {
const { stdout } = run(__dirname, ['--entry', './a.js', '--output', './binary'], false);
const { stdout } = run(__dirname, ['--entry', './a.js', '--output-path', './binary'], false);
// Should not print warning about config fallback, as we have production as default
expect(stdout).not.toContain('option has not been set, webpack will fallback to');
stat(resolve(__dirname, './binary/main.js'), (err, stats) => {
Expand All @@ -26,7 +26,7 @@ describe('output flag defaults', () => {
});

it('throw error on empty output flag', () => {
const { stderr } = run(__dirname, ['--entry', './a.js', '--output'], false);
expect(stderr).toContain("error: option '-o, --output <value>' argument missing");
const { stderr } = run(__dirname, ['--entry', './a.js', '--output-path'], false);
expect(stderr).toContain("error: option '-o, --output-path <value>' argument missing");
});
});
2 changes: 1 addition & 1 deletion test/devtool/array/source-map-array.test.js
Expand Up @@ -14,7 +14,7 @@ describe('source-map object', () => {
});
});
it('should override entire array on flag', (done) => {
const { stderr } = run(__dirname, ['--devtool', 'source-map', '--output', './binary'], false);
const { stderr } = run(__dirname, ['--devtool', 'source-map', '--output-path', './binary'], false);
expect(stderr).toBe('');
readdir(resolve(__dirname, 'binary'), (err, files) => {
expect(err).toBe(null);
Expand Down
2 changes: 1 addition & 1 deletion test/devtool/object/source-map-object.test.js
Expand Up @@ -24,7 +24,7 @@ describe('source-map object', () => {
});

it('should override config with source-map', (done) => {
run(__dirname, ['-c', './webpack.eval.config.js', '--devtool', 'source-map', '-o', './binary/dist-amd.js'], false);
run(__dirname, ['-c', './webpack.eval.config.js', '--devtool', 'source-map', '-o', './binary'], false);
stat(resolve(__dirname, 'binary/dist-amd.js.map'), (err, stats) => {
expect(err).toBe(null);
expect(stats.isFile()).toBe(true);
Expand Down
2 changes: 1 addition & 1 deletion test/entry/defaults-index/entry-multi-args.test.js
Expand Up @@ -18,7 +18,7 @@ describe('single entry flag index present', () => {
});

it('finds default index file, compiles and overrides with flags successfully', (done) => {
const { stderr } = run(__dirname, ['--output', 'bin/main.js']);
const { stderr } = run(__dirname, ['--output-path', 'bin']);
expect(stderr).toBeFalsy();

stat(resolve(__dirname, './bin/main.js'), (err, stats) => {
Expand Down
2 changes: 1 addition & 1 deletion test/node/node.test.js
Expand Up @@ -8,7 +8,7 @@ const { run } = require('../utils/test-utils');
// throws different error from what we manually see
describe('node flags', () => {
it('is able to pass the options flags to node js', async (done) => {
const { stdout, stderr } = await run(__dirname, ['--output', './bin/[name].bundle.js'], false, [
const { stdout, stderr } = await run(__dirname, ['--output-path', './bin'], false, [
`--require=${resolve(__dirname, 'bootstrap.js')}`,
`--require=${resolve(__dirname, 'bootstrap2.js')}`,
]);
Expand Down
2 changes: 1 addition & 1 deletion test/node/webpack.config.js
Expand Up @@ -4,6 +4,6 @@ module.exports = {
entry: './a.js',
output: {
path: resolve(__dirname, 'binary'),
filename: 'a.bundle.js',
filename: '[name].bundle.js',
},
};
14 changes: 5 additions & 9 deletions test/output/named-bundles/output-named-bundles.test.js
Expand Up @@ -5,27 +5,23 @@ const { run } = require('../../utils/test-utils');

describe('output flag named bundles', () => {
it('should output file given as flag instead of in configuration', () => {
const { stderr } = run(__dirname, ['-c', resolve(__dirname, 'webpack.config.js'), '--output', './binary/a.bundle.js'], false);
const { stderr } = run(__dirname, ['-c', resolve(__dirname, 'webpack.config.js'), '--output-path', './binary'], false);
expect(stderr).toBeFalsy();

const stats = statSync(resolve(__dirname, './binary/a.bundle.js'));
expect(stats.isFile()).toBe(true);
});

it('should resolve the path to binary/a.bundle.js as ./binary/a.bundle.js', () => {
const { stderr } = run(__dirname, ['-c', resolve(__dirname, 'webpack.config.js'), '--output', 'binary/a.bundle.js'], false);
const { stderr } = run(__dirname, ['-c', resolve(__dirname, 'webpack.config.js'), '--output-path', 'binary'], false);
expect(stderr).toBeFalsy();

const stats = statSync(resolve(__dirname, './binary/a.bundle.js'));
expect(stats.isFile()).toBe(true);
});

it('should create multiple bundles with an overriding flag', () => {
const { stderr } = run(
__dirname,
['-c', resolve(__dirname, 'webpack.single.config.js'), '--output', './bin/[name].bundle.js'],
false,
);
const { stderr } = run(__dirname, ['-c', resolve(__dirname, 'webpack.single.config.js'), '--output-path', './bin'], false);
expect(stderr).toBeFalsy();

let stats = statSync(resolve(__dirname, './bin/b.bundle.js'));
Expand All @@ -45,8 +41,8 @@ describe('output flag named bundles', () => {
});

it('should output file in bin directory using default webpack config with warning for empty output value', () => {
const { stdout, stderr, exitCode } = run(__dirname, ['--output'], false);
expect(stderr).toEqual("error: option '-o, --output <value>' argument missing");
const { stdout, stderr, exitCode } = run(__dirname, ['--output-path'], false);
expect(stderr).toEqual("error: option '-o, --output-path <value>' argument missing");
expect(exitCode).toEqual(1);
expect(stdout).toBeFalsy();
});
Expand Down
2 changes: 1 addition & 1 deletion test/output/named-bundles/webpack.config.js
Expand Up @@ -4,6 +4,6 @@ module.exports = {
entry: './a.js',
output: {
path: resolve(__dirname, 'bin'),
filename: 'bundle.js',
filename: 'a.bundle.js',
},
};
2 changes: 1 addition & 1 deletion test/output/named-bundles/webpack.single.config.js
Expand Up @@ -7,6 +7,6 @@ module.exports = {
},
output: {
path: resolve(__dirname, 'bin'),
filename: 'bundle.js',
filename: '[name].bundle.js',
},
};
6 changes: 3 additions & 3 deletions test/utils/test-utils.js
Expand Up @@ -24,7 +24,7 @@ const run = (testCase, args = [], setOutput = true, nodeArgs = [], env) => {

const outputPath = path.resolve(testCase, 'bin');
const processExecutor = nodeArgs.length ? execaNode : spawnSync;
const argsWithOutput = setOutput ? args.concat('--output', outputPath) : args;
const argsWithOutput = setOutput ? args.concat('--output-path', outputPath) : args;
const result = processExecutor(WEBPACK_PATH, argsWithOutput, {
cwd,
reject: false,
Expand All @@ -40,7 +40,7 @@ const runWatch = ({ testCase, args = [], setOutput = true, outputKillStr = 'Time
const cwd = path.resolve(testCase);

const outputPath = path.resolve(testCase, 'bin');
const argsWithOutput = setOutput ? args.concat('--output', outputPath) : args;
const argsWithOutput = setOutput ? args.concat('--output-path', outputPath) : args;

return new Promise((resolve, reject) => {
const watchPromise = execa(WEBPACK_PATH, argsWithOutput, {
Expand Down Expand Up @@ -76,7 +76,7 @@ const runAndGetWatchProc = (testCase, args = [], setOutput = true, input = '', f
const cwd = path.resolve(testCase);

const outputPath = path.resolve(testCase, 'bin');
const argsWithOutput = setOutput ? args.concat('--output', outputPath) : args;
const argsWithOutput = setOutput ? args.concat('--output-path', outputPath) : args;

const options = {
cwd,
Expand Down
8 changes: 4 additions & 4 deletions test/utils/test-utils.test.js
Expand Up @@ -38,7 +38,7 @@ describe('run function', () => {
// Executes the correct command
expect(command).toContain('cli.js');
// Should use apply a default output dir
expect(command).toContain('--output');
expect(command).toContain('--output-path');
expect(command).toContain('bin');
expect(stdout).toBeTruthy();
expect(stderr).toBeFalsy();
Expand All @@ -61,7 +61,7 @@ describe('run function', () => {
it('uses default output when output param is false', () => {
const { stdout, stderr, command } = run(__dirname, [], false);
// execution command contains info command
expect(command).not.toContain('--output');
expect(command).not.toContain('--output-path');
expect(stdout).toBeTruthy();
expect(stderr).toBeFalsy();
});
Expand All @@ -73,7 +73,7 @@ describe('runAndGetWatchProc function', () => {
// Executes the correct command
expect(command).toContain('cli.js');
// Should use apply a default output dir
expect(command).toContain('--output');
expect(command).toContain('--output-path');
expect(command).toContain('bin');
expect(stdout).toBeTruthy();
expect(stderr).toBeFalsy();
Expand All @@ -96,7 +96,7 @@ describe('runAndGetWatchProc function', () => {
it('uses default output when output param is false', async () => {
const { stdout, stderr, command } = await runAndGetWatchProc(__dirname, [], false);
// execution command contains info command
expect(command).not.toContain('--output');
expect(command).not.toContain('--output-path');
expect(stdout).toBeTruthy();
expect(stderr).toBeFalsy();
});
Expand Down

0 comments on commit 2f36b9d

Please sign in to comment.