From ae7d1625c3b47ff67166126b8b1804daafbc75e2 Mon Sep 17 00:00:00 2001 From: Kirill Nagaitsev Date: Thu, 11 Jul 2019 00:23:16 -0500 Subject: [PATCH 1/4] feat(server): add transport mode --- lib/Server.js | 30 +- lib/options.json | 41 +- lib/utils/getSocketClientPath.js | 10 +- lib/utils/getSocketServerImplementation.js | 12 +- test/e2e/Client.test.js | 7 +- test/e2e/ProvidePlugin.test.js | 3 +- ...ientMode.test.js => TransportMode.test.js} | 18 +- ...est.js.snap => TransportMode.test.js.snap} | 0 test/options.test.js | 50 +- test/ports-map.js | 5 +- .../serverMode-option.test.js.snap | 91 --- .../transportMode-option.test.js.snap | 112 +++ test/server/clientMode-option.test.js | 99 --- test/server/serverMode-option.test.js | 490 -------------- test/server/transportMode-option.test.js | 636 ++++++++++++++++++ test/server/utils/getSocketClientPath.test.js | 34 +- .../getSocketServerImplementation.test.js | 44 +- test/server/utils/updateCompiler.test.js | 24 +- 18 files changed, 928 insertions(+), 778 deletions(-) rename test/e2e/{ClientMode.test.js => TransportMode.test.js} (87%) rename test/e2e/__snapshots__/{ClientMode.test.js.snap => TransportMode.test.js.snap} (100%) delete mode 100644 test/server/__snapshots__/serverMode-option.test.js.snap create mode 100644 test/server/__snapshots__/transportMode-option.test.js.snap delete mode 100644 test/server/clientMode-option.test.js delete mode 100644 test/server/serverMode-option.test.js create mode 100644 test/server/transportMode-option.test.js diff --git a/lib/Server.js b/lib/Server.js index 0058ebfd47..53bf442229 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -61,15 +61,29 @@ class Server { this.log = _log || createLogger(options); - if (this.options.serverMode !== undefined) { - this.log.warn( - 'serverMode is an experimental option, meaning its usage could potentially change without warning' - ); - } + if (this.options.transportMode === undefined) { + this.options.transportMode = { + server: 'sockjs', + client: 'sockjs', + }; + } else { + switch (typeof this.options.transportMode) { + case 'string': + this.options.transportMode = { + server: this.options.transportMode, + client: this.options.transportMode, + }; + break; + // if not a string, it is an object + default: + this.options.transportMode.server = + this.options.transportMode.server || 'sockjs'; + this.options.transportMode.client = + this.options.transportMode.client || 'sockjs'; + } - if (this.options.clientMode !== undefined) { this.log.warn( - 'clientMode is an experimental option, meaning its usage could potentially change without warning' + 'transportMode is an experimental option, meaning its usage could potentially change without warning' ); } @@ -674,7 +688,7 @@ class Server { if (!headers) { this.log.warn( - 'serverMode implementation must pass headers to the callback of onConnection(f) ' + + 'transportMode.server implementation must pass headers to the callback of onConnection(f) ' + 'via f(connection, headers) in order for clients to pass a headers security check' ); } diff --git a/lib/options.json b/lib/options.json index 26dba30afc..c09c696fe8 100644 --- a/lib/options.json +++ b/lib/options.json @@ -48,9 +48,6 @@ "warning" ] }, - "clientMode": { - "type": "string" - }, "compress": { "type": "boolean" }, @@ -303,16 +300,6 @@ "serveIndex": { "type": "boolean" }, - "serverMode": { - "anyOf": [ - { - "type": "string" - }, - { - "instanceof": "Function" - } - ] - }, "serverSideRender": { "type": "boolean" }, @@ -364,6 +351,31 @@ } ] }, + "transportMode": { + "anyOf": [ + { + "type": "object", + "properties": { + "client": { + "type": "string" + }, + "server": { + "anyOf": [ + { + "type": "string" + }, + { + "instanceof": "Function" + } + ] + } + } + }, + { + "enum": ["sockjs", "ws"] + } + ] + }, "useLocalIp": { "type": "boolean" }, @@ -396,7 +408,6 @@ "ca": "should be {String|Buffer}", "cert": "should be {String|Buffer}", "clientLogLevel": "should be {String} and equal to one of the allowed values\n\n [ 'none', 'silent', 'info', 'debug', 'trace', 'error', 'warning', 'warn' ]\n\n (https://webpack.js.org/configuration/dev-server/#devserverclientloglevel)", - "clientMode": "should be {String} (https://webpack.js.org/configuration/dev-server/#devserverclientmode)", "compress": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devservercompress)", "contentBase": "should be {Number|String|Array} (https://webpack.js.org/configuration/dev-server/#devservercontentbase)", "disableHostCheck": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverdisablehostcheck)", @@ -438,7 +449,6 @@ "reporter": "should be {Function} (https://github.com/webpack/webpack-dev-middleware#reporter)", "requestCert": "should be {Boolean}", "serveIndex": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverserveindex)", - "serverMode": "should be {String|Function} (https://webpack.js.org/configuration/dev-server/#devserverservermode)", "serverSideRender": "should be {Boolean} (https://github.com/webpack/webpack-dev-middleware#serversiderender)", "setup": "should be {Function} (https://webpack.js.org/configuration/dev-server/#devserversetup)", "sockHost": "should be {String|Null} (https://webpack.js.org/configuration/dev-server/#devserversockhost)", @@ -447,6 +457,7 @@ "socket": "should be {String} (https://webpack.js.org/configuration/dev-server/#devserversocket)", "staticOptions": "should be {Object} (https://webpack.js.org/configuration/dev-server/#devserverstaticoptions)", "stats": "should be {Object|Boolean} (https://webpack.js.org/configuration/dev-server/#devserverstats-)", + "transportMode": "should be {String|Object} (https://webpack.js.org/configuration/dev-server/#devservertransportmode)", "useLocalIp": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserveruselocalip)", "warn": "should be {Function}", "watchContentBase": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverwatchcontentbase)", diff --git a/lib/utils/getSocketClientPath.js b/lib/utils/getSocketClientPath.js index 15f5de6dd5..680f9f9b96 100644 --- a/lib/utils/getSocketClientPath.js +++ b/lib/utils/getSocketClientPath.js @@ -3,17 +3,17 @@ function getSocketClientPath(options) { let ClientImplementation; let clientImplFound = true; - switch (typeof options.clientMode) { + switch (typeof options.transportMode.client) { case 'string': // could be 'sockjs', 'ws', or a path that should be required - if (options.clientMode === 'sockjs') { + if (options.transportMode.client === 'sockjs') { ClientImplementation = require('../../client/clients/SockJSClient'); - } else if (options.clientMode === 'ws') { + } else if (options.transportMode.client === 'ws') { ClientImplementation = require('../../client/clients/WebsocketClient'); } else { try { // eslint-disable-next-line import/no-dynamic-require - ClientImplementation = require(options.clientMode); + ClientImplementation = require(options.transportMode.client); } catch (e) { clientImplFound = false; } @@ -25,7 +25,7 @@ function getSocketClientPath(options) { if (!clientImplFound) { throw new Error( - "clientMode must be a string denoting a default implementation (e.g. 'sockjs', 'ws') or a full path to " + + "transportMode.client must be a string denoting a default implementation (e.g. 'sockjs', 'ws') or a full path to " + 'a JS file which exports a class extending BaseClient (webpack-dev-server/client-src/clients/BaseClient) ' + 'via require.resolve(...)' ); diff --git a/lib/utils/getSocketServerImplementation.js b/lib/utils/getSocketServerImplementation.js index aaff1843c3..4ff26ff042 100644 --- a/lib/utils/getSocketServerImplementation.js +++ b/lib/utils/getSocketServerImplementation.js @@ -3,17 +3,17 @@ function getSocketServerImplementation(options) { let ServerImplementation; let serverImplFound = true; - switch (typeof options.serverMode) { + switch (typeof options.transportMode.server) { case 'string': // could be 'sockjs', in the future 'ws', or a path that should be required - if (options.serverMode === 'sockjs') { + if (options.transportMode.server === 'sockjs') { ServerImplementation = require('../servers/SockJSServer'); - } else if (options.serverMode === 'ws') { + } else if (options.transportMode.server === 'ws') { ServerImplementation = require('../servers/WebsocketServer'); } else { try { // eslint-disable-next-line import/no-dynamic-require - ServerImplementation = require(options.serverMode); + ServerImplementation = require(options.transportMode.server); } catch (e) { serverImplFound = false; } @@ -22,7 +22,7 @@ function getSocketServerImplementation(options) { case 'function': // potentially do more checks here to confirm that the user implemented this properlly // since errors could be difficult to understand - ServerImplementation = options.serverMode; + ServerImplementation = options.transportMode.server; break; default: serverImplFound = false; @@ -30,7 +30,7 @@ function getSocketServerImplementation(options) { if (!serverImplFound) { throw new Error( - "serverMode must be a string denoting a default implementation (e.g. 'sockjs', 'ws'), a full path to " + + "transportMode.server must be a string denoting a default implementation (e.g. 'sockjs', 'ws'), a full path to " + 'a JS file which exports a class extending BaseServer (webpack-dev-server/lib/servers/BaseServer) ' + 'via require.resolve(...), or the class itself which extends BaseServer' ); diff --git a/test/e2e/Client.test.js b/test/e2e/Client.test.js index 2205f9c906..efc90dee1c 100644 --- a/test/e2e/Client.test.js +++ b/test/e2e/Client.test.js @@ -23,18 +23,17 @@ const cssFilePath = resolve(__dirname, '../fixtures/reload-config/main.css'); describe('reload', () => { const modes = [ { - title: 'hot with default clientMode (sockjs)', + title: 'hot with default transportMode.client (sockjs)', options: { hot: true, }, shouldRefresh: false, }, { - title: 'hot with clientMode ws', + title: 'hot with transportMode.client ws', options: { hot: true, - clientMode: 'ws', - serverMode: require.resolve('../../lib/servers/WebsocketServer'), + transportMode: 'ws', }, shouldRefresh: false, }, diff --git a/test/e2e/ProvidePlugin.test.js b/test/e2e/ProvidePlugin.test.js index 06cbb4cf14..228ced7f38 100644 --- a/test/e2e/ProvidePlugin.test.js +++ b/test/e2e/ProvidePlugin.test.js @@ -52,8 +52,7 @@ describe('ProvidePlugin', () => { port, host: '0.0.0.0', inline: true, - clientMode: 'ws', - serverMode: require.resolve('../../lib/servers/WebsocketServer'), + transportMode: 'ws', watchOptions: { poll: true, }, diff --git a/test/e2e/ClientMode.test.js b/test/e2e/TransportMode.test.js similarity index 87% rename from test/e2e/ClientMode.test.js rename to test/e2e/TransportMode.test.js index caaeacc941..8b9ec973c3 100644 --- a/test/e2e/ClientMode.test.js +++ b/test/e2e/TransportMode.test.js @@ -3,33 +3,35 @@ const testServer = require('../helpers/test-server'); const config = require('../fixtures/client-config/webpack.config'); const runBrowser = require('../helpers/run-browser'); -const port = require('../ports-map').ClientMode; +const port = require('../ports-map').TransportMode; const { initConsoleDelay, awaitServerCloseDelay, } = require('../helpers/puppeteer-constants'); -describe('clientMode', () => { +describe('transportMode client', () => { const modes = [ { title: 'sockjs', options: { - clientMode: 'sockjs', + transportMode: 'sockjs', }, }, { title: 'ws', options: { - clientMode: 'ws', - serverMode: require.resolve('../../lib/servers/WebsocketServer'), + transportMode: 'ws', }, }, { title: 'custom client', options: { - clientMode: require.resolve( - '../fixtures/custom-client/CustomSockJSClient' - ), + transportMode: { + server: 'sockjs', + client: require.resolve( + '../fixtures/custom-client/CustomSockJSClient' + ), + }, }, }, ]; diff --git a/test/e2e/__snapshots__/ClientMode.test.js.snap b/test/e2e/__snapshots__/TransportMode.test.js.snap similarity index 100% rename from test/e2e/__snapshots__/ClientMode.test.js.snap rename to test/e2e/__snapshots__/TransportMode.test.js.snap diff --git a/test/options.test.js b/test/options.test.js index 99fead2086..48e0f89752 100644 --- a/test/options.test.js +++ b/test/options.test.js @@ -147,10 +147,6 @@ describe('options', () => { ], failure: ['whoops!'], }, - clientMode: { - success: ['sockjs', require.resolve('../client/clients/SockJSClient')], - failure: [false], - }, compress: { success: [true], failure: [''], @@ -361,14 +357,6 @@ describe('options', () => { success: [true], failure: [''], }, - serverMode: { - success: [ - 'sockjs', - require.resolve('../lib/servers/SockJSServer'), - SockJSServer, - ], - failure: [false], - }, serverSideRender: { success: [true], failure: [''], @@ -410,6 +398,44 @@ describe('options', () => { ], failure: ['whoops!', null], }, + transportMode: { + success: [ + 'ws', + 'sockjs', + { + server: 'sockjs', + }, + { + server: require.resolve('../lib/servers/SockJSServer'), + }, + { + server: SockJSServer, + }, + { + client: 'sockjs', + }, + { + client: require.resolve('../client/clients/SockJSClient'), + }, + { + server: SockJSServer, + client: require.resolve('../client/clients/SockJSClient'), + }, + ], + failure: [ + 'nonexistent-implementation', + null, + { + notAnOption: true, + }, + { + server: false, + }, + { + client: () => {}, + }, + ], + }, useLocalIp: { success: [false], failure: [''], diff --git a/test/ports-map.js b/test/ports-map.js index 64a24cfad9..ac73aaa142 100644 --- a/test/ports-map.js +++ b/test/ports-map.js @@ -32,14 +32,13 @@ const portsList = { 'open-option': 1, 'port-option': 1, 'proxy-option': 4, - 'serverMode-option': 1, + 'transportMode-option': 1, 'sockPath-option': 1, 'stats-option': 1, ProvidePlugin: 1, WebsocketClient: 1, WebsocketServer: 1, - ClientMode: 1, - 'clientMode-option': 1, + TransportMode: 1, Progress: 1, 'progress-option': 1, 'profile-option': 1, diff --git a/test/server/__snapshots__/serverMode-option.test.js.snap b/test/server/__snapshots__/serverMode-option.test.js.snap deleted file mode 100644 index 01d175c0cc..0000000000 --- a/test/server/__snapshots__/serverMode-option.test.js.snap +++ /dev/null @@ -1,91 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`serverMode option passed to server with a bad host header results in an error 1`] = ` -Array [ - "open", - "{\\"type\\":\\"error\\",\\"data\\":\\"Invalid Host/Origin header\\"}", - "close", -] -`; - -exports[`serverMode option passed to server without a header results in an error 1`] = ` -Array [ - "open", - "{\\"type\\":\\"error\\",\\"data\\":\\"Invalid Host/Origin header\\"}", - "close", -] -`; - -exports[`serverMode option server should close client with bad headers 1`] = ` -Array [ - Array [ - [Function], - ], -] -`; - -exports[`serverMode option server should close client with bad headers 2`] = ` -Array [ - Array [ - Object { - "foo": "bar", - }, - "{\\"type\\":\\"error\\",\\"data\\":\\"Invalid Host/Origin header\\"}", - ], -] -`; - -exports[`serverMode option server should close client with bad headers 3`] = ` -Array [ - Array [ - Object { - "foo": "bar", - }, - ], -] -`; - -exports[`serverMode option server should use server implementation correctly 1`] = ` -Array [ - Object { - "foo": "bar", - }, -] -`; - -exports[`serverMode option server should use server implementation correctly 2`] = ` -Array [ - Array [ - [Function], - ], -] -`; - -exports[`serverMode option server should use server implementation correctly 3`] = ` -Array [ - Object { - "foo": "bar", - }, - "{\\"type\\":\\"liveReload\\"}", -] -`; - -exports[`serverMode option server should use server implementation correctly 4`] = ` -Array [ - Object { - "foo": "bar", - }, - "{\\"type\\":\\"ok\\"}", -] -`; - -exports[`serverMode option server should use server implementation correctly 5`] = ` -Array [ - Array [ - Object { - "foo": "bar", - }, - [Function], - ], -] -`; diff --git a/test/server/__snapshots__/transportMode-option.test.js.snap b/test/server/__snapshots__/transportMode-option.test.js.snap new file mode 100644 index 0000000000..0a07287ada --- /dev/null +++ b/test/server/__snapshots__/transportMode-option.test.js.snap @@ -0,0 +1,112 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`transportMode client is passed to getSocketClientPath correctly as a string ("sockjs") 1`] = ` +Object { + "client": "sockjs", + "server": "sockjs", +} +`; + +exports[`transportMode server is passed to getSocketServerImplementation correctly as a string ("sockjs") 1`] = ` +Object { + "client": "sockjs", + "server": "sockjs", +} +`; + +exports[`transportMode server is passed to getSocketServerImplementation correctly as a string ("ws") 1`] = ` +Object { + "client": "ws", + "server": "ws", +} +`; + +exports[`transportMode server passed to server with a bad host header results in an error 1`] = ` +Array [ + "open", + "{\\"type\\":\\"error\\",\\"data\\":\\"Invalid Host/Origin header\\"}", + "close", +] +`; + +exports[`transportMode server passed to server without a header results in an error 1`] = ` +Array [ + "open", + "{\\"type\\":\\"error\\",\\"data\\":\\"Invalid Host/Origin header\\"}", + "close", +] +`; + +exports[`transportMode server server should close client with bad headers 1`] = ` +Array [ + Array [ + [Function], + ], +] +`; + +exports[`transportMode server server should close client with bad headers 2`] = ` +Array [ + Array [ + Object { + "foo": "bar", + }, + "{\\"type\\":\\"error\\",\\"data\\":\\"Invalid Host/Origin header\\"}", + ], +] +`; + +exports[`transportMode server server should close client with bad headers 3`] = ` +Array [ + Array [ + Object { + "foo": "bar", + }, + ], +] +`; + +exports[`transportMode server server should use server implementation correctly 1`] = ` +Array [ + Object { + "foo": "bar", + }, +] +`; + +exports[`transportMode server server should use server implementation correctly 2`] = ` +Array [ + Array [ + [Function], + ], +] +`; + +exports[`transportMode server server should use server implementation correctly 3`] = ` +Array [ + Object { + "foo": "bar", + }, + "{\\"type\\":\\"liveReload\\"}", +] +`; + +exports[`transportMode server server should use server implementation correctly 4`] = ` +Array [ + Object { + "foo": "bar", + }, + "{\\"type\\":\\"ok\\"}", +] +`; + +exports[`transportMode server server should use server implementation correctly 5`] = ` +Array [ + Array [ + Object { + "foo": "bar", + }, + [Function], + ], +] +`; diff --git a/test/server/clientMode-option.test.js b/test/server/clientMode-option.test.js deleted file mode 100644 index 9c08c36a74..0000000000 --- a/test/server/clientMode-option.test.js +++ /dev/null @@ -1,99 +0,0 @@ -'use strict'; - -const config = require('../fixtures/simple-config/webpack.config'); -const port = require('../ports-map')['clientMode-option']; - -describe('clientMode option', () => { - let mockedTestServer; - let testServer; - let getSocketClientPath; - - const clientModes = [ - { - title: 'as a string ("sockjs")', - clientMode: 'sockjs', - shouldThrow: false, - }, - { - title: 'as a path ("sockjs")', - clientMode: require.resolve('../../client-src/clients/SockJSClient'), - shouldThrow: false, - }, - { - title: 'as a nonexistent path', - clientMode: '/bad/path/to/implementation', - shouldThrow: true, - }, - ]; - - describe('is passed to getSocketClientPath correctly', () => { - beforeEach(() => { - jest.mock('../../lib/utils/getSocketClientPath'); - getSocketClientPath = require('../../lib/utils/getSocketClientPath'); - }); - - afterEach((done) => { - jest.resetAllMocks(); - jest.resetModules(); - - mockedTestServer.close(done); - }); - - clientModes.forEach((data) => { - it(data.title, (done) => { - mockedTestServer = require('../helpers/test-server'); - mockedTestServer.start( - config, - { - inline: true, - clientMode: data.clientMode, - port, - }, - () => { - expect(getSocketClientPath.mock.calls.length).toEqual(1); - expect(getSocketClientPath.mock.calls[0].length).toEqual(1); - expect(getSocketClientPath.mock.calls[0][0].clientMode).toEqual( - data.clientMode - ); - done(); - } - ); - }); - }); - }); - - describe('passed to server', () => { - beforeAll(() => { - jest.unmock('../../lib/utils/getSocketClientPath'); - testServer = require('../helpers/test-server'); - }); - - afterEach((done) => { - testServer.close(done); - }); - - clientModes.forEach((data) => { - it(`${data.title} ${ - data.shouldThrow ? 'should throw' : 'should not throw' - }`, (done) => { - const res = () => { - testServer.start( - config, - { - inline: true, - clientMode: data.clientMode, - port, - }, - done - ); - }; - if (data.shouldThrow) { - expect(res).toThrow(/clientMode must be a string/); - done(); - } else { - expect(res).not.toThrow(); - } - }); - }); - }); -}); diff --git a/test/server/serverMode-option.test.js b/test/server/serverMode-option.test.js deleted file mode 100644 index c16dcb1be3..0000000000 --- a/test/server/serverMode-option.test.js +++ /dev/null @@ -1,490 +0,0 @@ -'use strict'; - -/* eslint-disable - class-methods-use-this -*/ -const request = require('supertest'); -const sockjs = require('sockjs'); -const SockJS = require('sockjs-client/dist/sockjs'); -const SockJSServer = require('../../lib/servers/SockJSServer'); -const config = require('../fixtures/simple-config/webpack.config'); -const BaseServer = require('../../lib/servers/BaseServer'); -const port = require('../ports-map')['serverMode-option']; - -describe('serverMode option', () => { - let mockedTestServer; - let testServer; - let server; - let req; - let getSocketServerImplementation; - let consoleMock; - - const serverModes = [ - { - title: 'as a string ("sockjs")', - serverMode: 'sockjs', - }, - { - title: 'as a path ("sockjs")', - serverMode: require.resolve('../../lib/servers/SockJSServer'), - }, - { - title: 'as a string ("ws")', - serverMode: 'ws', - }, - { - title: 'as a path ("ws")', - serverMode: require.resolve('../../lib/servers/WebsocketServer'), - }, - { - title: 'as a class (custom implementation)', - serverMode: class CustomServer {}, - }, - { - title: 'as a nonexistent path', - serverMode: '/bad/path/to/implementation', - }, - ]; - - beforeAll(() => { - consoleMock = jest.spyOn(console, 'log').mockImplementation(); - }); - - afterAll(() => { - consoleMock.mockRestore(); - }); - - describe('is passed to getSocketServerImplementation correctly', () => { - beforeEach(() => { - jest.mock('../../lib/utils/getSocketServerImplementation'); - getSocketServerImplementation = require('../../lib/utils/getSocketServerImplementation'); - getSocketServerImplementation.mockImplementation(() => { - return class MockServer { - // eslint-disable-next-line no-empty-function - onConnection() {} - }; - }); - }); - - afterEach((done) => { - jest.resetAllMocks(); - jest.resetModules(); - - mockedTestServer.close(done); - }); - - serverModes.forEach((data) => { - it(data.title, (done) => { - mockedTestServer = require('../helpers/test-server'); - mockedTestServer.start( - config, - { - serverMode: data.serverMode, - port, - }, - () => { - expect(getSocketServerImplementation.mock.calls.length).toEqual(1); - expect(getSocketServerImplementation.mock.calls[0].length).toEqual( - 1 - ); - expect( - getSocketServerImplementation.mock.calls[0][0].serverMode - ).toEqual(data.serverMode); - done(); - } - ); - }); - }); - }); - - describe('passed to server', () => { - beforeAll(() => { - jest.unmock('../../lib/utils/getSocketServerImplementation'); - testServer = require('../helpers/test-server'); - }); - - afterEach((done) => { - testServer.close(done); - req = null; - server = null; - }); - - describe('as a string ("sockjs")', () => { - beforeEach((done) => { - server = testServer.start( - config, - { - serverMode: 'sockjs', - port, - }, - done - ); - req = request(`http://localhost:${port}`); - }); - - it('sockjs path responds with a 200', (done) => { - req.get('/sockjs-node').expect(200, done); - }); - }); - - describe('as a path ("sockjs")', () => { - beforeEach((done) => { - server = testServer.start( - config, - { - serverMode: require.resolve('../../lib/servers/SockJSServer'), - port, - }, - done - ); - req = request(`http://localhost:${port}`); - }); - - it('sockjs path responds with a 200', (done) => { - req.get('/sockjs-node').expect(200, done); - }); - }); - - describe('as a class ("sockjs")', () => { - beforeEach((done) => { - server = testServer.start( - config, - { - serverMode: SockJSServer, - port, - }, - done - ); - req = request(`http://localhost:${port}`); - }); - - it('sockjs path responds with a 200', (done) => { - req.get('/sockjs-node').expect(200, done); - }); - }); - - describe('as a class (custom "sockjs" implementation)', () => { - let sockPath; - it('uses supplied server implementation', (done) => { - server = testServer.start( - config, - { - port, - sockPath: '/foo/test/bar/', - serverMode: class MySockJSServer extends BaseServer { - constructor(serv) { - super(serv); - this.socket = sockjs.createServer({ - // Use provided up-to-date sockjs-client - sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', - // Limit useless logs - log: (severity, line) => { - if (severity === 'error') { - this.server.log.error(line); - } else { - this.server.log.debug(line); - } - }, - }); - - this.socket.installHandlers(this.server.listeningApp, { - prefix: this.server.sockPath, - }); - - sockPath = server.options.sockPath; - } - - send(connection, message) { - connection.write(message); - } - - close(connection) { - connection.close(); - } - - onConnection(f) { - this.socket.on('connection', (connection) => { - f(connection, connection.headers); - }); - } - - onConnectionClose(connection, f) { - connection.on('close', f); - } - }, - }, - () => { - expect(sockPath).toEqual('/foo/test/bar/'); - done(); - } - ); - }); - }); - - describe('as a path with nonexistent path', () => { - it('should throw an error', () => { - expect(() => { - server = testServer.start( - config, - { - serverMode: '/bad/path/to/implementation', - port, - }, - () => {} - ); - }).toThrow(/serverMode must be a string/); - }); - }); - - describe('without a header', () => { - let mockWarn; - beforeAll((done) => { - server = testServer.start( - config, - { - port, - serverMode: class MySockJSServer extends BaseServer { - constructor(serv) { - super(serv); - this.socket = sockjs.createServer({ - // Use provided up-to-date sockjs-client - sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', - // Limit useless logs - log: (severity, line) => { - if (severity === 'error') { - this.server.log.error(line); - } else { - this.server.log.debug(line); - } - }, - }); - - this.socket.installHandlers(this.server.listeningApp, { - prefix: this.server.sockPath, - }); - } - - send(connection, message) { - connection.write(message); - } - - close(connection) { - connection.close(); - } - - onConnection(f) { - this.socket.on('connection', (connection) => { - f(connection); - }); - } - - onConnectionClose(connection, f) { - connection.on('close', f); - } - }, - }, - done - ); - - mockWarn = jest.spyOn(server.log, 'warn').mockImplementation(() => {}); - }); - - it('results in an error', (done) => { - const data = []; - const client = new SockJS(`http://localhost:${port}/sockjs-node`); - - client.onopen = () => { - data.push('open'); - }; - - client.onmessage = (e) => { - data.push(e.data); - }; - - client.onclose = () => { - data.push('close'); - }; - - setTimeout(() => { - expect(data).toMatchSnapshot(); - const calls = mockWarn.mock.calls; - mockWarn.mockRestore(); - - let foundWarning = false; - const regExp = /serverMode implementation must pass headers to the callback of onConnection\(f\)/; - calls.every((call) => { - if (regExp.test(call)) { - foundWarning = true; - return false; - } - return true; - }); - - expect(foundWarning).toBeTruthy(); - - done(); - }, 5000); - }); - }); - - describe('with a bad host header', () => { - beforeAll((done) => { - server = testServer.start( - config, - { - port, - serverMode: class MySockJSServer extends BaseServer { - constructor(serv) { - super(serv); - this.socket = sockjs.createServer({ - // Use provided up-to-date sockjs-client - sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', - // Limit useless logs - log: (severity, line) => { - if (severity === 'error') { - this.server.log.error(line); - } else { - this.server.log.debug(line); - } - }, - }); - - this.socket.installHandlers(this.server.listeningApp, { - prefix: this.server.sockPath, - }); - } - - send(connection, message) { - connection.write(message); - } - - close(connection) { - connection.close(); - } - - onConnection(f) { - this.socket.on('connection', (connection) => { - f(connection, { - host: null, - }); - }); - } - - onConnectionClose(connection, f) { - connection.on('close', f); - } - }, - }, - done - ); - }); - - it('results in an error', (done) => { - const data = []; - const client = new SockJS(`http://localhost:${port}/sockjs-node`); - - client.onopen = () => { - data.push('open'); - }; - - client.onmessage = (e) => { - data.push(e.data); - }; - - client.onclose = () => { - data.push('close'); - }; - - setTimeout(() => { - expect(data).toMatchSnapshot(); - done(); - }, 5000); - }); - }); - }); - - describe('server', () => { - let MockSockJSServer; - beforeEach((done) => { - jest.mock('../../lib/servers/SockJSServer'); - mockedTestServer = require('../helpers/test-server'); - MockSockJSServer = require('../../lib/servers/SockJSServer'); - - server = mockedTestServer.start( - config, - { - port, - }, - done - ); - }); - - afterEach((done) => { - mockedTestServer.close(done); - jest.resetAllMocks(); - jest.resetModules(); - - server = null; - }); - - it('should use server implementation correctly', () => { - const mockServerInstance = MockSockJSServer.mock.instances[0]; - - const connectionObj = { - foo: 'bar', - }; - // this simulates a client connecting to the server - mockServerInstance.onConnection.mock.calls[0][0](connectionObj, { - host: `localhost:${port}`, - origin: `http://localhost:${port}`, - }); - - expect(server.sockets.length).toEqual(1); - expect(server.sockets).toMatchSnapshot(); - - // this simulates a client leaving the server - mockServerInstance.onConnectionClose.mock.calls[0][1](connectionObj); - - expect(server.sockets.length).toEqual(0); - - // check that the dev server was passed to the socket server implementation constructor - expect(MockSockJSServer.mock.calls[0].length).toEqual(1); - expect(MockSockJSServer.mock.calls[0][0].options.port).toEqual(port); - - expect(mockServerInstance.onConnection.mock.calls).toMatchSnapshot(); - expect(mockServerInstance.send.mock.calls.length).toEqual(3); - // call 0 to the send() method is liveReload - expect(mockServerInstance.send.mock.calls[0]).toMatchSnapshot(); - // call 1 to the send() method is hash data, so we skip it - // call 2 to the send() method is the "ok" message - expect(mockServerInstance.send.mock.calls[2]).toMatchSnapshot(); - // close should not be called because the server never forcefully closes - // a successful client connection - expect(mockServerInstance.close.mock.calls.length).toEqual(0); - expect(mockServerInstance.onConnectionClose.mock.calls).toMatchSnapshot(); - }); - - it('should close client with bad headers', () => { - const mockServerInstance = MockSockJSServer.mock.instances[0]; - - // this simulates a client connecting to the server - mockServerInstance.onConnection.mock.calls[0][0]( - { - foo: 'bar', - }, - { - host: null, - } - ); - expect(server.sockets.length).toEqual(0); - expect(MockSockJSServer.mock.calls[0].length).toEqual(1); - expect(MockSockJSServer.mock.calls[0][0].options.port).toEqual(port); - expect(mockServerInstance.onConnection.mock.calls).toMatchSnapshot(); - // the only call to send() here should be an invalid header message - expect(mockServerInstance.send.mock.calls).toMatchSnapshot(); - expect(mockServerInstance.close.mock.calls).toMatchSnapshot(); - // onConnectionClose should never get called since the client should be closed first - expect(mockServerInstance.onConnectionClose.mock.calls.length).toEqual(0); - }); - }); -}); diff --git a/test/server/transportMode-option.test.js b/test/server/transportMode-option.test.js new file mode 100644 index 0000000000..d4a54ffc12 --- /dev/null +++ b/test/server/transportMode-option.test.js @@ -0,0 +1,636 @@ +'use strict'; + +/* eslint-disable + class-methods-use-this +*/ +const request = require('supertest'); +const sockjs = require('sockjs'); +const SockJS = require('sockjs-client/dist/sockjs'); +const SockJSServer = require('../../lib/servers/SockJSServer'); +const config = require('../fixtures/simple-config/webpack.config'); +const BaseServer = require('../../lib/servers/BaseServer'); +const port = require('../ports-map')['transportMode-option']; + +describe('transportMode', () => { + describe('server', () => { + let mockedTestServer; + let testServer; + let server; + let req; + let getSocketServerImplementation; + let consoleMock; + + const serverModes = [ + { + title: 'as a string ("sockjs")', + transportMode: 'sockjs', + }, + { + title: 'as a path ("sockjs")', + transportMode: { + server: require.resolve('../../lib/servers/SockJSServer'), + }, + }, + { + title: 'as a string ("ws")', + transportMode: 'ws', + }, + { + title: 'as a path ("ws")', + transportMode: { + server: require.resolve('../../lib/servers/WebsocketServer'), + }, + }, + { + title: 'as a class (custom implementation)', + transportMode: { + server: class CustomServer {}, + }, + }, + { + title: 'as a nonexistent path', + transportMode: { + server: '/bad/path/to/implementation', + }, + }, + ]; + + beforeAll(() => { + consoleMock = jest.spyOn(console, 'log').mockImplementation(); + }); + + afterAll(() => { + consoleMock.mockRestore(); + }); + + describe('is passed to getSocketServerImplementation correctly', () => { + beforeEach(() => { + jest.mock('../../lib/utils/getSocketServerImplementation'); + getSocketServerImplementation = require('../../lib/utils/getSocketServerImplementation'); + getSocketServerImplementation.mockImplementation(() => { + return class MockServer { + // eslint-disable-next-line no-empty-function + onConnection() {} + }; + }); + }); + + afterEach((done) => { + jest.resetAllMocks(); + jest.resetModules(); + + mockedTestServer.close(done); + }); + + serverModes.forEach((data) => { + it(data.title, (done) => { + mockedTestServer = require('../helpers/test-server'); + mockedTestServer.start( + config, + { + transportMode: data.transportMode, + port, + }, + () => { + expect(getSocketServerImplementation.mock.calls.length).toEqual( + 1 + ); + expect( + getSocketServerImplementation.mock.calls[0].length + ).toEqual(1); + + if (typeof data.transportMode === 'string') { + expect( + getSocketServerImplementation.mock.calls[0][0].transportMode + ).toMatchSnapshot(); + } else { + expect( + getSocketServerImplementation.mock.calls[0][0].transportMode + ).toEqual(data.transportMode); + } + + done(); + } + ); + }); + }); + }); + + describe('passed to server', () => { + beforeAll(() => { + jest.unmock('../../lib/utils/getSocketServerImplementation'); + testServer = require('../helpers/test-server'); + }); + + afterEach((done) => { + testServer.close(done); + req = null; + server = null; + }); + + describe('as a string ("sockjs")', () => { + beforeEach((done) => { + server = testServer.start( + config, + { + transportMode: { + server: 'sockjs', + }, + port, + }, + done + ); + req = request(`http://localhost:${port}`); + }); + + it('sockjs path responds with a 200', (done) => { + req.get('/sockjs-node').expect(200, done); + }); + }); + + describe('as a path ("sockjs")', () => { + beforeEach((done) => { + server = testServer.start( + config, + { + transportMode: { + server: require.resolve('../../lib/servers/SockJSServer'), + }, + port, + }, + done + ); + req = request(`http://localhost:${port}`); + }); + + it('sockjs path responds with a 200', (done) => { + req.get('/sockjs-node').expect(200, done); + }); + }); + + describe('as a class ("sockjs")', () => { + beforeEach((done) => { + server = testServer.start( + config, + { + transportMode: { + server: SockJSServer, + }, + port, + }, + done + ); + req = request(`http://localhost:${port}`); + }); + + it('sockjs path responds with a 200', (done) => { + req.get('/sockjs-node').expect(200, done); + }); + }); + + describe('as a class (custom "sockjs" implementation)', () => { + let sockPath; + it('uses supplied server implementation', (done) => { + server = testServer.start( + config, + { + port, + sockPath: '/foo/test/bar/', + transportMode: { + server: class MySockJSServer extends BaseServer { + constructor(serv) { + super(serv); + this.socket = sockjs.createServer({ + // Use provided up-to-date sockjs-client + sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', + // Limit useless logs + log: (severity, line) => { + if (severity === 'error') { + this.server.log.error(line); + } else { + this.server.log.debug(line); + } + }, + }); + + this.socket.installHandlers(this.server.listeningApp, { + prefix: this.server.sockPath, + }); + + sockPath = server.options.sockPath; + } + + send(connection, message) { + connection.write(message); + } + + close(connection) { + connection.close(); + } + + onConnection(f) { + this.socket.on('connection', (connection) => { + f(connection, connection.headers); + }); + } + + onConnectionClose(connection, f) { + connection.on('close', f); + } + }, + }, + }, + () => { + expect(sockPath).toEqual('/foo/test/bar/'); + done(); + } + ); + }); + }); + + describe('as a path with nonexistent path', () => { + it('should throw an error', () => { + expect(() => { + server = testServer.start( + config, + { + transportMode: { + server: '/bad/path/to/implementation', + }, + port, + }, + () => {} + ); + }).toThrow(/transportMode\.server must be a string/); + }); + }); + + describe('without a header', () => { + let mockWarn; + beforeAll((done) => { + server = testServer.start( + config, + { + port, + transportMode: { + server: class MySockJSServer extends BaseServer { + constructor(serv) { + super(serv); + this.socket = sockjs.createServer({ + // Use provided up-to-date sockjs-client + sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', + // Limit useless logs + log: (severity, line) => { + if (severity === 'error') { + this.server.log.error(line); + } else { + this.server.log.debug(line); + } + }, + }); + + this.socket.installHandlers(this.server.listeningApp, { + prefix: this.server.sockPath, + }); + } + + send(connection, message) { + connection.write(message); + } + + close(connection) { + connection.close(); + } + + onConnection(f) { + this.socket.on('connection', (connection) => { + f(connection); + }); + } + + onConnectionClose(connection, f) { + connection.on('close', f); + } + }, + }, + }, + done + ); + + mockWarn = jest + .spyOn(server.log, 'warn') + .mockImplementation(() => {}); + }); + + it('results in an error', (done) => { + const data = []; + const client = new SockJS(`http://localhost:${port}/sockjs-node`); + + client.onopen = () => { + data.push('open'); + }; + + client.onmessage = (e) => { + data.push(e.data); + }; + + client.onclose = () => { + data.push('close'); + }; + + setTimeout(() => { + expect(data).toMatchSnapshot(); + const calls = mockWarn.mock.calls; + mockWarn.mockRestore(); + + let foundWarning = false; + const regExp = /transportMode\.server implementation must pass headers to the callback of onConnection\(f\)/; + calls.every((call) => { + if (regExp.test(call)) { + foundWarning = true; + return false; + } + return true; + }); + + expect(foundWarning).toBeTruthy(); + + done(); + }, 5000); + }); + }); + + describe('with a bad host header', () => { + beforeAll((done) => { + server = testServer.start( + config, + { + port, + transportMode: { + server: class MySockJSServer extends BaseServer { + constructor(serv) { + super(serv); + this.socket = sockjs.createServer({ + // Use provided up-to-date sockjs-client + sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js', + // Limit useless logs + log: (severity, line) => { + if (severity === 'error') { + this.server.log.error(line); + } else { + this.server.log.debug(line); + } + }, + }); + + this.socket.installHandlers(this.server.listeningApp, { + prefix: this.server.sockPath, + }); + } + + send(connection, message) { + connection.write(message); + } + + close(connection) { + connection.close(); + } + + onConnection(f) { + this.socket.on('connection', (connection) => { + f(connection, { + host: null, + }); + }); + } + + onConnectionClose(connection, f) { + connection.on('close', f); + } + }, + }, + }, + done + ); + }); + + it('results in an error', (done) => { + const data = []; + const client = new SockJS(`http://localhost:${port}/sockjs-node`); + + client.onopen = () => { + data.push('open'); + }; + + client.onmessage = (e) => { + data.push(e.data); + }; + + client.onclose = () => { + data.push('close'); + }; + + setTimeout(() => { + expect(data).toMatchSnapshot(); + done(); + }, 5000); + }); + }); + }); + + describe('server', () => { + let MockSockJSServer; + beforeEach((done) => { + jest.mock('../../lib/servers/SockJSServer'); + mockedTestServer = require('../helpers/test-server'); + MockSockJSServer = require('../../lib/servers/SockJSServer'); + + server = mockedTestServer.start( + config, + { + port, + }, + done + ); + }); + + afterEach((done) => { + mockedTestServer.close(done); + jest.resetAllMocks(); + jest.resetModules(); + + server = null; + }); + + it('should use server implementation correctly', () => { + const mockServerInstance = MockSockJSServer.mock.instances[0]; + + const connectionObj = { + foo: 'bar', + }; + // this simulates a client connecting to the server + mockServerInstance.onConnection.mock.calls[0][0](connectionObj, { + host: `localhost:${port}`, + origin: `http://localhost:${port}`, + }); + + expect(server.sockets.length).toEqual(1); + expect(server.sockets).toMatchSnapshot(); + + // this simulates a client leaving the server + mockServerInstance.onConnectionClose.mock.calls[0][1](connectionObj); + + expect(server.sockets.length).toEqual(0); + + // check that the dev server was passed to the socket server implementation constructor + expect(MockSockJSServer.mock.calls[0].length).toEqual(1); + expect(MockSockJSServer.mock.calls[0][0].options.port).toEqual(port); + + expect(mockServerInstance.onConnection.mock.calls).toMatchSnapshot(); + expect(mockServerInstance.send.mock.calls.length).toEqual(3); + // call 0 to the send() method is liveReload + expect(mockServerInstance.send.mock.calls[0]).toMatchSnapshot(); + // call 1 to the send() method is hash data, so we skip it + // call 2 to the send() method is the "ok" message + expect(mockServerInstance.send.mock.calls[2]).toMatchSnapshot(); + // close should not be called because the server never forcefully closes + // a successful client connection + expect(mockServerInstance.close.mock.calls.length).toEqual(0); + expect( + mockServerInstance.onConnectionClose.mock.calls + ).toMatchSnapshot(); + }); + + it('should close client with bad headers', () => { + const mockServerInstance = MockSockJSServer.mock.instances[0]; + + // this simulates a client connecting to the server + mockServerInstance.onConnection.mock.calls[0][0]( + { + foo: 'bar', + }, + { + host: null, + } + ); + expect(server.sockets.length).toEqual(0); + expect(MockSockJSServer.mock.calls[0].length).toEqual(1); + expect(MockSockJSServer.mock.calls[0][0].options.port).toEqual(port); + expect(mockServerInstance.onConnection.mock.calls).toMatchSnapshot(); + // the only call to send() here should be an invalid header message + expect(mockServerInstance.send.mock.calls).toMatchSnapshot(); + expect(mockServerInstance.close.mock.calls).toMatchSnapshot(); + // onConnectionClose should never get called since the client should be closed first + expect(mockServerInstance.onConnectionClose.mock.calls.length).toEqual( + 0 + ); + }); + }); + }); + + describe('client', () => { + let mockedTestServer; + let testServer; + let getSocketClientPath; + + const clientModes = [ + { + title: 'as a string ("sockjs")', + transportMode: 'sockjs', + shouldThrow: false, + }, + { + title: 'as a path ("sockjs")', + transportMode: { + client: require.resolve('../../client-src/clients/SockJSClient'), + }, + shouldThrow: false, + }, + { + title: 'as a nonexistent path', + transportMode: { + client: '/bad/path/to/implementation', + }, + shouldThrow: true, + }, + ]; + + describe('is passed to getSocketClientPath correctly', () => { + beforeEach(() => { + jest.mock('../../lib/utils/getSocketClientPath'); + getSocketClientPath = require('../../lib/utils/getSocketClientPath'); + }); + + afterEach((done) => { + jest.resetAllMocks(); + jest.resetModules(); + + mockedTestServer.close(done); + }); + + clientModes.forEach((data) => { + it(data.title, (done) => { + mockedTestServer = require('../helpers/test-server'); + mockedTestServer.start( + config, + { + inline: true, + transportMode: data.transportMode, + port, + }, + () => { + expect(getSocketClientPath.mock.calls.length).toEqual(1); + expect(getSocketClientPath.mock.calls[0].length).toEqual(1); + if (typeof data.transportMode === 'string') { + expect( + getSocketClientPath.mock.calls[0][0].transportMode + ).toMatchSnapshot(); + } else { + expect( + getSocketClientPath.mock.calls[0][0].transportMode + ).toEqual(data.transportMode); + } + + done(); + } + ); + }); + }); + }); + + describe('passed to server', () => { + beforeAll(() => { + jest.unmock('../../lib/utils/getSocketClientPath'); + testServer = require('../helpers/test-server'); + }); + + afterEach((done) => { + testServer.close(done); + }); + + clientModes.forEach((data) => { + it(`${data.title} ${ + data.shouldThrow ? 'should throw' : 'should not throw' + }`, (done) => { + const res = () => { + testServer.start( + config, + { + inline: true, + transportMode: data.transportMode, + port, + }, + done + ); + }; + if (data.shouldThrow) { + expect(res).toThrow(/transportMode\.client must be a string/); + done(); + } else { + expect(res).not.toThrow(); + } + }); + }); + }); + }); +}); diff --git a/test/server/utils/getSocketClientPath.test.js b/test/server/utils/getSocketClientPath.test.js index a823f966db..162461ca7a 100644 --- a/test/server/utils/getSocketClientPath.test.js +++ b/test/server/utils/getSocketClientPath.test.js @@ -8,50 +8,60 @@ const sockjsClientPath = require.resolve( const baseClientPath = require.resolve('../../../client/clients/BaseClient'); describe('getSocketClientPath', () => { - it("should work with clientMode: 'sockjs'", () => { + it("should work with transportMode.client: 'sockjs'", () => { let result; expect(() => { result = getSocketClientPath({ - clientMode: 'sockjs', + transportMode: { + client: 'sockjs', + }, }); }).not.toThrow(); expect(result).toEqual(sockjsClientPath); }); - it('should work with clientMode: SockJSClient full path', () => { + it('should work with transportMode.client: SockJSClient full path', () => { let result; expect(() => { result = getSocketClientPath({ - clientMode: sockjsClientPath, + transportMode: { + client: sockjsClientPath, + }, }); }).not.toThrow(); expect(result).toEqual(sockjsClientPath); }); - it('should throw with clientMode: bad path', () => { + it('should throw with transportMode.client: bad path', () => { expect(() => { getSocketClientPath({ - clientMode: '/bad/path/to/implementation', + transportMode: { + client: '/bad/path/to/implementation', + }, }); - }).toThrow(/clientMode must be a string/); + }).toThrow(/transportMode\.client must be a string/); }); - it('should throw with clientMode: bad type', () => { + it('should throw with transportMode.client: bad type', () => { expect(() => { getSocketClientPath({ - clientMode: 1, + transportMode: { + client: 1, + }, }); - }).toThrow(/clientMode must be a string/); + }).toThrow(/transportMode\.client must be a string/); }); - it('should throw with clientMode: unimplemented client', () => { + it('should throw with transportMode.client: unimplemented client', () => { expect(() => { getSocketClientPath({ - clientMode: baseClientPath, + transportMode: { + client: baseClientPath, + }, }); }).toThrow('Client needs implementation'); }); diff --git a/test/server/utils/getSocketServerImplementation.test.js b/test/server/utils/getSocketServerImplementation.test.js index 1898b8addf..89174b55b5 100644 --- a/test/server/utils/getSocketServerImplementation.test.js +++ b/test/server/utils/getSocketServerImplementation.test.js @@ -5,83 +5,97 @@ const SockJSServer = require('../../../lib/servers/SockJSServer'); const WebsocketServer = require('../../../lib/servers/WebsocketServer'); describe('getSocketServerImplementation util', () => { - it("should work with string serverMode ('sockjs')", () => { + it("should work with string transportMode.server ('sockjs')", () => { let result; expect(() => { result = getSocketServerImplementation({ - serverMode: 'sockjs', + transportMode: { + server: 'sockjs', + }, }); }).not.toThrow(); expect(result).toEqual(SockJSServer); }); - it('should work with serverMode (SockJSServer class)', () => { + it('should work with transportMode.server (SockJSServer class)', () => { let result; expect(() => { result = getSocketServerImplementation({ - serverMode: SockJSServer, + transportMode: { + server: SockJSServer, + }, }); }).not.toThrow(); expect(result).toEqual(SockJSServer); }); - it('should work with serverMode (SockJSServer full path)', () => { + it('should work with transportMode.server (SockJSServer full path)', () => { let result; expect(() => { result = getSocketServerImplementation({ - serverMode: require.resolve('../../../lib/servers/SockJSServer'), + transportMode: { + server: require.resolve('../../../lib/servers/SockJSServer'), + }, }); }).not.toThrow(); expect(result).toEqual(SockJSServer); }); - it("should work with string serverMode ('ws')", () => { + it("should work with string transportMode.server ('ws')", () => { let result; expect(() => { result = getSocketServerImplementation({ - serverMode: 'ws', + transportMode: { + server: 'ws', + }, }); }).not.toThrow(); expect(result).toEqual(WebsocketServer); }); - it('should work with serverMode (WebsocketServer class)', () => { + it('should work with transportMode.server (WebsocketServer class)', () => { let result; expect(() => { result = getSocketServerImplementation({ - serverMode: WebsocketServer, + transportMode: { + server: WebsocketServer, + }, }); }).not.toThrow(); expect(result).toEqual(WebsocketServer); }); - it('should work with serverMode (WebsocketServer full path)', () => { + it('should work with transportMode.server (WebsocketServer full path)', () => { let result; expect(() => { result = getSocketServerImplementation({ - serverMode: require.resolve('../../../lib/servers/WebsocketServer'), + transportMode: { + server: require.resolve('../../../lib/servers/WebsocketServer'), + }, }); }).not.toThrow(); expect(result).toEqual(WebsocketServer); }); - it('should throw with serverMode (bad path)', () => { + it('should throw with transportMode.server (bad path)', () => { expect(() => { getSocketServerImplementation({ - serverMode: '/bad/path/to/implementation', + transportMode: { + server: '/bad/path/to/implementation', + }, }); - }).toThrow(/serverMode must be a string/); + }).toThrow(/transportMode.server must be a string/); }); }); diff --git a/test/server/utils/updateCompiler.test.js b/test/server/utils/updateCompiler.test.js index 40c21ddecb..796bc543d4 100644 --- a/test/server/utils/updateCompiler.test.js +++ b/test/server/utils/updateCompiler.test.js @@ -15,8 +15,10 @@ describe('updateCompiler', () => { const spy = jest.spyOn(compiler.hooks.entryOption, 'call'); updateCompiler(compiler, { - serverMode: 'sockjs', - clientMode: 'sockjs', + transportMode: { + server: 'sockjs', + client: 'sockjs', + }, inline: true, }); @@ -51,8 +53,10 @@ describe('updateCompiler', () => { const spy = jest.spyOn(compiler.hooks.entryOption, 'call'); updateCompiler(compiler, { - serverMode: 'sockjs', - clientMode: 'sockjs', + transportMode: { + server: 'sockjs', + client: 'sockjs', + }, inline: true, hot: true, }); @@ -88,8 +92,10 @@ describe('updateCompiler', () => { const spy = jest.spyOn(compiler.hooks.entryOption, 'call'); updateCompiler(compiler, { - serverMode: 'sockjs', - clientMode: 'sockjs', + transportMode: { + server: 'sockjs', + client: 'sockjs', + }, inline: true, hot: true, }); @@ -128,8 +134,10 @@ describe('updateCompiler', () => { }); updateCompiler(multiCompiler, { - serverMode: 'sockjs', - clientMode: 'sockjs', + transportMode: { + server: 'sockjs', + client: 'sockjs', + }, inline: true, hot: true, }); From d9afb645526ad44da9d9994ff42c9c4ae7fe4c49 Mon Sep 17 00:00:00 2001 From: Kirill Nagaitsev Date: Thu, 11 Jul 2019 08:59:02 -0500 Subject: [PATCH 2/4] fix(transport): update transport e2e test and rename tests --- test/e2e/ProvidePlugin.test.js | 4 ++-- test/e2e/__snapshots__/TransportMode.test.js.snap | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/e2e/ProvidePlugin.test.js b/test/e2e/ProvidePlugin.test.js index 228ced7f38..6f08d065be 100644 --- a/test/e2e/ProvidePlugin.test.js +++ b/test/e2e/ProvidePlugin.test.js @@ -8,7 +8,7 @@ const port = require('../ports-map').ProvidePlugin; const { beforeBrowserCloseDelay } = require('../helpers/puppeteer-constants'); describe('ProvidePlugin', () => { - describe('inline with default clientMode (sockjs)', () => { + describe('inline with default transportMode.client (sockjs)', () => { beforeAll((done) => { const options = { port, @@ -46,7 +46,7 @@ describe('ProvidePlugin', () => { }); }); - describe('inline with clientMode ws', () => { + describe('inline with transportMode.client ws', () => { beforeAll((done) => { const options = { port, diff --git a/test/e2e/__snapshots__/TransportMode.test.js.snap b/test/e2e/__snapshots__/TransportMode.test.js.snap index 1e49239690..b3c92fa7fd 100644 --- a/test/e2e/__snapshots__/TransportMode.test.js.snap +++ b/test/e2e/__snapshots__/TransportMode.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`clientMode custom client on browser client logs correctly 1`] = ` +exports[`transportMode client custom client on browser client logs correctly 1`] = ` Array [ "Hey.", "open", @@ -13,7 +13,7 @@ Array [ ] `; -exports[`clientMode sockjs on browser client logs correctly 1`] = ` +exports[`transportMode client sockjs on browser client logs correctly 1`] = ` Array [ "Hey.", "[WDS] Live Reloading enabled.", @@ -21,7 +21,7 @@ Array [ ] `; -exports[`clientMode ws on browser client logs correctly 1`] = ` +exports[`transportMode client ws on browser client logs correctly 1`] = ` Array [ "Hey.", "[WDS] Live Reloading enabled.", From 1b3e78d13fd5a086c6e12d5e8d5da40abfbc9f78 Mon Sep 17 00:00:00 2001 From: Kirill Nagaitsev Date: Thu, 11 Jul 2019 16:55:10 -0500 Subject: [PATCH 3/4] test(options): fixed options test --- lib/options.json | 3 ++- test/options.test.js | 38 ++++++++++++++++++++++++++++---------- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/lib/options.json b/lib/options.json index c09c696fe8..217ee5a631 100644 --- a/lib/options.json +++ b/lib/options.json @@ -369,7 +369,8 @@ } ] } - } + }, + "additionalProperties": false }, { "enum": ["sockjs", "ws"] diff --git a/test/options.test.js b/test/options.test.js index 48e0f89752..409dc07bec 100644 --- a/test/options.test.js +++ b/test/options.test.js @@ -403,36 +403,54 @@ describe('options', () => { 'ws', 'sockjs', { - server: 'sockjs', + transportMode: { + server: 'sockjs', + }, }, { - server: require.resolve('../lib/servers/SockJSServer'), + transportMode: { + server: require.resolve('../lib/servers/SockJSServer'), + }, }, { - server: SockJSServer, + transportMode: { + server: SockJSServer, + }, }, { - client: 'sockjs', + transportMode: { + client: 'sockjs', + }, }, { - client: require.resolve('../client/clients/SockJSClient'), + transportMode: { + client: require.resolve('../client/clients/SockJSClient'), + }, }, { - server: SockJSServer, - client: require.resolve('../client/clients/SockJSClient'), + transportMode: { + server: SockJSServer, + client: require.resolve('../client/clients/SockJSClient'), + }, }, ], failure: [ 'nonexistent-implementation', null, { - notAnOption: true, + transportMode: { + notAnOption: true, + }, }, { - server: false, + transportMode: { + server: false, + }, }, { - client: () => {}, + transportMode: { + client: () => {}, + }, }, ], }, From c648678c15bdb0673cd0ea053c8c01fc1eff4e65 Mon Sep 17 00:00:00 2001 From: Kirill Nagaitsev Date: Tue, 23 Jul 2019 15:44:02 -0500 Subject: [PATCH 4/4] refactor(server): move transport mode default setting into helper --- lib/Server.js | 22 +---- lib/utils/normalizeOptions.js | 27 ++++-- .../normalizeOptions.test.js.snap | 84 +++++++++++++++++-- test/server/utils/normalizeOptions.test.js | 57 +++++++++++++ 4 files changed, 153 insertions(+), 37 deletions(-) diff --git a/lib/Server.js b/lib/Server.js index 53bf442229..bd19774ff0 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -61,27 +61,7 @@ class Server { this.log = _log || createLogger(options); - if (this.options.transportMode === undefined) { - this.options.transportMode = { - server: 'sockjs', - client: 'sockjs', - }; - } else { - switch (typeof this.options.transportMode) { - case 'string': - this.options.transportMode = { - server: this.options.transportMode, - client: this.options.transportMode, - }; - break; - // if not a string, it is an object - default: - this.options.transportMode.server = - this.options.transportMode.server || 'sockjs'; - this.options.transportMode.client = - this.options.transportMode.client || 'sockjs'; - } - + if (this.options.transportMode !== undefined) { this.log.warn( 'transportMode is an experimental option, meaning its usage could potentially change without warning' ); diff --git a/lib/utils/normalizeOptions.js b/lib/utils/normalizeOptions.js index 443d2df94a..885f365687 100644 --- a/lib/utils/normalizeOptions.js +++ b/lib/utils/normalizeOptions.js @@ -9,14 +9,25 @@ function normalizeOptions(compiler, options) { options.contentBase = options.contentBase !== undefined ? options.contentBase : process.cwd(); - // set serverMode default - if (options.serverMode === undefined) { - options.serverMode = 'sockjs'; - } - - // set clientMode default - if (options.clientMode === undefined) { - options.clientMode = 'sockjs'; + // normalize transportMode option + if (options.transportMode === undefined) { + options.transportMode = { + server: 'sockjs', + client: 'sockjs', + }; + } else { + switch (typeof options.transportMode) { + case 'string': + options.transportMode = { + server: options.transportMode, + client: options.transportMode, + }; + break; + // if not a string, it is an object + default: + options.transportMode.server = options.transportMode.server || 'sockjs'; + options.transportMode.client = options.transportMode.client || 'sockjs'; + } } if (!options.watchOptions) { diff --git a/test/server/utils/__snapshots__/normalizeOptions.test.js.snap b/test/server/utils/__snapshots__/normalizeOptions.test.js.snap index 0658e08b4b..5741d85d9a 100644 --- a/test/server/utils/__snapshots__/normalizeOptions.test.js.snap +++ b/test/server/utils/__snapshots__/normalizeOptions.test.js.snap @@ -2,37 +2,105 @@ exports[`normalizeOptions contentBase array should set correct options 1`] = ` Object { - "clientMode": "sockjs", "contentBase": Array [ "/path/to/dist1", "/path/to/dist2", ], - "serverMode": "sockjs", + "transportMode": Object { + "client": "sockjs", + "server": "sockjs", + }, "watchOptions": Object {}, } `; exports[`normalizeOptions contentBase string should set correct options 1`] = ` Object { - "clientMode": "sockjs", "contentBase": "/path/to/dist", - "serverMode": "sockjs", + "transportMode": Object { + "client": "sockjs", + "server": "sockjs", + }, "watchOptions": Object {}, } `; exports[`normalizeOptions no options should set correct options 1`] = ` Object { - "clientMode": "sockjs", - "serverMode": "sockjs", + "transportMode": Object { + "client": "sockjs", + "server": "sockjs", + }, + "watchOptions": Object {}, +} +`; + +exports[`normalizeOptions transportMode custom client path should set correct options 1`] = ` +Object { + "transportMode": Object { + "client": "/path/to/custom/client/", + "server": "sockjs", + }, + "watchOptions": Object {}, +} +`; + +exports[`normalizeOptions transportMode custom server class should set correct options 1`] = ` +Object { + "transportMode": Object { + "client": "sockjs", + "server": [Function], + }, + "watchOptions": Object {}, +} +`; + +exports[`normalizeOptions transportMode custom server path should set correct options 1`] = ` +Object { + "transportMode": Object { + "client": "sockjs", + "server": "/path/to/custom/server/", + }, + "watchOptions": Object {}, +} +`; + +exports[`normalizeOptions transportMode sockjs string should set correct options 1`] = ` +Object { + "transportMode": Object { + "client": "sockjs", + "server": "sockjs", + }, + "watchOptions": Object {}, +} +`; + +exports[`normalizeOptions transportMode ws object should set correct options 1`] = ` +Object { + "transportMode": Object { + "client": "ws", + "server": "ws", + }, + "watchOptions": Object {}, +} +`; + +exports[`normalizeOptions transportMode ws string should set correct options 1`] = ` +Object { + "transportMode": Object { + "client": "ws", + "server": "ws", + }, "watchOptions": Object {}, } `; exports[`normalizeOptions watchOptions should set correct options 1`] = ` Object { - "clientMode": "sockjs", - "serverMode": "sockjs", + "transportMode": Object { + "client": "sockjs", + "server": "sockjs", + }, "watchOptions": Object { "poll": true, }, diff --git a/test/server/utils/normalizeOptions.test.js b/test/server/utils/normalizeOptions.test.js index c1f66f4939..1490b36088 100644 --- a/test/server/utils/normalizeOptions.test.js +++ b/test/server/utils/normalizeOptions.test.js @@ -37,6 +37,63 @@ describe('normalizeOptions', () => { }, optionsResults: null, }, + { + title: 'transportMode sockjs string', + multiCompiler: false, + options: { + transportMode: 'sockjs', + }, + optionsResults: null, + }, + { + title: 'transportMode ws string', + multiCompiler: false, + options: { + transportMode: 'ws', + }, + optionsResults: null, + }, + { + title: 'transportMode ws object', + multiCompiler: false, + options: { + transportMode: { + server: 'ws', + client: 'ws', + }, + }, + optionsResults: null, + }, + { + title: 'transportMode custom server path', + multiCompiler: false, + options: { + transportMode: { + server: '/path/to/custom/server/', + }, + }, + optionsResults: null, + }, + { + title: 'transportMode custom server class', + multiCompiler: false, + options: { + transportMode: { + server: class CustomServerImplementation {}, + }, + }, + optionsResults: null, + }, + { + title: 'transportMode custom client path', + multiCompiler: false, + options: { + transportMode: { + client: '/path/to/custom/client/', + }, + }, + optionsResults: null, + }, ]; cases.forEach((data) => {