Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(server): add serverMode option #1937

Merged
merged 7 commits into from Jun 1, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 16 additions & 2 deletions lib/Server.js
Expand Up @@ -30,8 +30,8 @@ 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');
const SockJSServer = require('./servers/SockJSServer');

// Workaround for node ^8.6.0, ^9.0.0
// DEFAULT_ECDH_CURVE is default to prime256v1 in these version
Expand Down Expand Up @@ -67,6 +67,19 @@ class Server {

this.log = _log || createLogger(options);

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.SocketServerImplementation is a class, so it must be instantiated before use
this.SocketServerImplementation = getSocketServerImplementation(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.SocketServerImplementation -> this.socketServerImplementation

this.options
);

this.originalStats =
this.options.stats && Object.keys(this.options.stats).length
? this.options.stats
Expand Down Expand Up @@ -655,7 +668,8 @@ class Server {
}

createSocketServer() {
this.socketServer = new SockJSServer(this);
const SocketServerImplementation = this.SocketServerImplementation;
this.socketServer = new SocketServerImplementation(this);

this.socketServer.onConnection((connection) => {
if (!connection) {
Expand Down
11 changes: 11 additions & 0 deletions lib/options.json
Expand Up @@ -297,6 +297,16 @@
"serveIndex": {
"type": "boolean"
},
"serverMode": {
"anyOf": [
{
"type": "string"
},
{
"instanceof": "Function"
}
]
},
"serverSideRender": {
"type": "boolean"
},
Expand Down Expand Up @@ -420,6 +430,7 @@
"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)",
Expand Down
41 changes: 41 additions & 0 deletions 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 (e.g. '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;
50 changes: 50 additions & 0 deletions 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/);
});
});
133 changes: 133 additions & 0 deletions test/ServerMode.test.js
@@ -0,0 +1,133 @@
'use strict';

/* eslint-disable
class-methods-use-this
*/
const request = require('supertest');
const sockjs = require('sockjs');
const SockJSServer = require('../lib/servers/SockJSServer');
const config = require('./fixtures/simple-config/webpack.config');
const testServer = require('./helpers/test-server');
const BaseServer = require('./../lib/servers/BaseServer');

describe('serverMode', () => {
let server;
let req;

afterEach((done) => {
testServer.close(done);
req = null;
server = null;
});
describe("supplying 'sockjs' as a string", () => {
beforeEach((done) => {
server = testServer.start(
config,
{
serverMode: 'sockjs',
},
done
);
req = request('http://localhost:8080');
});

it('sockjs path responds with a 200', (done) => {
req.get('/sockjs-node').expect(200, done);
});
});

describe('supplying path to sockjs implementation', () => {
beforeEach((done) => {
server = testServer.start(
config,
{
serverMode: require.resolve('../lib/servers/SockJSServer'),
},
done
);
req = request('http://localhost:8080');
});

it('sockjs path responds with a 200', (done) => {
req.get('/sockjs-node').expect(200, done);
});
});

describe('supplying sockjs implementation class', () => {
beforeEach((done) => {
server = testServer.start(
config,
{
serverMode: SockJSServer,
},
done
);
req = request('http://localhost:8080');
});

it('sockjs path responds with a 200', (done) => {
req.get('/sockjs-node').expect(200, done);
});
});

describe('custom sockjs implementation', () => {
it('uses supplied server implementation', (done) => {
server = testServer.start(
config,
{
sockPath: '/foo/test/bar/',
serverMode: class MySockJSServer extends BaseServer {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@evilebottnawi being able to pass in class also makes testing easier

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,
});

expect(server.options.sockPath).toEqual('/foo/test/bar/');
}

send(connection, message) {
connection.write(message);
}

close(connection) {
connection.close();
}

onConnection(f) {
this.socket.on('connection', f);
}
},
},
done
);
});
});

describe('supplying nonexistent path', () => {
it('should throw an error', () => {
expect(() => {
server = testServer.start(
config,
{
serverMode: '/bad/path/to/implementation',
},
() => {}
);
}).toThrow(/serverMode must be a string/);
});
});
});
9 changes: 9 additions & 0 deletions test/options.test.js
Expand Up @@ -6,6 +6,7 @@ const webpack = require('webpack');
const { createFsFromVolume, Volume } = require('memfs');
const Server = require('../lib/Server');
const options = require('../lib/options.json');
const SockJSServer = require('../lib/servers/SockJSServer');
const config = require('./fixtures/simple-config/webpack.config');

describe('options', () => {
Expand Down Expand Up @@ -340,6 +341,14 @@ describe('options', () => {
success: [true],
failure: [''],
},
serverMode: {
success: [
'sockjs',
require.resolve('../lib/servers/SockJSServer'),
SockJSServer,
],
failure: ['', false],
},
serverSideRender: {
success: [true],
failure: [''],
Expand Down