diff --git a/packages/core/src/api/getBlocksChangedByTransaction.test.ts b/packages/core/src/api/getBlocksChangedByTransaction.test.ts index 25f112b2d2..828894cf1d 100644 --- a/packages/core/src/api/getBlocksChangedByTransaction.test.ts +++ b/packages/core/src/api/getBlocksChangedByTransaction.test.ts @@ -2,7 +2,10 @@ import { describe, expect, it, beforeEach } from "vite-plus/test"; import { setupTestEnv } from "./blockManipulation/setupTestEnv.js"; import { getBlocksChangedByTransaction } from "./getBlocksChangedByTransaction.js"; +import { getBlockInfo } from "./getBlockInfoFromPos.js"; +import { getNodeById } from "./nodeUtil.js"; import { BlockNoteEditor } from "../editor/BlockNoteEditor.js"; +import { PartialBlock } from "../blocks/defaultBlocks.js"; const getEditor = setupTestEnv(); @@ -570,3 +573,211 @@ describe("getBlocksChangedByTransaction", () => { ); }); }); + +/** + * These exercise the ranged optimization: getBlocksChangedByTransaction only + * snapshots the range a transaction touched, not the whole document. In a large + * document the failure modes are (a) missing a real change and (b) reporting a + * block that didn't actually change. Each test edits a big document and asserts + * the exact set of reported changes. + */ +describe("getBlocksChangedByTransaction - ranged optimization", () => { + let editor: BlockNoteEditor; + + const LARGE = 200; + + function makeParagraphs(count: number): PartialBlock[] { + return Array.from({ length: count }, (_, i) => ({ + id: `p-${i}`, + type: "paragraph", + content: `Paragraph ${i}`, + })); + } + + function summarize(changes: Array<{ type: string; block: { id: string } }>) { + return changes.map((change) => ({ + type: change.type, + id: change.block.id, + })); + } + + beforeEach(() => { + editor = getEditor(); + }); + + it("reports only the changed block for a prop update deep in a large doc", () => { + editor.replaceBlocks(editor.document, makeParagraphs(LARGE)); + + const changes = editor.transact((tr) => { + editor.updateBlock("p-120", { props: { backgroundColor: "red" } }); + return getBlocksChangedByTransaction(tr); + }); + + expect(summarize(changes)).toEqual([{ type: "update", id: "p-120" }]); + }); + + it("reports only the edited block for a content insertion in a large doc", () => { + editor.replaceBlocks(editor.document, makeParagraphs(LARGE)); + + const changes = editor.transact((tr) => { + editor.setTextCursorPosition("p-77", "start"); + editor.insertInlineContent("Hello "); + return getBlocksChangedByTransaction(tr); + }); + + expect(summarize(changes)).toEqual([{ type: "update", id: "p-77" }]); + }); + + it("reports two distant prop updates without reporting the blocks between them", () => { + editor.replaceBlocks(editor.document, makeParagraphs(LARGE)); + + const changes = editor.transact((tr) => { + editor.updateBlock("p-10", { props: { backgroundColor: "red" } }); + editor.updateBlock("p-190", { props: { backgroundColor: "blue" } }); + return getBlocksChangedByTransaction(tr); + }); + + const summary = summarize(changes); + expect(summary).toContainEqual({ type: "update", id: "p-10" }); + expect(summary).toContainEqual({ type: "update", id: "p-190" }); + expect(summary).toHaveLength(2); + }); + + it("reports a mark-only change as an update (empty-map AddMarkStep)", () => { + editor.replaceBlocks(editor.document, makeParagraphs(LARGE)); + + const changes = editor.transact((tr) => { + const posInfo = getNodeById("p-140", tr.doc); + if (!posInfo) { + throw new Error("block not found"); + } + const info = getBlockInfo(posInfo); + if (!info.isBlockContainer) { + throw new Error("expected a block container"); + } + // Adding a mark produces an AddMarkStep, whose StepMap is empty — the case + // getChangedRange has to recover from the step's own from/to. + tr.addMark( + info.blockContent.beforePos + 1, + info.blockContent.afterPos - 1, + editor.pmSchema.marks.bold.create(), + ); + return getBlocksChangedByTransaction(tr); + }); + + expect(summarize(changes)).toEqual([{ type: "update", id: "p-140" }]); + }); + + it("reports mixed insert/update/delete across a large doc in one transaction", () => { + editor.replaceBlocks(editor.document, makeParagraphs(LARGE)); + + const changes = editor.transact((tr) => { + editor.updateBlock("p-20", { props: { backgroundColor: "red" } }); + editor.removeBlocks(["p-100"]); + editor.insertBlocks( + [{ id: "inserted", type: "paragraph", content: "new" }], + "p-180", + "after", + ); + return getBlocksChangedByTransaction(tr); + }); + + const summary = summarize(changes); + expect(summary).toContainEqual({ type: "update", id: "p-20" }); + expect(summary).toContainEqual({ type: "delete", id: "p-100" }); + expect(summary).toContainEqual({ type: "insert", id: "inserted" }); + expect(summary).toHaveLength(3); + }); + + it("reports an insert at the very start of a large doc", () => { + editor.replaceBlocks(editor.document, makeParagraphs(LARGE)); + + const changes = editor.transact((tr) => { + editor.insertBlocks( + [{ id: "new-first", type: "paragraph", content: "X" }], + "p-0", + "before", + ); + return getBlocksChangedByTransaction(tr); + }); + + expect(summarize(changes)).toEqual([{ type: "insert", id: "new-first" }]); + }); + + it("reports an insert at the very end of a large doc", () => { + editor.replaceBlocks(editor.document, makeParagraphs(LARGE)); + + const changes = editor.transact((tr) => { + editor.insertBlocks( + [{ id: "new-last", type: "paragraph", content: "X" }], + `p-${LARGE - 1}`, + "after", + ); + return getBlocksChangedByTransaction(tr); + }); + + expect(summarize(changes)).toEqual([{ type: "insert", id: "new-last" }]); + }); + + it("reports a delete in the middle without touching the blocks it shifts", () => { + editor.replaceBlocks(editor.document, makeParagraphs(LARGE)); + + const changes = editor.transact((tr) => { + editor.removeBlocks(["p-100"]); + return getBlocksChangedByTransaction(tr); + }); + + expect(summarize(changes)).toEqual([{ type: "delete", id: "p-100" }]); + }); + + it("reports a single move for a block moved across a large span", () => { + editor.replaceBlocks(editor.document, makeParagraphs(LARGE)); + + const changes = editor.transact((tr) => { + const block = editor.getBlock("p-5"); + editor.removeBlocks(["p-5"]); + editor.insertBlocks([{ ...block }], "p-195", "after"); + return getBlocksChangedByTransaction(tr); + }); + + expect(summarize(changes)).toEqual([{ type: "move", id: "p-5" }]); + }); + + it("does not report ancestor blocks when a deeply nested block changes", () => { + const blocks = makeParagraphs(100); + blocks[50] = { + id: "parent", + type: "paragraph", + content: "Parent", + children: [ + { + id: "child", + type: "paragraph", + content: "Child", + children: [ + { id: "grandchild", type: "paragraph", content: "Grandchild" }, + ], + }, + ], + }; + editor.replaceBlocks(editor.document, blocks); + + const changes = editor.transact((tr) => { + editor.updateBlock("grandchild", { props: { backgroundColor: "red" } }); + return getBlocksChangedByTransaction(tr); + }); + + expect(summarize(changes)).toEqual([{ type: "update", id: "grandchild" }]); + }); + + it("returns no changes for a selection-only transaction in a large doc", () => { + editor.replaceBlocks(editor.document, makeParagraphs(LARGE)); + + const changes = editor.transact((tr) => { + editor.setTextCursorPosition("p-100", "end"); + return getBlocksChangedByTransaction(tr); + }); + + expect(changes).toEqual([]); + }); +}); diff --git a/packages/core/src/api/getBlocksChangedByTransaction.ts b/packages/core/src/api/getBlocksChangedByTransaction.ts index 94b2bc1d3b..9157a86812 100644 --- a/packages/core/src/api/getBlocksChangedByTransaction.ts +++ b/packages/core/src/api/getBlocksChangedByTransaction.ts @@ -11,6 +11,7 @@ import { import type { BlockSchema } from "../schema/index.js"; import type { InlineContentSchema } from "../schema/inlineContent/types.js"; import type { StyleSchema } from "../schema/styles/types.js"; +import { getChangedRange } from "./getChangedRange.js"; import { getNodeId } from "./getBlockInfoFromPos.js"; import { nodeToBlock } from "./nodeConversions/nodeToBlock.js"; import { isNodeBlock } from "./nodeUtil.js"; @@ -20,14 +21,23 @@ import { isNodeBlock } from "./nodeUtil.js"; * * High-level algorithm used by getBlocksChangedByTransaction: * 1) Merge appended transactions into one document change. - * 2) Collect a snapshot of blocks before and after (flat map by id, and per-parent child order). - * 3) Emit inserts and deletes by diffing ids between snapshots. - * 4) For ids present in both snapshots: - * - If parentId changed, emit a move - * - Else if block changed (ignoring children), emit an update - * 5) Finally, detect same-parent sibling reorders by comparing child order per parent. - * We use an inlined O(n log n) LIS inside detectReorderedChildren to keep a - * longest already-ordered subsequence and mark only the remaining items as moved. + * 2) Compute the single range the transaction touched (in both the old and new + * doc) and only snapshot blocks within it, rather than walking the whole + * document. getChanges() runs per transaction, so a full-document snapshot + * made typing in large documents slow: every keystroke re-converted every block. + * 3) Snapshot blocks before and after within that range (flat map by id, and + * per-parent child order). + * 4) Emit inserts/deletes by diffing ids; for shared ids, emit a move (parent + * changed) or update (block changed, ignoring children). + * 5) Detect same-parent sibling reorders via an O(n log n) LIS in + * detectReorderedChildren, marking only items outside the longest ordered + * subsequence as moved. + * + * The range suffices because `changedRange()` spans from the first to the last + * changed position: any inserted/deleted/moved/updated/reordered block has its + * relevant positions inside it, and blocks outside are byte-for-byte identical in + * the same relative order. A moved block's parent contains it, so the parent + * overlaps the range too (and nodeToBlock converts its full subtree regardless). */ /** * Gets the parent block of a node, if it has one. @@ -144,14 +154,19 @@ type BlockSnapshot< }; /** - * Collects a snapshot of blocks and per-parent child order in a single traversal. - * Uses "__root__" to represent the root level where parentId is undefined. + * Snapshots blocks and per-parent child order for the block nodes overlapping the + * given range (uses "__root__" for the root level). Traversing only the range is + * what keeps this cheap per keystroke: nodeToBlock runs only for blocks that could + * have changed. */ function collectSnapshot< BSchema extends BlockSchema, ISchema extends InlineContentSchema, SSchema extends StyleSchema, ->(doc: Node): BlockSnapshot { +>( + doc: Node, + range: { from: number; to: number }, +): BlockSnapshot { const ROOT_KEY = "__root__"; const byId: Record< string, @@ -161,7 +176,12 @@ function collectSnapshot< } > = {}; const childrenByParent: Record = {}; - doc.descendants((node, pos) => { + // Clamp to valid positions; nodesBetween throws on out-of-range ones. + const from = Math.max(0, Math.min(range.from, doc.content.size)); + const to = Math.max(from, Math.min(range.to, doc.content.size)); + // nodesBetween visits every node overlapping [from, to] in document order, + // including ancestor blocks that contain the range. + doc.nodesBetween(from, to, (node, pos) => { if (!isNodeBlock(node)) { return true; } @@ -282,11 +302,27 @@ export function getBlocksChangedByTransaction< ...appendedTransactions, ]); + // Changed range in the new doc; null means nothing changed. + const newRange = getChangedRange(combinedTransaction); + if (!newRange) { + return []; + } + // Map it back to old-doc coordinates. The -1/+1 biases expand outwards so that + // for pure inserts/deletes (collapsed new range) the old range still covers the + // affected span. + const invertedMapping = combinedTransaction.mapping.invert(); + const oldRange = { + from: invertedMapping.map(newRange.from, -1), + to: invertedMapping.map(newRange.to, 1), + }; + const prevSnap = collectSnapshot( combinedTransaction.before, + oldRange, ); const nextSnap = collectSnapshot( combinedTransaction.doc, + newRange, ); const changes: BlocksChanged = []; diff --git a/packages/core/src/api/getChangedRange.ts b/packages/core/src/api/getChangedRange.ts new file mode 100644 index 0000000000..c6f78de8bc --- /dev/null +++ b/packages/core/src/api/getChangedRange.ts @@ -0,0 +1,54 @@ +import type { Transform } from "prosemirror-transform"; + +/** + * Like ProseMirror's `Transform.changedRange()`, but also accounts for + * position-preserving steps whose `StepMap` is empty — `AttrStep` (prop-only + * updates like a heading's `level`) and mark steps. `changedRange()` and tiptap's + * `getChangedRanges` both miss `AttrStep`, so anything that scopes work to the + * changed range would silently ignore prop-only updates. + * + * O(steps), like `changedRange()`. Returns null when nothing changed. + */ +export function getChangedRange( + transform: Transform, +): { from: number; to: number } | null { + const { mapping, steps } = transform; + let from = Number.POSITIVE_INFINITY; + let to = Number.NEGATIVE_INFINITY; + + for (let i = 0; i < mapping.maps.length; i++) { + const map = mapping.maps[i]; + // Advance the accumulated range into this step's coordinate space. + if (i) { + from = map.map(from, 1); + to = map.map(to, -1); + } + + let hadRange = false; + map.forEach((_oldFrom, _oldTo, newFrom, newTo) => { + hadRange = true; + from = Math.min(from, newFrom); + to = Math.max(to, newTo); + }); + + if (!hadRange) { + // Position-preserving step: recover the affected position from the step, + // since its map has no ranges. (DocAttrStep has none and affects no nodes.) + const step = steps[i] as { pos?: number; from?: number; to?: number }; + if (typeof step.pos === "number") { + // AttrStep + from = Math.min(from, step.pos); + to = Math.max(to, step.pos + 1); + } else if (typeof step.from === "number" && typeof step.to === "number") { + // AddMarkStep / RemoveMarkStep + from = Math.min(from, step.from); + to = Math.max(to, step.to); + } + } + } + + if (from === Number.POSITIVE_INFINITY) { + return null; + } + return { from, to }; +} diff --git a/packages/core/src/editor/performance.test.ts b/packages/core/src/editor/performance.test.ts index 74bde90473..ed490cc0b9 100644 --- a/packages/core/src/editor/performance.test.ts +++ b/packages/core/src/editor/performance.test.ts @@ -144,4 +144,49 @@ describe("Performance: transaction processing scales sub-linearly (#2595)", () = // Absolute time (~40ms) is comparable to begin (~32ms). expect(ratio).toBeLessThan(250); }); + + // getChanges() is lazy — it only diffs when a subscriber calls it, so the tests + // above don't cover getBlocksChangedByTransaction. This locks in that the diff + // scopes to the changed range, not the whole document. + it( + "getChanges() in onChange scales sub-linearly", + { timeout: 30_000 }, + () => { + function measureWithGetChanges( + editor: BlockNoteEditor, + pos: number, + ) { + // Run the diff every transaction, like an app reading changed blocks per keystroke. + // eslint-disable-next-line @typescript-eslint/unbound-method -- getChanges is destructured from callback parameter, not a class + const unsubscribe = editor.onChange((_editor, { getChanges }) => { + getChanges(); + }); + const avg = measureAvgInsertTime(editor, pos); + unsubscribe(); + return avg; + } + + const smallEditor = createEditorWithBlocks(SMALL, "paragraph"); + const largeEditor = createEditorWithBlocks(LARGE, "paragraph"); + + const smallAvg = measureWithGetChanges( + smallEditor, + smallEditor._tiptapEditor.view.state.doc.content.size - 2, + ); + const largeAvg = measureWithGetChanges( + largeEditor, + largeEditor._tiptapEditor.view.state.doc.content.size - 2, + ); + const ratio = largeAvg / smallAvg; + + // eslint-disable-next-line no-console + console.log( + `getChanges (end): ${SMALL}=${smallAvg.toFixed(3)}ms, ${LARGE}=${largeAvg.toFixed(3)}ms, ratio=${ratio.toFixed(2)}x`, + ); + + // The full-document snapshot made getChanges O(blocks) per keystroke, pushing + // this toward the block-count ratio (~50x); the ranged diff stays well below. + expect(ratio).toBeLessThan(50); + }, + ); }); diff --git a/packages/core/src/extensions/PreviousBlockType/PreviousBlockType.test.ts b/packages/core/src/extensions/PreviousBlockType/PreviousBlockType.test.ts index 6fa413ab99..b11dbadd4b 100644 --- a/packages/core/src/extensions/PreviousBlockType/PreviousBlockType.test.ts +++ b/packages/core/src/extensions/PreviousBlockType/PreviousBlockType.test.ts @@ -96,4 +96,25 @@ describe("PreviousBlockType: scoped traversal", () => { ).length; expect(trackedBlockCount).toBeLessThan(10); }); + + it("detects attribute-only changes (heading level) that emit an AttrStep", () => { + const editor = createEditorWithBlocks(100, "heading"); + + // Changing only a heading's level (type stays "heading") emits an AttrStep, + // whose StepMap is empty — so ProseMirror's changedRange() returns null and + // the change would be missed unless the scoped range also covers AttrSteps. + const firstBlock = editor.document[0]; + editor.updateBlock(firstBlock, { props: { level: 2 } } as any); + + const state = getPreviousBlockTypePluginState(editor); + + expect(state.updatedBlocks.size).toBe(1); + expect(state.updatedBlocks.has(firstBlock.id)).toBe(true); + + // Still scoped to the changed range, not all 100 blocks. + const trackedBlockCount = Object.keys( + state.currentTransactionOldBlockAttrs, + ).length; + expect(trackedBlockCount).toBeLessThan(10); + }); }); diff --git a/packages/core/src/extensions/PreviousBlockType/PreviousBlockType.ts b/packages/core/src/extensions/PreviousBlockType/PreviousBlockType.ts index 1523ae5363..026dc91ea8 100644 --- a/packages/core/src/extensions/PreviousBlockType/PreviousBlockType.ts +++ b/packages/core/src/extensions/PreviousBlockType/PreviousBlockType.ts @@ -2,6 +2,7 @@ import { findChildrenInRange } from "@tiptap/core"; import { Plugin, PluginKey } from "prosemirror-state"; import { Decoration, DecorationSet } from "prosemirror-view"; import { getNodeId } from "../../api/getBlockInfoFromPos.js"; +import { getChangedRange } from "../../api/getChangedRange.js"; import { createExtension } from "../../editor/BlockNoteExtension.js"; const PLUGIN_KEY = new PluginKey(`previous-blocks`); @@ -72,9 +73,10 @@ export const PreviousBlockTypeExtension = createExtension(() => { return prev; } - // Only check nodes affected by the transaction, not the entire document. - // changedRange() is O(steps) unlike tiptap's getChangedRanges which is O(steps²). - const newRange = transaction.changedRange(); + // Only check nodes in the changed range, not the whole document. + // getChangedRange() also covers attribute-only steps (AttrStep), so a + // block whose `level`/`index` changes with no content edit is caught. + const newRange = getChangedRange(transaction); if (!newRange) { return prev; }