diff --git a/lib/Server.js b/lib/Server.js index ff21ccee94..b69663c4d1 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -33,6 +33,7 @@ const historyApiFallback = require('connect-history-api-fallback'); const webpack = require('webpack'); const webpackDevMiddleware = require('webpack-dev-middleware'); +const updateCompiler = require('./utils/updateCompiler'); const createLogger = require('./utils/createLogger'); const createCertificate = require('./utils/createCertificate'); @@ -89,6 +90,8 @@ class Server { throw new Error("'filename' option must be set in lazy mode."); } + updateCompiler(compiler, options); + this.stats = options.stats && Object.keys(options.stats).length ? options.stats diff --git a/lib/utils/addEntries.js b/lib/utils/addEntries.js index f4aa210b39..a4d28d9830 100644 --- a/lib/utils/addEntries.js +++ b/lib/utils/addEntries.js @@ -8,6 +8,7 @@ function addEntries(config, options, server) { // we're stubbing the app in this method as it's static and doesn't require // a server to be supplied. createDomain requires an app with the // address() signature. + const app = server || { address() { return { port: options.port }; @@ -35,13 +36,22 @@ function addEntries(config, options, server) { const clone = {}; Object.keys(entry).forEach((key) => { - clone[key] = entries.concat(entry[key]); + // entry[key] should be a string here + clone[key] = prependEntry(entry[key]); }); return clone; } - return entries.concat(entry); + // 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) => { + if (!entriesClone.includes(newEntry)) { + entriesClone.push(newEntry); + } + }); + return entriesClone; }; // eslint-disable-next-line no-shadow diff --git a/lib/utils/updateCompiler.js b/lib/utils/updateCompiler.js new file mode 100644 index 0000000000..4f6619874a --- /dev/null +++ b/lib/utils/updateCompiler.js @@ -0,0 +1,67 @@ +'use strict'; + +/* eslint-disable + no-shadow, + no-undefined +*/ +const webpack = require('webpack'); +const addEntries = require('./addEntries'); + +function updateCompiler(compiler, options) { + if (options.inline !== false) { + const findHMRPlugin = (config) => { + if (!config.plugins) { + return undefined; + } + + return config.plugins.find( + (plugin) => plugin.constructor === webpack.HotModuleReplacementPlugin + ); + }; + + const compilers = []; + const compilersWithoutHMR = []; + let webpackConfig; + if (compiler.compilers) { + webpackConfig = []; + compiler.compilers.forEach((compiler) => { + webpackConfig.push(compiler.options); + compilers.push(compiler); + if (!findHMRPlugin(compiler.options)) { + compilersWithoutHMR.push(compiler); + } + }); + } else { + webpackConfig = compiler.options; + compilers.push(compiler); + if (!findHMRPlugin(compiler.options)) { + compilersWithoutHMR.push(compiler); + } + } + + // it's possible that we should clone the config before doing + // this, but it seems safe not to since it actually reflects + // the changes we are making to the compiler + // important: this relies on the fact that addEntries now + // prevents duplicate new entries. + addEntries(webpackConfig, options); + compilers.forEach((compiler) => { + const config = compiler.options; + compiler.hooks.entryOption.call(config.context, config.entry); + }); + + // do not apply the plugin unless it didn't exist before. + if (options.hot || options.hotOnly) { + compilersWithoutHMR.forEach((compiler) => { + // addDevServerEntrypoints above should have added the plugin + // to the compiler options + const plugin = findHMRPlugin(compiler.options); + if (plugin) { + plugin.apply(compiler); + } + }); + } + } +} + +module.exports = updateCompiler; diff --git a/package-lock.json b/package-lock.json index ee87d059ce..eb33db2233 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4674,8 +4674,7 @@ }, "ansi-regex": { "version": "2.1.1", - "bundled": true, - "optional": true + "bundled": true }, "aproba": { "version": "1.2.0", @@ -4693,13 +4692,11 @@ }, "balanced-match": { "version": "1.0.0", - "bundled": true, - "optional": true + "bundled": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -4712,18 +4709,15 @@ }, "code-point-at": { "version": "1.1.0", - "bundled": true, - "optional": true + "bundled": true }, "concat-map": { "version": "0.0.1", - "bundled": true, - "optional": true + "bundled": true }, "console-control-strings": { "version": "1.1.0", - "bundled": true, - "optional": true + "bundled": true }, "core-util-is": { "version": "1.0.2", @@ -4826,8 +4820,7 @@ }, "inherits": { "version": "2.0.3", - "bundled": true, - "optional": true + "bundled": true }, "ini": { "version": "1.3.5", @@ -4837,7 +4830,6 @@ "is-fullwidth-code-point": { "version": "1.0.0", "bundled": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -4850,20 +4842,17 @@ "minimatch": { "version": "3.0.4", "bundled": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } }, "minimist": { "version": "0.0.8", - "bundled": true, - "optional": true + "bundled": true }, "minipass": { "version": "2.3.5", "bundled": true, - "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -4880,7 +4869,6 @@ "mkdirp": { "version": "0.5.1", "bundled": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -4953,8 +4941,7 @@ }, "number-is-nan": { "version": "1.0.1", - "bundled": true, - "optional": true + "bundled": true }, "object-assign": { "version": "4.1.1", @@ -4964,7 +4951,6 @@ "once": { "version": "1.4.0", "bundled": true, - "optional": true, "requires": { "wrappy": "1" } @@ -5040,8 +5026,7 @@ }, "safe-buffer": { "version": "5.1.2", - "bundled": true, - "optional": true + "bundled": true }, "safer-buffer": { "version": "2.1.2", @@ -5071,7 +5056,6 @@ "string-width": { "version": "1.0.2", "bundled": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -5089,7 +5073,6 @@ "strip-ansi": { "version": "3.0.1", "bundled": true, - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -5128,13 +5111,11 @@ }, "wrappy": { "version": "1.0.2", - "bundled": true, - "optional": true + "bundled": true }, "yallist": { "version": "3.0.3", - "bundled": true, - "optional": true + "bundled": true } } }, diff --git a/test/Client.test.js b/test/Client.test.js index 641d059876..db317eaf07 100644 --- a/test/Client.test.js +++ b/test/Client.test.js @@ -3,7 +3,6 @@ const express = require('express'); const httpProxy = require('http-proxy-middleware'); const request = require('supertest'); -const addEntries = require('../lib/utils/addEntries'); const helper = require('./helper'); const config = require('./fixtures/client-config/webpack.config'); const runBrowser = require('./helpers/run-browser'); @@ -28,13 +27,13 @@ describe('Client code', () => { port: 9001, host: '0.0.0.0', disableHostCheck: true, + inline: true, hot: true, watchOptions: { poll: true, }, }; - addEntries(config, options); - helper.start(config, options, done); + helper.startAwaitingCompilation(config, options, done); }); afterAll(helper.close); @@ -65,8 +64,7 @@ describe('Client code', () => { expect(requestObj.url()).toMatch( /^http:\/\/localhost:9000\/sockjs-node/ ); - browser.close(); - done(); + browser.close().then(done); }); page.goto('http://localhost:9000/main'); }); diff --git a/test/Entry.test.js b/test/Entry.test.js index b6bbb76732..0cd67d76ce 100644 --- a/test/Entry.test.js +++ b/test/Entry.test.js @@ -254,6 +254,21 @@ describe('Entry', () => { ]); }); + it('can prevent duplicate entries from successive calls', () => { + const webpackOptions = Object.assign({}, config); + const devServerOptions = { hot: true }; + + addEntries(webpackOptions, devServerOptions); + addEntries(webpackOptions, devServerOptions); + + expect(webpackOptions.entry.length).toEqual(3); + + const result = webpackOptions.entry.filter((entry) => + normalize(entry).includes('webpack/hot/dev-server') + ); + expect(result.length).toEqual(1); + }); + it('supports entry as Function', () => { const webpackOptions = Object.assign({}, configEntryAsFunction); const devServerOptions = {}; diff --git a/test/Hot.test.js b/test/Hot.test.js new file mode 100644 index 0000000000..5db2b2038b --- /dev/null +++ b/test/Hot.test.js @@ -0,0 +1,246 @@ +'use strict'; + +const request = require('supertest'); +const helper = require('./helper'); +const config = require('./fixtures/client-config/webpack.config'); +const multiCompilerConfig = require('./fixtures/multi-compiler-config/webpack.config'); + +describe('Hot Module Replacement (hot/hotOnly options)', () => { + let server; + let req; + + describe('simple hot config entries', () => { + beforeAll((done) => { + const options = { + port: 9000, + inline: true, + hot: true, + watchOptions: { + poll: true, + }, + }; + server = helper.startAwaitingCompilation(config, options, done); + req = request(server.app); + }); + + afterAll(helper.close); + + it('should include hot script in the bundle', (done) => { + req.get('/main.js').expect(200, /webpack\/hot\/dev-server\.js/, done); + }); + }); + + describe('simple hotOnly config entries', () => { + beforeAll((done) => { + const options = { + port: 9000, + inline: true, + hotOnly: true, + watchOptions: { + poll: true, + }, + }; + server = helper.startAwaitingCompilation(config, options, done); + req = request(server.app); + }); + + afterAll(helper.close); + + it('should include hotOnly script in the bundle', (done) => { + req + .get('/main.js') + .expect(200, /webpack\/hot\/only-dev-server\.js/, done); + }); + }); + + describe('multi compiler hot config entries', () => { + beforeAll((done) => { + const options = { + port: 9000, + inline: true, + hot: true, + watchOptions: { + poll: true, + }, + }; + server = helper.startAwaitingCompilation( + multiCompilerConfig, + options, + done + ); + req = request(server.app); + }); + + afterAll(helper.close); + + it('should include hot script in the bundle', (done) => { + req.get('/main.js').expect(200, /webpack\/hot\/dev-server\.js/, done); + }); + }); + + describe('hot disabled entries', () => { + beforeAll((done) => { + const options = { + port: 9000, + inline: true, + hot: false, + watchOptions: { + poll: true, + }, + }; + server = helper.startAwaitingCompilation(config, options, done); + req = request(server.app); + }); + + afterAll(helper.close); + + it('should NOT include hot script in the bundle', (done) => { + req + .get('/main.js') + .expect(200) + .then(({ text }) => { + expect(text).not.toMatch(/webpack\/hot\/dev-server\.js/); + done(); + }); + }); + }); + + // the following cases check to make sure that the HMR + // plugin is actually added + + describe('simple hot config HMR plugin', () => { + it('should register the HMR plugin before compilation is complete', (done) => { + let pluginFound = false; + const options = { + port: 9000, + inline: true, + hot: true, + watchOptions: { + poll: true, + }, + }; + const fullSetup = helper.startAwaitingCompilationFullSetup( + config, + options, + () => { + expect(pluginFound).toBeTruthy(); + done(); + } + ); + + const compiler = fullSetup.compiler; + compiler.hooks.compilation.intercept({ + register: (tapInfo) => { + if (tapInfo.name === 'HotModuleReplacementPlugin') { + pluginFound = true; + } + return tapInfo; + }, + }); + }); + + afterAll(helper.close); + }); + + describe('simple hotOnly config HMR plugin', () => { + it('should register the HMR plugin before compilation is complete', (done) => { + let pluginFound = false; + const options = { + port: 9000, + inline: true, + hotOnly: true, + watchOptions: { + poll: true, + }, + }; + const fullSetup = helper.startAwaitingCompilationFullSetup( + config, + options, + () => { + expect(pluginFound).toBeTruthy(); + done(); + } + ); + + const compiler = fullSetup.compiler; + compiler.hooks.compilation.intercept({ + register: (tapInfo) => { + if (tapInfo.name === 'HotModuleReplacementPlugin') { + pluginFound = true; + } + return tapInfo; + }, + }); + }); + + afterAll(helper.close); + }); + + describe('multi compiler hot config HMR plugin', () => { + it('should register the HMR plugin before compilation is complete', (done) => { + let pluginFound = false; + const options = { + port: 9000, + inline: true, + hot: true, + watchOptions: { + poll: true, + }, + }; + const fullSetup = helper.startAwaitingCompilationFullSetup( + multiCompilerConfig, + options, + () => { + expect(pluginFound).toBeTruthy(); + done(); + } + ); + + const compiler = fullSetup.compiler.compilers[0]; + compiler.hooks.compilation.intercept({ + register: (tapInfo) => { + if (tapInfo.name === 'HotModuleReplacementPlugin') { + pluginFound = true; + } + return tapInfo; + }, + }); + }); + + afterAll(helper.close); + }); + + describe('hot disabled HMR plugin', () => { + it('should NOT register the HMR plugin before compilation is complete', (done) => { + let pluginFound = false; + const options = { + port: 9000, + inline: true, + hot: false, + watchOptions: { + poll: true, + }, + }; + const fullSetup = helper.startAwaitingCompilationFullSetup( + config, + options, + () => { + expect(pluginFound).toBeFalsy(); + done(); + } + ); + + const compiler = fullSetup.compiler; + compiler.hooks.compilation.intercept({ + register: (tapInfo) => { + if (tapInfo.name === 'HotModuleReplacementPlugin') { + pluginFound = true; + } + return tapInfo; + }, + }); + }); + + afterAll(helper.close); + }); +}); diff --git a/test/Inline.test.js b/test/Inline.test.js new file mode 100644 index 0000000000..948e2710b0 --- /dev/null +++ b/test/Inline.test.js @@ -0,0 +1,90 @@ +'use strict'; + +const request = require('supertest'); +const helper = require('./helper'); +const config = require('./fixtures/client-config/webpack.config'); +const multiCompilerConfig = require('./fixtures/multi-compiler-config/webpack.config'); + +describe('inline', () => { + let server; + let req; + + describe('simple inline config entries', () => { + beforeAll((done) => { + const options = { + port: 9000, + host: '0.0.0.0', + inline: true, + watchOptions: { + poll: true, + }, + }; + server = helper.startAwaitingCompilation(config, options, done); + req = request(server.app); + }); + + afterAll(helper.close); + + it('should include inline client script in the bundle', (done) => { + req + .get('/main.js') + .expect(200, /client\/index\.js\?http:\/\/0\.0\.0\.0:9000/, done); + }); + }); + + describe('multi compiler inline config entries', () => { + beforeAll((done) => { + const options = { + port: 9000, + host: '0.0.0.0', + inline: true, + watchOptions: { + poll: true, + }, + }; + server = helper.startAwaitingCompilation( + multiCompilerConfig, + options, + done + ); + req = request(server.app); + }); + + afterAll(helper.close); + + it('should include inline client script in the bundle', (done) => { + req + .get('/main.js') + .expect(200, /client\/index\.js\?http:\/\/0\.0\.0\.0:9000/, done); + }); + }); + + describe('inline disabled entries', () => { + beforeAll((done) => { + const options = { + port: 9000, + host: '0.0.0.0', + inline: false, + watchOptions: { + poll: true, + }, + }; + server = helper.startAwaitingCompilation(config, options, done); + req = request(server.app); + }); + + afterAll(helper.close); + + it('should NOT include inline client script in the bundle', (done) => { + req + .get('/main.js') + .expect(200) + .then(({ text }) => { + expect(text).not.toMatch( + /client\/index\.js\?http:\/\/0\.0\.0\.0:9000/ + ); + done(); + }); + }); + }); +}); diff --git a/test/Lazy.test.js b/test/Lazy.test.js index 21d733ceb1..c64487a8b4 100644 --- a/test/Lazy.test.js +++ b/test/Lazy.test.js @@ -15,7 +15,7 @@ describe('Lazy', () => { }); it('with filename option should not throw an error', (done) => { - helper.start( + helper.startBeforeCompilation( config, { lazy: true, diff --git a/test/helper.js b/test/helper.js index 412652cdaa..e45144f9d3 100644 --- a/test/helper.js +++ b/test/helper.js @@ -1,5 +1,8 @@ 'use strict'; +/* eslint-disable + no-undefined +*/ const webpack = require('webpack'); const Server = require('../lib/Server'); @@ -9,10 +12,26 @@ module.exports = { // start server, returning the full setup of the server // (both the server and the compiler) startFullSetup(config, options, done) { - // eslint-disable-next-line no-undefined if (options.quiet === undefined) { options.quiet = true; } + // originally, inline was not working by default for tests with the API + // if you need to test inline, it should be set explicitly, + // rather than expecting it to be defaulted to + // (the only test that relied on inline before this point was Client.test.js) + if ( + options.inline === undefined && + options.hot === undefined && + options.hotOnly === undefined + ) { + options.inline = false; + } + // defaulting to this will hopefully help with problems on OSX in tests + if (options.watchOptions === undefined) { + options.watchOptions = { + poll: true, + }; + } const compiler = webpack(config); server = new Server(compiler, options); @@ -28,7 +47,7 @@ module.exports = { compiler, }; }, - startAwaitingCompilation(config, options, done) { + startAwaitingCompilationFullSetup(config, options, done) { let readyCount = 0; const ready = () => { readyCount += 1; @@ -41,9 +60,21 @@ module.exports = { // wait for compilation, since dev server can start before this // https://github.com/webpack/webpack-dev-server/issues/847 fullSetup.compiler.hooks.done.tap('done', ready); - return fullSetup.server; + return fullSetup; + }, + startAwaitingCompilation(config, options, done) { + return this.startAwaitingCompilationFullSetup(config, options, done).server; }, start(config, options, done) { + // I suspect that almost all tests need to wait for compilation to + // finish, because not doing so leaves open handles for jest, + // in the case where a compilation didn't finish before destroying + // the server and moving on. Thus, the default "start" should wait + // for compilation, and only special cases where you don't expect + // a compilation happen should use startBeforeCompilation + return this.startAwaitingCompilation(config, options, done); + }, + startBeforeCompilation(config, options, done) { return this.startFullSetup(config, options, done).server; }, close(done) {