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

Encapsulate sockjs, introduce ws and build base for users to make own server implementation #1904

Merged
merged 7 commits into from May 28, 2019
Merged
45 changes: 5 additions & 40 deletions lib/Server.js
Expand Up @@ -12,7 +12,6 @@ const url = require('url');
const http = require('http');
const https = require('https');
const ip = require('ip');
const sockjs = require('sockjs');
const semver = require('semver');
const killable = require('killable');
const chokidar = require('chokidar');
Expand All @@ -32,26 +31,7 @@ const createDomain = require('./utils/createDomain');
const runBonjour = require('./utils/runBonjour');
const routes = require('./utils/routes');
const schema = require('./options.json');

// Workaround for sockjs@~0.3.19
// sockjs will remove Origin header, however Origin header is required for checking host.
// See https://github.com/webpack/webpack-dev-server/issues/1604 for more information
{
// eslint-disable-next-line global-require
const SockjsSession = require('sockjs/lib/transport').Session;
const decorateConnection = SockjsSession.prototype.decorateConnection;
SockjsSession.prototype.decorateConnection = function(req) {
decorateConnection.call(this, req);
const connection = this.connection;
if (
connection.headers &&
!('origin' in connection.headers) &&
'origin' in req.headers
) {
connection.headers.origin = req.headers.origin;
}
};
}
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 @@ -674,20 +654,9 @@ class Server {
}

createSocketServer() {
const 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.log.error(line);
} else {
this.log.debug(line);
}
},
});
this.socketServer = new SockJSServer(this);

socket.on('connection', (connection) => {
this.socketServer.onConnection((connection) => {
if (!connection) {
return;
}
Expand Down Expand Up @@ -739,10 +708,6 @@ class Server {

this._sendStats([connection], this.getStats(this._stats), true);
});

socket.installHandlers(this.listeningApp, {
prefix: this.sockPath,
});
}

listen(port, hostname, fn) {
Expand All @@ -765,7 +730,7 @@ class Server {

close(cb) {
this.sockets.forEach((socket) => {
socket.close();
this.socketServer.close(socket);
});

this.sockets = [];
Expand Down Expand Up @@ -926,7 +891,7 @@ class Server {
// eslint-disable-next-line
sockWrite(sockets, type, data) {
sockets.forEach((socket) => {
socket.write(JSON.stringify({ type, data }));
this.socketServer.send(socket, JSON.stringify({ type, data }));
});
}

Expand Down
9 changes: 9 additions & 0 deletions lib/servers/BaseServer.js
@@ -0,0 +1,9 @@
'use strict';

// base class that users should extend if they are making their own
// server implementation
module.exports = class BaseServer {
constructor(server) {
this.server = server;
}
};
64 changes: 64 additions & 0 deletions lib/servers/SockJSServer.js
@@ -0,0 +1,64 @@
'use strict';

/* eslint-disable
class-methods-use-this,
func-names
*/
const sockjs = require('sockjs');
const BaseServer = require('./BaseServer');

// Workaround for sockjs@~0.3.19
// sockjs will remove Origin header, however Origin header is required for checking host.
// See https://github.com/webpack/webpack-dev-server/issues/1604 for more information
{
// eslint-disable-next-line global-require
const SockjsSession = require('sockjs/lib/transport').Session;
const decorateConnection = SockjsSession.prototype.decorateConnection;
SockjsSession.prototype.decorateConnection = function(req) {
decorateConnection.call(this, req);
const connection = this.connection;
if (
connection.headers &&
!('origin' in connection.headers) &&
'origin' in req.headers
) {
connection.headers.origin = req.headers.origin;
}
};
}

module.exports = class SockJSServer extends BaseServer {
// options has: error (function), debug (function), server (http/s server), path (string)
constructor(server) {
super(server);
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();
}

// f should return the resulting connection
onConnection(f) {
this.socket.on('connection', f);
}
};
8 changes: 8 additions & 0 deletions lib/servers/WsServer.js
@@ -0,0 +1,8 @@
'use strict';

// const ws = require('ws');
const BaseServer = require('./BaseServer');

// ws server implementation under construction
// will need changes in the client as well to function
module.exports = class WsServer extends BaseServer {};
9 changes: 9 additions & 0 deletions lib/servers/baseServer.js
@@ -0,0 +1,9 @@
'use strict';
Copy link
Member

Choose a reason for hiding this comment

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

The file name should be changed from baseServer.js to BaseServer.


// base class that users should extend if they are making their own
// server implementation
module.exports = class BaseServer {
constructor(server) {
this.server = server;
}
};
64 changes: 64 additions & 0 deletions lib/servers/sockjsServer.js
@@ -0,0 +1,64 @@
'use strict';
Copy link
Member

Choose a reason for hiding this comment

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

The file name should be changed from sockjsServer.js to SockJSServer.


/* eslint-disable
class-methods-use-this,
func-names
*/
const sockjs = require('sockjs');
const BaseServer = require('./BaseServer');

// Workaround for sockjs@~0.3.19
// sockjs will remove Origin header, however Origin header is required for checking host.
// See https://github.com/webpack/webpack-dev-server/issues/1604 for more information
{
// eslint-disable-next-line global-require
const SockjsSession = require('sockjs/lib/transport').Session;
const decorateConnection = SockjsSession.prototype.decorateConnection;
SockjsSession.prototype.decorateConnection = function(req) {
decorateConnection.call(this, req);
const connection = this.connection;
if (
connection.headers &&
!('origin' in connection.headers) &&
'origin' in req.headers
) {
connection.headers.origin = req.headers.origin;
}
};
}

module.exports = class SockJSServer extends BaseServer {
// options has: error (function), debug (function), server (http/s server), path (string)
constructor(server) {
super(server);
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();
}

// f should return the resulting connection
onConnection(f) {
this.socket.on('connection', f);
}
};
8 changes: 8 additions & 0 deletions lib/servers/wsServer.js
@@ -0,0 +1,8 @@
'use strict';
Copy link
Member

Choose a reason for hiding this comment

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

The file name should be changed from wsServer.js to WsServer.


// const ws = require('ws');
const BaseServer = require('./BaseServer');

// ws server implementation under construction
// will need changes in the client as well to function
module.exports = class WsServer extends BaseServer {};
Copy link
Member

Choose a reason for hiding this comment

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

/cc @hiroppy What do you think is better:

  • WebsocketServer
  • WSServer
  • WsServer

Copy link
Member

Choose a reason for hiding this comment

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

I like WebsocketServer. For example, the plugin name of wasm(WebAssemblyPlugin) which webpack has is not omitted. But this example is webpack, not webpack-dev-server. In any case, I think that it is easier to understand if it is not omitted.

Copy link
Member

Choose a reason for hiding this comment

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

👍 let's use WebsocketServer

/cc @Loonride

29 changes: 8 additions & 21 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.