Malformed delta state when using applyDelta / toDelta

deeg commented 1 year ago

Please save me some time and use the following template. In 90% of all issues I can't reproduce the problem because I don't know what exactly you are doing, in which environment, or which y-* version is responsible. Just use the following template even if you think the problem is obvious.

Checklist

[X] Are you reporting a bug? Use github issues for bug reports and feature requests. For general questions, please use https://discuss.yjs.dev/
[X] 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

Applying delta operations to a yText document ends up with a different final state than if applying those same delta operations to a Delta object directly outside of YJS.

To Reproduce

Insert a table with some content after it

const initialTable = [
    {
        "attributes": {
            "block-id": "block-28eea923-9cbb-4b6f-a950-cf7fd82bc087"
        },
        "insert": "\n"
    },
    {
        "attributes": {
            "table-col": {
                "width": "150"
            }
        },
        "insert": "\n\n\n"
    },
    {
        "attributes": {
            "block-id": "block-9144be72-e528-4f91-b0b2-82d20408e9ea",
            "table-cell-line": {
                "rowspan": "1",
                "colspan": "1",
                "row": "row-6kv2ls",
                "cell": "cell-apba4k"
            },
            "row": "row-6kv2ls",
            "cell": "cell-apba4k",
            "rowspan": "1",
            "colspan": "1"
        },
        "insert": "\n"
    },
    {
        "attributes": {
            "block-id": "block-639adacb-1516-43ed-b272-937c55669a1c",
            "table-cell-line": {
                "rowspan": "1",
                "colspan": "1",
                "row": "row-6kv2ls",
                "cell": "cell-a8qf0r"
            },
            "row": "row-6kv2ls",
            "cell": "cell-a8qf0r",
            "rowspan": "1",
            "colspan": "1"
        },
        "insert": "\n"
    },
    {
        "attributes": {
            "block-id": "block-6302ca4a-73a3-4c25-8c1e-b542f048f1c6",
            "table-cell-line": {
                "rowspan": "1",
                "colspan": "1",
                "row": "row-6kv2ls",
                "cell": "cell-oi9ikb"
            },
            "row": "row-6kv2ls",
            "cell": "cell-oi9ikb",
            "rowspan": "1",
            "colspan": "1"
        },
        "insert": "\n"
    },
    {
        "attributes": {
            "block-id": "block-ceeddd05-330e-4f86-8017-4a3a060c4627",
            "table-cell-line": {
                "rowspan": "1",
                "colspan": "1",
                "row": "row-d1sv2g",
                "cell": "cell-dt6ks2"
            },
            "row": "row-d1sv2g",
            "cell": "cell-dt6ks2",
            "rowspan": "1",
            "colspan": "1"
        },
        "insert": "\n"
    },
    {
        "attributes": {
            "block-id": "block-37b19322-cb57-4e6f-8fad-0d1401cae53f",
            "table-cell-line": {
                "rowspan": "1",
                "colspan": "1",
                "row": "row-d1sv2g",
                "cell": "cell-qah2ay"
            },
            "row": "row-d1sv2g",
            "cell": "cell-qah2ay",
            "rowspan": "1",
            "colspan": "1"
        },
        "insert": "\n"
    },
    {
        "attributes": {
            "block-id": "block-468a69b5-9332-450b-9107-381d593de249",
            "table-cell-line": {
                "rowspan": "1",
                "colspan": "1",
                "row": "row-d1sv2g",
                "cell": "cell-fpcz5a"
            },
            "row": "row-d1sv2g",
            "cell": "cell-fpcz5a",
            "rowspan": "1",
            "colspan": "1"
        },
        "insert": "\n"
    },
    {
        "attributes": {
            "block-id": "block-26b1d252-9b2e-4808-9b29-04e76696aa3c",
            "table-cell-line": {
                "rowspan": "1",
                "colspan": "1",
                "row": "row-pflz90",
                "cell": "cell-zrhylp"
            },
            "row": "row-pflz90",
            "cell": "cell-zrhylp",
            "rowspan": "1",
            "colspan": "1"
        },
        "insert": "\n"
    },
    {
        "attributes": {
            "block-id": "block-6af97ba7-8cf9-497a-9365-7075b938837b",
            "table-cell-line": {
                "rowspan": "1",
                "colspan": "1",
                "row": "row-pflz90",
                "cell": "cell-s1q9nt"
            },
            "row": "row-pflz90",
            "cell": "cell-s1q9nt",
            "rowspan": "1",
            "colspan": "1"
        },
        "insert": "\n"
    },
    {
        "attributes": {
            "block-id": "block-107e273e-86bc-44fd-b0d7-41ab55aca484",
            "table-cell-line": {
                "rowspan": "1",
                "colspan": "1",
                "row": "row-pflz90",
                "cell": "cell-20b0j9"
            },
            "row": "row-pflz90",
            "cell": "cell-20b0j9",
            "rowspan": "1",
            "colspan": "1"
        },
        "insert": "\n"
    },
    {
        "attributes": {
            "block-id": "block-38161f9c-6f6d-44c5-b086-54cc6490f1e3"
        },
        "insert": "\n"
    },
    {
        "insert": "Content after table"
    },
    {
        "attributes": {
            "block-id": "block-15630542-ef45-412d-9415-88f0052238ce"
        },
        "insert": "\n"
    }
]

Insert a dash and then a space

const addingDash = [
    {
        "retain": 12
    },
    {
        "insert": "-"
    }
]

const addingSpace = [
    {
        "retain": 13
    },
    {
        "insert": " "
    }
]
]

This action removes the -characters and adds a list into the table cell

const addingList = [
    {
        "retain": 12
    },
    {
        "delete": 2
    },
    {
        "retain": 1,
        "attributes": {
           // Clear table line attribute
            "table-cell-line": null,
            // Add list attribute in place of table-cell-line
            "list": {
                "rowspan": "1",
                "colspan": "1",
                "row": "row-pflz90",
                "cell": "cell-20b0j9",
                "list": "bullet"
            }
        }
    }
]

Expected behavior

With a plain Delta object this returns the following end state

Notice how at the end the operation with the list attribute does not have any table-cell-line. There is also no table-cell-line below the operation with list attribute.

const deltaWithoutYjs = new Delta({ops: [...initialTable]}).compose(new Delta({ops: [...addingDash]})).compose(new Delta({ops: addingSpace})).compose(new Delta({ops: addingList}));

{
  "ops": [
    {
      "attributes": {
        "block-id": "block-28eea923-9cbb-4b6f-a950-cf7fd82bc087"
      },
      "insert": "\n"
    },
    {
      "attributes": {
        "table-col": {
          "width": "150"
        }
      },
      "insert": "\n\n\n"
    },
    {
      "attributes": {
        "block-id": "block-9144be72-e528-4f91-b0b2-82d20408e9ea",
        "table-cell-line": {
          "rowspan": "1",
          "colspan": "1",
          "row": "row-6kv2ls",
          "cell": "cell-apba4k"
        },
        "row": "row-6kv2ls",
        "cell": "cell-apba4k",
        "rowspan": "1",
        "colspan": "1"
      },
      "insert": "\n"
    },
    {
      "attributes": {
        "block-id": "block-639adacb-1516-43ed-b272-937c55669a1c",
        "table-cell-line": {
          "rowspan": "1",
          "colspan": "1",
          "row": "row-6kv2ls",
          "cell": "cell-a8qf0r"
        },
        "row": "row-6kv2ls",
        "cell": "cell-a8qf0r",
        "rowspan": "1",
        "colspan": "1"
      },
      "insert": "\n"
    },
    {
      "attributes": {
        "block-id": "block-6302ca4a-73a3-4c25-8c1e-b542f048f1c6",
        "table-cell-line": {
          "rowspan": "1",
          "colspan": "1",
          "row": "row-6kv2ls",
          "cell": "cell-oi9ikb"
        },
        "row": "row-6kv2ls",
        "cell": "cell-oi9ikb",
        "rowspan": "1",
        "colspan": "1"
      },
      "insert": "\n"
    },
    {
      "attributes": {
        "block-id": "block-ceeddd05-330e-4f86-8017-4a3a060c4627",
        "table-cell-line": {
          "rowspan": "1",
          "colspan": "1",
          "row": "row-d1sv2g",
          "cell": "cell-dt6ks2"
        },
        "row": "row-d1sv2g",
        "cell": "cell-dt6ks2",
        "rowspan": "1",
        "colspan": "1"
      },
      "insert": "\n"
    },
    {
      "attributes": {
        "block-id": "block-37b19322-cb57-4e6f-8fad-0d1401cae53f",
        "table-cell-line": {
          "rowspan": "1",
          "colspan": "1",
          "row": "row-d1sv2g",
          "cell": "cell-qah2ay"
        },
        "row": "row-d1sv2g",
        "cell": "cell-qah2ay",
        "rowspan": "1",
        "colspan": "1"
      },
      "insert": "\n"
    },
    {
      "attributes": {
        "block-id": "block-468a69b5-9332-450b-9107-381d593de249",
        "table-cell-line": {
          "rowspan": "1",
          "colspan": "1",
          "row": "row-d1sv2g",
          "cell": "cell-fpcz5a"
        },
        "row": "row-d1sv2g",
        "cell": "cell-fpcz5a",
        "rowspan": "1",
        "colspan": "1"
      },
      "insert": "\n"
    },
    {
      "attributes": {
        "block-id": "block-26b1d252-9b2e-4808-9b29-04e76696aa3c",
        "table-cell-line": {
          "rowspan": "1",
          "colspan": "1",
          "row": "row-pflz90",
          "cell": "cell-zrhylp"
        },
        "row": "row-pflz90",
        "cell": "cell-zrhylp",
        "rowspan": "1",
        "colspan": "1"
      },
      "insert": "\n"
    },
    {
      "attributes": {
        "block-id": "block-6af97ba7-8cf9-497a-9365-7075b938837b",
        "table-cell-line": {
          "rowspan": "1",
          "colspan": "1",
          "row": "row-pflz90",
          "cell": "cell-s1q9nt"
        },
        "row": "row-pflz90",
        "cell": "cell-s1q9nt",
        "rowspan": "1",
        "colspan": "1"
      },
      "insert": "\n"
    },
    {
      "insert": "\n",
      // This attibutes has only list and no table-cell-line
      "attributes": {
        "list": {
          "rowspan": "1",
          "colspan": "1",
          "row": "row-pflz90",
          "cell": "cell-20b0j9",
          "list": "bullet"
        },
        "block-id": "block-107e273e-86bc-44fd-b0d7-41ab55aca484",
        "row": "row-pflz90",
        "cell": "cell-20b0j9",
        "rowspan": "1",
        "colspan": "1"
      }
    },
   // No table-cell-line below here
    {
      "attributes": {
        "block-id": "block-38161f9c-6f6d-44c5-b086-54cc6490f1e3"
      },
      "insert": "\n"
    },
    {
      "insert": "Content after table"
    },
    {
      "attributes": {
        "block-id": "block-15630542-ef45-412d-9415-88f0052238ce"
      },
      "insert": "\n"
    }
  ]
}

End state with YJS

Notice how at the end the operation with the list attribute also has table-cell-line attribute. There are also table-cell-line attributes below that operation when there should not be.

const ydoc = new Y.Doc();
const ytext = ydoc.getText('id');
ytext.applyDelta([...initialTable]);
ytext.applyDelta([...addingDash]);
ytext.applyDelta([...addingSpace]);
ytext.applyDelta([...addingList]);
const deltaWithYjs = ydoc.getText('id').toDelta();

[
  {
    "insert": "\n",
    "attributes": {
      "block-id": "block-28eea923-9cbb-4b6f-a950-cf7fd82bc087"
    }
  },
  {
    "insert": "\n\n\n",
    "attributes": {
      "table-col": {
        "width": "150"
      }
    }
  },
  {
    "insert": "\n",
    "attributes": {
      "block-id": "block-9144be72-e528-4f91-b0b2-82d20408e9ea",
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-6kv2ls",
        "cell": "cell-apba4k"
      },
      "row": "row-6kv2ls",
      "cell": "cell-apba4k",
      "rowspan": "1",
      "colspan": "1"
    }
  },
  {
    "insert": "\n",
    "attributes": {
      "block-id": "block-639adacb-1516-43ed-b272-937c55669a1c",
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-6kv2ls",
        "cell": "cell-a8qf0r"
      },
      "row": "row-6kv2ls",
      "cell": "cell-a8qf0r",
      "rowspan": "1",
      "colspan": "1"
    }
  },
  {
    "insert": "\n",
    "attributes": {
      "block-id": "block-6302ca4a-73a3-4c25-8c1e-b542f048f1c6",
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-6kv2ls",
        "cell": "cell-oi9ikb"
      },
      "row": "row-6kv2ls",
      "cell": "cell-oi9ikb",
      "rowspan": "1",
      "colspan": "1"
    }
  },
  {
    "insert": "\n",
    "attributes": {
      "block-id": "block-ceeddd05-330e-4f86-8017-4a3a060c4627",
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-d1sv2g",
        "cell": "cell-dt6ks2"
      },
      "row": "row-d1sv2g",
      "cell": "cell-dt6ks2",
      "rowspan": "1",
      "colspan": "1"
    }
  },
  {
    "insert": "\n",
    "attributes": {
      "block-id": "block-37b19322-cb57-4e6f-8fad-0d1401cae53f",
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-d1sv2g",
        "cell": "cell-qah2ay"
      },
      "row": "row-d1sv2g",
      "cell": "cell-qah2ay",
      "rowspan": "1",
      "colspan": "1"
    }
  },
  {
    "insert": "\n",
    "attributes": {
      "block-id": "block-468a69b5-9332-450b-9107-381d593de249",
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-d1sv2g",
        "cell": "cell-fpcz5a"
      },
      "row": "row-d1sv2g",
      "cell": "cell-fpcz5a",
      "rowspan": "1",
      "colspan": "1"
    }
  },
  {
    "insert": "\n",
    "attributes": {
      "block-id": "block-26b1d252-9b2e-4808-9b29-04e76696aa3c",
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-pflz90",
        "cell": "cell-zrhylp"
      },
      "row": "row-pflz90",
      "cell": "cell-zrhylp",
      "rowspan": "1",
      "colspan": "1"
    }
  },
  {
    "insert": "\n",
    "attributes": {
      "block-id": "block-6af97ba7-8cf9-497a-9365-7075b938837b",
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-pflz90",
        "cell": "cell-s1q9nt"
      },
      "row": "row-pflz90",
      "cell": "cell-s1q9nt",
      "rowspan": "1",
      "colspan": "1"
    }
  },
  {
    "insert": "\n",
    // This attributes has table-cell-line and list
    "attributes": {
      "block-id": "block-107e273e-86bc-44fd-b0d7-41ab55aca484",
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-pflz90",
        "cell": "cell-s1q9nt"
      },
      "row": "row-pflz90",
      "cell": "cell-20b0j9",
      "rowspan": "1",
      "colspan": "1",
      "list": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-pflz90",
        "cell": "cell-20b0j9",
        "list": "bullet"
      }
    }
  },
// table-cell-line below this part which is incorrect
  {
    "insert": "\n",
    "attributes": {
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-pflz90",
        "cell": "cell-s1q9nt"
      },
      "block-id": "block-38161f9c-6f6d-44c5-b086-54cc6490f1e3"
    }
  },
  {
    "insert": "Content after table",
    "attributes": {
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-pflz90",
        "cell": "cell-s1q9nt"
      }
    }
  },
  {
    "insert": "\n",
    "attributes": {
      "table-cell-line": {
        "rowspan": "1",
        "colspan": "1",
        "row": "row-pflz90",
        "cell": "cell-s1q9nt"
      },
      "block-id": "block-15630542-ef45-412d-9415-88f0052238ce"
    }
  }
]

Diff between the two end states

{
  "ops": [
    {
      "retain": 12
    },
    {
      "retain": 22,
      "attributes": {
        "table-cell-line": {
          "rowspan": "1",
          "colspan": "1",
          "row": "row-pflz90",
          "cell": "cell-s1q9nt"
        }
      }
    }
  ]
}

Environment Information

Node 16
yjs 13.5.39

Code sample: yjs-playground.zip

_{Huly®: YJS-335}

dmonad commented 1 year ago

The Quill implementation of the Delta format has a lot of weird edge cases that are specific to the Quill editor. I don't expect them to be fully compatible. That's why in the y-quill editor binding, I do some sanity transformations before I apply Deltas to the Yjs document.

Here is an edge case that Yjs handles differently than Quill:

// assume you have a `ytext` with the rich-text content "a*b*" (b is bold).

ytext.applyDelta([{ retain: 1 }, { insert: 'X' }])

// Then both "a*Xb*" and "aX*b*" are valid outcomes because you didn't specify any formatting attributes in the delta.
// However, the below approach should always yield the same result

ytext.applyDelta([retain: 1}, {insert: 'X', attributes: {}}]) // specify that X shouldn't have attributes.

So I suggest you try setting empty attributes specifically, so they don't get inherited.

deeg commented 1 year ago

@dmonad , thanks for the quick response!

The Quill implementation of the Delta format has a lot of weird edge cases that are specific to the Quill editor. I don't expect them to be fully compatible. That's why in the y-quill editor binding, I do some sanity transformations before I apply Deltas to the Yjs document.

We are following all of the same formatting that you are doing int he y-quill binding.

The issue seems to be with this delta

const addingList = [
    {
        "retain": 12
    },
    {
        "delete": 2
    },
    {
        "retain": 1,
        "attributes": {
           // Clear table line attribute
            "table-cell-line": null,
            // Add list attribute in place of table-cell-line
            "list": {
                "rowspan": "1",
                "colspan": "1",
                "row": "row-pflz90",
                "cell": "cell-20b0j9",
                "list": "bullet"
            }
        }
    }
]

It doesn't like that table-cell-line attribute is null. If I split this delta up into two events, it works as expected:

Delta 1:

[ {
    'retain': 12,
    attributes: {}
  },
  {
    'delete': 2,
    attributes: {}
  }]

Delta 2:

[{
    'retain': 12,
    attributes: {}
  },
  {
    'retain': 1,
    'attributes': {
      'table-cell-line': null,
      'list': {
        'rowspan': '1',
        'colspan': '1',
        'row': 'row-pflz90',
        'cell': 'cell-20b0j9',
        'list': 'bullet'
      }
    }
  }]

So I suggest you try setting empty attributes specifically, so they don't get inherited.

I tried this, but it doesn't seem to fix the problem.

Do you have any other suggestions on how to ensure we always get the same end state with yDoc and Delta formats?

dmonad commented 1 year ago

FYI: Setting an attribute to null should remove the attribute. I'm not sure if that is still the case in the Quill Delta format.

If you set up some minimal reproduction case (in code), I'd have a look at it. Since the above Delta is huge with presumably a lot of edge-cases, I haven't run it yet.

deeg commented 1 year ago

@dmonad I will make a smaller example and send that too.

All of the original code is linked at the bottom in a Zip file and can be easily run.

Delta does remove the attribute correctly with null, and it still says it should work that way according to their docs.

YJS does handle it okay sometimes, but not in the case of this specific delta.

deeg commented 1 year ago

@dmonad here is a smaller example with a table with two cells.

If you unzip and run npm i you should be able to run the example.

I have also included JSON files of the different outputs to easily inspect them.

Please let me know if you would like me to provide anything else!

yjs-delta-conversion.zip

deeg commented 1 year ago

Seems like potentially related: https://github.com/yjs/yjs/issues/302

dmonad commented 1 year ago

Hi @deeg,

Thanks for the debugging case. This should be fixed in the latest release!

yjs / yjs

Malformed delta state when using applyDelta / toDelta #474