Repository navigation
Bug: LexicalNode.getTopLevelElement invariant does not allow DecoratorNode children of root nodes #6319
Description
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:
In the past it seems this was allowed (#2741), did this change on purpose?
Activity
etrepum commentedon Jun 18, 2024
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
$insertNodeToNearestRootfrom@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 anElementNode.etrepum commentedon Jun 18, 2024
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 commentedon Jul 2, 2024
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 commentedon Jul 2, 2024
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'tgetTopLevelNodeorgetTopLevelElementOrDecorator. In all cases where the given node isn't exactly a top-levelDecoratorNodeit is impossible for this method to return anything butElementNode.m1yon commentedon Jul 2, 2024
I agree; it makes sense for the fix to happen in
$insertNodeToNearestRootrather thangetTopLevelElementThe issue I'm having is when I try to call
$insertNodeToNearestRootwith a decorator node, I get the errorExpected node 25 to have a top parent element.from getTopLevelElementOrThrow.etrepum commentedon Aug 8, 2024
This should've been fixed a few weeks ago, see also: #6451 #6458