balderdashy / sails.io.js

Browser SDK for communicating w/ Sails via sockets
https://sailsjs.com/documentation/reference/web-sockets/socket-client
183 stars 118 forks source link

forceNew option support #72

Closed shvelo closed 8 years ago

shvelo commented 9 years ago

Support passing forceNew to socket.io to force opening a new connection.

See #53

elennaro commented 9 years ago

+1

elennaro commented 8 years ago

Please! Can you merge this?

sgress454 commented 8 years ago

I will take a look. It'd be good to have a test case for it though!

shvelo commented 8 years ago

I've run into a strange condition, after adding the test for forceNew, sails.lower was calling the callback twice.

shvelo commented 8 years ago

Any objections?

sgress454 commented 8 years ago

Couple of things:

1) You can already use multiplex: false to force a new connection. 2) Starting w/ socket.io-client v1.4, sockets connecting to the same namespace always get new connections (see https://github.com/socketio/socket.io/issues/1956). The core sockets hook that Sails uses only listens to the / namespace, so this will always be the case.

So it doesn't seem like this is a necessary change. If I'm mistaken, let me know how and I'll reconsider.