Repository navigation
[lexical-y] Bug Fix: Write a local update made before an unread remote change on the document it was made on - #9523
Merged
Conversation
zurfyx
requested review from
acywatson,
ivailop7 and
potatowagon
as code owners
October 29, 2026 03:40
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
This was referenced Oct 29, 2026
Merged
vjeux
approved these changes
Oct 29, 2026
zurfyx
force-pushed
the
zurfyx/update-error-rollback
branch
from
October 29, 2026 17:18
53ab2b2 to
e86a226
Compare
…e change on the document it was made on
## Description
A provider can apply a remote change in the same task as a local update made without `discrete: true`, before that update's commit: a server that edits and applies client updates in one message handler, or a test harness that delivers synchronously. Yjs integrates the remote change first. Before reading it, the binding commits the waiting local update and writes it (`commitPending`). It wrote that update on the document that already had the remote change, but the update was made on the document before it:
- Offsets shifted. After a remote delete of "t1z " before it, a local delete of "t7z" deleted "t8z" instead, on every client. Typed text and line splits landed four characters off.
- Rewrites undid the remote change. A local format change, or a line written from the editor, removed the remote change's formats or brought back text it deleted.
The self-check warned (`self-check: line 5: text "t2z t3z t4z t5z t6z t7z t9z" != ...`), but by then the document was wrong for everyone.
Now the binding writes such an update on the document as it was before the remote change, the one it was made on:
- The items the change inserted are not there, and the items it deleted still are. Deleting one of those only notes the delete, so undoing the local update does not bring back what the remote change deleted.
- An attribute is set or deleted at the value it had before the change. A set is placed after that value, so Yjs settles it against the change's value as it settles concurrent values, and a delete removes that value, never the change's.
- Yjs merges the write with the remote change as it merges any concurrent edits, and the change is read after the write, as before.
- A new internal module, `Unread`, holds that view: the document without every change the binding has not read yet, from the moment Yjs starts applying one (`beforeTransaction`) until the binding has read it. That includes a write another binding of the document makes from an observer of the remote change, before this binding reads either. The binding's writes and the selections it takes meanwhile are those of its editor, which shows none of those changes. Each binding runs its work, `getCursor` included, on its own view, and only for items under its root. So another binding of the document (a replica's, another root's, or another editor on the same root) sees it as its own editor shows it, whichever binding reads the change first. The read itself sees the document as it is, as before. The writer's liveness checks (`isGone`), deletes (`deleteItem`), attribute reads and writes (`attrOf`, `attrsOf`, `setAttr`, `deleteAttr`) and stored-type lookups go through it.
- The selection the read keeps is taken after those updates, on the same view, so the local caret stays where the local update left it.
- The self-check and the follow-up check skip such a write. The read after it is checked as usual.
This also settles two cases that were wrong before:
- A waiting local update that set or deleted an attribute the remote change had set used to replace or delete the change's value. The outcome now matches the one for the same edits made concurrently.
- A node the remote change replaced in place (its stored type) used to be rewritten from its old type by an unrelated waiting update, which dropped what the change inserted into it.
One known limit is left as an expected failure. When a binding reads a change, the read shows what changes applied since did to the lines it reads (a write another binding made from an observer of that change). The binding still treats those changes as unread until it reads them in turn, so a selection taken of its editor in between is taken on a document without them. That happens when another editor on the same document asks for it from a listener, or writes from one (the binding's caret then moves). The text is right; only the selection is off.
Why not commit the local update before the remote change instead:
- The binding has no hook before Yjs integrates a remote update: `beforeTransaction` already runs inside its transaction.
- Writing in that transaction would give the local change the provider's origin. Providers would not send it on, and undo would not track it.
- Committing every local content update at once removes the window, but drops the batching the binding documents ("several `editor.update()` calls made in one task are committed, written and sent together"). 291 tests change behavior with it.
## Changeset
`.changeset/lexical-y-write-before-unread-change.md`: `[fix]` for `@lexical/y`, with a `Verify:` paragraph.
## Test plan
New `packages/lexical-y/src/__tests__/unit/BatchedWrites.test.ts` (29 tests):
- 16 cases. A remote change (a delete before, an insert before, a delete after, a format) meets a local change (a delete, typing, a format, a split). The local change is made without `discrete` and is still waiting for its commit when the remote change arrives in the same task. Each case checks the document and that no self-check warns, and for typing, that the local caret stays after the typed text.
- An invariant over random batched edits, undos and offline peers, synced in the same task or later (25 seeds by default, more with `LEXICAL_Y_SEEDS`). Every peer's next edit must reach a fresh reader, once.
- Another binding while one writes on that view: writing on a replica of the document, capturing its selection on another root of the same document, keeping its selection on the same root, whichever binding reads the change first, and writing from its listener before the first binding has read the change.
- Attributes the remote change set or deleted: deleted, then reset and restored by two local updates; set, then deleted or set by a waiting local update, which must give the same result as the concurrent edits.
- A node the remote change replaced in place, with an unrelated waiting update.
- The known limit, as `test.fails`.
`UpdateErrors.test.ts`'s invariant now makes its edits discrete or batched.
`./node_modules/.bin/vitest --run --project unit packages/lexical-y/src/__tests__/unit/BatchedWrites.test.ts packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts` (with `LEXICAL_Y_SEEDS=200`)
### Before
On the previous commit, 27 of 40 fail: all 16 cases, both invariants, the attribute cases except the restoration and the set the remote change wins for client ids 1,2, the replaced node, and the other-binding cases (A's own write is still wrong there, B's caret moves when A reads the change first, and the self-check warns). The known limit's `test.fails` fails too, because its selection is right there:
```
× editors show the document and every later edit is written 520ms
× A deletes "t1z "; B deletes "t7z" 154ms
× A deletes "t1z "; B types "!" after "t7z" 60ms
× A deletes "t1z "; B bolds "t7z" 74ms
× A deletes "t1z "; B splits the line after "t5z" 81ms
× A inserts "t0z "; B deletes "t7z" 45ms
× A inserts "t0z "; B types "!" after "t7z" 36ms
× A inserts "t0z "; B bolds "t7z" 50ms
× A inserts "t0z "; B splits the line after "t5z" 47ms
× A deletes "t8z" after it; B deletes "t7z" 33ms
× A deletes "t8z" after it; B types "!" after "t7z" 36ms
× A deletes "t8z" after it; B bolds "t7z" 37ms
× A deletes "t8z" after it; B splits the line after "t5z" 40ms
× A bolds "t2z"; B deletes "t7z" 44ms
× A bolds "t2z"; B types "!" after "t7z" 40ms
× A bolds "t2z"; B bolds "t7z" 38ms
× A bolds "t2z"; B splits the line after "t5z" 44ms
× editors show the document and every edit is written 10977ms
× on a replica of the document, sees its document as it is 20ms
× on another root of the same document, sees it as it is 28ms
× another editor on the same root keeps its selection 34ms
× a change another binding writes while the remote change is applied is not in the view either 11ms
× a selection taken between reads of two changes is the editor's (known limit) 10ms
✓ deleted, then reset and restored by local updates (ids 1,2) 40ms
× set, then deleted by a waiting local update as by a concurrent one (ids 1,2) 54ms
✓ set, then set by a waiting local update as by a concurrent one (ids 1,2) 39ms
✓ deleted, then reset and restored by local updates (ids 2,1) 27ms
× set, then deleted by a waiting local update as by a concurrent one (ids 2,1) 39ms
× set, then set by a waiting local update as by a concurrent one (ids 2,1) 45ms
× a node the remote change replaced in place keeps what it inserted 69ms
FAIL |unit| packages/lexical-y/src/__tests__/unit/BatchedWrites.test.ts > a local update waiting for its commit when a remote change arrives in the same task > A deletes "t1z "; B deletes "t7z"
AssertionError: expected [ Array(1) ] to deeply equal []
+ "self-check: line 5: text \"t2z t3z t4z t5z t6z t7z t9z\" != \"t1z t2z t3z t4z t5z t6z t8z t9z\"",
FAIL |unit| packages/lexical-y/src/__tests__/unit/BatchedWrites.test.ts > the invariant, over random batched edits > editors show the document and every edit is written
- 0 of 200 seeds
+ 85 of 200 seeds
FAIL |unit| packages/lexical-y/src/__tests__/unit/BatchedWrites.test.ts > an attribute the remote change set or deleted > set, then deleted by a waiting local update as by a concurrent one (ids 2,1)
AssertionError: expected '' to be 'REMOTE' // Object.is equality
FAIL |unit| packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts > the invariant, over random edits, batched or not, with updates that throw > editors show the document and every later edit is written
Error: seed 7: 2:6:t4z 1:5:t6z 1:0:t3z 3:5:t3z 1:3:t8z 3:3:t2z 1:0:t4z 2:4:t6z 1:3:t6z 2:4:t3z 1:3:t8z 2:1:t2z
Tests 27 failed | 12 passed | 1 expected fail (40)
```
### After
```
✓ |unit| packages/lexical-y/src/__tests__/unit/BatchedWrites.test.ts (29 tests)
✓ |unit| packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts (11 tests)
Tests 38 passed | 2 expected fail (40)
```
- The batched invariant passes with 1,000 seeds, with and without undo, and the `UpdateErrors` invariant with 400.
- `@lexical/y` unit suite: 138 files passed, 4712 tests passed, 14 expected failures. `pnpm run ci-check` passes.
- Not run: browser-mode and e2e tests.
zurfyx
force-pushed
the
zurfyx/write-before-unread-change
branch
from
October 29, 2026 17:35
334bfea to
2dd6321
Compare
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Stack, part 2 of 10. Based on #9509 (
zurfyx/update-error-rollback). Review only this PR's own commits.The
@lexical/yfixes touch the same writer, cursor, reader and binding internals, so they are one linear stack: each PR is based on the one below it, each conflict is resolved once, in the PR whose change causes it, and every CI run tests the real combination.Merge order: bottom first, #9509 → #9523 → #9522 → #9505 → #9508 → #9502 → #9504 → #9527 → #9507 → #9510. With squash merges, rebase the next PR onto
mainbefore merging it (git rebase --onto main <old base branch>).The stack
zurfyx/update-error-rollbackmainzurfyx/write-before-unread-changezurfyx/y-keep-chars-at-format-boundaryzurfyx/y-undo-member-closerzurfyx/y-initial-state-bindingzurfyx/y-reorder-intentzurfyx/y-preserve-unknown-attrszurfyx/y-move-keeps-unknown-fieldszurfyx/y-composition-holdbackzurfyx/y-incremental-read-marksFollow-ups for after the stack (not in it):
Unread(changes Yjs applied that the binding has not read) and [lexical-playground][lexical-website][lexical-y] Bug Fix: Hold back remote changes to the line being composed until the composition ends #9507'sHidden(changes held back while a line is composed) answer one question, and call sites combine them by hand. One owner module (isGone,isShown,attrOf/attrsOfand the write primitives), an item-level held overlay instead of thewas/nowmaps, and a lint or grep test that allows raw.deleted/getAttr(s)only in the Reader and in named provenance scans. [lexical-website][lexical-y] Bug Fix: Stop a format, comment or link from spreading to the end of the document after an undo or delete races an edit inside it #9505's run-opener scans (liveItemBefore,runOpener,restatedEnd) still read raw deletedness and move there too.$groupReference,$priorGroupItem, the nearest neighbour's marker before the write,priorGroupMarker), with separate predecessor stores for markers and nodes. One stored-predecessor lookup would replace it.Description
A provider can apply a remote change in the same task as a local update made without
discrete: true, before that update's commit: a server that edits and applies client updates in one message handler, or a test harness that delivers synchronously. Yjs integrates the remote change first. Before reading it, the binding commits the waiting local update and writes it (commitPending). It wrote that update on the document that already had the remote change, but the update was made on the document before it:The self-check warned (
self-check: line 5: text "t2z t3z t4z t5z t6z t7z t9z" != ...), but by then the document was wrong for everyone.Now the binding writes such an update on the document as it was before the remote change, the one it was made on:
Unread, holds that view: the document without every change the binding has not read yet, from the moment Yjs starts applying one (beforeTransaction) until the binding has read it. That includes a write another binding of the document makes from an observer of the remote change, before this binding reads either. The binding's writes and the selections it takes meanwhile are those of its editor, which shows none of those changes. Each binding runs its work,getCursorincluded, on its own view, and only for items under its root. So another binding of the document (a replica's, another root's, or another editor on the same root) sees it as its own editor shows it, whichever binding reads the change first. The read itself sees the document as it is, as before. The writer's liveness checks (isGone), deletes (deleteItem), attribute reads and writes (attrOf,attrsOf,setAttr,deleteAttr) and stored-type lookups go through it.This also settles two cases that were wrong before:
One known limit is left as an expected failure. When a binding reads a change, the read shows what changes applied since did to the lines it reads (a write another binding made from an observer of that change). The binding still treats those changes as unread until it reads them in turn, so a selection taken of its editor in between is taken on a document without them. That happens when another editor on the same document asks for it from a listener, or writes from one (the binding's caret then moves). The text is right; only the selection is off.
Why not commit the local update before the remote change instead:
beforeTransactionalready runs inside its transaction.editor.update()calls made in one task are committed, written and sent together"). 291 tests change behavior with it.Restack (October 29)
Unchanged: still based on #9509, head
334bfea5. It now sits low in the stack, so the PRs above it adopt itsUnreadview instead of the other way round: #9504 reads a group's stored fields through it (storedItemAttrs), #9527 movesattrsBeforeintoUnread.tsas the one view-aware reader of "the attributes before this write", and #9507 drops a refresh of held marker values that read raw Yjs (it changed which concurrent value Yjs kept) and judges its hold withisGone. ThecreateYDocmigration ofBatchedWrites.test.tsis in #9510, the PR that brands the document.Changeset
.changeset/lexical-y-write-before-unread-change.md:[fix]for@lexical/y, with aVerify:paragraph.Test plan
New
packages/lexical-y/src/__tests__/unit/BatchedWrites.test.ts(29 tests):discreteand is still waiting for its commit when the remote change arrives in the same task. Each case checks the document and that no self-check warns, and for typing, that the local caret stays after the typed text.LEXICAL_Y_SEEDS). Every peer's next edit must reach a fresh reader, once.test.fails.UpdateErrors.test.ts's invariant now makes its edits discrete or batched../node_modules/.bin/vitest --run --project unit packages/lexical-y/src/__tests__/unit/BatchedWrites.test.ts packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts(withLEXICAL_Y_SEEDS=200)Before
On the previous commit, 27 of 40 fail: all 16 cases, both invariants, the attribute cases except the restoration and the set the remote change wins for client ids 1,2, the replaced node, and the other-binding cases (A's own write is still wrong there, B's caret moves when A reads the change first, and the self-check warns). The known limit's
test.failsfails too, because its selection is right there:After
UpdateErrorsinvariant with 400.@lexical/yunit suite: 138 files passed, 4712 tests passed, 14 expected failures.pnpm run ci-checkpasses.