From 907f37859a685dfac346bfc51f39471138f62fdb Mon Sep 17 00:00:00 2001 From: Kirill Nagaitsev Date: Thu, 30 May 2019 11:33:24 -0500 Subject: [PATCH] feat(server): move to util, more tests --- lib/Server.js | 38 +++------------- lib/utils/getSocketServerImplementation.js | 41 ++++++++++++++++++ test/GetSocketServerImplementation.test.js | 50 ++++++++++++++++++++++ test/options.test.js | 3 +- 4 files changed, 97 insertions(+), 35 deletions(-) create mode 100644 lib/utils/getSocketServerImplementation.js create mode 100644 test/GetSocketServerImplementation.test.js diff --git a/lib/Server.js b/lib/Server.js index 79a02131a1..9027b0c3fc 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -30,6 +30,7 @@ const status = require('./utils/status'); const createDomain = require('./utils/createDomain'); const runBonjour = require('./utils/runBonjour'); const routes = require('./utils/routes'); +const getSocketServerImplementation = require('./utils/getSocketServerImplementation'); const schema = require('./options.json'); // Workaround for node ^8.6.0, ^9.0.0 @@ -66,44 +67,15 @@ class Server { this.log = _log || createLogger(options); - if (this.options.serverMode !== undefined) { + if (this.options.serverMode === undefined) { + this.options.serverMode = 'sockjs'; + } else { this.log.warn( 'serverMode is an experimental option, meaning its usage could potentially change without warning' ); } - this.options.serverMode = this.options.serverMode || 'sockjs'; - let ServerImplementation; - let serverImplFound = true; - if (typeof this.options.serverMode === 'string') { - // could be 'sockjs', in the future 'ws', or a path that should be required - if (this.options.serverMode === 'sockjs') { - // eslint-disable-next-line global-require - ServerImplementation = require('./servers/SockJSServer'); - } else { - try { - // eslint-disable-next-line global-require, import/no-dynamic-require - ServerImplementation = require(this.options.serverMode); - } catch (e) { - serverImplFound = false; - } - } - } else if (typeof this.options.serverMode === 'function') { - // potentially do more checks here to confirm that the user implemented this properlly - // since errors could be difficult to understand - ServerImplementation = this.options.serverMode; - } else { - serverImplFound = false; - } - - if (!serverImplFound) { - throw new Error( - "serverMode must be a string denoting a default implementation (eg. 'sockjs'), 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' - ); - } - this.ServerImplementation = ServerImplementation; + this.ServerImplementation = getSocketServerImplementation(this.options); this.originalStats = this.options.stats && Object.keys(this.options.stats).length diff --git a/lib/utils/getSocketServerImplementation.js b/lib/utils/getSocketServerImplementation.js new file mode 100644 index 0000000000..572a6080e0 --- /dev/null +++ b/lib/utils/getSocketServerImplementation.js @@ -0,0 +1,41 @@ +'use strict'; + +function getSocketServerImplementation(options) { + let ServerImplementation; + let serverImplFound = true; + switch (typeof options.serverMode) { + case 'string': + // could be 'sockjs', in the future 'ws', or a path that should be required + if (options.serverMode === 'sockjs') { + // eslint-disable-next-line global-require + ServerImplementation = require('../servers/SockJSServer'); + } else { + try { + // eslint-disable-next-line global-require, import/no-dynamic-require + ServerImplementation = require(options.serverMode); + } catch (e) { + serverImplFound = false; + } + } + break; + 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; + break; + default: + serverImplFound = false; + } + + if (!serverImplFound) { + throw new Error( + "serverMode must be a string denoting a default implementation (eg. 'sockjs'), 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' + ); + } + + return ServerImplementation; +} + +module.exports = getSocketServerImplementation; diff --git a/test/GetSocketServerImplementation.test.js b/test/GetSocketServerImplementation.test.js new file mode 100644 index 0000000000..830f49513f --- /dev/null +++ b/test/GetSocketServerImplementation.test.js @@ -0,0 +1,50 @@ +'use strict'; + +const getSocketServerImplementation = require('../lib/utils/getSocketServerImplementation'); +const SockJSServer = require('../lib/servers/SockJSServer'); + +describe('getSocketServerImplementation', () => { + it("should work with serverMode: 'sockjs'", () => { + let result; + + expect(() => { + result = getSocketServerImplementation({ + serverMode: 'sockjs', + }); + }).not.toThrow(); + + expect(result).toEqual(SockJSServer); + }); + + it('should work with serverMode: SockJSServer class', () => { + let result; + + expect(() => { + result = getSocketServerImplementation({ + serverMode: SockJSServer, + }); + }).not.toThrow(); + + expect(result).toEqual(SockJSServer); + }); + + it('should work with serverMode: SockJSServer full path', () => { + let result; + + expect(() => { + result = getSocketServerImplementation({ + serverMode: require.resolve('../lib/servers/SockJSServer'), + }); + }).not.toThrow(); + + expect(result).toEqual(SockJSServer); + }); + + it('should throw with serverMode: bad path', () => { + expect(() => { + getSocketServerImplementation({ + serverMode: '/bad/path/to/implementation', + }); + }).toThrow(/serverMode must be a string/); + }); +}); diff --git a/test/options.test.js b/test/options.test.js index cc77a667a4..b9d5c0a278 100644 --- a/test/options.test.js +++ b/test/options.test.js @@ -343,12 +343,11 @@ describe('options', () => { }, serverMode: { success: [ - '', 'sockjs', require.resolve('../lib/servers/SockJSServer'), SockJSServer, ], - failure: [false], + failure: ['', false], }, serverSideRender: { success: [true],