[lexical][lexical-list] Bug Fix: Reuse DOM when reconciling cross-parent node moves#8441
Merged
etrepum merged 1 commit intofacebook:mainfrom May 1, 2026
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
10be67e to
f78db38
Compare
…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
f78db38 to
0561ef4
Compare
etrepum
approved these changes
May 1, 2026
Collaborator
etrepum
left a comment
There was a problem hiding this comment.
This seems like a nice optimization!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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. ForDecoratorNodethis swapped the React portal target, souseDecoratorsunmounted and remounted the renderedcomponent. 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)$createNodenow detects cross-parent moves by comparingprevNode.__parenttonode.__parent. When they differ and the prev DOM is still inactivePrevKeyToDOMMap, the existing DOM is reused and reconciled via$reconcileNodeinstead of being recreated. This keeps the DOM identity intact, which React portals and contentEditable selection both rely on. The reuse branch is gated onslot !== nullso call sites that pass a null slot (used for in-place replacements) keep their existing behavior, and$reconcileNodeis never invoked withparentDOM=null.Supporting changes:
$destroyNodeshort-circuits when the key still exists inactiveNextNodeMap(the node moved). It only detaches from the old parent; mutation marking and child destruction are skipped. The mutation marker stays'updated', sincesetMutatedNodealready folded'destroyed' + 'created'to'updated', and listeners that branch on mutation type now see a single event instead of a fold-pair.$reconcileNodere-evaluates element direction when__parentchanges, because$getReconciledDirection's implicit'auto'depends on the parent. Otherwise the reused DOM would keep its olddir="auto"after moving away from a root child.prev=1, next=1fast path in$reconcileChildrenfalls back toslot.insertChildwhenlastDOMwas reused insidereplacementDOM. At that pointlastDOM.parentNodeis no longer the slot element, soreplaceChildwould throw.lexical-list (
packages/lexical-list)The reuse path exposed two latent fragilities in
ListItemNode:$setListItemThemeClassNamesnow 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 freshcreateDOMcall, sinceclassList.addis a no-op for existing classes. After a list-type toggle (which reparents items), the reused DOM kept classes in their original insertion order.updateListItemCheckednow setsaria-checkedunconditionally inside theisCheckboxbranch, mirroringroleandtabIndex. The previous conditional only set the attribute when the node's own__checkedchanged, so a list item moved into a check list ended up missingaria-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 inLexicalReconciler.test.tscover decorator reuse, multi-levelelement subtree reuse, the single-
'updated'mutation event, theslot !== nullguard via a cross-parent swap withupdateDOM=true, and a same-parent reorder regression.pnpm test-e2e-chromium— local run ofList.spec.mjspasses the two scenarios that originally failed CI:
Nested List > Can toggle format for multi-line list of each type without losing indentation stateNested List > Can create check list, toggle it to bullet-list and back'updated'listener event.handles work.
Tab/Shift+Tab5x): text and cursorpreserved.
no DOM divergence.
Fixes: #8420