yorkie-js-sdk
yorkie-js-sdk copied to clipboard
Trigger incorrect TreeChange during concurrent tree.style editing
What happened:
While concurrently editing a document in tree style, the TreeChange is sent incorrectly.
- Initial State:
<doc><p italic="true">hello</p></doc> - ClientA and ClientB are editing concurrently:
- Client A:
t.style(0, 1, { bold: 'true' }) - Client B:
t.style(0, 1, { bold: 'false' })
- Client A:
- Final Document State:
<doc><p bold="false" italic="true">hello</p></doc>
Though the final document state converges correctly, the treeChange received by users through doc.subscribe is as follows:
As a result, in docB, the last bold: true is applied, causing divergence in the document displayed in the editor.
// docA
[
{
type: 'tree-style',
from: 0,
to: 1,
value: { bold: 'true' },
} as any,
{
type: 'tree-style',
from: 0,
to: 1,
value: { bold: 'false' },
} as any,
],
// docB
[
{
type: 'tree-style',
from: 0,
to: 1,
value: { bold: 'false' },
} as any,
{
type: 'tree-style',
from: 0,
to: 1,
value: { bold: 'true' },
} as any,
],
What you expected to happen:
If there are no document changes, a change should not be triggered.
How to reproduce it (as minimally and precisely as possible):
To reproduce the issue, you can run the following test code:
it.only('Concurrent style and style', async function ({ task }) {
await withTwoClientsAndDocuments<{ t: Tree }>(async (c1, d1, c2, d2) => {
d1.update((root) => {
root.t = new Tree({
type: 'doc',
children: [
{
type: 'p',
children: [
{
type: 'text',
value: 'hello',
},
],
attributes: { italic: 'true' },
},
],
});
});
await c1.sync();
await c2.sync();
assert.equal(d1.getRoot().t.toXML(), d2.getRoot().t.toXML());
assert.equal(
d1.getRoot().t.toXML(),
/*html*/ `<doc><p italic="true">hello</p></doc>`,
);
const [ops1, ops2] = subscribeDocs(d1, d2);
d1.update((r) => r.t.style(0, 1, { bold: 'true' }));
d2.update((r) => r.t.style(0, 1, { bold: 'false' }));
await c1.sync();
await c2.sync();
await c1.sync();
assert.equal(d1.getRoot().t.toXML(), d2.getRoot().t.toXML());
assert.equal(
d1.getRoot().t.toXML(),
/*html*/ `<doc><p bold="false" italic="true">hello</p></doc>`,
);
assert.deepEqual(
ops1.map((it) => {
return { type: it.type, from: it.from, to: it.to, value: it.value };
}),
[
{
type: 'tree-style',
from: 0,
to: 1,
value: { bold: 'true' },
} as any,
{
type: 'tree-style',
from: 0,
to: 1,
value: { bold: 'false' },
} as any,
],
);
assert.deepEqual(
ops2.map((it) => {
return { type: it.type, from: it.from, to: it.to, value: it.value };
}),
[
{
type: 'tree-style',
from: 0,
to: 1,
value: { bold: 'false' },
} as any,
// {
// type: 'tree-style',
// from: 0,
// to: 1,
// value: { bold: 'true' },
// } as any,
],
);
}, task.name);
});
Anything else we need to know?: Related: https://github.com/yorkie-team/yorkie-android-sdk/pull/159
Environment:
- Operating system:
- Browser and version:
- Yorkie version (use
yorkie version): - Yorkie JS SDK version:
Unlike text style, which stores nodes requiring style application in toBeStyleds using latestCreatedAt and adds changes only for those nodes, the tree style implementation currently lacks latestCreatedAt.
- https://github.com/yorkie-team/yorkie-js-sdk/blob/8b6ae9788f1b33c52930aae885319a4841eb84b1/src/document/crdt/text.ts#L266-L292
- https://github.com/yorkie-team/yorkie/pull/795
~~Another approach might involve comparing previous attribute values and omitting changes if there are no differences.~~
~Dup of https://github.com/yorkie-team/yorkie/issues/788.~