-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Encapsulate sockjs, introduce ws and build base for users to make own server implementation #1904
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
Changes from 4 commits
5a6b2a4
2f22843
62cc34c
f915ce5
8f6bd69
11b6522
bee6f83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
'use strict'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The file name should be changed from |
||
|
||
/* 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); | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
'use strict'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The file name should be changed from |
||
|
||
// 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 {}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. /cc @hiroppy What do you think is better:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 let's use /cc @Loonride |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
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
toBaseServer
.