Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Transports are not overwritten by default ones. #528

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
7 participants

When creating a new Socket, its default options are first set and then the user options are merged into this object using io.util.merge.

This works great but when we want to force a transport on the client this way:

io.connect(host, { "transports": ["jsonp-polling"] }

the options object will first be loaded with all the available transports and then the specified transports will be pushed into the array. The result: no matter what transports are specified on the call to io.connect, all the available transports will be used to negotiate with the server.

If, instead, we first load the specified transports and only if no one was specified we add the available ones, transport selection behavior will be as expected.

This change will fix issues #349 and #429.

I have the same problem on using socket.io-client with Dojo. This change solves the problem. Thank you!

This patch has literally just saved my life

Contributor

3rd-Eden commented Apr 16, 2013

Transports should be set serverside, not client side imho.

@3rd-Eden The problem is that in my case, I need to use different set of transports for different clients, based on conditions that are outside the socket.io server. A blanket set of transports that applies to ALL clients is not suitable for me. Some clients should have xhr polling or jsonp polling, while other clients must absolutely and only use jsonp polling.

The server setting should determine the maximum set of available transports, but I need a way to further restrict that set on a per client basis.

Contributor

3rd-Eden commented Apr 16, 2013

@deakster ok, that makes sense i guess.

Just one small tip, you can also just filter the allowed transports in the url;

/socket.io/socket.io+websocket+jsonp-polling.js

Will only load socket.io + websocket & polling support (they are removed from the source code so you'll have a smaller file size);

/socket.io/socket.io+jsonp-polling.js

Completely disables websockets.. Might be useful for you

@3rd-Eden Ok, well that seems useful, but in my setup I am hosting the static socket.io client js files from a CDN rather than serving them from the socket.io server, so that wouldn't work.

I suppose I could create those files manually by requesting them from socket.io, and then sending all those permutations to the CDN.

what version of socket.io-client is this for?

@dcbartlett This is for 0.9.x versions of client. Anyway, the original code that receives this pull request has been there with no changes for a long time.

This issue was closed.

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