Skip to content

[lexical][Breaking Change] Bug Fix: Change $copyNode to use afterCloneFrom#7609

Merged
etrepum merged 3 commits intofacebook:mainfrom
etrepum:copyNode-afterCloneFrom
Jun 7, 2025
Merged

[lexical][Breaking Change] Bug Fix: Change $copyNode to use afterCloneFrom#7609
etrepum merged 3 commits intofacebook:mainfrom
etrepum:copyNode-afterCloneFrom

Conversation

@etrepum
Copy link
Copy Markdown
Collaborator

@etrepum etrepum commented Jun 6, 2025

Breaking Changes

If you have code that assumes that newNode.afterCloneFrom(prevNode) will not be called as a result of $copyNode(prevNode), and would no longer work if it was, then it will need some refactoring. This is a very unlikely scenario.

Description

$copyNode is used when creating a shallow copy (not including children) of a node with a new key. It currently only really works as expected if the node doesn't use NodeState and has a clone method that directly sets all expected properties.

Discovered this issue while working through a #7258 refactor of ListNode. After replacing the clone boilerplate with afterCloneFrom the list type and tag would no longer be copied.

Test plan

New unit tests

Before

$copyNode would not copy all properties of a node

After

$copyNode will copy all properties of a node, including NodeState (but without creating an aliasing issue), but not including the LexicalNode and ElementNode NodeKey pointers and size cache (__prev, __next, __parent, __first, __last, __size)

@vercel
Copy link
Copy Markdown

vercel bot commented Jun 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 7, 2025 4:34am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 7, 2025 4:34am

@facebook-github-bot facebook-github-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 Jun 6, 2025
@etrepum etrepum added the extended-tests Run extended e2e tests on a PR label Jun 6, 2025
@etrepum etrepum added this pull request to the merge queue Jun 7, 2025
Merged via the queue into facebook:main with commit 9b771ea Jun 7, 2025
39 checks passed
@etrepum etrepum deleted the copyNode-afterCloneFrom branch June 7, 2025 19:14
@etrepum etrepum mentioned this pull request Jul 3, 2025
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