From 843f8568762a31f8aa79a50ffd5b392a72548a69 Mon Sep 17 00:00:00 2001 From: yousefed Date: Thu, 20 Aug 2026 16:05:57 +0200 Subject: [PATCH 1/3] fix(core): stop fixTables from rewriting rendered diffs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit prosemirror-tables' tableEditing() appends a normalizing fixTables transaction whenever it sees an inconsistent table — and a rendered diff shows inconsistent tables on purpose (deleted/inserted row and column copies). Letting it run while a diff is displayed rewrote the very diff being shown, wrote the normalization back into the suggestion Y-doc through the read-only merged viewer's binding (the phantom empty cells in the concurrent-table snapshots), and could crash the merge outright (the 'Delete row vs add column' gallery scenario: fixTables fed y-prosemirror a delta Yjs can't apply). Read-only editors shouldn't self-normalize at all: block fixTables transactions while the editor isn't editable, mirroring AIExtension's ai-writing gate. --- .../core/src/blocks/Table/TableExtension.ts | 18 ++++- .../tables.concurrent.test.tsx.snap | 76 ------------------- 2 files changed, 16 insertions(+), 78 deletions(-) diff --git a/packages/core/src/blocks/Table/TableExtension.ts b/packages/core/src/blocks/Table/TableExtension.ts index 70cea2ee9f..3e59b733d6 100644 --- a/packages/core/src/blocks/Table/TableExtension.ts +++ b/packages/core/src/blocks/Table/TableExtension.ts @@ -1,7 +1,8 @@ import { callOrReturn, Extension, getExtensionField } from "@tiptap/core"; -import { TextSelection } from "prosemirror-state"; +import { Plugin, PluginKey, TextSelection } from "prosemirror-state"; import { columnResizing, + fixTablesKey, goToNextCell, isInTable, moveCellForward, @@ -17,7 +18,8 @@ export const EMPTY_CELL_HEIGHT = 31; export const TableExtension = Extension.create({ name: "BlockNoteTableExtension", - addProseMirrorPlugins: () => { + addProseMirrorPlugins() { + const editor = this.editor; return [ columnResizing({ cellMinWidth: RESIZE_MIN_WIDTH, @@ -28,6 +30,18 @@ export const TableExtension = Extension.create({ View: null, }), tableEditing(), + new Plugin({ + key: new PluginKey("blocknote-fix-tables-gate"), + // `tableEditing()` appends a normalizing `fixTables` transaction + // whenever it sees an "inconsistent" table. A rendered diff shows + // inconsistent tables *on purpose* (deleted row/column copies next to + // their replacements), and the editor is read-only while it does — so + // letting the fix run would silently rewrite the very diff being + // displayed. A read-only editor shouldn't self-normalize at all: + // block `fixTables` transactions while the editor isn't editable. + filterTransaction: (tr) => + !(tr.getMeta(fixTablesKey) && !editor.isEditable), + }), ]; }, diff --git a/tests/src/end-to-end/y-prosemirror/__snapshots__/tables.concurrent.test.tsx.snap b/tests/src/end-to-end/y-prosemirror/__snapshots__/tables.concurrent.test.tsx.snap index 09afa5be55..f13cc3632a 100644 --- a/tests/src/end-to-end/y-prosemirror/__snapshots__/tables.concurrent.test.tsx.snap +++ b/tests/src/end-to-end/y-prosemirror/__snapshots__/tables.concurrent.test.tsx.snap @@ -290,16 +290,6 @@ exports[`concurrent: A adds a column, B adds a row 4`] = ` > B3 - - - @@ -413,15 +403,6 @@ exports[`concurrent: A adds a column, B adds a row 5`] = ` - - - @@ -720,16 +701,6 @@ exports[`concurrent: A adds a row, B adds a column 4`] = ` > B3 - - - @@ -843,15 +814,6 @@ exports[`concurrent: A adds a row, B adds a column 5`] = ` - - - @@ -1523,16 +1485,6 @@ exports[`sequential: A adds a column then a row, B adds a column 4`] = ` > C3 - - - @@ -1691,15 +1643,6 @@ exports[`sequential: A adds a column then a row, B adds a column 5`] = ` - - - @@ -2062,16 +2005,6 @@ exports[`sequential: A adds a row then a column, B adds a row 4`] = ` > D2 - - - @@ -2234,15 +2167,6 @@ exports[`sequential: A adds a row then a column, B adds a row 5`] = ` - - - From 859dcc4bdd2aac0e98d5d3d5b544edb971a4573b Mon Sep 17 00:00:00 2001 From: yousefed Date: Thu, 20 Aug 2026 16:05:59 +0200 Subject: [PATCH 2/3] fix(core): pair blocks by id in blockMatchNodes; drop table-dimension rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two containers with different block ids are different blocks, no matter how similar their content — pairing them diffed one block into the other in place, which rendered e.g. a full-document replacement as edits inside the first deleted block instead of a separately inserted one. Only enforced when both sides carry an id (delta attrs are op-wrapped; compare values), so id-less external content still pairs by shape. The table-dimension pairing rule (replace the table when both dimensions changed) is removed: with fixTables gated in diff mode the rendered inconsistent tables it guarded against are displayed as-is, and the rule produced no observable behavior differences in the suite. The DiffVersioning test fixture now gives its before/after docs a stable id — they model one block at two points in time, which is the only shape real versioning inputs have (snapshots preserve ids across versions). --- .../DiffVersioningExtension.test.ts | 8 +- .../core/src/y/extensions/blockMatchNodes.ts | 99 +++---------------- .../addRemoveBlocks.test.tsx.snap | 21 +++- 3 files changed, 36 insertions(+), 92 deletions(-) diff --git a/packages/core/src/y/extensions/DiffVersioningExtension.test.ts b/packages/core/src/y/extensions/DiffVersioningExtension.test.ts index 968193b2bd..8c8a988b8d 100644 --- a/packages/core/src/y/extensions/DiffVersioningExtension.test.ts +++ b/packages/core/src/y/extensions/DiffVersioningExtension.test.ts @@ -27,7 +27,13 @@ function createDiffEditor() { function blocksFromText(text: string): Block[] { const e = BlockNoteEditor.create(); e.mount(document.createElement("div")); - e.replaceBlocks(e.document, [{ type: "paragraph", content: text }]); + // Stable id: the before/after docs a diff compares represent the *same* + // logical block at two points in time, and blocks keep their id across + // edits. Minting a fresh id per call would make `blockMatchNodes` treat the + // two versions as different blocks (replace) instead of one edited block. + e.replaceBlocks(e.document, [ + { id: "diff-block", type: "paragraph", content: text }, + ]); const blocks = e.document; e.unmount(); return blocks; diff --git a/packages/core/src/y/extensions/blockMatchNodes.ts b/packages/core/src/y/extensions/blockMatchNodes.ts index 0e79597a3d..f02d2959bf 100644 --- a/packages/core/src/y/extensions/blockMatchNodes.ts +++ b/packages/core/src/y/extensions/blockMatchNodes.ts @@ -41,79 +41,6 @@ const hasBlockGroup = (d: schema.Unwrap): boolean => { return false; }; -function getTableDimensions( - d: schema.Unwrap, -): { rows: number; cols: number } | null { - if (d.name !== "table") { - return null; - } - - // Collect all rows with their cells' colspan/rowspan values. - const rows: Array> = []; - for (const op of (d as any).children) { - if (delta.$insertOp.check(op)) { - for (const tr of op.insert as Array< - schema.Unwrap - >) { - if (tr.name !== "tableRow") { - return null; - } - const cells: Array<{ colspan: number; rowspan: number }> = []; - for (const trOp of (tr as any).children) { - if (delta.$insertOp.check(trOp)) { - for (const td of trOp.insert as Array< - schema.Unwrap - >) { - if (td.name !== "tableCell" && td.name !== "tableHeader") { - return null; - } - cells.push({ - colspan: Number(td.attrs.colspan) || 1, - rowspan: Number(td.attrs.rowspan) || 1, - }); - } - } - } - rows.push(cells); - } - } - } - - if (rows.length === 0) { - return null; - } - - // Build an occupancy grid to determine the true column count. - // Each entry in `grid[r]` tracks which columns are already occupied - // (by a cell from a previous row with rowspan > 1). - const grid: boolean[][] = []; - for (let r = 0; r < rows.length; r++) { - if (!grid[r]) { - grid[r] = []; - } - let col = 0; - for (const cell of rows[r]) { - // Skip columns already occupied by a rowspan from above. - while (grid[r][col]) { - col++; - } - // Mark all slots this cell occupies. - for (let dr = 0; dr < cell.rowspan; dr++) { - if (!grid[r + dr]) { - grid[r + dr] = []; - } - for (let dc = 0; dc < cell.colspan; dc++) { - grid[r + dr][col + dc] = true; - } - } - col += cell.colspan; - } - } - - const numCols = Math.max(...grid.map((row) => row.length)); - return { rows: rows.length, cols: numCols }; -} - /** * BlockNote's node-pairing policy for y-prosemirror's `matchNodes` option * (forwarded to `lib0/delta.diff`). This is the schema-specific bit that lives @@ -148,6 +75,19 @@ export const blockMatchNodes = ( return true; } + // Two containers with *different* block ids are different blocks, no matter + // how similar their content — pairing them would diff one block into the + // other in place. That rendered e.g. a full-document replacement as edits + // *inside* the first deleted block instead of a separately inserted one. + // Only enforced when both sides carry an id, so content converted from + // outside the editor (which may not have ids yet) still pairs by shape. + // Delta attrs are op-wrapped ({ type, value }) — compare the values. + const idA = (a as any).attrs?.id?.value; + const idB = (b as any).attrs?.id?.value; + if (idA && idB && idA !== idB) { + return false; + } + const childA = firstChild(a); const childB = firstChild(b); @@ -165,18 +105,5 @@ export const blockMatchNodes = ( return false; } - if (childA?.name === "table" && childB?.name === "table") { - const dimA = getTableDimensions(childA); - const dimB = getTableDimensions(childB); - if ( - dimA !== null && - dimB !== null && - dimA.rows !== dimB.rows && - dimA.cols !== dimB.cols - ) { - return false; - } - } - return true; }; diff --git a/tests/src/end-to-end/y-prosemirror/__snapshots__/addRemoveBlocks.test.tsx.snap b/tests/src/end-to-end/y-prosemirror/__snapshots__/addRemoveBlocks.test.tsx.snap index 6c2bebcac8..d280ceacb8 100644 --- a/tests/src/end-to-end/y-prosemirror/__snapshots__/addRemoveBlocks.test.tsx.snap +++ b/tests/src/end-to-end/y-prosemirror/__snapshots__/addRemoveBlocks.test.tsx.snap @@ -978,11 +978,22 @@ exports[`suggestion mode: remove all blocks 2`] = ` exports[`suggestion mode: remove all blocks 3`] = ` " - - - Only block - - + + + + + Only block + + + + + + + + + + + " `; From c3ff80e0b71a178b203009ab507c9a16f3d37070 Mon Sep 17 00:00:00 2001 From: yousefed Date: Thu, 20 Aug 2026 16:06:01 +0200 Subject: [PATCH 3/3] docs(examples): refresh table and delete-all feedback notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 'Delete a whole document': resolved note removed (the replacement block now renders as its own insert below the deleted document). - 'Merge cells': phantom-deleted-column note removed (it was the fixTables write-back, now gated). - 'Delete row vs add column': no longer crashes with the fixTables gate (verified in-browser) — knownCrash flag and note removed. - All six concurrent table scenarios share one low-severity note: table merges aren't conflict-free (pre-existing collaboration limitation, not diff-specific; a dedicated table CRDT is the follow-up). --- .../14-suggestion-gallery/src/scenarios.ts | 49 ++++++++----------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts b/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts index df7f558f2b..8b2bfecb2c 100644 --- a/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts +++ b/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts @@ -90,6 +90,20 @@ export const IMG_SRC_BASE = export const IMG_SRC_NEW = "data:image/svg+xml;utf8,"; +// Shared note for the concurrent table scenarios: structural table merges are +// a known pre-existing limitation of the collaboration layer, independent of +// the suggestion/diff rendering. +const CONCURRENT_TABLE_FEEDBACK: Feedback[] = [ + { + severity: "low", + note: + "Concurrent structural edits to the same table don't merge " + + "conflict-free — simultaneous row/column changes can combine into an " + + "inconsistent table. A follow-up is a dedicated table CRDT in Yjs + " + + "BlockNote. (Pre-existing collaboration issue, not specific to diffs.)", + }, +]; + // Shared 2×2 table baseline used by most of the table scenarios. const TABLE_2X2 = { id: "table", @@ -895,12 +909,6 @@ export const scenarios: SuggestionScenario[] = [ { kind: "single", id: "table-merge-cells", - feedback: [ - { - severity: "low", - note: "The diff shows a phantom extra 'deleted column' that isn't actually part of the merge.", - }, - ], title: "Merge cells", category: "Tables", description: "Merge the two top-row cells into one (colspan 2).", @@ -1149,6 +1157,7 @@ export const scenarios: SuggestionScenario[] = [ { kind: "concurrent", id: "concurrent-table-row-and-column", + feedback: CONCURRENT_TABLE_FEEDBACK, title: "Add row vs add column", category: "Tables", description: "A adds a row while B adds a column.", @@ -1177,6 +1186,7 @@ export const scenarios: SuggestionScenario[] = [ { kind: "concurrent", id: "concurrent-table-addcol-vs-addrow", + feedback: CONCURRENT_TABLE_FEEDBACK, title: "Add column vs add row", category: "Tables", description: "A adds a column while B adds a row.", @@ -1205,18 +1215,10 @@ export const scenarios: SuggestionScenario[] = [ { kind: "concurrent", id: "concurrent-table-row-vs-column", - feedback: [ - { - severity: "high", - note: "Crashes — prosemirror-tables' fixTables treats the suggestion-marked table as malformed and feeds y-prosemirror a delta Yjs can't apply (lib0 'Unexpected case'). Confirmed via a fixTables on/off loop (25/25 crashes on, 0/25 off); fix is to block fixTablesKey transactions while suggestions are active, mirroring AIExtension during ai-writing.", - }, - ], + feedback: CONCURRENT_TABLE_FEEDBACK, title: "Delete row vs add column", category: "Tables", - description: - "A deletes a row while B adds a column — known to crash the merge " + - "(prosemirror-tables fixTables).", - knownCrash: true, + description: "A deletes a row while B adds a column.", initial: [TABLE_2X2], applyA: (editor) => editor.updateBlock("table", { @@ -1237,12 +1239,7 @@ export const scenarios: SuggestionScenario[] = [ kind: "concurrent", id: "concurrent-table-delcol-vs-addrow", title: "Delete column vs add row", - feedback: [ - { - severity: "high", - note: "Diff seems weird and A2 in wrong place", - }, - ], + feedback: CONCURRENT_TABLE_FEEDBACK, category: "Tables", description: "A deletes a column while B adds a row.", initial: [TABLE_2X2], @@ -1270,6 +1267,7 @@ export const scenarios: SuggestionScenario[] = [ { kind: "concurrent", id: "concurrent-table-seq-col-then-row", + feedback: CONCURRENT_TABLE_FEEDBACK, title: "A adds column then row, B adds column", category: "Tables", description: "A adds a column and then a row (two edits); B adds a column.", @@ -1306,6 +1304,7 @@ export const scenarios: SuggestionScenario[] = [ { kind: "concurrent", id: "concurrent-table-seq-row-then-col", + feedback: CONCURRENT_TABLE_FEEDBACK, title: "A adds row then column, B adds row", category: "Tables", description: "A adds a row and then a column (two edits); B adds a row.", @@ -1623,12 +1622,6 @@ export const scenarios: SuggestionScenario[] = [ editor.replaceBlocks(editor.document, [ { type: "paragraph", content: "(all content removed)" }, ]), - feedback: [ - { - severity: "high", - note: "the 'all content removed' paragraph should show up below or above the document, not inside it", - }, - ], }, // --- Merge / split ---