From cf4d0d07b8ffba9d28cd627b48f74e5f4f307fc6 Mon Sep 17 00:00:00 2001 From: Michael Rawlings Date: Wed, 17 Apr 2019 11:18:51 -0700 Subject: [PATCH] fix: only add client entry to web targets (#1775) --- lib/options.json | 20 +++ lib/utils/addEntries.js | 56 ++++++-- test/Entry.test.js | 125 ++++++++++++++++++ test/UniversalCompiler.test.js | 48 +++++++ .../universal-compiler-config/client.js | 3 + .../universal-compiler-config/server.js | 3 + .../webpack.config.js | 25 ++++ 7 files changed, 266 insertions(+), 14 deletions(-) create mode 100644 test/UniversalCompiler.test.js create mode 100644 test/fixtures/universal-compiler-config/client.js create mode 100644 test/fixtures/universal-compiler-config/server.js create mode 100644 test/fixtures/universal-compiler-config/webpack.config.js diff --git a/lib/options.json b/lib/options.json index e77b6edf06..c704721080 100644 --- a/lib/options.json +++ b/lib/options.json @@ -172,6 +172,26 @@ "inline": { "type": "boolean" }, + "injectClient": { + "anyOf": [ + { + "type": "boolean" + }, + { + "instanceof": "Function" + } + ] + }, + "injectHot": { + "anyOf": [ + { + "type": "boolean" + }, + { + "instanceof": "Function" + } + ] + }, "disableHostCheck": { "type": "boolean" }, diff --git a/lib/utils/addEntries.js b/lib/utils/addEntries.js index b126ee18b7..9754fbeb00 100644 --- a/lib/utils/addEntries.js +++ b/lib/utils/addEntries.js @@ -18,26 +18,31 @@ function addEntries(config, options, server) { const domain = createDomain(options, app); const sockPath = options.sockPath ? `&sockPath=${options.sockPath}` : ''; const sockPort = options.sockPort ? `&sockPort=${options.sockPort}` : ''; - const entries = [ - `${require.resolve('../../client/')}?${domain}${sockPath}${sockPort}`, - ]; + const clientEntry = `${require.resolve( + '../../client/' + )}?${domain}${sockPath}${sockPort}`; + let hotEntry; + if (options.hotOnly) { - entries.push(require.resolve('webpack/hot/only-dev-server')); + hotEntry = require.resolve('webpack/hot/only-dev-server'); } else if (options.hot) { - entries.push(require.resolve('webpack/hot/dev-server')); + hotEntry = require.resolve('webpack/hot/dev-server'); } - const prependEntry = (entry) => { - if (typeof entry === 'function') { - return () => Promise.resolve(entry()).then(prependEntry); + const prependEntry = (originalEntry, additionalEntries) => { + if (typeof originalEntry === 'function') { + return () => + Promise.resolve(originalEntry()).then((entry) => + prependEntry(entry, additionalEntries) + ); } - if (typeof entry === 'object' && !Array.isArray(entry)) { + if (typeof originalEntry === 'object' && !Array.isArray(originalEntry)) { const clone = {}; - Object.keys(entry).forEach((key) => { + Object.keys(originalEntry).forEach((key) => { // entry[key] should be a string here - clone[key] = prependEntry(entry[key]); + clone[key] = prependEntry(originalEntry[key], additionalEntries); }); return clone; @@ -45,8 +50,8 @@ function addEntries(config, options, server) { // in this case, entry is a string or an array. // make sure that we do not add duplicates. - const entriesClone = entries.slice(0); - [].concat(entry).forEach((newEntry) => { + const entriesClone = additionalEntries.slice(0); + [].concat(originalEntry).forEach((newEntry) => { if (!entriesClone.includes(newEntry)) { entriesClone.push(newEntry); } @@ -54,9 +59,32 @@ function addEntries(config, options, server) { return entriesClone; }; + // eslint-disable-next-line no-shadow + const checkInject = (option, config, defaultValue) => { + if (typeof option === 'boolean') return option; + if (typeof option === 'function') return option(config); + return defaultValue; + }; + // eslint-disable-next-line no-shadow [].concat(config).forEach((config) => { - config.entry = prependEntry(config.entry || './src'); + const webTarget = + config.target === 'web' || + config.target === 'webworker' || + config.target == null; + const additionalEntries = checkInject( + options.injectClient, + config, + webTarget + ) + ? [clientEntry] + : []; + + if (hotEntry && checkInject(options.injectHot, config, true)) { + additionalEntries.push(hotEntry); + } + + config.entry = prependEntry(config.entry || './src', additionalEntries); if (options.hot || options.hotOnly) { config.plugins = config.plugins || []; diff --git a/test/Entry.test.js b/test/Entry.test.js index 0cd67d76ce..7a6f354173 100644 --- a/test/Entry.test.js +++ b/test/Entry.test.js @@ -277,4 +277,129 @@ describe('Entry', () => { expect(typeof webpackOptions.entry === 'function').toBe(true); }); + + it('only prepends devServer entry points to web targets by default', () => { + const webpackOptions = [ + Object.assign({}, config), + Object.assign({ target: 'web' }, config), + Object.assign({ target: 'webworker' }, config), + Object.assign({ target: 'node' }, config) /* index:3 */, + ]; + + const devServerOptions = {}; + + addEntries(webpackOptions, devServerOptions); + + // eslint-disable-next-line no-shadow + webpackOptions.forEach((webpackOptions, index) => { + const expectInline = index !== 3; /* all but the node target */ + + expect(webpackOptions.entry.length).toEqual(expectInline ? 2 : 1); + + if (expectInline) { + expect( + normalize(webpackOptions.entry[0]).indexOf('client/index.js?') !== -1 + ).toBeTruthy(); + } + + expect(normalize(webpackOptions.entry[expectInline ? 1 : 0])).toEqual( + './foo.js' + ); + }); + }); + + it('allows selecting compilations to inline the client into', () => { + const webpackOptions = [ + Object.assign({}, config), + Object.assign({ target: 'web' }, config), + Object.assign({ name: 'only-include' }, config) /* index:2 */, + Object.assign({ target: 'node' }, config), + ]; + + const devServerOptions = { + injectClient: (compilerConfig) => compilerConfig.name === 'only-include', + }; + + addEntries(webpackOptions, devServerOptions); + + // eslint-disable-next-line no-shadow + webpackOptions.forEach((webpackOptions, index) => { + const expectInline = index === 2; /* only the "only-include" compiler */ + + expect(webpackOptions.entry.length).toEqual(expectInline ? 2 : 1); + + if (expectInline) { + expect( + normalize(webpackOptions.entry[0]).indexOf('client/index.js?') !== -1 + ).toBeTruthy(); + } + + expect(normalize(webpackOptions.entry[expectInline ? 1 : 0])).toEqual( + './foo.js' + ); + }); + }); + + it('when hot, prepends the hot runtime to all targets by default', () => { + const webpackOptions = [ + Object.assign({ target: 'web' }, config), + Object.assign({ target: 'node' }, config), + ]; + + const devServerOptions = { + // disable inlining the client so entry indexes match up + // and we can use the same assertions for both configs + injectClient: false, + hot: true, + }; + + addEntries(webpackOptions, devServerOptions); + + // eslint-disable-next-line no-shadow + webpackOptions.forEach((webpackOptions) => { + expect(webpackOptions.entry.length).toEqual(2); + + expect( + normalize(webpackOptions.entry[0]).includes('webpack/hot/dev-server') + ).toBeTruthy(); + + expect(normalize(webpackOptions.entry[1])).toEqual('./foo.js'); + }); + }); + + it('allows selecting which compilations to inject the hot runtime into', () => { + const webpackOptions = [ + Object.assign({ target: 'web' }, config), + Object.assign({ target: 'node' }, config), + ]; + + const devServerOptions = { + injectHot: (compilerConfig) => compilerConfig.target === 'node', + hot: true, + }; + + addEntries(webpackOptions, devServerOptions); + + // node target should have the client runtime but not the hot runtime + const webWebpackOptions = webpackOptions[0]; + + expect(webWebpackOptions.entry.length).toEqual(2); + + expect( + normalize(webWebpackOptions.entry[0]).indexOf('client/index.js?') !== -1 + ).toBeTruthy(); + + expect(normalize(webWebpackOptions.entry[1])).toEqual('./foo.js'); + + // node target should have the hot runtime but not the client runtime + const nodeWebpackOptions = webpackOptions[1]; + + expect(nodeWebpackOptions.entry.length).toEqual(2); + + expect( + normalize(nodeWebpackOptions.entry[0]).includes('webpack/hot/dev-server') + ).toBeTruthy(); + + expect(normalize(nodeWebpackOptions.entry[1])).toEqual('./foo.js'); + }); }); diff --git a/test/UniversalCompiler.test.js b/test/UniversalCompiler.test.js new file mode 100644 index 0000000000..1490a088c7 --- /dev/null +++ b/test/UniversalCompiler.test.js @@ -0,0 +1,48 @@ +'use strict'; + +const request = require('supertest'); +const helper = require('./helper'); +const config = require('./fixtures/universal-compiler-config/webpack.config'); + +describe('UniversalCompiler', () => { + let server; + let req; + beforeAll((done) => { + server = helper.start(config, { inline: true }, done); + req = request(server.app); + }); + + afterAll(helper.close); + + it('client bundle should have the inlined the client runtime', (done) => { + req + .get('/client.js') + .expect('Content-Type', 'application/javascript; charset=UTF-8') + .expect(200) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res.text).toContain('Hello from the client'); + expect(res.text).toContain('webpack-dev-server/client'); + done(); + }); + }); + + it('server bundle should NOT have the inlined the client runtime', (done) => { + // we wouldn't normally request a server bundle + // but we'll do it here to check the contents + req + .get('/server.js') + .expect('Content-Type', 'application/javascript; charset=UTF-8') + .expect(200) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res.text).toContain('Hello from the server'); + expect(res.text).not.toContain('webpack-dev-server/client'); + done(); + }); + }); +}); diff --git a/test/fixtures/universal-compiler-config/client.js b/test/fixtures/universal-compiler-config/client.js new file mode 100644 index 0000000000..1bae5186de --- /dev/null +++ b/test/fixtures/universal-compiler-config/client.js @@ -0,0 +1,3 @@ +'use strict'; + +console.log('Hello from the client'); diff --git a/test/fixtures/universal-compiler-config/server.js b/test/fixtures/universal-compiler-config/server.js new file mode 100644 index 0000000000..20b3c3356b --- /dev/null +++ b/test/fixtures/universal-compiler-config/server.js @@ -0,0 +1,3 @@ +'use strict'; + +console.log('Hello from the server'); diff --git a/test/fixtures/universal-compiler-config/webpack.config.js b/test/fixtures/universal-compiler-config/webpack.config.js new file mode 100644 index 0000000000..8b7ad99f3d --- /dev/null +++ b/test/fixtures/universal-compiler-config/webpack.config.js @@ -0,0 +1,25 @@ +'use strict'; + +module.exports = [ + { + mode: 'development', + context: __dirname, + entry: './client.js', + output: { + path: '/', + filename: 'client.js', + }, + node: false, + }, + { + mode: 'development', + context: __dirname, + target: 'node', + entry: './server.js', + output: { + path: '/', + filename: 'server.js', + }, + node: false, + }, +];