[Breaking Change][lexical] Fix: Preserve the first linebreak when passing inline nodes to insertNodes

[levensta]

Description

1. Changed normalization of inline nodes when inserting via the RangeSelection.insertNodes method. The first linebreak is always preserved
2. The attribute data-lexical-managed-linebreak="true" has been added to managed line break. This is done to make debugging and reading tests easier, where you need to distinguish where <br> is inserted from LineBreakNode, and where a managed line break is used to display an empty line.
   
   For example, this is what a paragraph containing a single line break would look like in the DOM

   <p>
     <br>
     <br data-managed-linebreak="true">
   </p>

Closes #3980

Test plan

Before

For example, if when calling $insertNodes() there are inline nodes, at least one block element, and the first node is a LineBreakNode

<br>
hello world
<p></p>

Then the linebreak is removed, and the remaining inlines are wrapped in a paragraph

 root
  ├ paragraph 
  | └ text "hello world"
  └ paragraph 

After

Now the first linebreak is preserved

 root
  ├ paragraph 
  | ├  linebreak 
  | └ text "hello world"
  └ paragraph 

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
lexical	Ready	Preview, Comment	Jun 11, 2026 10:37pm
lexical-playground	Ready	Preview, Comment	Jun 11, 2026 10:37pm

[levensta]

There are two <br> in the failed test because one of them is a managed line break and the other is a LineBreakNode

We can either settle for a small refactoring of wrapInlineNodes or try to solve this issue #3980

[levensta]

I'm also thinking about how we could export a common function to normalize inline elements for shadow root elements.

If we use this function as is, its use in a transformation would look like this:

  const children = node.getChildren();
  if (
    node.isEmpty() ||
    children.some(
      child =>
        $isLineBreakNode(child) ||
        $isInlineElementOrDecoratorNode(child) ||
        $isTextNode(child) ||
        child.isParentRequired(),
    )
  ) {
    node.append(...$wrapInlineNodes(children).getChildren());
  }

Which doesn't look effective

[potatowagon]

Review (automated — Navi for @potatowagon)

Status: Not ready to approve — CI still failing

What changed (latest commit 007cd16)

The code change is a clean simplification of $wrapInlineNodes:

• Removes the special-case that discarded the first LineBreakNode (the old if (isLineBreakNode) { continue; } path)
• Now appends LineBreakNodes to the parent element like any other inline node
• Uses $isInlineElementOrDecoratorNode() helper instead of manual checks (DRYer)
• Re-enables 4 previously skipped tests (referencing issue Bug: Pasting text immediately after an inline element (e.g. link) causes the text and its following siblings to be moved into the inline element #4295)
• Adds a regression test proving LineBreakNode is preserved when followed by a block element

Assessment

Logic correctness: The simplification is sound — there's no semantic reason to silently discard the first linebreak. The old behavior was a lossy workaround.

Concern: Unit tests are still failing (core-tests/unit on Node 22.x and 24.x). As the author notes, the failure is because LexicalSelectionHelpers.test.ts:2681 expects one <br> but gets two — one managed linebreak and one explicit LineBreakNode. This reflects a deeper issue (tracked in #3980) about duplicate line breaks.

Breaking change risk: This IS a breaking change — code that relied on the first linebreak being stripped will now see an extra linebreak. The author has correctly labeled it as such.

Recommendation: The code direction is correct but CI must be green before merging. Either:

1. Fix the failing test expectation if the new behavior is intentionally different, or
2. Address the underlying Bug: Line Breaks <br/> breaks markup #3980 managed-linebreak/LineBreakNode duplication

Will re-review once CI is green.

[potatowagon]

Review — Preserve First Linebreak in insertNodes

Assessment: Has concerns ⚠️

What I verified:

1. Logic correctness: The change to $wrapInlineNodes simplifies the wrapping logic — it now unconditionally appends LineBreakNodes into currentBlock rather than skipping them (the old code created a parent block but then continued, effectively dropping the LineBreak from the block's children). The refactored code is cleaner: it uses $isInlineElementOrDecoratorNode and always appends to currentBlock.

2. Breaking change assessment: This is marked as a breaking change. The behavior change is: previously, a leading LineBreakNode before inline content would result in an empty paragraph; now it's preserved inside the wrapping paragraph. This is arguably more correct behavior (issue #4295), but it may affect existing consumers who rely on the current insertNodes behavior.

3. Test coverage: Tests that were previously commented out (referencing issue #4295) are now enabled. A new regression test ($wrapInlineNodes regression) explicitly verifies LineBreakNode is preserved in the wrapping paragraph.

Concerns:

CI is failing: Unit tests on Node 22.x and 24.x are red. This needs to be addressed before merge. The e2e tests pass, but the core unit tests do not — which likely means the test expectations or the logic has a mismatch that needs investigation.

The fix should be verified once CI is green. The conceptual change is sound, but cannot recommend landing with failing tests.

— via Navi on behalf of potatowagon

[levensta]

I just corrected the test, in general it already reflects the correct behavior: 1 <br> is the required managed linebreak, and the second <br> is a visible line break in the form of a LineBreakNode node

[etrepum]

Maybe we should set something on the DOM on the managed line breaks like data-lexical-linebreak="true" like we do with the webkit hack IMG

[levensta]

I've thought about this, but it doesn't seem to serve much purpose. Maybe just as additional information that this is a special <br> and as a sort of self-documenting explanation in the tests. Or perhaps to maintain consistency with the WebKit branch

Or perhaps we should instead set the data-lexical-linebreak attribute for the LineBreakNode. Just as we already have the data-lexical-text and data-lexical-decorator attributes.

[etrepum]

data-lexical-managed-linebreak might be a better name for the attribute. I don't think setting data-lexical-linebreak on all LineBreakNode is worthwhile. The value here is mostly to help people with debugging and to be an easy target to unambiguously normalize them away (e.g. on import or on a serialization style html export)

[levensta]

Done, now all managed linebreaks have the attribute