From a3092ef2b51ece30221f7dd7b30a686626c1fd7a Mon Sep 17 00:00:00 2001 From: Alexander Akait <4567934+alexander-akait@users.noreply.github.com> Date: Wed, 30 Dec 2020 17:28:03 +0300 Subject: [PATCH] fix: respect the `output.publicPath` option for the `serve`command (#2271) --- .../__snapshots__/startDevServer.test.ts.snap | 5 + packages/serve/src/startDevServer.ts | 73 ++++++++------ packages/serve/src/types.ts | 1 + .../dev-server-output-public-path.config.js | 13 +++ ...ti-dev-server-output-public-path.config.js | 26 +++++ test/serve/basic/multi-dev-server.config.js | 32 +++++++ .../basic/multi-output-public-path.config.js | 23 +++++ test/serve/basic/multi.config.js | 25 +++++ .../serve/basic/multiple-dev-server.config.js | 29 ++++++ test/serve/basic/output-public-path.config.js | 10 ++ test/serve/basic/serve-basic.test.js | 96 ++++++++++++++++++- test/serve/basic/src/other.js | 1 + 12 files changed, 302 insertions(+), 32 deletions(-) create mode 100644 test/serve/basic/dev-server-output-public-path.config.js create mode 100644 test/serve/basic/multi-dev-server-output-public-path.config.js create mode 100644 test/serve/basic/multi-dev-server.config.js create mode 100644 test/serve/basic/multi-output-public-path.config.js create mode 100644 test/serve/basic/multi.config.js create mode 100644 test/serve/basic/multiple-dev-server.config.js create mode 100644 test/serve/basic/output-public-path.config.js create mode 100644 test/serve/basic/src/other.js diff --git a/packages/serve/__tests__/__snapshots__/startDevServer.test.ts.snap b/packages/serve/__tests__/__snapshots__/startDevServer.test.ts.snap index e6fdcbe3b3d..ad76801f1da 100644 --- a/packages/serve/__tests__/__snapshots__/startDevServer.test.ts.snap +++ b/packages/serve/__tests__/__snapshots__/startDevServer.test.ts.snap @@ -4,6 +4,7 @@ exports[`startDevServer should set default port and host if not provided 1`] = ` Object { "host": "localhost", "port": 8080, + "publicPath": "/", } `; @@ -22,6 +23,7 @@ Object { "hot": true, "port": 9000, "progress": true, + "publicPath": "/", } `; @@ -40,6 +42,7 @@ Object { "hot": true, "port": 9000, "progress": true, + "publicPath": "/", } `; @@ -56,6 +59,7 @@ Object { "host": "localhost", "port": 9000, "progress": true, + "publicPath": "/", } `; @@ -64,6 +68,7 @@ Object { "host": "localhost", "port": 9001, "progress": true, + "publicPath": "/", } `; diff --git a/packages/serve/src/startDevServer.ts b/packages/serve/src/startDevServer.ts index 08a9478cc76..dd5065248ca 100644 --- a/packages/serve/src/startDevServer.ts +++ b/packages/serve/src/startDevServer.ts @@ -11,10 +11,7 @@ import { devServerOptionsType } from './types'; * @returns {Object[]} array of resulting servers */ export default async function startDevServer(compiler, cliOptions, logger): Promise { - let isDevServer4 = false, - devServerVersion, - Server, - findPort; + let devServerVersion, Server, findPort; try { // eslint-disable-next-line node/no-extraneous-require @@ -28,24 +25,6 @@ export default async function startDevServer(compiler, cliOptions, logger): Prom process.exit(2); } - isDevServer4 = devServerVersion.startsWith('4'); - - const defaultOpts = {}; - const devServerOptions = []; - const compilers = compiler.compilers || [compiler]; - - compilers.forEach((compiler) => { - if (compiler.options.devServer) { - devServerOptions.push(compiler.options.devServer); - } - }); - - if (devServerOptions.length === 0) { - devServerOptions.push(defaultOpts); - } - - const servers = []; - const usedPorts: number[] = []; const mergeOptions = (cliOptions: devServerOptionsType, devServerOptions: devServerOptionsType): devServerOptionsType => { // CLI options should take precedence over devServer options, // and CLI options should have no default values included @@ -60,35 +39,69 @@ export default async function startDevServer(compiler, cliOptions, logger): Prom return options; }; - for (const devServerOpts of devServerOptions) { - const options = mergeOptions(cliOptions, devServerOpts); + const isMultiCompiler = Boolean(compiler.compilers); + + let compilersWithDevServerOption; + + if (isMultiCompiler) { + compilersWithDevServerOption = compiler.compilers.filter((compiler) => compiler.options.devServer); + + // No compilers found with the `devServer` option, let's use first compiler + if (compilersWithDevServerOption.length === 0) { + compilersWithDevServerOption = [compiler.compilers[0]]; + } + } else { + compilersWithDevServerOption = [compiler]; + } + + const isDevServer4 = devServerVersion.startsWith('4'); + const usedPorts = []; + const devServersOptions = []; + + for (const compilerWithDevServerOption of compilersWithDevServerOption) { + const options = mergeOptions(cliOptions, compilerWithDevServerOption.options.devServer || {}); if (isDevServer4) { options.port = await findPort(options.port); options.client = options.client || {}; options.client.port = options.client.port || options.port; } else { + if (!options.publicPath) { + options.publicPath = + typeof compilerWithDevServerOption.options.output.publicPath === 'undefined' || + compilerWithDevServerOption.options.output.publicPath === 'auto' + ? '/' + : compilerWithDevServerOption.options.output.publicPath; + } + options.host = options.host || 'localhost'; options.port = options.port || 8080; } if (options.port) { - const portNum = +options.port; + const portNumber = Number(options.port); - if (usedPorts.find((port) => portNum === port)) { + if (usedPorts.find((port) => portNumber === 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); + usedPorts.push(portNumber); } + devServersOptions.push({ compiler, options }); + } + + const servers = []; + + for (const devServerOptions of devServersOptions) { + const { compiler, options } = devServerOptions; const server = new Server(compiler, options); - server.listen(options.port, options.host, (err): void => { - if (err) { - throw err; + server.listen(options.port, options.host, (error): void => { + if (error) { + throw error; } }); diff --git a/packages/serve/src/types.ts b/packages/serve/src/types.ts index 61c59c5543a..f4a1b276fd0 100644 --- a/packages/serve/src/types.ts +++ b/packages/serve/src/types.ts @@ -27,6 +27,7 @@ export type devServerOptionsType = { static?: boolean | string | object | (string | object)[]; transportMode?: object | string; useLocalIp?: boolean; + publicPath?: undefined; }; type devServerClientOptions = { diff --git a/test/serve/basic/dev-server-output-public-path.config.js b/test/serve/basic/dev-server-output-public-path.config.js new file mode 100644 index 00000000000..e84e9137dd6 --- /dev/null +++ b/test/serve/basic/dev-server-output-public-path.config.js @@ -0,0 +1,13 @@ +const WebpackCLITestPlugin = require('../../utils/webpack-cli-test-plugin'); + +module.exports = { + mode: 'development', + devtool: false, + output: { + publicPath: '/my-public-path/', + }, + devServer: { + publicPath: '/dev-server-my-public-path/', + }, + plugins: [new WebpackCLITestPlugin(['mode', 'output'], false, 'hooks.compilation.taps')], +}; diff --git a/test/serve/basic/multi-dev-server-output-public-path.config.js b/test/serve/basic/multi-dev-server-output-public-path.config.js new file mode 100644 index 00000000000..56409276d4e --- /dev/null +++ b/test/serve/basic/multi-dev-server-output-public-path.config.js @@ -0,0 +1,26 @@ +const WebpackCLITestPlugin = require('../../utils/webpack-cli-test-plugin'); + +module.exports = [ + { + name: 'one', + mode: 'development', + devtool: false, + entry: './src/other.js', + output: { + filename: 'first-output/[name].js', + }, + }, + { + name: 'two', + mode: 'development', + devtool: false, + output: { + publicPath: '/my-public-path/', + filename: 'second-output/[name].js', + }, + devServer: { + publicPath: '/dev-server-my-public-path/', + }, + plugins: [new WebpackCLITestPlugin(['mode', 'output'], false, 'hooks.compilation.taps')], + }, +]; diff --git a/test/serve/basic/multi-dev-server.config.js b/test/serve/basic/multi-dev-server.config.js new file mode 100644 index 00000000000..59bb8f16133 --- /dev/null +++ b/test/serve/basic/multi-dev-server.config.js @@ -0,0 +1,32 @@ +const getPort = require('get-port'); + +const WebpackCLITestPlugin = require('../../utils/webpack-cli-test-plugin'); + +module.exports = async () => [ + { + name: 'one', + mode: 'development', + devtool: false, + output: { + filename: 'first-output/[name].js', + }, + devServer: { + port: await getPort(), + publicPath: '/one-dev-server-my-public-path/', + }, + plugins: [new WebpackCLITestPlugin(['mode', 'output'], false, 'hooks.compilation.taps')], + }, + { + name: 'two', + mode: 'development', + devtool: false, + entry: './src/other.js', + output: { + filename: 'second-output/[name].js', + }, + devServer: { + port: await getPort(), + publicPath: '/two-dev-server-my-public-path/', + }, + }, +]; diff --git a/test/serve/basic/multi-output-public-path.config.js b/test/serve/basic/multi-output-public-path.config.js new file mode 100644 index 00000000000..64286a1c2ef --- /dev/null +++ b/test/serve/basic/multi-output-public-path.config.js @@ -0,0 +1,23 @@ +const WebpackCLITestPlugin = require('../../utils/webpack-cli-test-plugin'); + +module.exports = [ + { + name: 'one', + mode: 'development', + devtool: false, + output: { + publicPath: '/my-public-path/', + filename: 'first-output/[name].js', + }, + plugins: [new WebpackCLITestPlugin(['mode', 'output'], false, 'hooks.compilation.taps')], + }, + { + name: 'two', + mode: 'development', + devtool: false, + entry: './src/other.js', + output: { + filename: 'second-output/[name].js', + }, + }, +]; diff --git a/test/serve/basic/multi.config.js b/test/serve/basic/multi.config.js new file mode 100644 index 00000000000..e344db6c72b --- /dev/null +++ b/test/serve/basic/multi.config.js @@ -0,0 +1,25 @@ +const WebpackCLITestPlugin = require('../../utils/webpack-cli-test-plugin'); + +module.exports = [ + { + name: 'one', + mode: 'development', + devtool: false, + output: { + filename: 'first-output/[name].js', + }, + devServer: { + publicPath: '/dev-server-my-public-path/', + }, + plugins: [new WebpackCLITestPlugin(['mode', 'output'], false, 'hooks.compilation.taps')], + }, + { + name: 'two', + mode: 'development', + devtool: false, + entry: './src/other.js', + output: { + filename: 'second-output/[name].js', + }, + }, +]; diff --git a/test/serve/basic/multiple-dev-server.config.js b/test/serve/basic/multiple-dev-server.config.js new file mode 100644 index 00000000000..154a03b2012 --- /dev/null +++ b/test/serve/basic/multiple-dev-server.config.js @@ -0,0 +1,29 @@ +const WebpackCLITestPlugin = require('../../utils/webpack-cli-test-plugin'); + +module.exports = [ + { + name: 'one', + mode: 'development', + devtool: false, + output: { + filename: 'first-output/[name].js', + }, + devServer: { + publicPath: '/dev-server-my-public-path/', + }, + plugins: [new WebpackCLITestPlugin(['mode', 'output'], false)], + }, + { + name: 'two', + mode: 'development', + devtool: false, + entry: './src/other.js', + output: { + filename: 'first-output/[name].js', + }, + devServer: { + publicPath: '/dev-server-my-public-path/', + }, + plugins: [new WebpackCLITestPlugin(['mode', 'output'], false)], + }, +]; diff --git a/test/serve/basic/output-public-path.config.js b/test/serve/basic/output-public-path.config.js new file mode 100644 index 00000000000..0b9f7f04547 --- /dev/null +++ b/test/serve/basic/output-public-path.config.js @@ -0,0 +1,10 @@ +const WebpackCLITestPlugin = require('../../utils/webpack-cli-test-plugin'); + +module.exports = { + mode: 'development', + devtool: false, + output: { + publicPath: '/my-public-path/', + }, + plugins: [new WebpackCLITestPlugin(['mode', 'output'], false, 'hooks.compilation.taps')], +}; diff --git a/test/serve/basic/serve-basic.test.js b/test/serve/basic/serve-basic.test.js index e6754e52658..f9f68736d73 100644 --- a/test/serve/basic/serve-basic.test.js +++ b/test/serve/basic/serve-basic.test.js @@ -2,7 +2,7 @@ const path = require('path'); const getPort = require('get-port'); -const { runServe, isDevServer4 } = require('../../utils/test-utils'); +const { runServe, isWebpack5, isDevServer4 } = require('../../utils/test-utils'); const testPath = path.resolve(__dirname); @@ -34,6 +34,29 @@ describe('basic serve usage', () => { expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull(); }); + it('should work in multi compiler mode', async () => { + const { stderr, stdout } = await runServe(['serve', '--config', 'multi.config.js', '--port', port], __dirname); + + expect(stderr).toBeFalsy(); + expect(stdout).toContain('one'); + expect(stdout).toContain('first-output/main.js'); + expect(stdout).toContain('two'); + expect(stdout).toContain('second-output/main.js'); + expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull(); + }); + + // TODO need fix in future, edge case + it.skip('should work in multi compiler mode with multiple dev servers', async () => { + const { stderr, stdout } = await runServe(['serve', '--config', 'multi-dev-server.config.js'], __dirname); + + expect(stderr).toBeFalsy(); + expect(stdout).toContain('one'); + expect(stdout).toContain('first-output/main.js'); + expect(stdout).toContain('two'); + expect(stdout).toContain('second-output/main.js'); + expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull(); + }); + it('should work with the "--mode" option', async () => { const { stderr, stdout } = await runServe([], __dirname); @@ -141,7 +164,60 @@ describe('basic serve usage', () => { expect(stdout.match(/HotModuleReplacementPlugin/g)).toHaveLength(1); }); - it.only('should work with the "--open" option', async () => { + it('should work with the default "publicPath" option', async () => { + const { stderr, stdout } = await runServe(['serve'], __dirname); + + expect(stderr).toBeFalsy(); + expect(stdout).toContain('main.js'); + expect(stdout).toContain('from /'); + expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull(); + }); + + it('should work with the "--output-public-path" option', async () => { + const { stderr, stdout } = await runServe(['serve', '--output-public-path', '/my-public-path/'], __dirname); + + if (isWebpack5) { + expect(stderr).toBeFalsy(); + expect(stdout).toContain('main.js'); + expect(stdout).toContain('/my-public-path/'); + expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull(); + } else { + expect(stderr).toContain("unknown option '--output-public-path'"); + expect(stdout).toBeFalsy(); + } + }); + + it('should respect the "publicPath" option from configuration', async () => { + const { stderr, stdout } = await runServe(['serve', '--config', 'output-public-path.config.js'], __dirname); + + expect(stderr).toBeFalsy(); + expect(stdout).toContain('main.js'); + expect(stdout).toContain('/my-public-path/'); + expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull(); + }); + + it('should respect the "publicPath" option from configuration using multi compiler mode', async () => { + const { stderr, stdout } = await runServe(['serve', '--config', 'multi-output-public-path.config.js', '--port', port], __dirname); + + expect(stderr).toBeFalsy(); + expect(stdout).toContain('one'); + expect(stdout).toContain('first-output/main.js'); + expect(stdout).toContain('two'); + expect(stdout).toContain('second-output/main.js'); + expect(stdout).toContain('/my-public-path/'); + expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull(); + }); + + it('should respect the "publicPath" option from configuration (from the "devServer" options)', async () => { + const { stderr, stdout } = await runServe(['serve', '--config', 'dev-server-output-public-path.config.js'], __dirname); + + expect(stderr).toBeFalsy(); + expect(stdout).toContain('main.js'); + expect(stdout).toContain('/dev-server-my-public-path/'); + expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull(); + }); + + it('should work with the "--open" option', async () => { const { stdout, stderr } = await runServe(['--open', '--port', port], testPath); expect(stderr).toBeFalsy(); @@ -149,6 +225,22 @@ describe('basic serve usage', () => { expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull(); }); + it('should respect the "publicPath" option from configuration using multi compiler mode (from the "devServer" options)', async () => { + const { stderr, stdout } = await runServe( + ['serve', '--config', 'multi-dev-server-output-public-path.config.js', '--port', port], + __dirname, + ); + + expect(stderr).toBeFalsy(); + expect(stderr).toBeFalsy(); + expect(stdout).toContain('one'); + expect(stdout).toContain('first-output/main.js'); + expect(stdout).toContain('two'); + expect(stdout).toContain('second-output/main.js'); + expect(stdout).toContain('/dev-server-my-public-path/'); + expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull(); + }); + it('should log and error on unknown flag', async () => { const { exitCode, stdout, stderr } = await runServe(['--port', port, '--unknown-flag'], testPath); diff --git a/test/serve/basic/src/other.js b/test/serve/basic/src/other.js new file mode 100644 index 00000000000..2457f618e17 --- /dev/null +++ b/test/serve/basic/src/other.js @@ -0,0 +1 @@ +console.log('HERE');