From 41bd8a95442119cbc52566b077435f12f2db4926 Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Tue, 6 Oct 2026 21:31:16 +0000 Subject: [PATCH 1/2] fix(core): preserve undo boundaries for block conversions and Yjs 13 --- .../commands/updateBlock/updateBlock.test.ts | 89 ++++++++++++++++++- .../commands/updateBlock/updateBlock.ts | 26 +++++- .../core/src/yjs/extensions/YUndo.test.ts | 84 +++++++++++++++++ packages/core/src/yjs/extensions/YUndo.ts | 17 +++- 4 files changed, 212 insertions(+), 4 deletions(-) create mode 100644 packages/core/src/yjs/extensions/YUndo.test.ts diff --git a/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.test.ts b/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.test.ts index e44e4a6380..aefb941b69 100644 --- a/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.test.ts +++ b/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.test.ts @@ -1,13 +1,45 @@ +import { closeHistory } from "@tiptap/pm/history"; +import { ReplaceStep, Transform } from "prosemirror-transform"; import { describe, expect, it } from "vite-plus/test"; -import type { PartialBlock } from "../../../../blocks/defaultBlocks.js"; +import type { + DefaultBlockSchema, + DefaultInlineContentSchema, + DefaultStyleSchema, + PartialBlock, +} from "../../../../blocks/defaultBlocks.js"; import { getBlockInfo } from "../../../getBlockInfoFromPos.js"; import { getNodeById } from "../../../nodeUtil.js"; import { setupTestEnv } from "../../setupTestEnv.js"; -import { updateBlock } from "./updateBlock.js"; +import { updateBlock, updateBlockTr } from "./updateBlock.js"; const getEditor = setupTestEnv(); +describe("Test updateBlock undo/redo", () => { + it.each([ + ["paragraph", "divider"], + ["heading", "divider"], + ["codeBlock", "image"], + ["divider", "paragraph"], + ["paragraph", "heading"], + ] as const)("can undo and redo %s → %s", (from, to) => { + const editor = getEditor(); + editor.updateBlock("empty-paragraph", { type: from }); + const before = editor.prosemirrorState.doc; + // Isolate the conversion from fixture preparation. + editor.prosemirrorView.dispatch(closeHistory(editor.prosemirrorState.tr)); + editor.updateBlock("empty-paragraph", { type: to }); + const after = editor.prosemirrorState.doc; + expect(editor.getBlock("empty-paragraph")?.type).toBe(to); + after.check(); + + expect(editor.undo()).toBe(true); + expect(editor.prosemirrorState.doc.eq(before)).toBe(true); + expect(editor.redo()).toBe(true); + expect(editor.prosemirrorState.doc.eq(after)).toBe(true); + }); +}); + describe("Test updateBlock typing", () => { it("Type is inferred correctly", () => { try { @@ -626,6 +658,59 @@ describe("Test updateBlock", () => { * only a small part changed. */ describe("Test updateBlock minimal steps", () => { + it.each([ + ["paragraph", "divider"], + ["heading", "divider"], + ["codeBlock", "image"], + ] as const)( + "%s → %s replaces only the empty content node and can be inverted", + (from, to) => { + const editor = getEditor(); + editor.updateBlock("paragraph-with-children", { + content: [], + props: { backgroundColor: "red" }, + }); + editor.updateBlock("paragraph-with-children", { type: from }); + const before = editor.prosemirrorState.doc; + const original = getNodeById("paragraph-with-children", before)!; + const contentPos = original.posBeforeNode + 1; + const tr = new Transform(before); + updateBlockTr< + DefaultBlockSchema, + DefaultInlineContentSchema, + DefaultStyleSchema + >(tr, original.posBeforeNode, { type: to }); + + // Only the paragraph's two structural tokens become one leaf token. + // Its enclosing container and nonempty children must not be replaced. + expect(tr.steps).toHaveLength(1); + const step = tr.steps[0]; + expect(step).toBeInstanceOf(ReplaceStep); + if (!(step instanceof ReplaceStep)) { + throw new Error("Expected a single content-node ReplaceStep"); + } + expect(step.from).toBe(contentPos); + expect(step.to).toBe(contentPos + 2); + expect(step.slice.size).toBe(1); + expect(step.slice.content.firstChild?.type.name).toBe(to); + const updated = getNodeById("paragraph-with-children", tr.doc)!; + expect(updated.node.attrs).toEqual(original.node.attrs); + expect(updated.node.child(1)).toBe(original.node.child(1)); + expect(getNodeById("paragraph-0", tr.doc)!.node).toBe( + getNodeById("paragraph-0", before)!.node, + ); + expect(getNodeById("paragraph-9", tr.doc)!.node).toBe( + getNodeById("paragraph-9", before)!.node, + ); + tr.doc.check(); + + const inverse = new Transform(tr.doc); + inverse.step(step.invert(before)); + expect(inverse.doc.eq(before)).toBe(true); + inverse.doc.check(); + }, + ); + // Runs `updateBlock` in a throwaway transaction and returns the resulting // steps as JSON for inspection. const getSteps = (blockId: string, update: PartialBlock) => { diff --git a/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.ts b/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.ts index 6edfc434d5..0c24b7a6d3 100644 --- a/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.ts +++ b/packages/core/src/api/blockManipulation/commands/updateBlock/updateBlock.ts @@ -435,7 +435,7 @@ function replaceContentMinimal( * for each changed attribute. These are minimal steps that don't touch the * node's content. * - If the type changes, `setNodeMarkup` is used, which keeps the node's content - * via a `ReplaceAroundStep`. + * via a `ReplaceAroundStep`, except when converting a non-leaf into a leaf. */ function setNodeMarkupMinimal( tr: Transform, @@ -471,6 +471,30 @@ function setNodeMarkupMinimal( return; } + if (!node.isLeaf && newType.isLeaf) { + // setNodeMarkup preserves a non-leaf's content gap via a ReplaceAroundStep. + // After conversion to a leaf, that gap sits outside the node and the inverse + // step fails its structure check. A leaf accepts only empty content, so + // there is no gap to preserve: changing these two structural tokens into + // one leaf token is the smallest possible replacement. Leave the enclosing + // block container, its children, and adjacent blocks untouched. Use a strict + // step so invalid parent content is rejected instead of fitted/coerced. + tr.step( + new ReplaceStep( + pos, + pos + node.nodeSize, + new Slice( + Fragment.from( + newType.createChecked(mergedAttrs, node.content, node.marks), + ), + 0, + 0, + ), + ), + ); + return; + } + // Type changed - setNodeMarkup keeps the content via a ReplaceAroundStep. tr.setNodeMarkup(pos, newType, mergedAttrs); } diff --git a/packages/core/src/yjs/extensions/YUndo.test.ts b/packages/core/src/yjs/extensions/YUndo.test.ts new file mode 100644 index 0000000000..9bb1547383 --- /dev/null +++ b/packages/core/src/yjs/extensions/YUndo.test.ts @@ -0,0 +1,84 @@ +import { closeHistory } from "@tiptap/pm/history"; +import { + afterEach, + beforeEach, + describe, + expect, + it, + vi, +} from "vite-plus/test"; +import { yUndoPluginKey } from "y-prosemirror"; +import * as Y from "yjs"; + +import { BlockNoteEditor } from "../../editor/BlockNoteEditor.js"; +import { withCollaboration } from "../index.js"; + +describe("YUndo history boundaries", () => { + let doc: Y.Doc; + let editor: BlockNoteEditor; + let undoManager: Y.UndoManager; + + beforeEach(() => { + doc = new Y.Doc(); + const fragment = doc.getXmlFragment("document"); + editor = BlockNoteEditor.create( + withCollaboration({ + collaboration: { + fragment, + user: { name: "Test", color: "#ff0000" }, + provider: undefined, + }, + }), + ); + editor.mount(document.createElement("div")); + const undoState = yUndoPluginKey.getState(editor.prosemirrorState); + if (!undoState) { + throw new Error("Missing Yjs undo plugin in collaborative test editor"); + } + undoManager = undoState.undoManager; + undoManager.clear(); + undoManager.stopCapturing(); + // Both edits occur within one capture window, regardless of test speed. + vi.spyOn(Date, "now").mockReturnValue(Date.now()); + }); + + afterEach(() => { + vi.restoreAllMocks(); + undoManager.destroy(); + editor._tiptapEditor.destroy(); + doc.destroy(); + }); + + function insert(value: string) { + editor.prosemirrorView.dispatch( + editor.prosemirrorState.tr.insertText(value), + ); + } + + it("groups consecutive edits without a history boundary", () => { + insert("first"); + insert("second"); + + undoManager.undo(); + expect(editor.prosemirrorState.doc.textContent).toBe(""); + expect(undoManager.canUndo()).toBe(false); + undoManager.redo(); + expect(editor.prosemirrorState.doc.textContent).toBe("firstsecond"); + }); + + it("separates edits across closeHistory for undo and redo", () => { + insert("first"); + editor.prosemirrorView.dispatch(closeHistory(editor.prosemirrorState.tr)); + insert("second"); + + undoManager.undo(); + expect(editor.prosemirrorState.doc.textContent).toBe("first"); + undoManager.undo(); + expect(editor.prosemirrorState.doc.textContent).toBe(""); + expect(undoManager.canUndo()).toBe(false); + undoManager.redo(); + expect(editor.prosemirrorState.doc.textContent).toBe("first"); + undoManager.redo(); + expect(editor.prosemirrorState.doc.textContent).toBe("firstsecond"); + }); +}); diff --git a/packages/core/src/yjs/extensions/YUndo.ts b/packages/core/src/yjs/extensions/YUndo.ts index c3b65a55bd..a961c757a1 100644 --- a/packages/core/src/yjs/extensions/YUndo.ts +++ b/packages/core/src/yjs/extensions/YUndo.ts @@ -1,10 +1,25 @@ +import { Plugin } from "prosemirror-state"; import { redoCommand, undoCommand, yUndoPlugin } from "y-prosemirror"; import { createExtension } from "../../editor/BlockNoteExtension.js"; export const YUndoExtension = createExtension(() => { + const undoPlugin = yUndoPlugin(); + const historyBoundaryPlugin = new Plugin({ + appendTransaction(transactions, _oldState, newState) { + // @handlewithcare/prosemirror-inputrules dispatches closeHistory between + // inserting the literal trigger and applying its conversion. Yjs doesn't + // read that marker, so otherwise both changes enter the same undo item. + // prosemirror-history doesn't export the marker's PluginKey; keep this + // compatibility dependency here, guarded by the history-boundary tests. + if (transactions.some((tr) => tr.getMeta("closeHistory$"))) { + undoPlugin.getState(newState)?.undoManager.stopCapturing(); + } + return null; + }, + }); return { key: "yUndo", - prosemirrorPlugins: [yUndoPlugin()], + prosemirrorPlugins: [undoPlugin, historyBoundaryPlugin], dependsOn: ["yCursor", "ySync"], undoCommand: undoCommand, redoCommand: redoCommand, From 56b4a96734796ebfab59a64c0c64e657792ec0fa Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Wed, 7 Oct 2026 04:18:38 +0000 Subject: [PATCH 2/2] test(core): destroy collaborative editor after thread store test --- .../yjs/comments/RESTYjsThreadStore.test.ts | 77 ++++++++++--------- 1 file changed, 41 insertions(+), 36 deletions(-) diff --git a/packages/core/src/yjs/comments/RESTYjsThreadStore.test.ts b/packages/core/src/yjs/comments/RESTYjsThreadStore.test.ts index 41623fb779..b91fe1cb2b 100644 --- a/packages/core/src/yjs/comments/RESTYjsThreadStore.test.ts +++ b/packages/core/src/yjs/comments/RESTYjsThreadStore.test.ts @@ -40,46 +40,51 @@ describe("RESTYjsThreadStore", () => { it("sends resolvable yjs positions along with the thread", async () => { const { editor, doc, fragment } = createCollabEditor(); - const requests: any[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation((async ( - _url: any, - init: any, - ) => { - requests.push(JSON.parse(init.body)); - return new Response("{}", { - status: 200, - headers: { "Content-Type": "application/json" }, - }); - }) as any); + try { + const requests: any[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation((async ( + _url: any, + init: any, + ) => { + requests.push(JSON.parse(init.body)); + return new Response("{}", { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }) as any); - const store = new RESTYjsThreadStore( - "https://example.com/threads", - {}, - doc.getMap("threads"), - new DefaultThreadStoreAuth("user-1", "editor"), - ); + const store = new RESTYjsThreadStore( + "https://example.com/threads", + {}, + doc.getMap("threads"), + new DefaultThreadStoreAuth("user-1", "editor"), + ); - await store.addThreadToDocument({ - threadId: "thread-1", - selection: { anchor: 3, head: 8 }, - editor, - }); + await store.addThreadToDocument({ + threadId: "thread-1", + selection: { anchor: 3, head: 8 }, + editor, + }); - expect(requests).toHaveLength(1); - const { yjs } = requests[0].selection; - expect(yjs).toBeDefined(); + expect(requests).toHaveLength(1); + const { yjs } = requests[0].selection; + expect(yjs).toBeDefined(); - // the relative positions must resolve back to the positions we passed in - const state = ySyncPluginKey.getState(editor.prosemirrorState) as any; - const resolve = (relPos: any) => - relativePositionToAbsolutePosition( - fragment.doc!, - state.binding.type, - Y.createRelativePositionFromJSON(relPos), - state.binding.mapping, - ); + // the relative positions must resolve back to the positions we passed in + const state = ySyncPluginKey.getState(editor.prosemirrorState) as any; + const resolve = (relPos: any) => + relativePositionToAbsolutePosition( + fragment.doc!, + state.binding.type, + Y.createRelativePositionFromJSON(relPos), + state.binding.mapping, + ); - expect(resolve(yjs.anchor)).toBe(3); - expect(resolve(yjs.head)).toBe(8); + expect(resolve(yjs.anchor)).toBe(3); + expect(resolve(yjs.head)).toBe(8); + } finally { + editor._tiptapEditor.destroy(); + doc.destroy(); + } }); });