-
Notifications
You must be signed in to change notification settings - Fork 10.1k
socket.rooms is a useless object #2890
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
Comments
Well, it seems the documentation is not up-to-date here, the Note: I guess we could use a boolean as value. |
I guess this is a feature request then. Make socket.io great again: bring back the arrays! My goal here is to get an array of rooms the client is in. With an array, I assume the var rooms = socket.rooms.slice(1); Using the current implementation: var roomKeys = Object.keys(socket.rooms);
var socketIdIndex = roomKeys.indexOf( socket.id );
var rooms = roomKeys.splice( socketIdIndex, 1 ); If the var rooms = Object.keys(socket.rooms); With booleans, I would be able to filter out the true values, like this (right?): var roomKeys = Object.keys(socket.rooms);
var rooms = roomKeys.filter(function(key){
return socket.rooms[key]==true;
}); Which is just as slow as iterating with It would be helpful to include rooms that the client isn't in (such as to show chat rooms it can join). I just think that information would be better elsewhere, like the var allRooms = io.rooms;
var myRooms = socket.rooms; Anyone else have an opinion about this? |
Yah this seems unnecessarily complicated. I'm really struggling to understand why the user's socket id constitutes being a "room"?, or why key:value pairs are needed. It just needs to be a list - whatever rooms are present, you are in, done.
And as above, io.rooms to get them all, either, also as a string array, or in this case as an object with room names as keys and an array of socket ids as their value.
|
I think the main reason is the lookup performance. Refer to my answer on stackoverflow
To check if user has joined a room just a simple like this: If it's array type then we need to use indexOf or iterate every array: But I totally agree if the the key value is changed to a boolean: It may help saving some memory usage |
I wouldn't mind changing socket.rooms so that the values are booleans. Is it okay to work on this? I'm not sure what the contributing process is as I didn't see a CONTRIBUTING.md file. |
Wouldn't a
The structure would be simply |
I agree as long as the set's time complexity is constant. I believe in javascript it is. |
In Socket.IO v3, |
The value stored in the adapter will now be used, instead of duplicating it in the Socket class. Breaking change: Socket#rooms is now a Set instead of an object Closes socketio#2890
This is concerning the socket.rooms object, documented as:
I don't know what that means, but the example is clear:
When I run
console.log(socket.rooms);
, I want that pretty array (so I can slice off <socket.id> and get just the rooms), but I get this ugly object back:... which I then have to turn into an array with
.keys()
and then pick out thesocket.id
by hand (since it might not be first).Is this a bug? I'm using 1.7.3.
The text was updated successfully, but these errors were encountered: