Skip to content

Guard findNodeKeyUp against out-of-range positions#162

Merged
coolov merged 4 commits into
mainfrom
create-find-node-key-up-check
Apr 1, 2026
Merged

Guard findNodeKeyUp against out-of-range positions#162
coolov merged 4 commits into
mainfrom
create-find-node-key-up-check

Conversation

@coolov
Copy link
Copy Markdown
Contributor

@coolov coolov commented Apr 1, 2026

Issue

findNodeKeyUp calls doc.resolve(pos) without validating that pos is within the document bounds, causing a "RangeError: Position -1 out of range" crash when a node view's getPos() returns -1 (the standard sentinel for nodes that are no longer part of the document)

Fix

This PR adds a bounds check pos < 0 that returns ROOT_NODE_KEY early, if the node has no valid position, there are no ancestors to find

@coolov coolov requested a review from a team as a code owner April 1, 2026 16:10
const pluginState = reactPluginKey.getState(editorView.state);
if (!pluginState) return ROOT_NODE_KEY;

if (pos < 0) return ROOT_NODE_KEY;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment here about the -1 sentinel/why we have this check?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, also we could make this slightly more self-explanatory by doing an equality check to -1. but that's a nit.

@saranrapjs
Copy link
Copy Markdown
Member

I think we use the yarn berry release process (e.g. yarn version check -i) to manage releases, but maybe this doesn't matter if we're aiming to publish this in the same step as we're making the change?

@coolov coolov merged commit cf4fe97 into main Apr 1, 2026
2 checks passed
@coolov coolov deleted the create-find-node-key-up-check branch April 1, 2026 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants