From ab8d2f9d06eae6f92d073a1599c4824cc0828501 Mon Sep 17 00:00:00 2001 From: yousefed Date: Thu, 8 Oct 2026 16:09:55 +0200 Subject: [PATCH 1/4] fix(versioning): diff structural changes made by the old Yjs binding The old binding changed a block's type in place, keeping the old and the new content in one block, which a diff renders as schema-invalid content that is then dropped. Before rendering a diff, replace each block that changed structurally since the earlier version with a copy, as the current binding stores such a change, so it shows as a deleted and an inserted block. The copy and the deletion keep the change's authors. The versioning snapshot shows that this also splits tables two users resize concurrently (to revisit). --- .../y/extensions/legacyYjsDocBinding.test.ts | 3 +- .../core/src/y/extensions/snapshotPreview.ts | 132 ++++++++++- .../__snapshots__/versioning.test.tsx.snap | 221 ++++++++++++------ 3 files changed, 281 insertions(+), 75 deletions(-) diff --git a/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts b/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts index 5eb2355209..c4444fdfab 100644 --- a/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts +++ b/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts @@ -458,8 +458,7 @@ describe("legacy Yjs document binding", () => { }; } - // To be fixed by #3173. - it.fails.each(structuralChanges)( + it.each(structuralChanges)( "diffs $name made with the old binding like one made with the new binding", ({ blocks, change }) => { const { current, old } = diffsOfBothBindings(blocks, change); diff --git a/packages/core/src/y/extensions/snapshotPreview.ts b/packages/core/src/y/extensions/snapshotPreview.ts index b619689062..7d4721deb4 100644 --- a/packages/core/src/y/extensions/snapshotPreview.ts +++ b/packages/core/src/y/extensions/snapshotPreview.ts @@ -2,11 +2,123 @@ import { configureYProsemirror } from "@y/prosemirror"; import * as Y from "@y/y"; import type { BlockNoteEditor } from "../../editor/BlockNoteEditor.js"; +import { findTypeInOtherYdoc } from "../utils.js"; +import { blockMatchNodes } from "./blockMatchNodes.js"; import { decodeFragmentUpdate, destroyDecodedFragment, } from "./snapshotCodec.js"; +/** Whether the item with this id was already in `baseline`. */ +function inBaseline(baseline: Y.Doc, id: Y.ID): boolean { + const last = baseline.store.clients.get(id.client)?.at(-1); + return last !== undefined && id.clock < last.id.clock + last.length; +} + +/** The distinct attributions recorded for `node` and its descendants. */ +function subtreeAttributions( + node: Y.Node, + map: Y.IdMap, +): Y.ContentAttribute[] { + const found = new Map>(); + for (const structs of node.doc!.store.clients.values()) { + for (const item of structs) { + if ( + item instanceof Y.Item && + (item === node._item || Y.isParentOf(node, item)) + ) { + for (const range of map.slice( + item.id.client, + item.id.clock, + item.length, + )) { + for (const attr of range.attrs ?? []) { + found.set(`${attr.name}:${String(attr.val)}`, attr); + } + } + } + } + } + return [...found.values()]; +} + +/** + * Replace each block that changed structurally since `baseline` with a fresh + * copy, where "structurally" is what `blockMatchNodes` treats as a different + * block (e.g. a type change). The old collaboration binding stored such changes + * inside the same container, which a diff renders as schema-invalid content + * that is then dropped. A fresh container diffs as a deleted block next to an + * inserted one, as the current binding stores it. The copy and the deletion + * take over the original's attributions, so the change keeps its author. + */ +function splitChangedBlocks( + node: Y.Node, + baseline: Y.Doc, + attributions: Y.ContentMap | undefined, + added: Y.ContentMap, +): void { + for (let index = 0; index < node.length; index++) { + const child = node.get(index); + if (!(child instanceof Y.Node)) { + continue; + } + if (child.name === "blockContainer" && child._item) { + // A block created after the baseline has no previous version. + const before = inBaseline(baseline, child._item.id) + ? findTypeInOtherYdoc(child, baseline) + : undefined; + if ( + before && + !blockMatchNodes(before.toDeltaDeep(), child.toDeltaDeep()) + ) { + const inserted = attributions + ? subtreeAttributions(child, attributions.inserts) + : []; + const deleted = attributions + ? subtreeAttributions(child, attributions.deletes) + : []; + const doc = child.doc!; + // A change that only inserted (or only deleted) still has one author + // for both sides of the split, under that side's attribution kind. + function as( + kind: "insert" | "delete", + attrs: Y.ContentAttribute[], + ) { + return attrs.map((attr) => Y.createContentAttribute(kind, attr.val)); + } + const authors = inserted.length ? inserted : deleted; + function record(tr: Y.Transaction) { + if (authors.length) { + Y.insertIntoIdMap( + added.inserts, + Y.createIdMapFromIdSet(tr.insertSet, as("insert", authors)), + ); + Y.insertIntoIdMap( + added.deletes, + Y.createIdMapFromIdSet( + tr.deleteSet, + as("delete", deleted.length ? deleted : authors), + ), + ); + } + } + doc.on("beforeObserverCalls", record); + try { + doc.transact(() => { + const copy = child.clone(); + node.delete(index); + node.insert(index, [copy]); + }); + } finally { + doc.off("beforeObserverCalls", record); + } + continue; + } + } + splitChangedBlocks(child, baseline, attributions, added); + } +} + /** * Decode a snapshot, diff it against a baseline if given, and render it. * @@ -33,6 +145,22 @@ export function showSnapshotPreview( try { const snapshot = decodeFragmentUpdate(fragment, snapshotContent); try { + let renderAttributions = attributions; + if (baseline) { + const added = Y.createContentMap(); + splitChangedBlocks( + snapshot.fragment, + baseline.doc, + attributions, + added, + ); + if (attributions) { + renderAttributions = Y.createContentMap( + Y.mergeIdMaps([attributions.inserts, added.inserts]), + Y.mergeIdMaps([attributions.deletes, added.deletes]), + ); + } + } editor.exec( configureYProsemirror({ ytype: snapshot.fragment, @@ -40,7 +168,9 @@ export function showSnapshotPreview( ? Y.createDiffRenderer( baseline.doc, snapshot.doc, - attributions ? { attributions } : undefined, + renderAttributions + ? { attributions: renderAttributions } + : undefined, ) : undefined, }), diff --git a/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap b/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap index 243de25f9c..21d3c7ebef 100644 --- a/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap +++ b/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap @@ -2,28 +2,45 @@ exports[`versioning diff: A adds column then row, B adds column 1`] = ` [ - "insert A", - "insert A", - "insert "C1" A", - "insert B", - "insert B", - "insert "D1" B", - "insert A", - "insert A", - "insert "C2" A", - "insert B", - "insert B", - "insert "D2" B", - "insert A", - "insert A", - "insert A", - "insert "A3" A", - "insert A", - "insert A", - "insert "B3" A", - "insert A", - "insert A", - "insert "C3" A", + "delete block "A1B1A2B2" B,A", + "insert block "A1B1C1D1A2B2C2D2A3B3C3" B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A1" B,A", + "insert B,A", + "insert B,A", + "insert "B1" B,A", + "insert B,A", + "insert B,A", + "insert "C1" B,A", + "insert B,A", + "insert B,A", + "insert "D1" B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A2" B,A", + "insert B,A", + "insert B,A", + "insert "B2" B,A", + "insert B,A", + "insert B,A", + "insert "C2" B,A", + "insert B,A", + "insert B,A", + "insert "D2" B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A3" B,A", + "insert B,A", + "insert B,A", + "insert "B3" B,A", + "insert B,A", + "insert B,A", + "insert "C3" B,A", "insert ", "insert ", ] @@ -31,29 +48,46 @@ exports[`versioning diff: A adds column then row, B adds column 1`] = ` exports[`versioning diff: A adds row then column, B adds row 1`] = ` [ - "insert A", - "insert A", - "insert "C1" A", - "insert A", - "insert A", - "insert "C2" A", - "insert A", - "insert A", - "insert A", - "insert "A3" A", - "insert A", - "insert A", - "insert "B3" A", - "insert A", - "insert A", - "insert "C3" A", - "insert B", - "insert B", - "insert B", - "insert "D1" B", - "insert B", - "insert B", - "insert "D2" B", + "delete block "A1B1A2B2" B,A", + "insert block "A1B1C1A2B2C2A3B3C3D1D2" B,A", + "insert
B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A1" B,A", + "insert B,A", + "insert B,A", + "insert "B1" B,A", + "insert B,A", + "insert B,A", + "insert "C1" B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A2" B,A", + "insert B,A", + "insert B,A", + "insert "B2" B,A", + "insert B,A", + "insert B,A", + "insert "C2" B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A3" B,A", + "insert B,A", + "insert B,A", + "insert "B3" B,A", + "insert B,A", + "insert B,A", + "insert "C3" B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "D1" B,A", + "insert B,A", + "insert B,A", + "insert "D2" B,A", "insert ", "insert ", ] @@ -415,19 +449,36 @@ exports[`versioning diff: Add column 1`] = ` exports[`versioning diff: Add column vs add row 1`] = ` [ - "insert A", - "insert A", - "insert "C1" A", - "insert A", - "insert A", - "insert "C2" A", - "insert B", - "insert B", - "insert B", - "insert "A3" B", - "insert B", - "insert B", - "insert "B3" B", + "delete block "A1B1A2B2" B,A", + "insert block "A1B1C1A2B2C2A3B3" B,A", + "insert
B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A1" B,A", + "insert B,A", + "insert B,A", + "insert "B1" B,A", + "insert B,A", + "insert B,A", + "insert "C1" B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A2" B,A", + "insert B,A", + "insert B,A", + "insert "B2" B,A", + "insert B,A", + "insert B,A", + "insert "C2" B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A3" B,A", + "insert B,A", + "insert B,A", + "insert "B3" B,A", "insert ", "insert ", ] @@ -496,19 +547,36 @@ exports[`versioning diff: Add row 1`] = ` exports[`versioning diff: Add row vs add column 1`] = ` [ - "insert B", - "insert B", - "insert "C1" B", - "insert B", - "insert B", - "insert "C2" B", - "insert A", - "insert A", - "insert A", - "insert "A3" A", - "insert A", - "insert A", - "insert "B3" A", + "delete block "A1B1A2B2" B,A", + "insert block "A1B1C1A2B2C2A3B3" B,A", + "insert
B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A1" B,A", + "insert B,A", + "insert B,A", + "insert "B1" B,A", + "insert B,A", + "insert B,A", + "insert "C1" B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A2" B,A", + "insert B,A", + "insert B,A", + "insert "B2" B,A", + "insert B,A", + "insert B,A", + "insert "C2" B,A", + "insert B,A", + "insert B,A", + "insert B,A", + "insert "A3" B,A", + "insert B,A", + "insert B,A", + "insert "B3" B,A", "insert ", "insert ", ] @@ -697,10 +765,19 @@ exports[`versioning diff: Delete parent with mixed children 1`] = ` exports[`versioning diff: Delete row vs add column 1`] = ` [ + "delete block "A1B1A2B2" A", + "insert block "A1B1C1" B", + "insert
B", + "insert B", + "insert B", + "insert B", + "insert "A1" B", + "insert B", + "insert B", + "insert "B1" B", "insert B", "insert B", "insert "C1" B", - "delete A", ] `; From ccd46e0d32e66ad7368d0a71d9e83827c675600b Mon Sep 17 00:00:00 2001 From: yousefed Date: Thu, 8 Oct 2026 21:26:15 +0200 Subject: [PATCH 2/4] fix(collaboration): stop replacing tables that one edit reshapes both ways Replacing such a table silently dropped concurrent edits to it. Version diffs compare all the changes between two versions, so two concurrent one-way reshapes (a row and a column) replaced the whole table there too. Reshapes now merge in place, as one-way reshapes always did. --- .../core/src/y/extensions/blockMatchNodes.ts | 13 +- .../y/extensions/legacyYjsDocBinding.test.ts | 3 +- .../src/y/extensions/tableReshape.test.ts | 3 +- .../__snapshots__/versioning.test.tsx.snap | 221 ++++++------------ 4 files changed, 86 insertions(+), 154 deletions(-) diff --git a/packages/core/src/y/extensions/blockMatchNodes.ts b/packages/core/src/y/extensions/blockMatchNodes.ts index 0e79597a3d..aef824b422 100644 --- a/packages/core/src/y/extensions/blockMatchNodes.ts +++ b/packages/core/src/y/extensions/blockMatchNodes.ts @@ -136,6 +136,8 @@ function getTableDimensions( * @param b inserted (new) node * @returns whether `a` and `b` are the same node (diff in place) vs different (replace) */ +const replaceReshapedTables = false; + export const blockMatchNodes = ( a: schema.Unwrap, b: schema.Unwrap, @@ -165,7 +167,16 @@ export const blockMatchNodes = ( return false; } - if (childA?.name === "table" && childB?.name === "table") { + // Disabled: replacing a table that one edit reshapes both ways silently + // dropped concurrent edits to it, and version diffs compare all the changes + // between two versions, so concurrent one-way reshapes (a row and a column) + // replaced the whole table there. Reshapes merge in place, as one-way ones + // always did. + if ( + replaceReshapedTables && + childA?.name === "table" && + childB?.name === "table" + ) { const dimA = getTableDimensions(childA); const dimB = getTableDimensions(childB); if ( diff --git a/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts b/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts index c4444fdfab..0a4b98ce03 100644 --- a/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts +++ b/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts @@ -467,8 +467,7 @@ describe("legacy Yjs document binding", () => { }, ); - // To be fixed by #3173. - it.fails("diffs a table resize made with the old binding like one made with the new binding", () => { + it("diffs a table resize made with the old binding like one made with the new binding", () => { const { current, old } = diffsOfBothBindings( [table(2, 2)], (editor) => editor.updateBlock("table", table(3, 3)), diff --git a/packages/core/src/y/extensions/tableReshape.test.ts b/packages/core/src/y/extensions/tableReshape.test.ts index 780069874e..a1f622ee66 100644 --- a/packages/core/src/y/extensions/tableReshape.test.ts +++ b/packages/core/src/y/extensions/tableReshape.test.ts @@ -95,8 +95,7 @@ it("stores a table that one edit reshapes in two directions", () => { ); }); -// To be fixed by #3173. -it.fails("keeps a concurrent cell edit when a different user reshapes the table", () => { +it("keeps a concurrent cell edit when a different user reshapes the table", () => { const { a, b, sync } = twoUsers(); a.updateBlock("table", table(grown)); b.updateBlock( diff --git a/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap b/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap index 21d3c7ebef..243de25f9c 100644 --- a/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap +++ b/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap @@ -2,45 +2,28 @@ exports[`versioning diff: A adds column then row, B adds column 1`] = ` [ - "delete block "A1B1A2B2" B,A", - "insert block "A1B1C1D1A2B2C2D2A3B3C3" B,A", - "insert
B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A1" B,A", - "insert B,A", - "insert B,A", - "insert "B1" B,A", - "insert B,A", - "insert B,A", - "insert "C1" B,A", - "insert B,A", - "insert B,A", - "insert "D1" B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A2" B,A", - "insert B,A", - "insert B,A", - "insert "B2" B,A", - "insert B,A", - "insert B,A", - "insert "C2" B,A", - "insert B,A", - "insert B,A", - "insert "D2" B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A3" B,A", - "insert B,A", - "insert B,A", - "insert "B3" B,A", - "insert B,A", - "insert B,A", - "insert "C3" B,A", + "insert A", + "insert A", + "insert "C1" A", + "insert B", + "insert B", + "insert "D1" B", + "insert A", + "insert A", + "insert "C2" A", + "insert B", + "insert B", + "insert "D2" B", + "insert A", + "insert A", + "insert A", + "insert "A3" A", + "insert A", + "insert A", + "insert "B3" A", + "insert A", + "insert A", + "insert "C3" A", "insert ", "insert ", ] @@ -48,46 +31,29 @@ exports[`versioning diff: A adds column then row, B adds column 1`] = ` exports[`versioning diff: A adds row then column, B adds row 1`] = ` [ - "delete block "A1B1A2B2" B,A", - "insert block "A1B1C1A2B2C2A3B3C3D1D2" B,A", - "insert
B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A1" B,A", - "insert B,A", - "insert B,A", - "insert "B1" B,A", - "insert B,A", - "insert B,A", - "insert "C1" B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A2" B,A", - "insert B,A", - "insert B,A", - "insert "B2" B,A", - "insert B,A", - "insert B,A", - "insert "C2" B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A3" B,A", - "insert B,A", - "insert B,A", - "insert "B3" B,A", - "insert B,A", - "insert B,A", - "insert "C3" B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "D1" B,A", - "insert B,A", - "insert B,A", - "insert "D2" B,A", + "insert A", + "insert A", + "insert "C1" A", + "insert A", + "insert A", + "insert "C2" A", + "insert A", + "insert A", + "insert A", + "insert "A3" A", + "insert A", + "insert A", + "insert "B3" A", + "insert A", + "insert A", + "insert "C3" A", + "insert B", + "insert B", + "insert B", + "insert "D1" B", + "insert B", + "insert B", + "insert "D2" B", "insert ", "insert ", ] @@ -449,36 +415,19 @@ exports[`versioning diff: Add column 1`] = ` exports[`versioning diff: Add column vs add row 1`] = ` [ - "delete block "A1B1A2B2" B,A", - "insert block "A1B1C1A2B2C2A3B3" B,A", - "insert
B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A1" B,A", - "insert B,A", - "insert B,A", - "insert "B1" B,A", - "insert B,A", - "insert B,A", - "insert "C1" B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A2" B,A", - "insert B,A", - "insert B,A", - "insert "B2" B,A", - "insert B,A", - "insert B,A", - "insert "C2" B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A3" B,A", - "insert B,A", - "insert B,A", - "insert "B3" B,A", + "insert A", + "insert A", + "insert "C1" A", + "insert A", + "insert A", + "insert "C2" A", + "insert B", + "insert B", + "insert B", + "insert "A3" B", + "insert B", + "insert B", + "insert "B3" B", "insert ", "insert ", ] @@ -547,36 +496,19 @@ exports[`versioning diff: Add row 1`] = ` exports[`versioning diff: Add row vs add column 1`] = ` [ - "delete block "A1B1A2B2" B,A", - "insert block "A1B1C1A2B2C2A3B3" B,A", - "insert
B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A1" B,A", - "insert B,A", - "insert B,A", - "insert "B1" B,A", - "insert B,A", - "insert B,A", - "insert "C1" B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A2" B,A", - "insert B,A", - "insert B,A", - "insert "B2" B,A", - "insert B,A", - "insert B,A", - "insert "C2" B,A", - "insert B,A", - "insert B,A", - "insert B,A", - "insert "A3" B,A", - "insert B,A", - "insert B,A", - "insert "B3" B,A", + "insert B", + "insert B", + "insert "C1" B", + "insert B", + "insert B", + "insert "C2" B", + "insert A", + "insert A", + "insert A", + "insert "A3" A", + "insert A", + "insert A", + "insert "B3" A", "insert ", "insert ", ] @@ -765,19 +697,10 @@ exports[`versioning diff: Delete parent with mixed children 1`] = ` exports[`versioning diff: Delete row vs add column 1`] = ` [ - "delete block "A1B1A2B2" A", - "insert block "A1B1C1" B", - "insert
B", - "insert B", - "insert B", - "insert B", - "insert "A1" B", - "insert B", - "insert B", - "insert "B1" B", "insert B", "insert B", "insert "C1" B", + "delete A", ] `; From a5918441a871ddbd44d58a9478af690ddc29dc0a Mon Sep 17 00:00:00 2001 From: yousefed Date: Thu, 8 Oct 2026 22:32:58 +0200 Subject: [PATCH 3/4] fix(versioning): credit an old-binding structural change only to whoever made it The split credited every attribution found anywhere in the block to both sides, timestamps included: `insertAt` became an "insert" by a user named like the time. It now credits the content nodes and child groups that changed the block, with their own attributions. The split only changes the decoded snapshot; a test checks that the document stays unchanged. --- .../y/extensions/legacyYjsDocBinding.test.ts | 3 +- .../core/src/y/extensions/snapshotPreview.ts | 97 ++++++++++--------- 2 files changed, 54 insertions(+), 46 deletions(-) diff --git a/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts b/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts index 0a4b98ce03..08d8aa4ff8 100644 --- a/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts +++ b/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts @@ -494,8 +494,7 @@ describe("legacy Yjs document binding", () => { expect(Y.encodeStateAsUpdateV2(opened.doc)).toEqual(stored); }); - // To be fixed by #3173. - it.fails("credits a type change made with the old binding only to whoever made it", () => { + it("credits a type change made with the old binding only to whoever made it", () => { const legacy = createLegacyEditor(); legacy.editor.replaceBlocks(legacy.editor.document, [ { id: "changed", type: "paragraph", content: "Text" }, diff --git a/packages/core/src/y/extensions/snapshotPreview.ts b/packages/core/src/y/extensions/snapshotPreview.ts index 7d4721deb4..7075eebe52 100644 --- a/packages/core/src/y/extensions/snapshotPreview.ts +++ b/packages/core/src/y/extensions/snapshotPreview.ts @@ -15,31 +15,46 @@ function inBaseline(baseline: Y.Doc, id: Y.ID): boolean { return last !== undefined && id.clock < last.id.clock + last.length; } -/** The distinct attributions recorded for `node` and its descendants. */ -function subtreeAttributions( - node: Y.Node, - map: Y.IdMap, -): Y.ContentAttribute[] { - const found = new Map>(); - for (const structs of node.doc!.store.clients.values()) { - for (const item of structs) { - if ( - item instanceof Y.Item && - (item === node._item || Y.isParentOf(node, item)) - ) { - for (const range of map.slice( - item.id.client, - item.id.clock, - item.length, - )) { - for (const attr of range.attrs ?? []) { - found.set(`${attr.name}:${String(attr.val)}`, attr); - } - } +/** + * The attributions of what changed `block`'s structure since `baseline`: its + * content nodes and child groups that are new, or deleted, since then. Its + * other content (e.g. text typed in it) doesn't make the change. + */ +function structuralAttributions( + block: Y.Node, + baseline: Y.Doc, + attributions: Y.ContentMap, +): { inserted: Y.ContentAttribute[]; deleted: Y.ContentAttribute[] } { + const inserted = new Map>(); + const deleted = new Map>(); + for (let item = block._start; item !== null; item = item.right) { + const isNew = !inBaseline(baseline, item.id); + if (!isNew && !item.deleted) { + continue; + } + const [found, map] = isNew + ? [inserted, attributions.inserts] + : [deleted, attributions.deletes]; + for (const range of map.slice(item.id.client, item.id.clock, item.length)) { + for (const attr of range.attrs ?? []) { + found.set(`${attr.name}:${String(attr.val)}`, attr); } } } - return [...found.values()]; + return { inserted: [...inserted.values()], deleted: [...deleted.values()] }; +} + +/** Attributions as the other kind: `insert`/`insertAt` as `delete`/`deleteAt`. */ +function asKind( + attrs: Y.ContentAttribute[], + kind: "insert" | "delete", +): Y.ContentAttribute[] { + return attrs.map((attr) => + Y.createContentAttribute( + attr.name.replace(/^(insert|delete)/, kind), + attr.val, + ), + ); } /** @@ -49,7 +64,8 @@ function subtreeAttributions( * inside the same container, which a diff renders as schema-invalid content * that is then dropped. A fresh container diffs as a deleted block next to an * inserted one, as the current binding stores it. The copy and the deletion - * take over the original's attributions, so the change keeps its author. + * are credited to whoever changed the block's structure (see + * {@link structuralAttributions}). */ function splitChangedBlocks( node: Y.Node, @@ -71,34 +87,27 @@ function splitChangedBlocks( before && !blockMatchNodes(before.toDeltaDeep(), child.toDeltaDeep()) ) { - const inserted = attributions - ? subtreeAttributions(child, attributions.inserts) - : []; - const deleted = attributions - ? subtreeAttributions(child, attributions.deletes) - : []; + const { inserted, deleted } = attributions + ? structuralAttributions(child, baseline, attributions) + : { inserted: [], deleted: [] }; + // `node` is a decoded snapshot's, never the live document, so the split + // doesn't reach the stored document. const doc = child.doc!; - // A change that only inserted (or only deleted) still has one author - // for both sides of the split, under that side's attribution kind. - function as( - kind: "insert" | "delete", - attrs: Y.ContentAttribute[], - ) { - return attrs.map((attr) => Y.createContentAttribute(kind, attr.val)); - } - const authors = inserted.length ? inserted : deleted; + // A change that only inserted (or only deleted) still credits both + // sides of the split, under that side's attribution kind. + const insertedAs = inserted.length + ? inserted + : asKind(deleted, "insert"); + const deletedAs = deleted.length ? deleted : asKind(inserted, "delete"); function record(tr: Y.Transaction) { - if (authors.length) { + if (insertedAs.length) { Y.insertIntoIdMap( added.inserts, - Y.createIdMapFromIdSet(tr.insertSet, as("insert", authors)), + Y.createIdMapFromIdSet(tr.insertSet, insertedAs), ); Y.insertIntoIdMap( added.deletes, - Y.createIdMapFromIdSet( - tr.deleteSet, - as("delete", deleted.length ? deleted : authors), - ), + Y.createIdMapFromIdSet(tr.deleteSet, deletedAs), ); } } From a7e23b9ebb10de23c7224dec60cf19810e41b2bb Mon Sep 17 00:00:00 2001 From: yousefed Date: Fri, 9 Oct 2026 16:14:57 +0200 Subject: [PATCH 4/4] fix(versioning): credit text typed after an old-binding structural change to its writer The split replaced the block with a clone and credited all of the clone to whoever changed the block's structure, including text that someone else typed into the block later. The clone is now paired with the original unit by unit (it has the same content in the same order), and content added after the baseline keeps its own author. --- .../y/extensions/legacyYjsDocBinding.test.ts | 34 ++++++++ .../core/src/y/extensions/snapshotPreview.ts | 86 ++++++++++++++++++- 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts b/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts index 08d8aa4ff8..9f067ca9c2 100644 --- a/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts +++ b/packages/core/src/y/extensions/legacyYjsDocBinding.test.ts @@ -526,6 +526,40 @@ describe("legacy Yjs document binding", () => { ]); }); + it("credits text typed after an old-binding type change to whoever typed it", () => { + const legacy = createLegacyEditor(); + legacy.editor.replaceBlocks(legacy.editor.document, [ + { id: "changed", type: "paragraph", content: "Text" }, + ]); + const before = Y1.encodeStateAsUpdateV2(legacy.doc); + legacy.editor.updateBlock("changed", { type: "heading" }); + const byBob = Y1.encodeStateAsUpdateV2(legacy.doc); + legacy.editor.setTextCursorPosition("changed", "end"); + legacy.editor.insertInlineContent(" by Alice"); + const after = Y1.encodeStateAsUpdateV2(legacy.doc); + const opened = openWithNewBinding(after); + + const changed = diffBlocks( + opened.editor, + opened.doc, + before, + after, + "all", + attributionsOfSteps(before, [ + { state: byBob, user: "bob", time: 2000 }, + { state: after, user: "alice", time: 3000 }, + ]), + ); + expect( + changed + .filter(({ type }) => type === "text") + .map(({ change, text, users }) => [change, text, users]), + ).toEqual([ + ["y-attributed-insert", "Text", ["bob"]], + ["y-attributed-insert", " by Alice", ["alice"]], + ]); + }); + it("diffs a text edit made with the old binding in place", () => { const legacy = createLegacyEditor(); legacy.editor.replaceBlocks(legacy.editor.document, [ diff --git a/packages/core/src/y/extensions/snapshotPreview.ts b/packages/core/src/y/extensions/snapshotPreview.ts index 7075eebe52..7f801cf2e0 100644 --- a/packages/core/src/y/extensions/snapshotPreview.ts +++ b/packages/core/src/y/extensions/snapshotPreview.ts @@ -44,6 +44,59 @@ function structuralAttributions( return { inserted: [...inserted.values()], deleted: [...deleted.values()] }; } +/** + * The units of a node's live content, in order: one per character or embed, + * and one per child node. + */ +function liveUnits(node: Y.Node): Array<{ id: Y.ID; node?: Y.Node }> { + const units: Array<{ id: Y.ID; node?: Y.Node }> = []; + for (let item = node._start; item !== null; item = item.right) { + if (item.deleted) { + continue; + } + if (item.content instanceof Y.ContentType) { + units.push({ id: item.id, node: item.content.type }); + continue; + } + for (let i = 0; i < item.length; i++) { + units.push({ id: Y.createID(item.id.client, item.id.clock + i) }); + } + } + return units; +} + +/** + * Pair each unit and attribute of `original` with the same one in `copy`, its + * clone. Returns false if they don't line up, which a clone always should. + */ +function pairClone( + original: Y.Node, + copy: Y.Node, + pairs: Array<[Y.ID, Y.ID]>, +): boolean { + for (const [key, item] of original._map) { + const cloned = copy._map.get(key); + if (!item.deleted) { + if (!cloned) { + return false; + } + pairs.push([item.id, cloned.id]); + } + } + const a = liveUnits(original); + const b = liveUnits(copy); + if (a.length !== b.length) { + return false; + } + return a.every((unit, i) => { + const other = b[i]; + pairs.push([unit.id, other.id]); + return unit.node && other.node + ? pairClone(unit.node, other.node, pairs) + : !unit.node && !other.node; + }); +} + /** Attributions as the other kind: `insert`/`insertAt` as `delete`/`deleteAt`. */ function asKind( attrs: Y.ContentAttribute[], @@ -99,11 +152,21 @@ function splitChangedBlocks( ? inserted : asKind(deleted, "insert"); const deletedAs = deleted.length ? deleted : asKind(inserted, "delete"); + // Copied content added after the baseline keeps its own author (e.g. + // text typed after the structural change). + const own: Y.IdMap = Y.createIdMap(); + const owned = Y.createIdSet(); function record(tr: Y.Transaction) { if (insertedAs.length) { Y.insertIntoIdMap( added.inserts, - Y.createIdMapFromIdSet(tr.insertSet, insertedAs), + Y.mergeIdMaps([ + Y.diffIdMap( + Y.createIdMapFromIdSet(tr.insertSet, insertedAs), + owned, + ), + own, + ]), ); Y.insertIntoIdMap( added.deletes, @@ -115,8 +178,27 @@ function splitChangedBlocks( try { doc.transact(() => { const copy = child.clone(); + node.insert(index + 1, [copy]); + // Paired while the original is still live. + const pairs: Array<[Y.ID, Y.ID]> = []; + if (attributions && pairClone(child, copy, pairs)) { + for (const [original, cloned] of pairs) { + const attrs = inBaseline(baseline, original) + ? undefined + : attributions.inserts.slice( + original.client, + original.clock, + 1, + )[0]?.attrs; + if (attrs) { + const ids = Y.createIdSet(); + ids.add(cloned.client, cloned.clock, 1); + owned.add(cloned.client, cloned.clock, 1); + Y.insertIntoIdMap(own, Y.createIdMapFromIdSet(ids, attrs)); + } + } + } node.delete(index); - node.insert(index, [copy]); }); } finally { doc.off("beforeObserverCalls", record);