Skip to content

Bind routes to express-ws instance to make it easier to broadcast - #122

Open
aral wants to merge 7 commits into
HenningM:masterfrom
aral:referential-routes
Open

Bind routes to express-ws instance to make it easier to broadcast#122
aral wants to merge 7 commits into
HenningM:masterfrom
aral:referential-routes

Conversation

@aral

@aral aral commented Jul 26, 2019

Copy link
Copy Markdown

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

  1. Binds WebSocket routes (middlewares) to the express-ws “instance” so that you can use this within a route to a reference to the getWss() function and the Express app instance.

  2. Documents this as well as documenting the missing route parameter for the getWss() 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

@aral

aral commented Jul 26, 2019

Copy link
Copy Markdown
Author

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!) :)

@sberlinches

Copy link
Copy Markdown

This is what exactly I was looking for.
Looking forward to seeing it in the next release 👍

@aral

aral commented Sep 8, 2019

Copy link
Copy Markdown
Author

@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.

@sberlinches

sberlinches commented Sep 24, 2019

Copy link
Copy Markdown

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

@sberlinches

Copy link
Copy Markdown

@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.

@aral

aral commented Oct 3, 2019

Copy link
Copy Markdown
Author

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

@aral

aral commented Oct 3, 2019

Copy link
Copy Markdown
Author

@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'}.`)
  })
})

@aral

aral commented Oct 4, 2019

Copy link
Copy Markdown
Author

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/

@aral

aral commented Oct 4, 2019

Copy link
Copy Markdown
Author

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

Ron-SSG commented Jan 28, 2020

Copy link
Copy Markdown

So when will this be released?

@aral

aral commented Jan 28, 2020

Copy link
Copy Markdown
Author

@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 :)

@Ron-SSG

Ron-SSG commented Jan 29, 2020

Copy link
Copy Markdown

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?

@aral

aral commented Feb 15, 2021

Copy link
Copy Markdown
Author

@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.)

@ForrestFire0

Copy link
Copy Markdown

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants