-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
a single ws connection per a window #1771
Conversation
@svenefftinge It is still in the progress, but you can try it out. |
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.
Cool! Worked really well for me and the code looks good, too.
const { id } = message; | ||
const channel = this.channels.get(id); | ||
if (channel) { | ||
channel.fireOpen(); |
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.
we should at least log if there is a message without a channel
fireOpen: () => void = () => { }; | ||
onOpen(cb: () => void): void { | ||
if (this.toDispose.disposed) { | ||
return; |
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.
is registering callbacks on disposed channel not something we want at least log about? Maybe even throw an error?
protected handleConnection(socket: ws, request: http.IncomingMessage): void { | ||
const pathname = request.url && url.parse(request.url).pathname; | ||
if (pathname !== '/services') { | ||
// TODO shoule we allow other ws connections? |
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.
should
, but, yes, why not allowing multiple different websockets?
Signed-off-by: Anton Kosiakov <anton.kosyakov@typefox.io>
75b0109
to
02d7371
Compare
Signed-off-by: Anton Kosiakov <anton.kosyakov@typefox.io>
02d7371
to
0334978
Compare
@svenefftinge I've addressed your comments and opened issues for remaining tasks. It is ready for the review. |
This PR should make sure that only one ws connection is used per a window. Fixes #1672
TODO: