[lexical-yjs] Bug Fix: Properly sync when emptying document via undo

[moughxyz]

Description

shouldBootstrap is a parameter passed to the CollaborationPlugin. When true, it initializes the editor with an empty paragraph (or some other specified state). shouldBootstrap = true should only be used by Lexical clients when creating new documents/notes/comments/etc. If we expect to populate existing data from a server, which is a very common use case, it should be set to false.

When a document is not bootstrapped, the document only initializes the initial paragraph node upon the first user interaction. Then, both a new paragraph as well as the user-typed character are inserted as a single Yjs change. However, when the user undos this initial change via a single undo interaction, the entire change is undone wholesale, and the document now has no initial paragraph node.

The existing syncYjsChangesToLexical attempts to addresses this by doing a check: $getRoot().getChildrenSize() === 0) and if true, inserts a new paragraph node to the root. However, this insertion was previously being done in an editor.update block that had either the tag 'collaboration' or 'historic' tag.

Then, when syncLexicalUpdateToYjs was called, because one of these tags were present, the function would early-return, and this change would thus not be synced to other clients, causing permanent desync and corruption of the doc for both users.

Not only was the change not syncing to other clients, but even the initiating client was not notified via the proper callbacks, and the change would fall through from persistence, causing permanent desync.

The fix is to move the insertion of the paragraph node outside of the editor.update block that included the 'collaboration' or 'historic' tag, and instead insert it in a separate editor.update block.

Test plan

A unit test was added that should clearly demonstrate the problem and how simple it is to achieve. It now passes with the fix, but fails without the fix.

Before

You can corrupt a document by following these steps:

1. Initialize an editor with the CollaborationPlugin with shouldBootstrap set to false.
2. Type a character into the editor
3. Undo by pressing Cmd/Ctrl + Z
4. Type another character

At this point, any character you type in step 4 will not properly sync and changes will be lost. This is because the Lexical state is now out of sync with the Yjs state. A paragraph was inserted by Lexical, but Yjs was not notified.

The following video shows this in action. What is shown is a single editing session with no browser refreshes. Every keystroke I type into the interactive editor is rebuilt by the playback editor below by reconstructing the playback editor from scratch localStorage Yjs deltas.

After the undo, change callbacks stop being propagated to the interactive editor, and thus no more changes are persisted, and the document is permanently corrupted.

before.mov

After

after.mov

---

This should also close #6374, as it is a less invasive solution to the same problem.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 22, 2024 5:05pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 22, 2024 5:05pm

[github-actions]

size-limit report 📦

Path	Size
lexical - cjs	29.38 KB (0%)
lexical - esm	29.24 KB (0%)
@lexical/rich-text - cjs	37.74 KB (0%)
@lexical/rich-text - esm	31.05 KB (0%)
@lexical/plain-text - cjs	36.41 KB (0%)
@lexical/plain-text - esm	28.42 KB (0%)
@lexical/react - cjs	39.64 KB (0%)
@lexical/react - esm	32.51 KB (0%)

[moughxyz]

Looks like the failing e2e tests are unrelated to this change, as they are failing in other open PRs as well.

[moughxyz]

@ivailop7 could you please review this? I'd appreciate any feedback on how we can get this to a mergeable state, as we've invested a lot of effort in this.

[ivailop7]

We'll fix the overflow tests later this week and look into merging this one.

[ivailop7]

LGTM from my side, could I get a second pair of eyes from someone. @StyleT, @etrepum , @zurfyx , @potatowagon

[etrepum]

LGTM, thank you for all of the hard work tracking this down and writing tests for this issue!

[moughxyz]

Thanks @ivailop7 and @etrepum, really appreciate all your effort!