Bug: `LexicalNode.getTopLevelElement` invariant does not allow `DecoratorNode` children of root nodes

[decademoon]

Lexical version: 0.16.0

Steps To Reproduce

In the playground, start with an empty editor and then insert a horizontal rule.

root
  ├ (49) paragraph 
  ├ (50) horizontalrule 
  └ (51) paragraph

So then it seems DecoratorNode can be a top level element (as expected), however the invariant in getTopLevelElement disallows this:

https://github.com/facebook/lexical/blob/fcf1ae6d1b98dd4c7e288ba2e0a0f1f23f520a63/packages/lexical/src/LexicalNode.ts#L351-L365

In the past it seems this was allowed (#2741), did this change on purpose?

[etrepum]

This change was probably my fault in #5223. I couldn't find any existing code that used DecoratorNode as children of the root and it seemed that there was assumptions that all children of the root were ElementNode.

As far as I can tell the only call of this method from the monorepo that could possibly return a decorator is $insertNodeToNearestRoot from @lexical/utils. I think it would be better to special case the rare case where a decorator might be a root child rather than have to handle the decorator case at every call site where the node legitimately expects a parent to be an ElementNode.

[etrepum]

Note also that the Flow types were changed to explicitly declare that these methods should only return ElementNode or null back in #2261 (and have not been changed since), so the change I made also agrees with this choice.

[m1yon]

I couldn't find any existing code that used DecoratorNode as children of the root and it seemed that there was assumptions that all children of the root were ElementNode.

In the playground, both the YoutubeNode and TweetNode are decorator nodes that can be inserted at the root level yeah?

Forcing root-level DecoratorNodes to be wrapped in ParagraphNodes or some other ElementNode doesn't make much sense unless I'm missing something.

[etrepum]

I think the best fix for this would be to handle top-level DecoratorNodes separately from whatever code is currently calling node.getTopLevelElement() ($insertNodeToNearestRoot? it's unclear since the original issue did not include a repro or traceback) rather than to change the signature of this method. The name isn't getTopLevelNode or getTopLevelElementOrDecorator. In all cases where the given node isn't exactly a top-level DecoratorNode it is impossible for this method to return anything but ElementNode.

[m1yon]

I think the best fix for this would be to handle top-level DecoratorNodes separately from whatever code is currently calling node.getTopLevelElement() ($insertNodeToNearestRoot? it's unclear since the original issue did not include a repro or traceback) rather than to change the signature of this method. The name isn't getTopLevelNode or getTopLevelElementOrDecorator. In all cases where the given node isn't exactly a top-level DecoratorNode it is impossible for this method to return anything but ElementNode.

I agree; it makes sense for the fix to happen in $insertNodeToNearestRoot rather than getTopLevelElement

The issue I'm having is when I try to call $insertNodeToNearestRoot with a decorator node, I get the error Expected node 25 to have a top parent element. from getTopLevelElementOrThrow.

[etrepum]

This should've been fixed a few weeks ago, see also: #6451 #6458