eui icon indicating copy to clipboard operation
eui copied to clipboard

[EuiMarkdownEditor] `getCursorNode` function throws error in some cases

Open i-a-n opened this issue 3 years ago • 0 comments

while developing our own custom plugin for EuiMarkdownEditor we noticed that any markdown AST node that 1. has children and 2. isn't of type === 'text' will throw errors in the console. this can be replicated most simply by creating a multiline blockquote and then typing around in it:

> foo
> bar

Screen Shot 2021-12-22 at 9 29 23 AM

the getCursorNode function fails here: https://github.com/elastic/eui/blob/main/src/components/markdown_editor/markdown_editor.tsx#L300

for (let i = 0; i < node.children.length; i++) {
              const child = node.children[i];
              if (
                child.position.start.offset < selectionStart &&

...as it assumes that every non-text node with children should be drilled down into, and each of those child nodes is expected to have .position.start.offset present. this isn't the case for nodes like, say blockquote, which can have child nodes like type: 'break'. we discovered this during development of our own plugin because it also creates child nodes who have no position.

my first instinct would be to safeguard those object properties, something like child.position?.start?.offset, which would hopefully just cause that node to be bypassed. but I'm not totally familiar with AST traversal and its implementation here, so I wanted to make sure developers who know this component better than me (like @miukimiu and @chandlerprall ) would approve of this fix, or have other ideas.

thanks!

i-a-n avatar Dec 22 '21 17:12 i-a-n