From 1c7435511dd33b78498979ecde5dfdeb10f6d4e0 Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Tue, 18 Aug 2026 16:02:09 +0200 Subject: [PATCH] perf(core): resolve block changes from changed range only getBlocksChangedByTransaction snapshotted the entire document (a nodeToBlock conversion of every block, twice) on every transaction. Since apps read getChanges() on each keystroke, typing lagged in large documents. It now diffs only the range the transaction touched. Extract a shared getChangedRange() helper that, unlike ProseMirror's changedRange(), also covers attribute-only steps (AttrStep) and mark steps, at the same O(steps) cost. PreviousBlockType now uses it too, fixing a latent bug where attribute-only changes (e.g. a heading's level) were silently missed by its ranged diff. --- .../src/api/getBlocksChangedByTransaction.ts | 60 +++++++++++++++---- packages/core/src/api/getChangedRange.ts | 54 +++++++++++++++++ packages/core/src/editor/performance.test.ts | 45 ++++++++++++++ .../PreviousBlockType.test.ts | 21 +++++++ .../PreviousBlockType/PreviousBlockType.ts | 8 ++- 5 files changed, 173 insertions(+), 15 deletions(-) create mode 100644 packages/core/src/api/getChangedRange.ts 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; }