BjornTheProgrammer / react-codemirror-collab-sockets

An example of a react-codemirror implementation of the codemirror collab package, with cursor and multiple document examples.
MIT License
17 stars 3 forks source link

Socket Rooms #1

Closed bloggrammer closed 1 year ago

bloggrammer commented 1 year ago

I love your react codemirror collab implementation.

Could you add support for socket rooms? Instead of emitting the data to all connected clients, the data is emitted to clients in a particular room. socket.to("room 1").emit(...)

Look forward to hearing from you soon.

BjornTheProgrammer commented 1 year ago

Sure, I can implement socket rooms. This makes sense to do. It shouldn't be hard to implement this in the current, multi-docs branch. I'll just need a day or two to do so since I'm pretty busy right now.

bloggrammer commented 1 year ago

Wow! That would be great @BjornTheProgrammer

I tried to add the room functionality in the cursors branch but the cursor positions were not updating in real-time as before. I gave up lol.

socket.to(roomId).emit(...) on the server side prevented the CodeMirror editor from rendering as the event was not firing. io.to(roomId).emit(...) worked but the cursor behavior was lagging and the socket connection was not persistent. I had to refresh the page each time this happens to get it working again

BjornTheProgrammer commented 1 year ago

Well, upon thinking about it, it doesn't necessarily make sense to use socket rooms. You stated that "instead of emitting the data to all connected clients, the data [should be] emitted to clients in a particular room." However, in the current implementation, there are no broadcast emits, which means that data is only emitted directly from client -> sever and server -> client. There are no updates sent to other connections. Adding rooms wouldn't add any benefit to the code base from my understanding.

Additionally, since the push and pull methods from the peerExtension function/class in frontend/src/utils/collab.ts relies on the current state of the document in order to get updates, to me it seems impossible to use broadcast updates of any sort using the @codemirror/collab package.

async push() {
    let updates = sendableUpdates(this.view.state);
    if (this.pushing || !updates.length) return;
    this.pushing = true;
    let version = getSyncedVersion(this.view.state);
    await pushUpdates(socket, docName, version, updates);
    this.pushing = false;
    // Regardless of whether the push failed or new updates came in
    // while it was running, try again if there's updates remaining
    if (sendableUpdates(this.view.state).length)
        setTimeout(() => this.push(), 100);
}

async pull() {
    while (!this.done) {
        let version = getSyncedVersion(this.view.state)
        let updates = await pullUpdates(socket, docName, version)
        let newUpdates = receiveUpdates(this.view.state, updates)
        this.view.dispatch(newUpdates)
    }
}

If you haven't already I recommend checking out the multi-docs implementation, as it adds pseudo-rooms functionality with just client -> sever and server -> client emits.

If I'm misunderstanding or missing a benefit, please let me know, I'll see if it's possible while utilizing the collab package for CodeMirror.

BjornTheProgrammer commented 1 year ago

Additionally, the multi-docs branch has cursor support built-in.

bloggrammer commented 1 year ago

Thanks for your detailed explanation.

I have taken a look at the multi-docs and it seems to do the trick.

By adding rooms functionality, I mean instead of the socket on the server emitting data to all the connected sockets with socket.emit(...) we can have a room functionality where only the clients in that room can collaborate not all the connected clients. This is possible via io.to("room name").emit(...) on the server side after joining the room.

io.on("connection", socket => {
  socket.join("some room");
});

io.to("some room").emit("some event");

This link explains better: https://socket.io/docs/v3/rooms/

BjornTheProgrammer commented 1 year ago

Yeah, I looked at the socket.io docs already. I think the point of confusion here is the fact that there is no emitting data to all the connected clients currently. socket.emit(...) does not emit data to all connect clients, just to the client which requested. If you want to send data to all other connected clients you would use socket.broadcast.emit(...).

Hence, what you are describing as a problem, currently is not. There is no place in the codebase where it would make sense to implement a socket room, since we don't want to send data to any other connected clients, only to the one which requested.

bloggrammer commented 1 year ago

I agree with you, @BjornTheProgrammer.

However, I was able to modify the codebase in the multi-docs branch to mimick socket rooms connection.

Thanks.

BjornTheProgrammer commented 1 year ago

Sounds good,

Have a great rest of your day!