-
Notifications
You must be signed in to change notification settings - Fork 47
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
Handle multiple web sockets at once #57
Conversation
This doesn't pass the build and has a merge conflict. Why do you want to handle multiple WebSockets? |
Hi. The user may have a URL open in more than one tab or window, either deliberately or accidentally. Without handling multiple sockets, the module will only update one of those tabs or windows – which not only limits the potential use cases but is confusing if the active tab or window is hidden from view. |
The behavior you described is already handled by reload. As long as every webpage includes the reload script tag. The client side will connect to the socket the server has created in every client instance. I just tested this now again in one of my projects with two different tabs in two different windows |
Sometimes I (accidentally or deliberately) have more than one browser window or tab open to my app, and in that case I would like all of them to refresh when I change the code. The old behaviour is that just one client, the last to connect to the websocket server, would refresh when the `reloadServer.reload()` method is called. Note that this only changes the behaviour when the `reloadServer.reload()` function is used: it would always behave correctly if the clients are signalled by stopping the server. This PR changes (an undocumented property of) the public API, in that the `.connection` object is replaced by a `.connections` object. HOWEVER, this change will not break any existing code, for the simple reason that previously the `.connection` object was always undefined! The code was returning it before it had been assigned: it was assigned in the handler for the wss "connection" event.
Hi @alallier! I’ve rebased this against the current master, and added a proper explanation in the commit message. The short answer to your question is that indeed multiple windows have always been able to be signalled by stopping the server. The PS. I’ve just pushed a second commit that adjust the coding style so it passes your tests. |
Ahh okay I understand now! Thanks for clarifying. Will review soon. Sorry for the time delay in general on this. |
Pasting commit message from 9b7fd0e for future clarification
|
My pull request totally solves this in a more practically manor by using global socket messages. Please review and I think you will agree my pull request supersedes : #62 |
Sorry this took so long, but thank you! |
Reviewed the code. Why wouldn't you all just use the built in clients array to broadcast to all websocket clients??? var wss = new WebSocketServer({ server: httpServer })
var reloader = function(){
wss.clients.forEach(function each(client){
if (client.readyState === WebSocket.OPEN){
client.send('reload')
}
})
} Seems soooooo much simplier and does not require event listening on sockets close. |
Thanks, @AckerApple! I hadn’t spotted that WebSocketServer maintains an array of connected clients by default. Out of interest, can you reproduce a situation where a client is in the array but doesn’t have an |
@robinhouston, you're welcome. This documentation, for ws, should help give you the info you seek: https://www.npmjs.com/package/ws#broadcast-example @robinhouston, I forked this reload library and made a large pull request for the betterment of this package but it was ultimately denied. I'm a little bitter, little rubbed the wrong way. I have published my fork on npm here : https://www.npmjs.com/package/ack-reload Such as life. Try ack-reload. Tell your friends, I can't keep "trolling" this package. |
Thanks @AckerApple for pointing this out. We can re-factor to use the native array. |
No description provided.