Always initialize with the same yjs document if no state is present

[juliusknorr]

Fix #5574
Fix #4881

When restructuring the y.js state handling we changed the behaviour to only apply the initial content for the first session. Since we no longer cleanup the document state we introduced an issue where a second read only document would no longer show content as the steps the first session tried to push to set the content never appeared (as the read only session cannot push any steps). Apart from this there might still be possible breakage of documents if pushing the steps fails or is not happening for some reason.

I tried to summarize the problems of the current approach in a small diagram:

This is a new approach that fixes the above issue and also cleans up the way we generally initialize the y.js document (which as a side effect also fixes #4881). While this is not the ideal approach from reading up on y.js handling the initial document state, this seems the most reasonable given the technical limitation we have on the backend where we cannot build a y.js document based on the markdown content right now.

With this change we now always initialize a idempotent y.js document state if there is no state file present. This allows us to have the same state accross sessions, which can then pick up syncing their edits immediately and no longer require a step to be pushed and synced from the first to other sessions to have the initial content. On the first save the y.js state is persisted on the server and will be used for any further sessions.

Tests

Some additional cypress tests cover the different scenarios to pick up a document that either starts from the client created initial state or from the server stored one.

Testing concurrent sessions is not that straight forward with cypress, but we could think about recording some edits and replying the API requests while having one tab open showing the changes and asserting the document. However this is left for another time for now.

Additional jest test I used for exploring options, not entirely sure if that is actually useful yet to have in our codebase

diff --git a/src/tests/yjs.spec.js b/src/tests/yjs.spec.js
index a56725ec0..a8f2fa01f 100644
--- a/src/tests/yjs.spec.js
+++ b/src/tests/yjs.spec.js
@@ -1,10 +1,17 @@
 import recorded from './fixtures/recorded'
-import { Doc, encodeStateAsUpdate } from 'yjs'
+import Y, { Doc, encodeStateAsUpdate } from 'yjs'
 import { decodeArrayBuffer } from '../helpers/base64.js'
 import * as decoding from 'lib0/decoding'
 import * as encoding from 'lib0/encoding'
 import * as syncProtocol from 'y-protocols/sync'
 
+import createEditor from '../EditorFactory'
+import markdownit from '../markdownit'
+import { generateJSON } from '@tiptap/html'
+import { Node } from '@tiptap/pm/model'
+import { prosemirrorJSONToYDoc, prosemirrorToYXmlFragment, yDocToProsemirror } from 'y-prosemirror'
+import { createMarkdownSerializer } from '../extensions/Markdown.js'
+
 describe('recorded session', () => {
 	const flattened = recorded.flat()
 	const sync = flattened.filter(step => step.startsWith('AA'))
@@ -86,6 +93,68 @@ describe('recorded session', () => {
 		expect(size(updates)).toBe(103024)
 	})
 
+	test('create two documents from same initial state', () => {
+		const editor = createEditor({
+			enableRichEditing: true
+		})
+		const extensions = editor.extensionManager.extensions
+		const html = markdownit.render('# Hello world')
+
+		const getYDoc = (html) => {
+			const json = generateJSON(html, extensions)
+
+			// Attempt 1
+			//const ydoc = prosemirrorJSONToYDoc(editor.schema, json)
+			// return ydoc
+
+			// Attempt 2
+			// https://github.com/yjs/y-prosemirror/blob/8db24263770c2baaccb08e08ea9ef92dbcf8a9da/src/lib.js#L205
+			const doc = Node.fromJSON(editor.schema, json)
+			const ydoc = new Doc()
+			ydoc.clientID = 0
+			const type = /** @type {Y.XmlFragment} */ (ydoc.get('prosemirror', Y.XmlFragment))
+			if (!type.doc) {
+				return ydoc
+			}
+
+			prosemirrorToYXmlFragment(doc, type)
+			return type.doc
+		}
+
+		const ydoc1 = getYDoc(html)
+		const ydoc2 = getYDoc(html)
+		expect(encodeStateAsUpdate(ydoc1)).toEqual(encodeStateAsUpdate(ydoc2))
+		const responses = sync.map(step => {
+			const response1 = processStep(ydoc1, step)
+			const response2 = processStep(ydoc2, step)
+			expect(response1.length).toBe(response2.length)
+			return response1
+		})
+		expect(encodeStateAsUpdate(ydoc1)).toEqual(encodeStateAsUpdate(ydoc2))
+
+		const baseYdoc = getYDoc(html)
+		const ydocC1 = new Doc()
+		const ydocC2 = new Doc()
+		const baseUpdate = encodeStateAsUpdate(baseYdoc)
+		// apply update to both clients
+		Y.applyUpdate(ydocC1, baseUpdate)
+		Y.applyUpdate(ydocC2, baseUpdate)
+
+		expect(encodeStateAsUpdate(ydocC1)).toEqual(encodeStateAsUpdate(ydocC2))
+		const responses2 = sync.map(step => {
+			const response1 = processStep(ydocC1, step)
+			const response2 = processStep(ydocC2, step)
+			expect(response1.length).toBe(response2.length)
+			return response1
+		})
+		expect(encodeStateAsUpdate(ydocC1)).toEqual(encodeStateAsUpdate(ydocC2))
+
+		const pm1 = yDocToProsemirror(editor.schema, ydocC1)
+		const pm2 = yDocToProsemirror(editor.schema, ydocC2)
+
+		expect(pm1).toEqual(pm2)
+	})
+
 	test('read messages', () => {
 		const ydoc = new Doc()
 		const responses = sync.map(step => processStep(ydoc, step))

Further references

• https://morizbuesing.com/blog/initializing-a-yjs-document-with-a-common-value/
• https://discuss.yjs.dev/t/exclude-initial-loading-step-from-transcript-history/519
• https://discuss.yjs.dev/t/how-to-initialize-document-in-prosemirror-with-y-js/468/17
• New Feature: Generate JSON from HTML ueberdosis/tiptap#1273
• https://discuss.yjs.dev/t/initial-offline-value-of-a-shared-document/465/13

[juliusknorr]

/backport to stable29

[mejo-]

Thanks a lot for diving into this. The explanations seem reasonable to me and sound like a good approach to address the underlying problem.

For now I only have minor comments.

[mejo-]

I don't understand this part.

[juliusknorr]

Clarified also with a comment in the code

[mejo-]

Thanks for the code walkthrough in the call.