fix(core): optimize plugin traversals for large documents BLO-1111#2600
Merged
nperez0111 merged 4 commits intomainfrom Apr 1, 2026
Merged
fix(core): optimize plugin traversals for large documents BLO-1111#2600nperez0111 merged 4 commits intomainfrom
nperez0111 merged 4 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughNarrowed plugin traversals to transaction-changed ranges, replaced recursive numbered-list indexing with an iterative cached algorithm, bumped several Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as Editor (BlockNote)
participant Tr as Transaction
participant Plugin as Plugin (Indexing / PreviousBlockType)
participant Cache as Cache / DecorationSet
Editor->>Tr: user input / programmatic edit
Tr->>Plugin: apply(transaction)
Plugin->>Tr: tr.changedRange()
alt changedRange exists
Plugin->>Cache: read cached indices / decorations for range
Plugin->>Plugin: backward scan predecessors (build chain)
Plugin->>Plugin: forward assign indices -> update cache
Plugin->>Cache: build DecorationSet for changed nodes
else no changedRange
Plugin->>Cache: reuse previous DecorationSet (no recompute)
end
Plugin->>Editor: commit decorations / plugin state
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/core/src/editor/performance.test.ts (1)
22-38: Consider cleaning up editors after each test to prevent memory leaks.The created editors are never destroyed after use. While this may not cause issues in isolated test runs, it's good practice to clean up resources, especially when creating editors with 10k blocks.
♻️ Suggested cleanup pattern
function createEditorWithBlocks( blockCount: number, blockType: "heading" | "paragraph" | "numberedListItem" = "heading", ) { const editor = BlockNoteEditor.create(); editor.mount(document.createElement("div")); const blocks = []; for (let i = 0; i < blockCount; i++) { blocks.push({ type: blockType as any, content: `Block number ${i} with some text to simulate a real document`, ...(blockType === "heading" ? { props: { level: 1 } } : {}), }); } editor.replaceBlocks(editor.document, blocks as any); return editor; }Then in tests, consider using
afterEachor callingeditor._tiptapEditor.destroy()when done.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/editor/performance.test.ts` around lines 22 - 38, The tests create BlockNoteEditor instances via createEditorWithBlocks and never destroy them, risking memory leaks for large block counts; update the test suite to destroy the underlying tiptap editor after each test (e.g., call editor._tiptapEditor.destroy() or similar) — either modify createEditorWithBlocks to register created editors for cleanup or ensure each test calls the destroy method, and add an afterEach hook to iterate tracked editors and call _tiptapEditor.destroy() to guarantee cleanup.packages/core/src/blocks/ListItem/NumberedListItem/IndexingPlugin.ts (1)
111-115: Consider adding a brief comment explaining theisFirstlogic.The expression
chain.length === 1 ? isFirst || predecessorIndex === undefined : falseis correct but dense. A short comment would aid future maintainability.📝 Suggested comment
+ // isFirst is true only when this is the very first item in a new list: + // - chain.length > 1 means we found predecessor list items, so not first + // - otherwise, check if we started as first (no predecessor) or found one via cache return { index, isFirst: chain.length === 1 ? isFirst || predecessorIndex === undefined : false, hasStart, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/blocks/ListItem/NumberedListItem/IndexingPlugin.ts` around lines 111 - 115, Add a concise inline comment next to the isFirst computed field in IndexingPlugin.ts explaining the logic: that when chain.length === 1 we consider the item first if either the original isFirst flag is true or there is no predecessorIndex (undefined), otherwise for longer chains isFirst is false; place the comment adjacent to the expression `chain.length === 1 ? isFirst || predecessorIndex === undefined : false` so future readers understand the special-case for single-item chains and the predecessorIndex check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/blocks/ListItem/NumberedListItem/IndexingPlugin.test.ts`:
- Around line 336-340: Replace the dynamic require with a static import: add
"import { Selection } from 'prosemirror-state';" at the top of the test file and
remove the inline "const { Selection } = require('prosemirror-state');" line in
the test; keep the rest of the test logic (the Selection.near(...) call, tr =
view.state.tr.setSelection(...), and view.dispatch(tr)) unchanged so ESLint no
longer flags `@typescript-eslint/no-var-requires`.
---
Nitpick comments:
In `@packages/core/src/blocks/ListItem/NumberedListItem/IndexingPlugin.ts`:
- Around line 111-115: Add a concise inline comment next to the isFirst computed
field in IndexingPlugin.ts explaining the logic: that when chain.length === 1 we
consider the item first if either the original isFirst flag is true or there is
no predecessorIndex (undefined), otherwise for longer chains isFirst is false;
place the comment adjacent to the expression `chain.length === 1 ? isFirst ||
predecessorIndex === undefined : false` so future readers understand the
special-case for single-item chains and the predecessorIndex check.
In `@packages/core/src/editor/performance.test.ts`:
- Around line 22-38: The tests create BlockNoteEditor instances via
createEditorWithBlocks and never destroy them, risking memory leaks for large
block counts; update the test suite to destroy the underlying tiptap editor
after each test (e.g., call editor._tiptapEditor.destroy() or similar) — either
modify createEditorWithBlocks to register created editors for cleanup or ensure
each test calls the destroy method, and add an afterEach hook to iterate tracked
editors and call _tiptapEditor.destroy() to guarantee cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f24cb5e9-d70c-4613-8407-d47a5bd64f40
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
packages/core/package.jsonpackages/core/src/blocks/ListItem/NumberedListItem/IndexingPlugin.test.tspackages/core/src/blocks/ListItem/NumberedListItem/IndexingPlugin.tspackages/core/src/editor/performance.test.tspackages/core/src/extensions/PreviousBlockType/PreviousBlockType.test.tspackages/core/src/extensions/PreviousBlockType/PreviousBlockType.tspackages/xl-ai/package.jsonpackages/xl-multi-column/package.json
packages/core/src/blocks/ListItem/NumberedListItem/IndexingPlugin.test.ts
Outdated
Show resolved
Hide resolved
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
YousefED
changed the title
nperez0111
reviewed
Mar 30, 2026
Contributor
There was a problem hiding this comment.
This change is heavily stateful now, I'm going to need some time to experiment to see if I can break it or not.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes typing/echo lag in documents with many blocks (~50k chars) by optimizing several plugins that were performing O(n) full-document traversals on every transaction.
Closes #2595
Rationale
Several plugins (
PreviousBlockType,NumberedListIndexingPlugin) traversed the entire document on every keystroke. At ~500+ blocks, this caused noticeable input lag. The fix scopes traversals to only the changed ranges.Changes
findChildren(full doc) withfindChildrenInRangescoped totransaction.changedRange(). Maps new-doc range back to old-doc coordinates for accurate comparison.DecorationSet.map()+ incremental updates instead of rebuilding all decorationscompletedGroupspattern — once a decoration matches past the changed range in a blockGroup, skip remaining siblingscalculateListItemIndexto iterative to prevent stack overflow at 10k+ blocksDecorationSet.find()range to the numberedListItem node only (not the full blockContainer subtree) to avoid false matches from nested listsImpact
DecorationSet.map()which is inherently O(n))getChangedRanges→changedRange()change was evaluated and not included (validated to have zero measurable impact: ~3.5μs difference)Testing
Checklist
Summary by CodeRabbit
Chores
Performance
Bug Fixes
Tests