Skip to content

[Breaking Change][lexical] Bug Fix: Backspace at block start preserves the current block#8493

Merged
etrepum merged 1 commit into
facebook:mainfrom
mayrang:fix/4359-backspace-heading-preserve-type
May 11, 2026
Merged

[Breaking Change][lexical] Bug Fix: Backspace at block start preserves the current block#8493
etrepum merged 1 commit into
facebook:mainfrom
mayrang:fix/4359-backspace-heading-preserve-type

Conversation

@mayrang

@mayrang mayrang commented May 10, 2026

Copy link
Copy Markdown
Contributor

Description

When a HeadingNode (or any non-empty block ElementNode) sits directly after an empty ParagraphNode, pressing Backspace at the heading's start merges the heading's children into the empty paragraph and removes the heading. The heading's type gets discarded as part of the merge. Separately, when a non-empty HeadingNode is the first block in the document (no previous sibling), pressing Backspace at its start unconditionally replaces it with a brand-new HeadingNode of the same tag — the visible markup is unchanged but the node identity (__key) churns on every Backspace. The Google Docs convention many users expect is the opposite: in the first case, remove the empty previous block and preserve the heading; in the second, leave the heading untouched.

Breaking Change

Two scenarios change at Backspace at a block start:

  • Heading-after-empty-paragraph. Previously the cross-block merge ran and discarded the heading's type. Now the empty paragraph is removed and the heading is preserved. Applies to any non-empty block ElementNode (Heading, Quote, list items, custom subclasses). Observable only in this exact shape; direct removeText() calls and selections spanning non-empty blocks are unchanged.
  • Non-empty heading with no previous block. Previously HeadingNode.collapseAtStart self-replaced with a fresh HeadingNode of the same tag every time Backspace was pressed, churning the node __key and firing a phantom destroyed + created mutation pair even though the visible markup was unchanged. Now the heading is left in place: no __key churn, no mutation pair, no resulting history entry. Mutation listeners on HeadingNode (e.g. LexicalTableOfContentsPlugin) and any external code tracking heading keys stop seeing these phantom events; the visible DOM is unchanged in both old and new behavior.

Fix

Two narrow exceptions, both at the start of a block on Backspace.

  1. Heading after an empty paragraph. RangeSelection.deleteCharacter reaches the merge-block branch with state.caret.origin pointing at the empty adjacent block and state.block pointing at the (non-empty) anchor block. The branch then calls removeText(), and the cross-block merge in $removeTextFromCaretRange always wins on the earlier block's type — that's where the heading is lost. A narrow exception is added at the top of the merge-block branch: when the adjacent block is empty and the anchor block is not, remove just that empty adjacent block and leave the selection where it is. removeText() isn't called, so the cross-block merge never runs and the anchor block's type is preserved. Type-agnostic: applies to Heading, Quote, list items, custom subclasses without per-type branching. The exception is also limited to adjacent blocks at the same nesting level (caret.origin.getParent() === block.getParent()), so a structural wrapper like a ListNode whose only child is an empty ListItemNode falls through to the default cross-block merge — the ListNode is not treated as empty just because its only child is. This mirrors Google Docs' Backspace-at-heading behavior next to an empty bullet (the heading text is pulled into the list item).

  2. Non-empty heading with no previous block. RangeSelection.deleteCharacter falls through the merge logic (no previous block to merge with) and ultimately invokes $collapseAtStart, which walks ancestors calling collapseAtStart until one returns true. HeadingNode.collapseAtStart previously self-replaced with a fresh HeadingNode of the same tag (and moved children over) even when non-empty — semantically a no-op but observable as __key churn. The non-empty branch now just signals "handled" by returning true without replacing. The empty-heading branch (heading → paragraph) is unchanged. QuoteNode and ListItemNode collapseAtStart are intentionally untouched: they implement escape semantics ("Backspace at start escapes to paragraph" / list outdent), so the second exception stays Heading-specific, matching levensta's report.

Scope

Two adjacent cases are still out of scope:

  • The general removeText merge policy. Changing the cross-block merge itself (e.g. "later block wins" or "non-empty wins") would also affect direct removeText() calls and user-initiated selections that span non-empty blocks. That's a broader observable behavior change for any caller relying on the current "earlier block's type wins" rule.
  • The trailing LineBreakNode case. levensta noted on the issue thread that a non-empty paragraph ending in a LineBreakNode triggers the same merge. Treating that as "empty enough" to qualify for the exception is a judgment call about what counts as empty, and seems worth its own discussion.

If a broader scope is preferred, drop a comment and I'll fold either (or both) into this PR.

Refs #4359

Test plan

  • pnpm vitest run --project unit — 113 files / 2493 tests pass. New Backspace at start of heading (#4359) describe block in LexicalHeadingNode.test.ts:
    • Five test.for cases for the merge-block exception: heading preserved after empty paragraph; quote preserved after empty paragraph (type-agnostic); only the empty paragraph is removed between two non-empty siblings; legacy cross-block merge still applies when previous block is non-empty (out-of-scope marker); a ListNode whose only child is an empty ListItemNode falls through to the default cross-block merge (heading text is pulled into the list item), pinning the same-nesting-level limit on the exception.
    • Four direct HeadingNode.collapseAtStart cases for the second exception: non-empty heading preserved with same getKey() (catches the recreate regression — the previous code self-replaced and the test fails without the fix); preserved when the heading is wrapped inside another ElementNode; preserved when the heading is followed by a non-paragraph sibling (e.g. a list); empty heading still converts to paragraph (existing behavior).
  • pnpm tsc clean.
  • e2e Headings/ on chromium — 4/4 pass. The existing '... stays as a heading when you backspace at the start of a heading with no previous sibling nodes present' test covers the DOM-level behavior of the second exception; the getKey() check sits in the unit suite because the regression is invisible at the DOM level. Firefox/webkit weren't re-run this revision — no e2e files changed.
  • Manual playground sanity check:
    • empty paragraph + H1 ("Welcome") with caret at heading start, Backspace → empty paragraph removed, H1 preserved.
    • same shape with a Quote block instead of H1 → Quote preserved.
    • non-empty paragraph + H1 with caret at heading start, Backspace → existing behavior (merged into the previous paragraph, type discarded) — confirms no regression for the out-of-scope case.
    • heading-only at root start, caret at heading start, Backspace pressed multiple times → heading stays, no visible churn; typing afterwards prepends cleanly.

Note: the new direct HeadingNode.collapseAtStart tests invoke the method directly because jsdom doesn't implement Selection.modify, which deleteCharacter falls through to when no previous block exists. The end-to-end keyboard path is covered by the existing e2e test in the Headings/ suite.

@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 May 10, 2026
@etrepum etrepum added the extended-tests Run extended e2e tests on a PR label May 10, 2026
@etrepum

etrepum commented May 10, 2026

Copy link
Copy Markdown
Collaborator

I think this does make sense to scope down, but since it's more of a behavior change than a bug fix we should call it out with a [Breaking Change] in the title and add a ## Breaking Change section to the PR description so we can easily call it out in the release notes

@mayrang mayrang changed the title [lexical] Bug Fix: Preserve current block's type when backspacing past an empty previous block [Breaking Change][lexical] Bug Fix: Preserve current block's type when backspacing past an empty previous block May 10, 2026
@mayrang

mayrang commented May 10, 2026

Copy link
Copy Markdown
Contributor Author

Updated — [Breaking Change] added to the title, dedicated ## Breaking Change section added to the body for release notes. ### Scope kept for the parts intentionally left out.

@etrepum

etrepum commented May 11, 2026

Copy link
Copy Markdown
Collaborator

For some reason the deployments aren't showing up for this branch, I don't have access to introspect vercel but I can't merge without some final QA just in case

@vercel

vercel Bot commented May 11, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
lexical Ready Ready Preview, Comment May 11, 2026 4:25pm
lexical-playground Ready Ready Preview, Comment May 11, 2026 4:25pm

Request Review

@mayrang

mayrang commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

The Vercel deployment seems to have gotten dropped somewhere between the title/body edit on the day I opened this and the breaking-change update — not sure why; happy to do whatever helps if you have a way to retrigger.

In the meantime, ran through manual QA locally (Chromium and Safari) on top of the e2e CI runs that already covered chromium/firefox/webkit on macOS. Six scenarios:

  1. Empty paragraph + H1 with caret at heading start, Backspace → empty paragraph removed, H1 preserved.
  2. Same shape with a Quote block instead of H1 → Quote preserved (type-agnostic).
  3. Before / empty / H1 / After with caret at heading start, Backspace → only the empty paragraph is removed.
  4. Before / H1 (no empty between) with caret at heading start, Backspace → legacy merge into the previous paragraph, heading type discarded (out-of-scope case unchanged, as intended).
  5. H1 with no previous sibling, Backspace at start → heading stays (existing HeadingsBackspaceAtStart flow, different code path).
  6. Two empty paragraphs + H1, Backspace at heading start → one empty paragraph removed; second Backspace removes the other.

All matched expected. Just pushed a no-op commit to nudge Vercel into rebuilding the preview.

@levensta

Copy link
Copy Markdown
Contributor

By the way, this PR reminded me of a problem: when the cursor is at the beginning of a heading and there are no sibling elements before it, pressing Backspace causes the heading to recreate. This doesn't happen with other elements

Screen.Recording.2026-05-11.at.13.55.21.mov

I'm not sure if it's appropriate to fix this bug as part of this PR, but it seems to me that you're pretty close to a solution

@mayrang mayrang changed the title [Breaking Change][lexical] Bug Fix: Preserve current block's type when backspacing past an empty previous block [Breaking Change][lexical] Bug Fix: Backspace at block start preserves the current block May 11, 2026
@mayrang mayrang force-pushed the fix/4359-backspace-heading-preserve-type branch from 580ed62 to 8b6944e Compare May 11, 2026 15:11
@mayrang mayrang requested a review from etrepum May 11, 2026 15:15
@mayrang

mayrang commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

@levensta — thanks for spotting this. Confirmed and folded in (8b6944ea3).

Root cause: HeadingNode.collapseAtStart was self-replacing with a fresh HeadingNode of the same tag every time, even when non-empty. Visible markup didn't change but the node __key churned on each Backspace, firing a phantom destroyed + created mutation pair for any HeadingNode listener (e.g. LexicalTableOfContentsPlugin). Fix in packages/lexical-rich-text/src/index.ts: the non-empty branch now returns true without replacing; the empty-heading-to-paragraph branch is unchanged.

QuoteNode and ListItemNode collapseAtStart are intentionally left alone — they have their own escape semantics (Backspace at start drops to paragraph; list outdent), so the second exception stays Heading-specific.

Tests added in LexicalHeadingNode.test.ts:

  • heading-only at root, Backspace → same getKey() after (catches the recreate)
  • heading wrapped inside another ElementNode → preserved
  • heading followed by a non-paragraph sibling → preserved

While addressing this, a closer look at the merge-block exception surfaced a related cascade gap: caret.origin.remove(...) was being called with preserveEmptyParent=true, which left an empty wrapper behind when the previous block was an empty <ul> containing an empty <li>. Switched to the default and added a test.for case that fails on the previous code.

Updated PR title/body to cover both scopes.

etrepum
etrepum previously approved these changes May 11, 2026

@etrepum etrepum left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

On closer inspection I'm not sure we want this to happen when the preceding block is a ListNode, at least if we're mirroring what google docs does. I think the ListNode should not be considered empty because it has a ListItemNode

@etrepum etrepum dismissed their stale review May 11, 2026 15:54

ListNode behavior

@mayrang

mayrang commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

@etrepum — agreed on the ListNode framing. Narrowed the exception to siblings at the same nesting level (caret.origin.getParent() === block.getParent()), so a ListNode with an empty ListItemNode is no longer treated as an empty previous block. With the narrow in place the list case falls through to the default cross-block merge, which pulls the heading's text into the empty list item — matches Google Docs from what I saw locally.

Pinned that behavior with a test.for case in LexicalHeadingNode.test.ts (list with an empty item: cross-block merge pulls the heading text into the item). If you had a different outcome in mind (e.g. caret moves into the list item with the heading kept), let me know and I'll swap the test. Pushed as 01f7e49a4.

@etrepum etrepum added this pull request to the merge queue May 11, 2026
Merged via the queue into facebook:main with commit eddd735 May 11, 2026
41 checks passed
@etrepum etrepum mentioned this pull request May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

3 participants