HenningM
Bind routes to express-ws instance to make it easier to broadcast
Use case
I have my routes defined in external files and they don’t have access to the original express-ws “instance” (the object returned by the expressWs() function).
What this patch does
-
Binds WebSocket routes (
middlewares) to the express-ws “instance” so that you can usethiswithin a route to a reference to thegetWss()function and the Expressappinstance. -
Documents this as well as documenting the missing
routeparameter for thegetWss()function.
Example of use
From examples/chat.js:
var express = require('express');
var expressWs = require('..')
var expressWs = expressWs(express());
var app = expressWs.app;
app.ws('/chat', function(ws, req) {
ws.on('message', message => {
this.getWss('/chat').clients.forEach(client => {
client.send(message)
})
})
});
app.listen(3000)
Related issues
#7, #32, #80, #96
Note: the only bit I’m not sure of in this is whether the applyTo() function will have the correct scope for this (I’m not using this/haven’t tested). @HenningM, could you possibly have a look at that if you’re otherwise happy with the pull request? Thanks :) (And thanks for express-ws – it’s a lifesaver!) :)
This is what exactly I was looking for. Looking forward to seeing it in the next release :+1:
@sberlinches If you need it right now, please feel free to use it from my fork until it’s in master.
I’m using it in production with Site.js to power our funding/patronage pages at Small Technology Foundation.
@aral Thanks!
~~I have been testing it, and it works incredibly.~~
After implementing the second route, I realized that it doesn't work.
this.getWss('/chat').clients and this.getWss('/another_route').clients are the same object, thus, they have the same clients.
Did I do something wrong?
app.ws('/chat', function(ws, req) {
ws.on('message', message => {
this.getWss('/chat').clients.forEach(client => {
client.send(message)
})
})
});
app.ws('/another_route', function(ws, req) {
ws.on('message', message => {
this.getWss('/another_route').clients.forEach(client => {
client.send(message)
})
})
});
@aral My apologies for mentioning you again. It is just that I don't know if Github notifies to about an edited comment to the involved people.
No worries and thanks for the report. I’ll investigate today and update when I have something.
Update: Verified; now tracking here: https://source.ind.ie/site.js/app/issues/127
@sberlinches Hey Sergio, so I don’t know where I got it into my head that the getWss() function takes a route parameter (probably here). It doesn’t. And there’s no built in “room”-style functionality in either express-ws or ws.
I will revert the documentation change and update my pull request when I get a moment.
In the meanwhile, here’s how you would get your example working with the current version. (Also note that in the code below the message is not sent to the client that it originated from either.) Hope this helps.
app.ws('/chat', function(client, request) {
client.room = request.url.replace('/.websocket', '')
client.on('message', message => {
let count = 0
this.getWss().clients.forEach(c => {
if (c !== client && c.room === '/chat' && client.readyState === 1 /* WebSocket.OPEN */) {
c.send(message)
count++
}
})
console.log(`/chat broadcast to ${count} client${count === 1 ? '': 's'}.`)
})
})
app.ws('/another_route', function(client, request) {
client.room = request.url.replace('/.websocket', '')
client.on('message', message => {
let count = 0
this.getWss().clients.forEach(c => {
if (c !== client && c.room === '/another_route' && client.readyState === 1 /* WebSocket.OPEN */) {
c.send(message)
count++
}
})
console.log(`/another_route broadcast to ${count} client${count === 1 ? '': 's'}.`)
})
})
Right, I‘ve pushed two more commits. The first fixes the documentation regression I’d introduced by stating that the getWss() function had an optional route parameter and the second introduces linting for the examples and fixes all the linted errors and improves them with better broadcast and chat examples that demonstrate the ability to use “rooms” (broadcast to clients on a specific route).
I’m happy with the state of this pull request and I don’t imagine any further additions to it. Please feel free to merge or not at your leisure.
I’ve merged these changes into my fork, which is available from https://github.com/aral/express-ws/
OK, I lied, so I just pushed another commit that makes broadcast filtering (i.e., limiting broadcasts to just the clients on a certain route – for example to implement rooms in a chat app) quite elegant.
With the latest commit, the example above becomes:
const express = require('express');
const expressWs = require('express-ws')(express());
const app = expressWs.app;
function roomHandler(client, request) {
client.room = this.setRoom(request);
console.log(`New client connected to ${client.room}`);
client.on('message', (message) => {
const numberOfRecipients = this.broadcast(client, message);
console.log(`${client.room} message broadcast to ${numberOfRecipients} recipient${numberOfRecipients === 1 ? '' : 's'}.`);
});
}
app.ws('/room1', roomHandler);
app.ws('/room2', roomHandler);
app.listen(3000, () => {
console.log('\nChat server running on http://localhost:3000\n\nFor Room 1, connect to http://localhost:3000/room1\nFor Room 2, connect to http://localhost:3000/room2\n');
});
I’m going to integrate this into my master branch and into Site.js over the weekend.
@Ron-SSG Hey Ron, can’t speak for this project but I’ve been using it from my fork at https://github.com/aral/express-ws and it’s been part of Site.js for a while (https://sitejs.org). Hope that helps :)
Hey @aral , started working with your fork, but it seems that when using it with a router i can't get access to thisRoom() because this is basically the the function i call with my router.
I know that it's suppoused to be the expressWs instance but i can't get it to go through to the router and from there to my controller method.
Any ideas?
@Ron-SSG Sorry, I don’t know why I’m not getting notifications properly here. I also don’t think I entirely understand the problem you’re running into. With Site.js, along with my fork, I simply add the websocket routes (at the end of the router chain): https://github.com/small-tech/site.js/blob/master/index.js#L780 ← does that help at all? (I’m going to assume not, most likely.)
For those wondering what the current state of express-ws installed out of the box is: If you have two different routes declared with app.ws(path), and call getWss(path) for each route, the clients for each Wss object will be the same. If one client connects to one route, and one client to the other, they will both appear on both Wss objects list of clients.
However, an easy work-around is just to add the clients to an array when they connect to a route, and remove them from the array when they disconnect. A 'send to all' function just loops through the array of the route. Not too hard.
