Skip to content

[lexical-link] Bug Fix: Toggle links with nested children#8078

Merged
etrepum merged 1 commit intofacebook:mainfrom
patrick-atticus:links-nested-children
Jan 16, 2026
Merged

[lexical-link] Bug Fix: Toggle links with nested children#8078
etrepum merged 1 commit intofacebook:mainfrom
patrick-atticus:links-nested-children

Conversation

@patrick-atticus
Copy link
Copy Markdown
Contributor

@patrick-atticus patrick-atticus commented Jan 15, 2026

Description

When a link has nested children, the TOGGLE_LINK_COMMAND does nothing (it should remove the link). This PR adds handling for such cases.

Here's an example of the problematic lexical state:

 root
  └ (47) paragraph 
    └ (48) link "https://lexical.dev/"
      └ (49) heading 
>       └ (50) text "Lexical"

This can be reproduced by copying the title of a google search result and pasting it into the editor.

Changes

The link removal logic only checks the immediate parent of selected nodes, so it cant find the LinkNode if the parent is a HeadingNode.

  1. Changed link detection in RangeSelection to use $findMatchingParent() instead of getParent(), allowing it to traverse up and find ancestor LinkNodes.
  2. Updated $splitLinkAtSelection() to identify children that contain extracted nodes, not just children that are extracted nodes.

Test plan

Captured the behaviour in a unit test, which now passes

@vercel
Copy link
Copy Markdown

vercel bot commented Jan 15, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
lexical Ready Ready Preview, Comment Jan 15, 2026 4:40am
lexical-playground Ready Ready Preview, Comment Jan 15, 2026 4:40am

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 15, 2026
@etrepum etrepum added the extended-tests Run extended e2e tests on a PR label Jan 15, 2026
etrepum
etrepum reviewed Jan 15, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@etrepum etrepum left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This structure is a normalization failure, LinkNode is only supposed to wrap inline nodes as currently implemented. It's probably fine to do a little more extra work here so that it does work, but the real bug is that this structure was created in the first place (due to lack of normalization code to handle paste, probably).

Generally speaking there are three kinds of ElementNode and that determines what kind of children they are allowed to have:

  • Roots (RootNode and shadow roots like TableNode, TableRowNode, TableCellNode) must be non-inline and must only have block ElementNode (or DecoratorNode) children
  • Blocks (any other non-inline node, like ParagraphNode or HeadingNode; non-inline DecoratorNode are blocks too, like HorizontalRuleNode, but they can not have children), if it is an ElementNode it must only have inline children
  • Inline (like LinkNode, ListNode, ListItemNode, or MarkNode), must only have inline children

If that general rule about ElementNodes isn't followed, you'll hit edge cases because there are tacit assumptions that this structure is followed all over the place.

@patrick-atticus
Copy link
Copy Markdown
Contributor Author

patrick-atticus commented Jan 15, 2026 •
edited
Loading

I have actually worked on the normalization already - did you want me to include it in this PR or a separate change?

@etrepum
Copy link
Copy Markdown
Collaborator

etrepum commented Jan 16, 2026

That's not necessary, having normalization be separate from this makes more sense. I was just leaving some comments while I was waiting to make sure the extended test suite still passed

@etrepum etrepum added this pull request to the merge queue Jan 16, 2026
Merged via the queue into facebook:main with commit dbe2fcc Jan 16, 2026
46 checks passed
@levensta levensta mentioned this pull request Jan 17, 2026
Closed
@etrepum etrepum mentioned this pull request Jan 31, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@etrepum etrepum etrepum approved these changes

@zurfyx zurfyx Awaiting requested review from zurfyx zurfyx is a code owner

@fantactuka fantactuka Awaiting requested review from fantactuka fantactuka is a code owner

@acywatson acywatson Awaiting requested review from acywatson acywatson is a code owner

@ivailop7 ivailop7 Awaiting requested review from ivailop7 ivailop7 is a code owner

@potatowagon potatowagon Awaiting requested review from potatowagon potatowagon is a code owner

@takuyakanbr takuyakanbr Awaiting requested review from takuyakanbr takuyakanbr is a code owner

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@patrick-atticus @etrepum