[potatowagon]

Bug

#8640 ("Port node classes to the $config() protocol") introduced a silent backwards-compatibility break in the LexicalNode clone contract, found while syncing the change into Meta's internal www codebase (D112781184).

What breaks

When a node drops its explicit static clone() in favor of $config() (e.g. HeadingNode), getStaticNodeConfig auto-synthesizes a zero-arg clone:

klass.clone = (prevNode) => {
  setPendingNodeToClone(prevNode);
  return new klass();
};

That synthesized clone only produces a correct instance when it is driven through the internal $cloneWithProperties / $copyNode wrappers, because those wrappers are what call afterCloneFrom(prevNode) afterwards (which is where subclasses copy their own state). The synthesized clone itself only preserves __key (via setPendingNodeToClone), not properties.

Any caller that invokes NodeClass.clone(node) / node.constructor.clone(node) directly — a documented, idiomatic pattern before #8640 — now silently gets a default-constructed node with lost properties. For HeadingNode, __tag is copied only in afterCloneFrom, so a direct clone of an h2 returns a node whose tag has reverted to the constructor default 'h1'.

This is a silent data-correctness bug, not a throw, so it's easy to miss in review. It affects the 25+ core nodes that rely on auto-synthesized clone (ParagraphNode, TextNode, ListNode, TableNode, HeadingNode, LinkNode, …).

Fix

Close the gap at the source rather than relying on every caller using the "right" entry point: the synthesized clone now calls afterCloneFrom() itself when it is not being driven by an internal wrapper, so it is correct regardless of call path.

The subtlety is that afterCloneFrom is not guaranteed idempotent — some nodes deliberately accumulate state there (the existing clone() with no boilerplate unit test uses a node that increments a __version counter in afterCloneFrom). So afterCloneFrom must run exactly once per clone. A cloneWrapperDepth guard achieves this:

• $cloneWithProperties and $copyNode wrap their constructor.clone(node) call in runInCloneWrapper(...) and remain solely responsible for calling afterCloneFrom (unchanged behavior for them).
• The synthesized clone checks isInCloneWrapper(): inside a wrapper it does not self-apply (the wrapper will); called directly it self-applies.

This preserves the existing contract for explicit static clone() implementations (they still must not call afterCloneFrom; the wrappers do) and fixes direct callers without any double-application.

Tests

Added two regression tests in LexicalNode.test.ts:

1. direct clone() of an auto-synthesized node preserves properties — asserts a direct NodeClass.clone(node) on a $config()-based node keeps its custom property (fails before this change, passes after), and that $cloneWithProperties still works.
2. afterCloneFrom runs exactly once for auto-synthesized clone — a node that increments a version counter in afterCloneFrom ends at version 1 (not 2) after one clone, guarding against double-application from this change.

Verified locally:

• LexicalNode.test.ts: 96 passed
• full packages/lexical unit suite: 1204 passed (no regressions from double-afterCloneFrom)
• packages/lexical-rich-text (incl. 19 HeadingNode tests): 112 passed
• prettier --check and tsc --noEmit clean on the changed files

Follow-up (not in this PR): stricter importJSON base signature

While syncing, a second BC break was found: #8640 also tightened the base LexicalNode.importJSON return type to non-nullable LexicalNode. Subclasses that intentionally return null from importJSON (signaling "this node type does not support JSON import") now fail type-checking.

I deliberately left this out of this PR. Naively relaxing the base return type to LexicalNode | null ripples into the deserializer ($parseSerializedNodeImpl in LexicalUpdates.ts) and its recursive child-append path, forcing every importJSON consumer to handle null and changing deserialization semantics — a larger API decision than a clone-correctness fix should carry. The cleaner alternative is an explicit $config() opt-out (e.g. importable: false) with matching deserializer support. Happy to open a separate PR / discussion once maintainers weigh in on the preferred shape. In the meantime the www-side flow break can be unblocked with a local flow annotation on the specific non-importable subclass.

Context

Reported by the Lexical → fbsource sync oncall. Concrete internal regressions that surfaced this (Meta-internal): CaaSLexicalInsertLineNode.js flow break and an MLCNestingEnforcementPlugin heading-nesting test failure (h1>h2>text unwrapping h2 to h1) — both explained by the direct-clone property loss above. See D112781184 for the full internal analysis.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
lexical	Ready	Preview	Jul 20, 2026 2:37pm
lexical-playground	Ready	Preview	Jul 20, 2026 2:37pm

[etrepum]

I think a less problematic solution here is to add an optional internal argument to clone that can be used at the internal call sites that do not expect the implicit $afterCloneFrom. There are correctness issues here if a clone happens to call another node’s clone for some reason (even in another editor since this adds a module global)

[potatowagon]

Great catch — you're right on both counts, and the reentrancy issue with the module global is real. I put together a small model of exactly the case you described (a clone/constructor that clones another node while the outer clone is still in flight) and the depth-counter version silently drops the inner node's properties, while a per-call signal keeps it correct.

I've reworked it along the lines you suggested: the auto-synthesized clone now takes an optional internal second argument, and the internal call sites ($cloneWithProperties / $copyNode) pass it to opt out of the implicit afterCloneFrom (they still call it themselves). Direct callers pass nothing and get afterCloneFrom applied for them.

Specifics:

• The signal is a module-private unique symbol sentinel rather than a boolean, so external callers can't accidentally (or intentionally) pass it — the public clone(_data) signature is unchanged and the sentinel isn't exported.
• Because it's per-call, there's no shared/global state, so a nested clone (including across editors) can't inherit an outer node's "skip" state.
• afterCloneFrom still runs exactly once on every path, which matters since it isn't idempotent (the existing clone() with no boilerplate test relies on that).

Added a regression test for the reentrancy case (a direct clone performed from within another node's afterCloneFrom still preserves its own properties), alongside the direct-clone and exactly-once tests. Pushed the update — packages/lexical unit suite is green (1316 tests). Let me know if you'd prefer a different shape for the internal opt-out.

_{🤖 Raised and iterated with Navi on my behalf.}