Fix null dereference in updateOverlay when after-node uses a custom NodeView

nosooqua commented

2026-04-27 19:24:37 +02:00

First-time contributor

nodeDOM() can return null for nodes rendered with custom NodeViews (e.g.
React-based). The existing null check on line 89 guards the before-node,
but the after-node lookup inside if (before && after) was unguarded.

nodeDOM() can return null for nodes rendered with custom NodeViews (e.g. React-based). The existing null check on line 89 guards the before-node, but the after-node lookup inside `if (before && after)` was unguarded.

nosooqua added 1 commit

2026-04-27 19:24:37 +02:00

Fix null dereference in updateOverlay when after-node uses a custom NodeView e3d717ed80

nodeDOM() can return null for nodes rendered with custom NodeViews (e.g.
React-based). The existing null check on line 89 guards the before-node,
but the after-node lookup inside `if (before && after)` was unguarded.

marijn commented

2026-04-27 19:33:39 +02:00

Owner

nodeDOM can return null when pointed at a text node, at a position where there is no node, or at a position inside an opaque node's inner structure. This code runs only for block positions, so a text node isn't possible. The fact that after is non-null means that there is a node there, and the fact that the node before has a representation means that we're not inside an opaque node.

So theoretically this is not a thing that can happen. Did you run into it? What do you mean by 'React-based node views'? Those will also have an outer node, and should not affect nodeDOM.

`nodeDOM` can return null when pointed at a text node, at a position where there is no node, or at a position inside an opaque node's inner structure. This code runs only for block positions, so a text node isn't possible. The fact that `after` is non-null means that there is a node there, and the fact that the node before has a representation means that we're not inside an opaque node. So theoretically this is not a thing that can happen. Did you run into it? What do you mean by 'React-based node views'? Those will also have an outer node, and should not affect `nodeDOM`.

nosooqua commented

2026-04-28 20:54:55 +02:00

Author

First-time contributor

We found the root cause — it was indeed a bug in our implementation. A transaction was being dispatched synchronously during a NodeView’s initialization (during a drag-and-drop remount), which caused the DOM state to be inconsistent.

However, I’d still suggest adding a null guard for nodeDOM. Currently, the as HTMLElement cast bypasses TypeScript's safety, and if the DOM tree is temporarily out of sync, it leads to a hard crash. Falling back to the coordsAtPos logic instead of crashing would make the editor much more resilient to such edge cases.

We found the root cause — it was indeed a bug in our implementation. A transaction was being dispatched synchronously during a NodeView’s initialization (during a drag-and-drop remount), which caused the DOM state to be inconsistent. However, I’d still suggest adding a null guard for nodeDOM. Currently, the as HTMLElement cast bypasses TypeScript's safety, and if the DOM tree is temporarily out of sync, it leads to a hard crash. Falling back to the coordsAtPos logic instead of crashing would make the editor much more resilient to such edge cases.

marijn commented

2026-04-29 10:04:53 +02:00

Owner

I prefer not to insert checks that paper over issues that originate elsewhere like that. I feel it's generally better to have a crash point you towards a bug than to have it silently sit there.

marijn closed this pull request

2026-04-29 10:04:53 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

Rows
Columns

Fix null dereference in updateOverlay when after-node uses a custom NodeView #23

Pull request closed