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: only set output path on passing flag #1855

Merged
merged 5 commits into from Oct 3, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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
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 @@ function 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 @@ function 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 @@ function 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 @@ -69,7 +69,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 @@ -92,7 +92,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 @@ -104,7 +104,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 @@ -127,7 +127,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