Different outcomes for Y.Text when artificially delaying text attribute updates

[raedle]

Checklist

• (UNSURE) Are you reporting a bug? Use github issues for bug reports and feature requests. For general questions, please use https://discuss.yjs.dev/
• Try to report your issue in the correct repository. Yjs consists of many modules. When in doubt, report it to https://github.com/yjs/yjs/issues/

Describe the bug
I want to start off that I am unsure if what I am reporting is a bug or an expected behavior.

I am seeing different outcomes when remote updates for a Y.Text struct (specifically attribute updates) are applied locally but after another local change happened to text attributes.

The following code has three cases:

1. Single text struct leading to the expected outcome
2. Two text structs where remote updates are applied immediately and leading to the expected outcome
3. Two text structs where remote updates are applied after another local change and leading to an unexpected outcome

To Reproduce
Steps to reproduce the behavior:

1. Execute code or go to https://codesandbox.io/s/busy-cannon-59l6t?file=/src/index.ts
2. Open console
3. Check differences in logged text deltas

Example code:

import * as Y from "yjs";

// Create doc with text and initial text
const doc = new Y.Doc();
const text = doc.getText("text");
text.insert(0, "helloworld");
const initialState = Y.encodeStateAsUpdate(doc);

function getDoc(): Y.Doc {
  const doc = new Y.Doc();
  Y.applyUpdate(doc, initialState);
  return doc;
}

// 1. Single text struct
function oneText() {
  const doc1 = getDoc();
  const text1 = doc1.getText("text");

  text1.format(3, 3, { bold: true });
  text1.format(0, 4, { bold: true });

  console.log({
    test: "oneText",
    text1: text1.toDelta()
  });
}

// 2. Two text structs synced locally without and with a
// timeout.
//
// The timeout will delay applying the change on the other
// struct by putting it in a macro task. This means that
// both local changes will be applied before the update from
// the respective other struct.
//
// Without timeout
// text1 will apply format 3, 3, { bold: true }
// text2 will receive format 3, 3, { bold: true }
// text2 will apply format 0, 4, { bold: true }
// text1 will receive format 0, 4, { bold: true }
// 
// Expected output: [{"insert":"hellow","attributes":{"bold":true}},{"insert":"orld"}]
// Actual output: [{"insert":"hellow","attributes":{"bold":true}},{"insert":"orld"}]
//
// With timeout
// text1 will apply format 3, 3, { bold: true }
// text2 will apply format 0, 4, { bold: true }
// text2 will receive format 3, 3, { bold: true }
// text1 will receive format 0, 4, { bold: true }
//
// Expected output: [{"insert":"hellow","attributes":{"bold":true}},{"insert":"orld"}]
// Actual ouput: [{"insert":"hell","attributes":{"bold":true}},{"insert":"oworld"}]
function twoText(withTimeout: boolean = false) {
  const doc1 = getDoc();
  const text1 = doc1.getText("text");
  const doc2 = getDoc();
  const text2 = doc2.getText("text");

  doc1.on("update", (update: Uint8Array) => {
    if (withTimeout) {
      setTimeout(() => {
        Y.applyUpdate(doc2, update);
      }, 0);
    } else {
      Y.applyUpdate(doc2, update);
    }
  });
  doc2.on("update", (update: Uint8Array) => {
    if (withTimeout) {
      setTimeout(() => {
        Y.applyUpdate(doc1, update);
      }, 0);
    } else {
      Y.applyUpdate(doc1, update);
    }
  });

  doc1.transact(() => {
    text1.format(3, 3, { bold: true });
  });
  doc2.transact(() => {
    text2.format(0, 4, { bold: true });
  });

  if (withTimeout) {
    setTimeout(() => {
      console.log({
        test: "twoText",
        text1: text1.toDelta(),
        text2: text2.toDelta()
      });
    }, 0);
  } else {
    console.log({
      test: "twoText",
      text1: text1.toDelta(),
      text2: text2.toDelta()
    });
  }
}

oneText();
twoText();
twoText(true);

Screenshots
The screenshot shows the rendered console output. Compare the top text1/text2 combo with the bottom text1/text2 combo. The expected outcome is that the bottom combo would be the same as the top combo.

Environment Information

• Browser Chrome Version 89.0.4389.114 (Official Build) (x86_64)
• Node.js v12.16.3
• Yjs 13.5.3

[raedle]

The issue described above leads to inconsistent document states in Quill.

To Reproduce

Steps to reproduce the behavior:

1. Clone yjs-demos project to yjs-demos
2. (cd yjs-demos/demo-server && npm install)
3. (cd yjs-demos/quill && npm install)
4. (cd yjs-demos/demo-server && npm install lib0)
5. Copy y-websocket/bin/utils.js to yjs-demos/demo-server/utils.js
6. Copy y-websocket/bin/callback.js to yjs-demos/demo-server/callback.js
7. Change const setupWSConnection = require('y-websocket/bin/utils.js').setupWSConnection in yjs-demos/demo-server/demo-server.js to const setupWSConnection = require('./utils.js').setupWSConnection
8. Wrap send in utils.js in a setTimeout with 15000 ms to artificially delay sending messages to peer clients.
9. (cd yjs-demos/demo-server && npm start)
10. Change the WebsocketProvider in yjs-demos/quill/quill.js to connect to your local websocket server (i.e. const provider = new WebsocketProvider('ws://localhost:8080', 'quill-demo', ydoc)
11. (cd yjs-demos/quill && npm start)
12. Open Chrome and Firefox and navigate to http://localhost:8081/quill.html in both browsers
13. Type helloworld in one Quill editor and wait for the text to be synchronized to the other
14. Select text range from 3 with length 3 and make text bold (i.e., make low in helloworld bold)
15. Before the 15 seconds pass, select text range from 0 with length 4 in the other Quill editor and make text bold (i.e., make hell in helloworld bold)
16. Wait for both changes to synchronize and verify diverged states

Expected behavior
It is expected that both Quill editors show the same content. From a user perspective, I would have expected hellow in helloworld to be bold in both Quill editors.

Screencast

yjs-attribute-divergence.mp4

[dmonad]

Hi @raedle,

I fixed this in Yjs@13.5.5. Can you please confirm that this is fixed?

[raedle]

Thanks for the swift response, @dmonad !!

I did a quick test with the linked codesandbox.io. The results for the attributes with and without a timeout are still different.

In the test in rev 3a7a324 (in v13.5.5), the second format call is a subset of the first format call. This works!

However, if the calls would be switched, it leads to different results (i.e., if the first call is a subset and the second call).

Example 1: works

text0.format(0, 3, { bold: true })
text1.format(2, 2, { bold: true })

Example 2: doesn't work

text0.format(2, 2, { bold: true })
text1.format(0, 3, { bold: true })

The expected outcome of Example 2 would be the actual outcome of Example 1.

Here is a crude test that would eventually test for these cases:

/**
 * @param {t.TestCase} tc
 */
export const testDeltaAfterConcurrentFormattingInCluster = tc => {
  const { text0: text00, text1: text01, testConnector: testConnector0 } = init(tc, { users: 2 })
  const { text0: text10, text1: text11, testConnector: testConnector1 } = init(tc, { users: 2 })
  // client cluster1
  text00.insert(0, 'abcde')
  testConnector0.flushAllMessages()
  text00.format(0, 3, { bold: true })
  text01.format(2, 2, { bold: true })
  testConnector0.flushAllMessages()
  t.compare(text00.toDelta(), text01.toDelta())

  // client cluster2
  text10.insert(0, 'abcde')
  testConnector1.flushAllMessages()
  text01.format(2, 2, { bold: true })
  text11.format(0, 3, { bold: true })
  testConnector1.flushAllMessages()
  t.compare(text10.toDelta(), text11.toDelta())

  // compare texts of cluster1 to texts in cluster2
  t.compare(text00.toDelta(), text10.toDelta());
  t.compare(text01.toDelta(), text11.toDelta());
  t.compare(text00.toDelta(), text11.toDelta());
  t.compare(text01.toDelta(), text10.toDelta());
}