Repository navigation
[lexical-website][lexical-y] Bug Fix: Keep the fields an older client does not know in a block it moves - #9527
Merged
Merged
Conversation
zurfyx
requested review from
acywatson,
etrepum,
ivailop7 and
potatowagon
as code owners
October 29, 2026 04:14
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Oct 29, 2026
Merged
zurfyx
force-pushed
the
zurfyx/y-move-keeps-unknown-fields
branch
from
October 29, 2026 05:53
d475e2d to
a4a2302
Compare
zurfyx
changed the base branch from
zurfyx/stack-9502-9504
to
zurfyx/y-preserve-unknown-attrs
October 29, 2026 05:53
This was referenced Oct 29, 2026
zurfyx
force-pushed
the
zurfyx/y-move-keeps-unknown-fields
branch
from
October 29, 2026 06:15
a4a2302 to
89665c7
Compare
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: |
zurfyx
force-pushed
the
zurfyx/y-move-keeps-unknown-fields
branch
from
October 29, 2026 06:39
89665c7 to
c552868
Compare
zurfyx
force-pushed
the
zurfyx/y-preserve-unknown-attrs
branch
from
October 29, 2026 10:47
f0e50c0 to
f58c9a3
Compare
zurfyx
force-pushed
the
zurfyx/y-move-keeps-unknown-fields
branch
from
October 29, 2026 10:47
c552868 to
b2da1ef
Compare
vjeux
approved these changes
Oct 29, 2026
zurfyx
force-pushed
the
zurfyx/y-preserve-unknown-attrs
branch
from
October 29, 2026 19:14
f58c9a3 to
81f87e8
Compare
… does not know in a block it moves ## Description An older client of an app (one whose class lacks a field that a newer version of the class added) keeps that field when it edits a node (#9504). A block it moves was different: Yjs has no moves, so the writer deletes the moved block and writes it anew from what the client shows (#9502 chooses which block). The new copy started empty, so the moved block lost every field the client does not own, for every client and for a fresh reader, with no concurrency involved: - a line's fields on its marker (a note's `tone`), and the fields of its list and outer lists on the marker of every item; - a block node's or decorator's fields on its Y.Node, and those of everything it holds (lines, lists, inline nodes, slots), which are written anew too; - the same when the client splits such a line. With this change, a unit written anew takes what its node stored before the write: - **Lines.** A marker the write makes for a line that had one takes the fields the old marker stored that the client does not own (`$markerBefore`, read with `attrsBefore`: the attributes as they were before the transaction, which Yjs still holds for a marker the transaction deleted). A line that holds lines (stored as parts) reads each part's old marker, or the whole line's for a new part. Where a diff of a container gives a moved line the marker of another line there, the marker takes the moved line's fields and drops the other line's. - **Groups.** A list item new to its list, or written anew, takes the list's fields (`g:`) from the item next to it that kept its marker and was in the list before (#9504's rule, which now also requires the second part), so an item that joins another list takes that list's fields. Where no such item exists, the list moved with the item or lost all its items: the item takes the fields the list stored before, from its own old marker if it was in the list, else from another item's, else from an item the list had then (`$priorGroupItem`). So a list moved with an item added, swapped list items, and two lists an older client shows as one all keep their own fields. An outer level takes the fields of its item (`@`) from where that item's were stored, so a nested list moved into a new item takes none of the old item's. - **Nodes.** A node given new storage takes the fields its old Y.Node stored that the client does not own (`nodeBefore` in `$fillNode`). What each lookup finds is kept for the whole write, so a node written twice in one write (moved into a container that is itself written anew) still takes what it had before the first. Only names the client does not own are copied; it writes and deletes its own as before. Ownership now also counts a flat NodeState with no schema (a plain `parse` function) where the client writes it as a field: on a node stored as its own Y.Node, and on text (`jsonFieldsOf`). With #9504, such a state reset to its default was never deleted there, so every other client and a fresh reader kept the old value. On `main` it was deleted. Clients of one version store what they stored before, with one exception, in names that no class of theirs declares on a line (a list item's `checked` left on a paragraph): those now stay with the line when a diff gives it another line's marker, where before they stayed with the marker. Remaining limits, as `test.fails` and in y.md: - The text of a moved block is still written from what the client shows, so it loses the text formats, text-class fields and link fields the client does not know. Keeping them needs the old characters' formats carried into the new runs, character by character. - A nested list whose item moves into another list does not take that list's fields: the outer level would need them from the items of the list it joins. Not changed here: an item that keeps its marker while its list changes (an indent or outdent of a list's first item) keeps the list fields of its old place, as with #9504. ## Changeset `.changeset/lexical-y-moved-blocks-keep-fields.md`: `[fix]` for `@lexical/y`, with a `Verify:` paragraph. ## Test plan `regressions/SchemaSkew.test.ts`: #9504's `test.fails('limit: a block an older client moves keeps the fields it does not know')` is now a passing test. A new F2 group compares each move made by an older client with the same move made by a newer one: lines, lists, block nodes, decorators, nested content, nested lists and inline nodes, moved down, up and into a new container; splits; slots; items moved within and between lists; swapped list items; lists shown as one; lists moved with new items; nested lists moved into new or other items; moves into containers that move too; a line that gets another line's marker; a line holding lines. Another group checks a flat NodeState with no schema, reset in place and on a move. `./node_modules/.bin/vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SchemaSkew.test.ts` ### Before On the merge of #9502 and #9504: ``` FAIL |unit| packages/lexical-y/src/__tests__/unit/regressions/SchemaSkew.test.ts > F2: an older client that moves a block keeps the fields it does not know > a block node moved down past a paragraph AssertionError: expected { root: { children: [ …(2) ], …(5) } } to deeply equal { root: { children: [ …(2) ], …(5) } } - "tone": "warm", + "tone": "", FAIL |unit| packages/lexical-y/src/__tests__/unit/regressions/SchemaSkew.test.ts > S7-3, F2: a flat NodeState with no schema > on a block node, reset, stays reset + "test_skew_memo": "old", Test Files 1 failed (1) Tests 41 failed | 66 passed | 4 expected fail (111) ``` ### After ``` ✓ |unit| packages/lexical-y/src/__tests__/unit/regressions/SchemaSkew.test.ts (111 tests) Test Files 1 passed (1) Tests 107 passed | 4 expected fail (111) ``` - `@lexical/y` unit tests: 151 files passed and 2 skipped; 5,144 tests passed, 44 expected fail, 2 skipped (on the merge: 151 files passed and 2 skipped; 5,094 tests passed, 42 expected fail, 2 skipped). - Golden corpus: the fuzz's five lanes (no moves, block ids, gc, moves, block ids with moves), 250 seeds each, compared with the merge for every peer's Yjs state bytes, editor JSON, warnings, fuzz findings, node keys and selection. Editor JSON, warnings, findings, keys and selection are identical on all 1,250 seeds. Bytes differ on 3 seeds in each lane without moves and 2 in each lane with moves, all from the exception above. - `pnpm run ci-check` passes.
…in the document its editor shows
## Description
A block an older client moves keeps the fields it does not know: the write
reads what the block's marker or node stored before this transaction
(`attrsBefore`) and carries the fields it does not own to the block's new
storage. That read walked raw Yjs. When the local update waited for a
remote change the binding had not read yet (see `Unread`), the moved block
took the unread change's values, so the room ended differently from the same
edits written before the change arrived.
`attrsBefore` moves to `Unread.ts`, next to the other reads of the document
the write sees. It skips what an unread change inserted, as it skips what
this transaction inserted, and counts as there before the transaction what
the transaction deleted or only noted as deleted (`deleteItem`, for an item
an unread change deleted too), but not what an earlier write noted. It is
the one owner of "the attributes before this write"; `$markerBefore` and
`nodeBefore` read through it.
## Test plan
### Before
```
$ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/SchemaSkew.test.ts -t N2
✓ an item appended to it, in a list a remote change retones
✓ an item added before its first item, in a list a remote change retones
× a note that a remote change retones, moved down
× a card that a remote change retones, moved down
× a list that a remote change retones, moved down
```
With a removal any earlier write noted counted as there:
```
$ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/Unread.test.ts
× N2: before a write, an attribute an earlier write removed is gone, one this write removed is not
AssertionError: expected { 'p:future': 'keep' } to deeply equal {}
```
### After
```
$ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/SchemaSkew.test.ts packages/lexical-y/src/__tests__/unit/Unread.test.ts
Test Files 2 passed (2)
```
… an older client splits keeps a field it does not know on both lines
## Description
A client that splits a line whose marker stores a field none of its classes
declare cannot tell whether the newer class copies that field to the new
line. With this PR's rule, the write keeps the field on both lines: one line
keeps the old marker, and the marker the write makes for the other takes
what the old one stored (`$markerBefore`). The empty line Enter makes at the
end of a line is the exception: it has what the editor shows. Keeping the
field on both lines loses nothing; storing it once would need to know the
newer class's split behaviour.
The rule had no test for a split that keeps the line's type (the F2 split
tests use a note, whose split makes a paragraph and drops the field), and
y.md did not say it. This adds both. A client that composes as it splits
stores the same (see the composition regression tests).
## Test plan
### After
```
$ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/SchemaSkew.test.ts -t N1
✓ |unit| packages/lexical-y/src/__tests__/unit/regressions/SchemaSkew.test.ts (119 tests | 116 skipped) 309ms
Test Files 1 passed (1)
Tests 3 passed | 116 skipped (119)
```
Enter at the start, inside and at the end of the line store the field on 2,
2 and 1 lines, for a client that does not declare it in either version.
zurfyx
force-pushed
the
zurfyx/y-move-keeps-unknown-fields
branch
from
October 29, 2026 19:33
b2da1ef to
4054696
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 8 of 10. Based on #9504 (
zurfyx/y-preserve-unknown-attrs). 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
An older client of an app (one whose class lacks a field that a newer version of the class added) keeps that field when it edits a node (#9504). A block it moves was different: Yjs has no moves, so the writer deletes the moved block and writes it anew from what the client shows (#9502 chooses which block). The new copy started empty, so the moved block lost every field the client does not own, for every client and for a fresh reader, with no concurrency involved:
tone), and the fields of its list and outer lists on the marker of every item;With this change, a unit written anew takes what its node stored before the write:
$markerBefore, read withattrsBefore: the attributes as they were before the transaction, which Yjs still holds for a marker the transaction deleted). A line that holds lines (stored as parts) reads each part's old marker, or the whole line's for a new part. Where a diff of a container gives a moved line the marker of another line there, the marker takes the moved line's fields and drops the other line's.g:) from the item next to it that kept its marker and was in the list before ([lexical-website][lexical-y] Bug Fix: An older client keeps the fields and content it cannot read #9504's rule, which now also requires the second part), so an item that joins another list takes that list's fields. Where no such item exists, the list moved with the item or lost all its items: the item takes the fields the list stored before, from its own old marker if it was in the list, else from another item's, else from an item the list had then ($priorGroupItem). So a list moved with an item added, swapped list items, and two lists an older client shows as one all keep their own fields. An outer level takes the fields of its item (@) from where that item's were stored, so a nested list moved into a new item takes none of the old item's.nodeBeforein$fillNode).What each lookup finds is kept for the whole write, so a node written twice in one write (moved into a container that is itself written anew) still takes what it had before the first. Only names the client does not own are copied; it writes and deletes its own as before.
Ownership now also counts a flat NodeState with no schema (a plain
parsefunction) where the client writes it as a field: on a node stored as its own Y.Node, and on text (jsonFieldsOf). With #9504, such a state reset to its default was never deleted there, so every other client and a fresh reader kept the old value. Onmainit was deleted.Clients of one version store what they stored before, with one exception, in names that no class of theirs declares on a line (a list item's
checkedleft on a paragraph): those now stay with the line when a diff gives it another line's marker, where before they stayed with the marker.Remaining limits, as
test.failsand in y.md:Not changed here: an item that keeps its marker while its list changes (an indent or outdent of a list's first item) keeps the list fields of its old place, as with #9504.
Restack (October 29)
Rebased from
zurfyx/stack-9502-9504(#9502 with #9504 merged in) onto #9504, which now sits on #9502, so the merge branch drops out; no conflicts.New commits:
attrsBeforemoves toUnread.tsand reads the document the write sees: it skips what an unread remote change inserted, as it skips what this transaction inserted, and counts as there before the transaction only what that transaction removed (Unread.test.ts). A block an older client moves while a remote change waits to be read keeps what its editor shows, so the room ends as when the move is written before the change arrives (N2:move cases inSchemaSkew.test.ts). It is the one owner of "the attributes before this write";$markerBefore,nodeBeforeand [lexical-playground][lexical-website][lexical-y] Bug Fix: Hold back remote changes to the line being composed until the composition ends #9507's held-attribute check read through it.N1:tests inSchemaSkew.test.tspin it.Important
Decision for Gerard (N1): a split by a client that does not own a field. Kept: this PR's rule, the field stays on both halves. Under #9504's ownership rule that loses nothing: the client may not delete the old marker's field, and it cannot tell whether the newer class copies the field to the new line.
The alternative is one owner per stored marker: take the stored predecessor only where the line's old marker did not survive the write, so the field stays once, on the line that kept the old marker. To switch: in
Writer#$writeMarker, takebeforefor a marker the write made only where the line's old end (endsBefore) was deleted by the transaction or no longer ends a live unit; change theN1:expectations from 2 to 1; drop the y.md sentence. #9507's MF-1 test compares composing with not composing, so it holds under either rule.Changeset
.changeset/lexical-y-moved-blocks-keep-fields.md:[fix]for@lexical/y, with aVerify:paragraph.Test plan
regressions/SchemaSkew.test.ts: #9504'stest.fails('limit: a block an older client moves keeps the fields it does not know')is now a passing test. A new F2 group compares each move made by an older client with the same move made by a newer one: lines, lists, block nodes, decorators, nested content, nested lists and inline nodes, moved down, up and into a new container; splits; slots; items moved within and between lists; swapped list items; lists shown as one; lists moved with new items; nested lists moved into new or other items; moves into containers that move too; a line that gets another line's marker; a line holding lines. Another group checks a flat NodeState with no schema, reset in place and on a move../node_modules/.bin/vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SchemaSkew.test.tsBefore
On the merge of #9502 and #9504:
After
@lexical/yunit tests: 151 files passed and 2 skipped; 5,144 tests passed, 44 expected fail, 2 skipped (on the merge: 151 files passed and 2 skipped; 5,094 tests passed, 42 expected fail, 2 skipped).pnpm run ci-checkpasses.