yjs / y-websocket

Websocket Connector for Yjs
https://docs.yjs.dev/ecosystem/connection-provider/y-websocket
MIT License
492 stars 255 forks source link

In bin/utils.js, closeConn(..) doesn't handle correctly if persistence is null #176

Closed yingl closed 5 months ago

yingl commented 5 months ago

The original code only destroy the YDoc object and delete it from the docs when doc.conns.size == 0 and persistence is not null.

In my code, since I don't set the env process.env.YPERSISTENCE so persistence is null, even I called yDoc.destroy() at dosconnection, next time I connect and get the yDOc object using the same room id, the original content is still there, I tried with console.log(JSON.stringfy(yDoc)). I guess the reason is that this doc is not removed from the docs so there is still a reference to it.

I suggest to change the code this way, this looks fine for my scenario:

if (doc.conns.size === 0) {
    if (persistence !== null) {
        // if persisted, we store state and destroy ydocument
        persistence.writeState(doc.name, doc).then(() => {
          doc.destroy()
        })
    } else {
        doc.destroy()
    }

    docs.delete(doc.name)
}

Huly®: YJS-570

dmonad commented 5 months ago

That is by design. You probably don't want to run y-websocket without persistence in production. However, it can be really helpful for testing. For testing, the server should behave as if it has persistence. So, the document should be kept in memory until the process is stopped.

I don't understand why you'd want the server to forget content. Even if there is a good reason, the current implementation is favorable in my opinion. Feel free to adapt y-websocket for your own purposes.