Socket.io: Don't reuse same-namespace connections

Created on 21 Jan 2015 · 21Comments · Source: socketio/socket.io

io('http://localhost/')
io('http://localhost/')

will create two connections moving forward.

Source

rauchg

Most helpful comment

@rauchg , this breaking (and frankly, surprising) change never made it anywhere in the API documentation, or in the changelog of 1.4.0 for that matter.

I don't think you realize that this approach to development is hostile to your users. Please consider my request at #2425 to at least let us mitigate the damage. I was personally bit by this undocumented change. I can't trust your changelogs (history.md), and I can't even rely on the official site doc.

manad777 on 26 Feb 2016

👍7

All 21 comments

@michael-luo

rauchg on 21 Jan 2015

would the changes be in socket.io-client constructor instead of socket.io because I was calling the socket.io-client constructor in middleware.js with forceNew: true?

i'm wondering if this is the right approach, it works with our insights project when i disable forceNew:

https://github.com/michael-luo/socket.io-client/commit/d409db22420bff14ab6fb0feff5dc3103c810ebb

michael-luo on 21 Jan 2015

Correct. I just keep a single milestone instead of a milestone spread out in many different repos

rauchg on 23 Jan 2015

PR: https://github.com/Automattic/socket.io-client/pull/808

michael-luo on 23 Jan 2015

Can you clarify why this only applies to the root namespace? It seems weird that we still have to forceNew for other namespaces. Why is '/' special?

cha0s on 31 Jan 2015

@cha0s it shouldn't be. It should be for any existing namespace that has already been initialized, a subsequent io call will yield a new connection

rauchg on 31 Jan 2015

The problem that I see with the patch now is that it's not clear where a subsequent new namespace would go to: the first socket or the second

rauchg on 31 Jan 2015

Actually that's a non-issue since namespaces are completely independent.

rauchg on 31 Jan 2015

@cha0s in https://github.com/Automattic/socket.io-client/pull/808 you should see we're not special-casing /

rauchg on 31 Jan 2015

Thanks, I misunderstood and thought that https://github.com/michael-luo/socket.io-client/commit/d409db22420bff14ab6fb0feff5dc3103c810ebb#diff-6d186b954a58d5bb740f73d84fe39073R57 was the latest

cha0s on 31 Jan 2015

Could someone elaborate on this choice? This is quite a shift. (I'm not saying it's a bad one.) This means that clients will have to keep a record / cache of their clients themselves from 1.4.0 on.
What happens with the forceNew option? Will there be a option to get a existing connection using another option?
What does moving forward mean? Is this already implemented in the git version of socket.io-client? It's not (yet) documented in the socket.io.client readme.

peteruithoven on 4 Feb 2015

It's not yet landed in any tag, and still up for discussion.

rauchg on 4 Feb 2015

As mentioned in the above referenced issue, the great benefit of this approach is that you can change query parameters.

peteruithoven on 17 Feb 2015

After thinking about this, I think this is the right path forward. Least number of surprises. Some surprises will happen to existing users of this particular implementation detail, but in a non-breaking way (only in a potentially unexpected more resource-consuming way).

rauchg on 4 Apr 2015

cc @michael-luo thanks for the great patch!

rauchg on 4 Apr 2015

:+1: I think this is a big change but right one.

nkzawa on 4 Apr 2015

peteruithoven on 5 Apr 2015

https://github.com/Automattic/socket.io-client/commit/ffc12af6476ad11fd33517c841a27dcad6e1df0f

rauchg on 5 Apr 2015

@rauchg , this breaking (and frankly, surprising) change never made it anywhere in the API documentation, or in the changelog of 1.4.0 for that matter.

manad777 on 26 Feb 2016

👍7

@rauchg @michael-luo

Please: Updating Documentation A Good Idea

Likely having a similar experience to @manad777 , I just spent hours debugging what seemed like a bug according to documentation. After narrowing it down to the sameNamespace variable I was able to track this back to this thread. I think to save other's time in the future, this important default behaviour (and why it can't be overridden, the benefits - which seem to be the ability to use different query parameters, etc.) should be documented to save time?

Suggestion: Make Default and Overridable

Further, on a separate note, it may be likely that additional query parameters may not be something some folks are using. It appears that since this bug fix this is the default behaviour (i.e. same namespace connections requests result in new connections). However can I suggest that we have the ability to override this behaviour explicitly (i.e. change default behaviour only, but allow user to specify, for example, sameNamespaceMultiplex:true in the options)? This way the developer at least has choice vs. being forced to keep track of connection objects.

Since we have much discussion on this thread, can we have others who were previously involved weigh in on my suggestion above?

Many thanks.