From 2cdf5ce159f63ac65b33f4aca4c82fa1e959fef5 Mon Sep 17 00:00:00 2001 From: Loonride Date: Wed, 26 Aug 2020 14:56:29 -0500 Subject: [PATCH] fix(serve): merge CLI and devServer options correctly (#1649) --- .../__snapshots__/parseArgs.test.ts.snap | 49 ++++++ .../__snapshots__/startDevServer.test.ts.snap | 84 +++++++++ .../__tests__/args-to-camel-case.test.ts | 19 --- packages/serve/__tests__/createConfig.test.ts | 43 +++++ .../__tests__/getDevServerOptions.test.ts | 51 ++++++ packages/serve/__tests__/mergeOptions.test.ts | 33 ++++ packages/serve/__tests__/parseArgs.test.ts | 49 ++++++ .../serve/__tests__/startDevServer.test.ts | 161 ++++++++++++++++++ packages/serve/src/args-to-camel-case.ts | 31 ---- packages/serve/src/createConfig.ts | 29 ++++ packages/serve/src/getDevServerOptions.ts | 28 +++ packages/serve/src/index.ts | 27 +-- packages/serve/src/mergeOptions.ts | 24 +++ packages/serve/src/parseArgs.ts | 56 ++++++ packages/serve/src/startDevServer.ts | 59 ++++--- packages/serve/src/types.ts | 30 ++++ .../webpack-cli/__tests__/arg-parser.test.js | 134 ++++++++++++++- packages/webpack-cli/lib/utils/arg-parser.js | 60 +++++-- 18 files changed, 856 insertions(+), 111 deletions(-) create mode 100644 packages/serve/__tests__/__snapshots__/parseArgs.test.ts.snap create mode 100644 packages/serve/__tests__/__snapshots__/startDevServer.test.ts.snap delete mode 100644 packages/serve/__tests__/args-to-camel-case.test.ts create mode 100644 packages/serve/__tests__/createConfig.test.ts create mode 100644 packages/serve/__tests__/getDevServerOptions.test.ts create mode 100644 packages/serve/__tests__/mergeOptions.test.ts create mode 100644 packages/serve/__tests__/parseArgs.test.ts create mode 100644 packages/serve/__tests__/startDevServer.test.ts delete mode 100644 packages/serve/src/args-to-camel-case.ts create mode 100644 packages/serve/src/createConfig.ts create mode 100644 packages/serve/src/getDevServerOptions.ts create mode 100644 packages/serve/src/mergeOptions.ts create mode 100644 packages/serve/src/parseArgs.ts create mode 100644 packages/serve/src/types.ts diff --git a/packages/serve/__tests__/__snapshots__/parseArgs.test.ts.snap b/packages/serve/__tests__/__snapshots__/parseArgs.test.ts.snap new file mode 100644 index 00000000000..a3a3f3ad9a2 --- /dev/null +++ b/packages/serve/__tests__/__snapshots__/parseArgs.test.ts.snap @@ -0,0 +1,49 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`parseArgs handles hot arg 1`] = ` +Object { + "devServerArgs": Object { + "clientLogLevel": "info", + "hot": true, + "inline": true, + "liveReload": true, + "serveIndex": true, + }, + "webpackArgs": Object { + "color": true, + "config": Array [], + "hot": true, + "mode": "production", + }, +} +`; + +exports[`parseArgs handles unknown args 1`] = ` +Array [ + Array [ + "Unknown argument: --unknown-arg", + ], + Array [ + "Unknown argument: --unknown-arg-2", + ], +] +`; + +exports[`parseArgs parses webpack and dev server args 1`] = ` +Object { + "devServerArgs": Object { + "bonjour": true, + "clientLogLevel": "info", + "inline": true, + "liveReload": true, + "port": 8080, + "serveIndex": true, + }, + "webpackArgs": Object { + "color": true, + "config": Array [], + "mode": "production", + "target": "node", + }, +} +`; diff --git a/packages/serve/__tests__/__snapshots__/startDevServer.test.ts.snap b/packages/serve/__tests__/__snapshots__/startDevServer.test.ts.snap new file mode 100644 index 00000000000..e6fdcbe3b3d --- /dev/null +++ b/packages/serve/__tests__/__snapshots__/startDevServer.test.ts.snap @@ -0,0 +1,84 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`startDevServer should set default port and host if not provided 1`] = ` +Object { + "host": "localhost", + "port": 8080, +} +`; + +exports[`startDevServer should set default port and host if not provided 2`] = ` +Array [ + 8080, + "localhost", + [Function], +] +`; + +exports[`startDevServer should start dev server correctly for multi compiler with 1 devServer config 1`] = ` +Object { + "bonjour": true, + "host": "my.host", + "hot": true, + "port": 9000, + "progress": true, +} +`; + +exports[`startDevServer should start dev server correctly for multi compiler with 1 devServer config 2`] = ` +Array [ + 9000, + "my.host", + [Function], +] +`; + +exports[`startDevServer should start dev server correctly for single compiler 1`] = ` +Object { + "bonjour": true, + "host": "my.host", + "hot": true, + "port": 9000, + "progress": true, +} +`; + +exports[`startDevServer should start dev server correctly for single compiler 2`] = ` +Array [ + 9000, + "my.host", + [Function], +] +`; + +exports[`startDevServer should start dev servers correctly for multi compiler with 2 devServer configs 1`] = ` +Object { + "host": "localhost", + "port": 9000, + "progress": true, +} +`; + +exports[`startDevServer should start dev servers correctly for multi compiler with 2 devServer configs 2`] = ` +Object { + "host": "localhost", + "port": 9001, + "progress": true, +} +`; + +exports[`startDevServer should start dev servers correctly for multi compiler with 2 devServer configs 3`] = ` +Array [ + 9000, + "localhost", + [Function], +] +`; + +exports[`startDevServer should start dev servers correctly for multi compiler with 2 devServer configs 4`] = ` +Array [ + 9001, + "localhost", + [Function], +] +`; diff --git a/packages/serve/__tests__/args-to-camel-case.test.ts b/packages/serve/__tests__/args-to-camel-case.test.ts deleted file mode 100644 index ac91d2c55e5..00000000000 --- a/packages/serve/__tests__/args-to-camel-case.test.ts +++ /dev/null @@ -1,19 +0,0 @@ -'use strict'; - -import argsToCamelCase from '../src/args-to-camel-case'; - -describe('args-to-camel-case helper', () => { - it('converts arguments with multiple dashes to camel case', () => { - const obj = { - 'multi-word-arg': true, - 'multi-word-arg-2': 'hello world', - argument: 0, - }; - const result = { - multiWordArg: true, - multiWordArg2: 'hello world', - argument: 0, - }; - expect(argsToCamelCase(obj)).toEqual(result); - }); -}); diff --git a/packages/serve/__tests__/createConfig.test.ts b/packages/serve/__tests__/createConfig.test.ts new file mode 100644 index 00000000000..3f23534a8cd --- /dev/null +++ b/packages/serve/__tests__/createConfig.test.ts @@ -0,0 +1,43 @@ +'use strict'; + +import createConfig from '../src/createConfig'; + +describe('createConfig', () => { + it('creates config with arguments', () => { + const args = { + hot: true, + openPage: 'main', + }; + expect(createConfig(args)).toEqual(args); + }); + + it('sets client object using clientLogging argument', () => { + const args = { + clientLogging: 'verbose', + }; + expect(createConfig(args)).toEqual({ + client: { + logging: 'verbose', + }, + }); + }); + + it('sets hot using hotOnly argument', () => { + const args = { + hotOnly: true, + }; + expect(createConfig(args)).toEqual({ + hot: 'only', + }); + }); + + it('overrides hot with hotOnly', () => { + const args = { + hot: true, + hotOnly: true, + }; + expect(createConfig(args)).toEqual({ + hot: 'only', + }); + }); +}); diff --git a/packages/serve/__tests__/getDevServerOptions.test.ts b/packages/serve/__tests__/getDevServerOptions.test.ts new file mode 100644 index 00000000000..5304c617cd6 --- /dev/null +++ b/packages/serve/__tests__/getDevServerOptions.test.ts @@ -0,0 +1,51 @@ +'use strict'; + +import getDevServerOptions from '../src/getDevServerOptions'; + +describe('getDevServerOptions', () => { + // eslint-disable-next-line @typescript-eslint/no-var-requires, node/no-extraneous-require + const webpack = require('webpack'); + + it('gets dev server options from single compiler', () => { + const compiler = webpack({ + devServer: { + hot: true, + host: 'my.host', + }, + }); + expect(getDevServerOptions(compiler)).toEqual([ + { + hot: true, + host: 'my.host', + }, + ]); + }); + + it('gets dev server options from multi compiler', () => { + const compiler = webpack([ + { + devServer: { + hot: true, + host: 'my.host', + }, + }, + { + devServer: { + hot: false, + host: 'other.host', + }, + }, + ]); + + expect(getDevServerOptions(compiler)).toEqual([ + { + hot: true, + host: 'my.host', + }, + { + hot: false, + host: 'other.host', + }, + ]); + }); +}); diff --git a/packages/serve/__tests__/mergeOptions.test.ts b/packages/serve/__tests__/mergeOptions.test.ts new file mode 100644 index 00000000000..308c4c11be1 --- /dev/null +++ b/packages/serve/__tests__/mergeOptions.test.ts @@ -0,0 +1,33 @@ +'use strict'; + +import mergeOptions from '../src/mergeOptions'; + +describe('mergeOptions', () => { + it('merges CLI and devServer options correctly', () => { + const cliOptions = { + client: { + logging: 'verbose', + }, + hot: true, + bonjour: true, + }; + const devServerOptions = { + client: { + host: 'localhost', + logging: 'none', + }, + hot: false, + liveReload: false, + }; + // CLI should take priority + expect(mergeOptions(cliOptions, devServerOptions)).toEqual({ + client: { + host: 'localhost', + logging: 'verbose', + }, + hot: true, + bonjour: true, + liveReload: false, + }); + }); +}); diff --git a/packages/serve/__tests__/parseArgs.test.ts b/packages/serve/__tests__/parseArgs.test.ts new file mode 100644 index 00000000000..976d2bfd81d --- /dev/null +++ b/packages/serve/__tests__/parseArgs.test.ts @@ -0,0 +1,49 @@ +'use strict'; + +// TODO: update snapshots once we update to webpack-dev-server@4 + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const errorMock: any = jest.fn(); +jest.mock('webpack-cli/lib/utils/logger', () => { + return { + error: errorMock, + }; +}); + +import WebpackCLI from 'webpack-cli'; +import parseArgs from '../src/parseArgs'; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const processExitSpy: any = jest.spyOn(process, 'exit'); +// eslint-disable-next-line @typescript-eslint/no-empty-function +processExitSpy.mockImplementation(() => {}); + +describe('parseArgs', () => { + const cli = new WebpackCLI(); + + beforeEach(() => { + errorMock.mockClear(); + processExitSpy.mockClear(); + }); + + it('parses webpack and dev server args', () => { + const args = parseArgs(cli, ['--bonjour', '--target=node', '--port', '8080']); + expect(args).toMatchSnapshot(); + expect(errorMock.mock.calls.length).toEqual(0); + expect(processExitSpy.mock.calls.length).toEqual(0); + }); + + it('handles hot arg', () => { + const args = parseArgs(cli, ['--hot']); + expect(args).toMatchSnapshot(); + expect(errorMock.mock.calls.length).toEqual(0); + expect(processExitSpy.mock.calls.length).toEqual(0); + }); + + it('handles unknown args', () => { + parseArgs(cli, ['--unknown-arg', '--unknown-arg-2']); + expect(errorMock.mock.calls).toMatchSnapshot(); + expect(processExitSpy.mock.calls.length).toEqual(1); + expect(processExitSpy.mock.calls[0]).toEqual([2]); + }); +}); diff --git a/packages/serve/__tests__/startDevServer.test.ts b/packages/serve/__tests__/startDevServer.test.ts new file mode 100644 index 00000000000..2590063db8d --- /dev/null +++ b/packages/serve/__tests__/startDevServer.test.ts @@ -0,0 +1,161 @@ +'use strict'; + +import startDevServer from '../src/startDevServer'; + +jest.mock('webpack-dev-server/lib/Server'); + +describe('startDevServer', () => { + // eslint-disable-next-line @typescript-eslint/no-var-requires, node/no-extraneous-require + const webpack = require('webpack'); + // eslint-disable-next-line @typescript-eslint/no-var-requires + const DevServer = require('webpack-dev-server/lib/Server'); + + beforeEach(() => { + DevServer.mockClear(); + }); + + it('should start dev server correctly for single compiler', () => { + const config = { + devServer: { + port: 9000, + hot: false, + bonjour: true, + }, + }; + const compiler = webpack(config); + + const servers = startDevServer(compiler, { + host: 'my.host', + hot: true, + progress: true, + }); + + expect(servers.length).toEqual(1); + expect(servers).toEqual(DevServer.mock.instances); + + // this is the constructor + expect(DevServer.mock.calls.length).toEqual(1); + // the 2nd argument is the options + expect(DevServer.mock.calls[0][1]).toMatchSnapshot(); + + // the server should listen on correct host and port + expect(DevServer.mock.instances[0].listen.mock.calls.length).toEqual(1); + expect(DevServer.mock.instances[0].listen.mock.calls[0]).toMatchSnapshot(); + }); + + it('should set default port and host if not provided', () => { + const config = { + devServer: {}, + }; + const compiler = webpack(config); + + const servers = startDevServer(compiler, {}); + + expect(servers.length).toEqual(1); + expect(servers).toEqual(DevServer.mock.instances); + + // this is the constructor + expect(DevServer.mock.calls.length).toEqual(1); + // the 2nd argument is the options + expect(DevServer.mock.calls[0][1]).toMatchSnapshot(); + + // the server should listen on correct host and port + expect(DevServer.mock.instances[0].listen.mock.calls.length).toEqual(1); + expect(DevServer.mock.instances[0].listen.mock.calls[0]).toMatchSnapshot(); + }); + + it('should start dev server correctly for multi compiler with 1 devServer config', () => { + const config = [ + { + devServer: { + port: 9000, + hot: false, + bonjour: true, + }, + }, + {}, + ]; + const compiler = webpack(config); + + const servers = startDevServer(compiler, { + host: 'my.host', + hot: true, + progress: true, + }); + + expect(servers.length).toEqual(1); + expect(servers).toEqual(DevServer.mock.instances); + + // this is the constructor + expect(DevServer.mock.calls.length).toEqual(1); + // the 2nd argument is the options + expect(DevServer.mock.calls[0][1]).toMatchSnapshot(); + + // the server should listen on correct host and port + expect(DevServer.mock.instances[0].listen.mock.calls.length).toEqual(1); + expect(DevServer.mock.instances[0].listen.mock.calls[0]).toMatchSnapshot(); + }); + + it('should start dev servers correctly for multi compiler with 2 devServer configs', () => { + const config = [ + { + devServer: { + port: 9000, + // here to show that it will be overridden + progress: false, + }, + }, + { + devServer: { + port: 9001, + }, + }, + ]; + const compiler = webpack(config); + + const servers = startDevServer(compiler, { + // this progress CLI flag should override progress: false above + progress: true, + }); + + // there are 2 devServer configs, so both are run + expect(servers.length).toEqual(2); + expect(servers).toEqual(DevServer.mock.instances); + + // this is the constructor + expect(DevServer.mock.calls.length).toEqual(2); + // the 2nd argument is the options + expect(DevServer.mock.calls[0][1]).toMatchSnapshot(); + expect(DevServer.mock.calls[1][1]).toMatchSnapshot(); + + // both servers should listen on correct host and port + + expect(DevServer.mock.instances[0].listen.mock.calls.length).toEqual(1); + expect(DevServer.mock.instances[0].listen.mock.calls[0]).toMatchSnapshot(); + + expect(DevServer.mock.instances[1].listen.mock.calls.length).toEqual(1); + expect(DevServer.mock.instances[1].listen.mock.calls[0]).toMatchSnapshot(); + }); + + it('should handle 2 multi compiler devServer configs with conflicting ports', () => { + expect(() => { + const config = [ + { + devServer: { + port: 9000, + }, + }, + { + devServer: { + port: 9000, + }, + }, + ]; + const compiler = webpack(config); + + startDevServer(compiler, {}); + }).toThrow( + 'Unique ports must be specified for each devServer option in your webpack configuration. Alternatively, run only 1 devServer config using the --config-name flag to specify your desired config.', + ); + }); +}); diff --git a/packages/serve/src/args-to-camel-case.ts b/packages/serve/src/args-to-camel-case.ts deleted file mode 100644 index 995d9111fbb..00000000000 --- a/packages/serve/src/args-to-camel-case.ts +++ /dev/null @@ -1,31 +0,0 @@ -/** - * - * Converts dash-separated strings to camel case - * - * @param {String} str - the string to convert - * - * @returns {String} - new camel case string - */ -function dashesToCamelCase(str): string { - return str.replace(/-([a-z0-9])/g, (g): string => g[1].toUpperCase()); -} - -/** - * - * Converts CLI args to camel case from dash-separated words - * - * @param {Object} args - argument object parsed by command-line-args - * - * @returns {Object} - the same args object as passed in, with new keys - */ -export default function argsToCamelCase(args): object { - Object.keys(args).forEach((key): void => { - const newKey = dashesToCamelCase(key); - if (key !== newKey) { - const arg = args[key]; - delete args[key]; - args[newKey] = arg; - } - }); - return args; -} diff --git a/packages/serve/src/createConfig.ts b/packages/serve/src/createConfig.ts new file mode 100644 index 00000000000..bf5d23d909e --- /dev/null +++ b/packages/serve/src/createConfig.ts @@ -0,0 +1,29 @@ +import { devServerOptionsType } from './types'; + +/** + * + * Creates a devServer config from CLI args + * + * @param {Object} args - devServer args + * + * @returns {Object} valid devServer options object + */ +export default function createConfig(args): devServerOptionsType { + const options = { ...args }; + + if (options.clientLogging) { + options.client = { + logging: options.clientLogging, + }; + // clientLogging is not a valid devServer option + delete options.clientLogging; + } + + if (options.hotOnly) { + options.hot = 'only'; + // hotOnly is not a valid devServer option + delete options.hotOnly; + } + + return options; +} diff --git a/packages/serve/src/getDevServerOptions.ts b/packages/serve/src/getDevServerOptions.ts new file mode 100644 index 00000000000..3a7bdd6b788 --- /dev/null +++ b/packages/serve/src/getDevServerOptions.ts @@ -0,0 +1,28 @@ +import { devServerOptionsType } from './types'; + +/** + * + * Get the devServer option from the user's compiler options + * + * @param {Object} compiler - webpack compiler + * @param {Object} webpackArgs - webpack args + * + * @returns {Object} + */ +export default function getDevServerOptions(compiler): devServerOptionsType[] { + const defaultOpts = {}; + const devServerOptions = []; + const compilers = compiler.compilers || [compiler]; + + compilers.forEach((comp) => { + if (comp.options.devServer) { + devServerOptions.push(comp.options.devServer); + } + }); + + if (devServerOptions.length === 0) { + devServerOptions.push(defaultOpts); + } + + return devServerOptions; +} diff --git a/packages/serve/src/index.ts b/packages/serve/src/index.ts index 852c4fb2b1e..c56ebacc4f4 100644 --- a/packages/serve/src/index.ts +++ b/packages/serve/src/index.ts @@ -1,6 +1,6 @@ import WebpackCLI from 'webpack-cli'; -import logger from 'webpack-cli/lib/utils/logger'; import startDevServer from './startDevServer'; +import parseArgs from './parseArgs'; /** * @@ -10,33 +10,10 @@ import startDevServer from './startDevServer'; * @returns {Function} invokes the devServer API */ export default function serve(...args: string[]): void { - let devServerFlags: object[]; - try { - devServerFlags = require('webpack-dev-server/bin/cli-flags').devServer; - } catch (err) { - throw new Error(`You need to install 'webpack-dev-server' for running 'webpack serve'.\n${err}`); - } const cli = new WebpackCLI(); const core = cli.getCoreFlags(); - const parsedDevServerArgs = cli.argParser(devServerFlags, args, true); - const devServerArgs = parsedDevServerArgs.opts; - const parsedWebpackArgs = cli.argParser(core, parsedDevServerArgs.unknownArgs, true, process.title); - const webpackArgs = parsedWebpackArgs.opts; - - // pass along the 'hot' argument to the dev server if it exists - if (webpackArgs && webpackArgs.hot !== undefined) { - devServerArgs['hot'] = webpackArgs.hot; - } - - if (parsedWebpackArgs.unknownArgs.length > 0) { - parsedWebpackArgs.unknownArgs - .filter((e) => e) - .forEach((unknown) => { - logger.error(`Unknown argument: ${unknown}`); - }); - process.exit(2); - } + const { webpackArgs, devServerArgs } = parseArgs(cli, args); cli.getCompiler(webpackArgs, core).then((compiler): void => { startDevServer(compiler, devServerArgs); diff --git a/packages/serve/src/mergeOptions.ts b/packages/serve/src/mergeOptions.ts new file mode 100644 index 00000000000..6022dc4ffb8 --- /dev/null +++ b/packages/serve/src/mergeOptions.ts @@ -0,0 +1,24 @@ +import { devServerOptionsType } from './types'; + +/** + * + * Merges CLI options and devServer options from config file + * + * @param {Object} cliOptions - devServer CLI args + * @param {Object} devServerOptions - devServer config options + * + * @returns {Object} merged options object + */ +export default function mergeOptions(cliOptions: devServerOptionsType, devServerOptions: devServerOptionsType): devServerOptionsType { + // CLI options should take precedence over devServer options, + // and CLI options should have no default values included + const options = { ...devServerOptions, ...cliOptions }; + + if (devServerOptions.client && cliOptions.client) { + // the user could set some client options in their devServer config, + // then also specify client options on the CLI + options.client = { ...devServerOptions.client, ...cliOptions.client }; + } + + return options; +} diff --git a/packages/serve/src/parseArgs.ts b/packages/serve/src/parseArgs.ts new file mode 100644 index 00000000000..b37a7a3e9f5 --- /dev/null +++ b/packages/serve/src/parseArgs.ts @@ -0,0 +1,56 @@ +import logger from 'webpack-cli/lib/utils/logger'; + +type WebpackCLIType = { + getCoreFlags: Function; + argParser: Function; +}; + +type ArgsType = { + devServerArgs: object; + webpackArgs: object; +}; + +/** + * + * Parses raw dev server CLI args + * + * @param {Object} cli - webpack CLI object + * @param {Object[]} devServerFlags - devServer flags + * @param {String[]} args - unparsed devServer args processed from the CLI + * + * @returns {Object} parsed webpack args and dev server args objects + */ +export default function parseArgs(cli: WebpackCLIType, args: string[]): ArgsType { + let devServerFlags: object[]; + try { + devServerFlags = require('webpack-dev-server/bin/cli-flags').devServer; + } catch (err) { + throw new Error(`You need to install 'webpack-dev-server' for running 'webpack serve'.\n${err}`); + } + + const core = cli.getCoreFlags(); + + const parsedDevServerArgs = cli.argParser(devServerFlags, args, true); + const devServerArgs = parsedDevServerArgs.opts; + const parsedWebpackArgs = cli.argParser(core, parsedDevServerArgs.unknownArgs, true, process.title); + const webpackArgs = parsedWebpackArgs.opts; + + // pass along the 'hot' argument to the dev server if it exists + if (webpackArgs && webpackArgs.hot !== undefined) { + devServerArgs['hot'] = webpackArgs.hot; + } + + if (parsedWebpackArgs.unknownArgs.length > 0) { + parsedWebpackArgs.unknownArgs + .filter((e) => e) + .forEach((unknown) => { + logger.error(`Unknown argument: ${unknown}`); + }); + process.exit(2); + } + + return { + devServerArgs, + webpackArgs, + }; +} diff --git a/packages/serve/src/startDevServer.ts b/packages/serve/src/startDevServer.ts index 00142402c5c..e225d685274 100644 --- a/packages/serve/src/startDevServer.ts +++ b/packages/serve/src/startDevServer.ts @@ -1,34 +1,49 @@ +import createConfig from './createConfig'; +import getDevServerOptions from './getDevServerOptions'; +import mergeOptions from './mergeOptions'; + /** * * Starts the devServer * * @param {Object} compiler - a webpack compiler - * @param {Object} options - devServer options + * @param {Object} devServerArgs - devServer args * - * @returns {Void} + * @returns {Object[]} array of resulting servers */ -export default function startDevServer(compiler, options): void { +export default function startDevServer(compiler, devServerArgs): object[] { // eslint-disable-next-line @typescript-eslint/no-var-requires const Server = require('webpack-dev-server/lib/Server'); - const firstWpOpt = compiler.compilers ? compiler.compilers[0].options : compiler.options; - const devServerOptions = firstWpOpt.devServer || {}; - - const host = options.host || devServerOptions.host || 'localhost'; - const port = options.port || devServerOptions.port || 8080; - // socket should not have a default value, because it should only be used if the - // user explicitly provides it - const socket = options.socket || devServerOptions.socket; - - options.host = host; - options.port = port; - if (socket) { - options.socket = socket; - } - - const server = new Server(compiler, options); - server.listen(socket || port, host, (err): void => { - if (err) { - throw err; + const cliOptions = createConfig(devServerArgs); + const devServerOptions = getDevServerOptions(compiler); + + const servers = []; + + const usedPorts: number[] = []; + devServerOptions.forEach((devServerOpts): void => { + const options = mergeOptions(cliOptions, devServerOpts); + + options.host = options.host || 'localhost'; + options.port = options.port || 8080; + + const portNum = +options.port; + + if (usedPorts.find((port) => portNum === port)) { + throw new Error( + 'Unique ports must be specified for each devServer option in your webpack configuration. Alternatively, run only 1 devServer config using the --config-name flag to specify your desired config.', + ); } + usedPorts.push(portNum); + + const server = new Server(compiler, options); + server.listen(options.port, options.host, (err): void => { + if (err) { + throw err; + } + }); + + servers.push(server); }); + + return servers; } diff --git a/packages/serve/src/types.ts b/packages/serve/src/types.ts new file mode 100644 index 00000000000..7857236a663 --- /dev/null +++ b/packages/serve/src/types.ts @@ -0,0 +1,30 @@ +export type devServerOptionsType = { + bonjour?: boolean; + client?: object; + compress?: boolean; + dev?: object; + firewall?: boolean | string[]; + headers?: object; + historyApiFallback?: boolean | object; + host?: string | null; + hot?: boolean | string; + http2?: boolean; + https?: boolean | object; + injectClient?: boolean | Function; + injectHot?: boolean | Function; + liveReload?: boolean; + onAfterSetupMiddleware?: Function; + onBeforeSetupMiddleware?: Function; + onListening?: Function; + open?: string | boolean | object; + openPage?: string | string[]; + overlay?: boolean | object; + port?: number | string | null; + profile?: boolean; + progress?: boolean; + proxy?: object | (object | Function)[]; + public?: string; + static?: boolean | string | object | (string | object)[]; + transportMode?: object | string; + useLocalIp?: boolean; +}; diff --git a/packages/webpack-cli/__tests__/arg-parser.test.js b/packages/webpack-cli/__tests__/arg-parser.test.js index 8648ccf4129..dca85eb4031 100644 --- a/packages/webpack-cli/__tests__/arg-parser.test.js +++ b/packages/webpack-cli/__tests__/arg-parser.test.js @@ -4,7 +4,8 @@ jest.mock('../lib/utils/logger', () => { warn: warnMock, }; }); -jest.spyOn(process, 'exit').mockImplementation(() => {}); +const processExitSpy = jest.spyOn(process, 'exit').mockImplementation(() => {}); +const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); const argParser = require('../lib/utils/arg-parser'); const { core } = require('../lib/utils/cli-flags'); @@ -33,6 +34,7 @@ const basicOptions = [ }, { name: 'string-flag', + alias: 's', usage: '--string-flag ', type: String, description: 'string flag', @@ -46,10 +48,34 @@ const basicOptions = [ }, { name: 'multi-type', + alias: 'm', usage: '--multi-type | --multi-type ', type: [String, Boolean], + negative: true, description: 'flag with multiple types', }, + { + name: 'multi-type-different-order', + usage: '--multi-type-different-order | --multi-type-different-order ', + // duplicates and a different ordering should be handled correctly + type: [Boolean, String, Boolean], + description: 'flag with multiple types in different order', + }, + { + name: 'multi-type-empty', + usage: '--multi-type-empty', + // should default to Boolean type + type: [], + description: 'flag with empty multi type array', + }, + { + name: 'multi-type-number', + usage: '--multi-type-number', + // should use only Number type (the first in the array), + // because Number and Boolean together are not supported + type: [Number, Boolean], + description: 'flag with number multi type', + }, { name: 'custom-type-flag', usage: '--custom-type-flag ', @@ -87,6 +113,8 @@ helpAndVersionOptions.push( describe('arg-parser', () => { beforeEach(() => { warnMock.mockClear(); + processExitSpy.mockClear(); + consoleErrorSpy.mockClear(); }); it('parses no flags', () => { @@ -145,6 +173,16 @@ describe('arg-parser', () => { expect(warnMock.mock.calls.length).toEqual(0); }); + it('parses string flag alias', () => { + const res = argParser(basicOptions, ['-s', 'string-value'], true); + expect(res.unknownArgs.length).toEqual(0); + expect(res.opts).toEqual({ + stringFlag: 'string-value', + stringFlagWithDefault: 'default-value', + }); + expect(warnMock.mock.calls.length).toEqual(0); + }); + it('parses multi type flag as Boolean', () => { const res = argParser(basicOptions, ['--multi-type'], true); expect(res.unknownArgs.length).toEqual(0); @@ -165,6 +203,91 @@ describe('arg-parser', () => { expect(warnMock.mock.calls.length).toEqual(0); }); + it('parses multi type flag alias as Boolean', () => { + const res = argParser(basicOptions, ['-m'], true); + expect(res.unknownArgs.length).toEqual(0); + expect(res.opts).toEqual({ + multiType: true, + stringFlagWithDefault: 'default-value', + }); + expect(warnMock.mock.calls.length).toEqual(0); + }); + + it('parses multi type flag alias as String', () => { + const res = argParser(basicOptions, ['-m', 'value'], true); + expect(res.unknownArgs.length).toEqual(0); + expect(res.opts).toEqual({ + multiType: 'value', + stringFlagWithDefault: 'default-value', + }); + expect(warnMock.mock.calls.length).toEqual(0); + }); + + it('parses negated multi type flag as Boolean', () => { + const res = argParser(basicOptions, ['--no-multi-type'], true); + expect(res.unknownArgs.length).toEqual(0); + expect(res.opts).toEqual({ + multiType: false, + stringFlagWithDefault: 'default-value', + }); + expect(warnMock.mock.calls.length).toEqual(0); + }); + + it('parses multi type flag with different ordering as Boolean', () => { + const res = argParser(basicOptions, ['--multi-type-different-order'], true); + expect(res.unknownArgs.length).toEqual(0); + expect(res.opts).toEqual({ + multiTypeDifferentOrder: true, + stringFlagWithDefault: 'default-value', + }); + expect(warnMock.mock.calls.length).toEqual(0); + }); + + it('parses multi type flag with different ordering as String', () => { + const res = argParser(basicOptions, ['--multi-type-different-order', 'value'], true); + expect(res.unknownArgs.length).toEqual(0); + expect(res.opts).toEqual({ + multiTypeDifferentOrder: 'value', + stringFlagWithDefault: 'default-value', + }); + expect(warnMock.mock.calls.length).toEqual(0); + }); + + it('parses empty multi type flag array as Boolean', () => { + const res = argParser(basicOptions, ['--multi-type-empty'], true); + expect(res.unknownArgs.length).toEqual(0); + expect(res.opts).toEqual({ + multiTypeEmpty: true, + stringFlagWithDefault: 'default-value', + }); + expect(warnMock.mock.calls.length).toEqual(0); + }); + + it('parses multi type flag (Number, Boolean) as Number', () => { + const res = argParser(basicOptions, ['--multi-type-number', '1.1'], true); + expect(res.unknownArgs.length).toEqual(0); + expect(res.opts).toEqual({ + multiTypeNumber: 1.1, + stringFlagWithDefault: 'default-value', + }); + expect(warnMock.mock.calls.length).toEqual(0); + }); + + it('parsing multi type flag (Number, Boolean) as Boolean fails', () => { + // this should fail because a multi type of Number and Boolean is + // not supported + argParser(basicOptions, ['--multi-type-number'], true); + expect(warnMock.mock.calls.length).toEqual(0); + + // by default, commander handles the error with a process.exit(1) + // along with an error message + expect(processExitSpy.mock.calls.length).toEqual(1); + expect(processExitSpy.mock.calls[0]).toEqual([1]); + + expect(consoleErrorSpy.mock.calls.length).toEqual(1); + expect(consoleErrorSpy.mock.calls[0][0]).toContain("option '--multi-type-number ' argument missing"); + }); + it('warns on usage of both flag alias and same negated flag, setting it to true', () => { const res = argParser(basicOptions, ['--no-neg-flag', '-n'], true); expect(res.unknownArgs.length).toEqual(0); @@ -291,4 +414,13 @@ describe('arg-parser', () => { expect(warnMock.mock.calls.length).toEqual(1); expect(warnMock.mock.calls[0][0]).toContain('You provided both --neg-flag and --no-neg-flag'); }); + + it('handles unknown flag', () => { + const res = argParser(basicOptions, ['--unknown-flag'], true); + expect(res.unknownArgs).toEqual(['--unknown-flag']); + expect(res.opts).toEqual({ + stringFlagWithDefault: 'default-value', + }); + expect(warnMock.mock.calls.length).toEqual(0); + }); }); diff --git a/packages/webpack-cli/lib/utils/arg-parser.js b/packages/webpack-cli/lib/utils/arg-parser.js index 7e79694d3b4..e81600b9879 100644 --- a/packages/webpack-cli/lib/utils/arg-parser.js +++ b/packages/webpack-cli/lib/utils/arg-parser.js @@ -58,27 +58,61 @@ function argParser(options, args, argsOnly = false, name = '', helpFunction = un // Register options on the parser options.reduce((parserInstance, option) => { - const flags = option.alias ? `-${option.alias}, --${option.name}` : `--${option.name}`; - let flagsWithType = option.type !== Boolean ? flags + ' ' : flags; - if (option.type === Boolean || option.type === String) { - if (!option.multiple) { - // Prevent default behavior for standalone options - parserInstance.option(flagsWithType, option.description, option.defaultValue).action(() => {}); + let optionType = option.type; + let isStringOrBool = false; + if (Array.isArray(optionType)) { + // filter out duplicate types + optionType = optionType.filter((type, index) => { + return optionType.indexOf(type) === index; + }); + + // the only multi type currently supported is String and Boolean, + // if there is a case where a different multi type is needed it + // must be added here + if (optionType.length === 0) { + // if no type is provided in the array fall back to Boolean + optionType = Boolean; + } else if (optionType.length === 1 || optionType.length > 2) { + // treat arrays with 1 or > 2 args as a single type + optionType = optionType[0]; } else { + // only String and Boolean multi type is supported + if (optionType.includes(Boolean) && optionType.includes(String)) { + isStringOrBool = true; + } else { + optionType = optionType[0]; + } + } + } + + const flags = option.alias ? `-${option.alias}, --${option.name}` : `--${option.name}`; + let flagsWithType = flags; + if (isStringOrBool) { + // commander recognizes [value] as an optional placeholder, + // making this flag work either as a string or a boolean + flagsWithType = `${flags} [value]`; + } else if (optionType !== Boolean) { + // is a required placeholder for any non-Boolean types + flagsWithType = `${flags} `; + } + + if (isStringOrBool || optionType === Boolean || optionType === String) { + if (option.multiple) { + // a multiple argument parsing function const multiArg = (value, previous = []) => previous.concat([value]); parserInstance.option(flagsWithType, option.description, multiArg, option.defaultValue).action(() => {}); + } else { + // Prevent default behavior for standalone options + parserInstance.option(flagsWithType, option.description, option.defaultValue).action(() => {}); } - } else if (option.type === Number) { + } else if (optionType === Number) { + // this will parse the flag as a number parserInstance.option(flagsWithType, option.description, Number, option.defaultValue); } else { // in this case the type is a parsing function - if (option.type.length > 1) { - flagsWithType = flags + ' [value]'; - parserInstance.option(flagsWithType, option.description, option.type[0], option.defaultValue).action(() => {}); - } else { - parserInstance.option(flagsWithType, option.description, option.type, option.defaultValue).action(() => {}); - } + parserInstance.option(flagsWithType, option.description, optionType, option.defaultValue).action(() => {}); } + if (option.negative) { // commander requires explicitly adding the negated version of boolean flags const negatedFlag = `--no-${option.name}`;