Skip to content

[lexical][lexical-list] Bug Fix: Reuse DOM when reconciling cross-parent node moves#8441

Merged
etrepum merged 1 commit intofacebook:mainfrom
mayrang:fix/reconciler-cross-parent-move-dom-reuse
May 1, 2026
Merged

[lexical][lexical-list] Bug Fix: Reuse DOM when reconciling cross-parent node moves#8441
etrepum merged 1 commit intofacebook:mainfrom
mayrang:fix/reconciler-cross-parent-move-dom-reuse

Conversation

@mayrang
Copy link
Copy Markdown
Contributor

@mayrang mayrang commented May 1, 2026

Description

When a node moves to a different parent in the same transaction (for example, wrapping an image in a link via $toggleLink), the reconciler destroyed the old DOM and created a fresh one in the new parent. For DecoratorNode this swapped the React portal target, so useDecorators unmounted and remounted the rendered
component. On Safari this shows up as a 1-frame flicker for image links (#8420).

The same destroy-then-create produced a secondary symptom too: floating-UI plugins that measured the reference element during this window saw the wrapper span collapsed to its line-height, so the toolbar appeared at the wrong position on the first toggle.

Fix

Reconciler (packages/lexical)

$createNode now detects cross-parent moves by comparing prevNode.__parent to node.__parent. When they differ and the prev DOM is still in activePrevKeyToDOMMap, the existing DOM is reused and reconciled via $reconcileNode instead of being recreated. This keeps the DOM identity intact, which React portals and contentEditable selection both rely on. The reuse branch is gated on slot !== null so call sites that pass a null slot (used for in-place replacements) keep their existing behavior, and $reconcileNode is never invoked with parentDOM=null.

Supporting changes:

  • $destroyNode short-circuits when the key still exists in activeNextNodeMap (the node moved). It only detaches from the old parent; mutation marking and child destruction are skipped. The mutation marker stays 'updated', since setMutatedNode already folded 'destroyed' + 'created' to 'updated', and listeners that branch on mutation type now see a single event instead of a fold-pair.
  • $reconcileNode re-evaluates element direction when __parent changes, because $getReconciledDirection's implicit 'auto' depends on the parent. Otherwise the reused DOM would keep its old dir="auto" after moving away from a root child.
  • The prev=1, next=1 fast path in $reconcileChildren falls back to slot.insertChild when lastDOM was reused inside replacementDOM. At that point lastDOM.parentNode is no longer the slot element, so replaceChild would throw.

lexical-list (packages/lexical-list)

The reuse path exposed two latent fragilities in ListItemNode:

  • $setListItemThemeClassNames now removes all variable theme classes (listitemChecked, listitemUnchecked, nested.listitem) before re-adding the ones that should be present. Previously, the className string only ended up in canonical order on a fresh createDOM call, since classList.add is a no-op for existing classes. After a list-type toggle (which reparents items), the reused DOM kept classes in their original insertion order.
  • updateListItemChecked now sets aria-checked unconditionally inside the isCheckbox branch, mirroring role and tabIndex. The previous conditional only set the attribute when the node's own __checked changed, so a list item moved into a check list ended up missing aria-checked.

While #8420 is the immediate motivation, the change applies to all cross-parent moves, including drag/drop into a different paragraph, list indent/outdent, and list-type toggles.

Test plan

  • pnpm test-unit — 2386 pass. New tests in
    LexicalReconciler.test.ts cover decorator reuse, multi-level
    element subtree reuse, the single-'updated' mutation event, the
    slot !== null guard via a cross-parent swap with
    updateDOM=true, and a same-parent reorder regression.
  • pnpm test-e2e-chromium — local run of List.spec.mjs
    passes the two scenarios that originally failed CI:
    • Nested List > Can toggle format for multi-line list of each type without losing indentation state
    • Nested List > Can create check list, toggle it to bullet-list and back
  • Manual playground (Safari + Chrome):
    • Image link wrap/unwrap (5x toggle): no flicker, single
      'updated' listener event.
    • Image drag and drop across paragraphs: image preserved, resize
      handles work.
    • List indent / outdent (Tab / Shift+Tab 5x): text and cursor
      preserved.
    • Image link undo / redo (5x, repeated with accumulated state):
      no DOM divergence.
    • Korean IME input followed by link toggle: no character loss.
    • Equation node wrapped in a link: no flicker.
  • CI e2e (chromium / firefox / webkit, including collab).

Fixes: #8420

@vercel
Copy link
Copy Markdown

vercel Bot commented May 1, 2026

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

Project Deployment Actions Updated (UTC)
lexical Ready Ready Preview, Comment May 1, 2026 0:26am
lexical-playground Ready Ready Preview, Comment May 1, 2026 0:26am

Request Review

@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 1, 2026
@mayrang mayrang force-pushed the fix/reconciler-cross-parent-move-dom-reuse branch from 10be67e to f78db38 Compare May 1, 2026 12:18
@mayrang mayrang changed the title [lexical] Bug Fix: Reuse DOM when reconciling cross-parent node moves [lexical][lexical-list] Bug Fix: Reuse DOM when reconciling cross-parent node moves May 1, 2026
…ent node moves

When a node moves to a different parent in the same transaction
(for example, wrapping an image in a link via $toggleLink), the
reconciler destroyed the old DOM and created a fresh one in the
new parent. For DecoratorNode this swapped the React portal
target, forcing useDecorators to unmount and remount the rendered
component (visible as a 1-frame flicker on Safari for image
links, facebook#8420).

$createNode now detects cross-parent moves by comparing
prevNode.__parent to node.__parent. When they differ and the prev
DOM is still in activePrevKeyToDOMMap, the existing DOM is reused
and reconciled via $reconcileNode. The reuse branch is gated on
slot !== null so call sites that pass a null slot keep their
existing behavior and $reconcileNode is never invoked with
parentDOM=null.

Supporting changes in the reconciler:
- $destroyNode short-circuits when the key still exists in
  activeNextNodeMap so it only detaches from the old parent;
  mutation marking and child destruction are skipped. The
  resulting marker is still 'updated' (setMutatedNode already
  folded 'destroyed' + 'created'), now emitted as a single event.
- $reconcileNode re-evaluates element direction when __parent
  changes, since $getReconciledDirection's implicit 'auto'
  depends on the parent.
- The prev=1, next=1 fast path falls back to slot.insertChild
  when lastDOM was reused inside replacementDOM.

Supporting changes in lexical-list (exposed by the reuse path):
- $setListItemThemeClassNames now removes all variable theme
  classes (checked/unchecked, nested) before re-adding so the
  className string stays in canonical order regardless of how
  the dom got there. Without this, list items reused across a
  list-type toggle ended up with classes in insertion order
  rather than a stable order.
- updateListItemChecked now sets aria-checked unconditionally
  inside the isCheckbox branch, mirroring role and tabIndex.
  The previous conditional skipped the attribute when a node
  moved into a check list without its own __checked changing.

Refs: facebook#8420
Copy link
Copy Markdown
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

This seems like a nice optimization!

@etrepum etrepum added this pull request to the merge queue May 1, 2026
Merged via the queue into facebook:main with commit 8e1912f May 1, 2026
44 checks passed
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.

Bug: Safari-specific rendering issues with FloatingLinkEditor on image links

2 participants