Skip to content

Commit e8c0831

Browse files
committed
fix: don't crash when a node view's getPos() is stale
`getPos()` is derived from ProseMirror's view-desc tree, but `updateStateInner` assigns the new state before it reconciles that tree. Anything rendering part-way through reconciliation - a re-entrant dispatch from a node view effect, TipTap's `flushSync` while mounting a node view - therefore reads a position that no longer lines up with `view.state.doc`. Resolving it threw `RangeError: Position N out of range`, or `Node should be a bnBlock, but is instead: doc` when the position was in range but pointed at the wrong node. Under React 19 that tears down the consumer's tree rather than being rethrown. Upstream considers this TipTap's problem (ProseMirror/prosemirror#1532) and TipTap considers `flushSync` unavoidable, so recover on our side: - core `getBlockFromNodeView`: falls back to building the block from the node alone. Reached only when a re-entrant dispatch superseded the document, so the node is gone from it and there is no container to read an id from - `type`, `props` and `content` are correct, `id` is freshly generated and belongs to no block. - react `useNodeViewBlock`: falls back to the last block it resolved, seeded from the one core resolved at construction. Both are silent and transient; ProseMirror rebuilds the node view against the current document immediately after, so a stale frame is invisible where a throw is not.
1 parent 95e1f24 commit e8c0831

8 files changed

Lines changed: 656 additions & 9 deletions

File tree

‎packages/core/src/schema/blocks/createSpec.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
import { nonFormattingMarks } from "../markGroups.js";
1616
import { PropSchema } from "../propTypes.js";
1717
import {
18-
getBlockFromPos,
18+
getBlockFromNodeView,
1919
propsToAttributes,
2020
wrapInBlockStructure,
2121
} from "./internal.js";
@@ -247,8 +247,15 @@ export function addNodeAndExtensionsToSpec<
247247
return (props) => {
248248
// Gets the BlockNote editor instance
249249
const editor = this.options.editor;
250-
// Gets the block
251-
const block = getBlockFromPos(props.getPos, props.view.state.doc);
250+
// Gets the block. Resolving this can't rely on `getPos()` alone —
251+
// node views are constructed part-way through ProseMirror's
252+
// reconciliation, where positions don't always line up with
253+
// `view.state.doc` yet (see `getBlockFromNodeView`).
254+
const block = getBlockFromNodeView(
255+
props.getPos,
256+
props.node,
257+
props.view.state.doc,
258+
);
252259
// Gets the custom HTML attributes for `blockContent` nodes
253260
const blockContentDOMAttributes =
254261
this.options.domAttributes?.blockContent || {};
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import { beforeEach, describe, expect, it } from "vite-plus/test";
2+
3+
import { getNodeById } from "../../api/nodeUtil.js";
4+
import { BlockNoteEditor } from "../../editor/BlockNoteEditor.js";
5+
import { getBlockFromNodeView } from "./internal.js";
6+
7+
describe("getBlockFromNodeView", () => {
8+
let editor: BlockNoteEditor;
9+
10+
beforeEach(() => {
11+
editor = BlockNoteEditor.create({
12+
trailingBlock: false,
13+
initialContent: [
14+
{ type: "paragraph", content: "first" },
15+
{ type: "paragraph", content: "second" },
16+
],
17+
});
18+
});
19+
20+
it("resolves from the position when it is valid", () => {
21+
const doc = editor.prosemirrorState.doc;
22+
const target = editor.document[1];
23+
// `getBlockFromPos` resolves the *parent* of the position, so the position
24+
// sits just inside the block container — which is what ProseMirror hands a
25+
// node view.
26+
const pos = getNodeById(target.id, doc)!.posBeforeNode + 1;
27+
const block = getBlockFromNodeView(() => pos, doc.nodeAt(pos)!, doc) as any;
28+
29+
expect(block.id).toBe(target.id);
30+
});
31+
32+
it("builds a standalone block when the node is no longer in the document", () => {
33+
// The only way `getPos()` fails at node view construction is a re-entrant
34+
// dispatch superseding `view.state.doc`, which takes the node with it — so
35+
// the node genuinely isn't in the document any more. See #2937.
36+
const doc = editor.prosemirrorState.doc;
37+
const orphan = editor.pmSchema.nodes.paragraph.create(
38+
{ textAlignment: "right" },
39+
editor.pmSchema.text("orphaned content"),
40+
);
41+
const block = getBlockFromNodeView(
42+
() => doc.content.size + 1000,
43+
orphan,
44+
doc,
45+
) as any;
46+
47+
// Everything readable off the node is correct...
48+
expect(block.type).toBe("paragraph");
49+
expect(block.props.textAlignment).toBe("right");
50+
expect(block.content[0].text).toBe("orphaned content");
51+
// ...but the id belongs to no block in the document, so it must not
52+
// collide with a real one.
53+
expect(editor.document.map((b) => b.id)).not.toContain(block.id);
54+
});
55+
56+
it("recovers from an in-range position that resolves to the wrong node", () => {
57+
// Position 0 resolves to the doc rather than a block container — the shape
58+
// a bounds check alone would not catch.
59+
const doc = editor.prosemirrorState.doc;
60+
const orphan = editor.pmSchema.nodes.paragraph.create(
61+
null,
62+
editor.pmSchema.text("orphaned content"),
63+
);
64+
expect(() => getBlockFromNodeView(() => 0, orphan, doc)).not.toThrow();
65+
});
66+
67+
it("recovers when the position is undefined", () => {
68+
const doc = editor.prosemirrorState.doc;
69+
const orphan = editor.pmSchema.nodes.paragraph.create(
70+
null,
71+
editor.pmSchema.text("orphaned content"),
72+
);
73+
expect(() =>
74+
getBlockFromNodeView(() => undefined, orphan, doc),
75+
).not.toThrow();
76+
});
77+
});

‎packages/core/src/schema/blocks/internal.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,74 @@ export function getBlockFromPos(getPos: () => number | undefined, doc: PMNode) {
9393
return block;
9494
}
9595

96+
/**
97+
* Resolves the block a node view should render, tolerating a `getPos()` that
98+
* can't be trusted.
99+
*
100+
* ProseMirror derives `getPos()` from its view-desc tree, but
101+
* `EditorView.updateStateInner` assigns the new state *before* it reconciles
102+
* that tree. Anything that runs partway through reconciliation - a re-entrant
103+
* dispatch from a node view's effect, TipTap's `flushSync` while mounting a
104+
* node view - therefore sees positions that no longer line up with
105+
* `view.state.doc`. The position then either lands out of range or, just as
106+
* bad, in range but pointing at the wrong node.
107+
*
108+
* Node views are constructed inside that same window, so this applies at
109+
* construction just as much as on re-render. The state is always transient:
110+
* ProseMirror finishes reconciling and rebuilds the node view against the
111+
* current document immediately after. So we degrade instead of throwing,
112+
* because a stale frame is invisible where a throw is not.
113+
*
114+
* See issues #2937, #2682 and #2621.
115+
*/
116+
export function getBlockFromNodeView(
117+
getPos: () => number | undefined,
118+
node: PMNode,
119+
doc: PMNode,
120+
) {
121+
let positionError: unknown;
122+
123+
try {
124+
return getBlockFromPos(getPos, doc);
125+
} catch (e) {
126+
// Kept only to rethrow below if there turns out to be nothing to render.
127+
// Otherwise deliberately silent: this is expected and self-correcting, and
128+
// there is nothing a consumer could do about it in the meantime.
129+
positionError = e;
130+
}
131+
132+
// Whenever this is reached, the node is not in `doc` — a re-entrant dispatch
133+
// superseded the document ProseMirror is building node views for, and the
134+
// node went with it. So there is no container to read an id from, and the
135+
// block has to be built from the node alone.
136+
//
137+
// `type`, `props` and `content` are read off the node and are correct.
138+
// `children` is empty and `id` is freshly generated, i.e. it belongs to no
139+
// block in the document — callers must not treat it as addressable. (It can't
140+
// collide with a real block: ids are uuids, and the deterministic test-mode
141+
// generator shares one monotonic counter with real ids.) This is short-lived;
142+
// ProseMirror rebuilds the node view against the real document right after.
143+
//
144+
// The alternatives are worse. Throwing is the crash this exists to prevent.
145+
// Returning an empty placeholder node view can leave the block *permanently*
146+
// blank, since vanilla node views don't implement `update()` and so are only
147+
// rebuilt when something else changes the document.
148+
//
149+
// `createAndFill` rather than `create` so an unexpected node shape yields
150+
// `null` instead of throwing over the top of the original failure.
151+
const standalone = doc.type.schema.nodes["blockContainer"]?.createAndFill(
152+
null,
153+
node,
154+
);
155+
if (standalone) {
156+
return nodeToBlock(standalone, doc);
157+
}
158+
159+
// Nothing left to render from. Surface the original failure rather than
160+
// inventing a block that isn't grounded in anything.
161+
throw positionError;
162+
}
163+
96164
// Function that wraps the `dom` element returned from 'blockConfig.render' in a
97165
// `blockContent` div, which contains the block type and props as HTML
98166
// attributes. If `blockConfig.render` also returns a `contentDOM`, it also adds

‎packages/react/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ export * from "./hooks/useEditorState.js";
139139
export * from "./schema/ReactBlockSpec.js";
140140
export * from "./schema/ReactInlineContentSpec.js";
141141
export * from "./schema/ReactStyleSpec.js";
142+
export * from "./schema/useNodeViewBlock.js";
142143

143144
export * from "./icons.js";
144145

‎packages/react/src/schema/ReactBlockSpec.tsx‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
Extension,
1111
ExtensionFactoryInstance,
1212
ExtractBlockConfigFromConfigOrCreator,
13-
getBlockFromPos,
1413
mergeCSSClasses,
1514
Props,
1615
PropSchema,
@@ -23,6 +22,7 @@ import {
2322
} from "@tiptap/react";
2423
import { FC, ReactNode } from "react";
2524
import { renderToDOMSpec } from "./@util/ReactRenderUtil.js";
25+
import { useNodeViewBlock } from "./useNodeViewBlock.js";
2626

2727
// this file is mostly analogoues to `customBlocks.ts`, but for React blocks
2828

@@ -267,17 +267,20 @@ export function createReactBlockSpec<
267267
},
268268
render(block, editor) {
269269
if (this.renderType === "nodeView") {
270+
// The block core's `addNodeView` resolved when this node view was
271+
// constructed (itself guarded, via `getBlockFromNodeView`). Seeds
272+
// the fallback below so there is always something to render.
273+
const initialBlock = block;
274+
270275
return ReactNodeViewRenderer(
271276
(props: NodeViewProps) => {
272277
// Vanilla JS node views are recreated on each update. However,
273278
// using `ReactNodeViewRenderer` makes it so the node view is
274279
// only created once, so the block we get in the node view will
275280
// be outdated. Therefore, we have to get the block in the
276-
// `ReactNodeViewRenderer` instead.
277-
const block = getBlockFromPos(
278-
props.getPos,
279-
props.view.state.doc,
280-
);
281+
// `ReactNodeViewRenderer` instead. That position can be stale,
282+
// so resolving it is guarded (see `useNodeViewBlock`).
283+
const block = useNodeViewBlock(props, initialBlock);
281284

282285
const ref = useReactNodeView().nodeViewContentRef;
283286

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { Block, getBlockFromPos } from "@blocknote/core";
2+
import type { NodeViewProps } from "@tiptap/react";
3+
import { useRef } from "react";
4+
5+
/**
6+
* Resolves the `Block` that a React node view should render.
7+
*
8+
* A node view's `getPos()` is not always usable. ProseMirror computes it by
9+
* walking the view-desc tree, but `EditorView.updateStateInner` assigns the new
10+
* state *before* it reconciles that tree, so any render that happens partway
11+
* through reconciliation sees positions computed against a half-updated tree
12+
* while `view.state.doc` is already the new document. React node views get
13+
* rendered in exactly that window: TipTap's `ReactRenderer` calls `flushSync`
14+
* while mounting a node view, and anything that runs from the resulting commit
15+
* (a layout effect, a re-entrant dispatch, another node view's pending update)
16+
* renders with a position that no longer matches the document. `getPos()` can
17+
* also return `undefined` outright once a desc has been detached.
18+
*
19+
* The resulting position is either out of range (`RangeError: Position N out of
20+
* range`) or in range but pointing at the wrong node (`Node should be a
21+
* bnBlock, but is instead: doc`) — both thrown out of the render path, which
22+
* under React 19 tears down the consumer's tree rather than being rethrown.
23+
*
24+
* Upstream considers this TipTap's problem (ProseMirror/prosemirror#1532) and
25+
* TipTap considers `flushSync` unavoidable, so we recover here instead, by
26+
* reusing the last block we rendered. The bad position is always transient —
27+
* ProseMirror finishes reconciling and re-renders the node view with a valid
28+
* one immediately after — so a stale frame is invisible, whereas a throw is
29+
* not.
30+
*
31+
* See BlockNote issues #2937, #2682 and #2621.
32+
*/
33+
export function useNodeViewBlock(
34+
props: NodeViewProps,
35+
/**
36+
* The block core's `addNodeView` resolved when the node view was constructed
37+
* — via `getBlockFromNodeView`, so it is already guarded against the same
38+
* problem. Seeds the fallback so there is always a block to render.
39+
*/
40+
initialBlock: Block<any, any, any>,
41+
): Block<any, any, any> {
42+
const lastBlockRef = useRef(initialBlock);
43+
const doc = props.view.state.doc;
44+
45+
try {
46+
// Deliberate render-phase write: a monotonic "last good value" cache, so a
47+
// repeated render (e.g. StrictMode's double invoke) recomputes the same
48+
// thing.
49+
lastBlockRef.current = getBlockFromPos(props.getPos, doc);
50+
} catch {
51+
// Expected and self-correcting, so deliberately silent: ProseMirror
52+
// re-renders the node view with a usable position immediately after, and
53+
// there is nothing a consumer could do about it in the meantime.
54+
}
55+
56+
return lastBlockRef.current;
57+
}

0 commit comments

Comments
 (0)