[lexical] Feature: Add mutatedNodes to UpdateListener payload #7321
Conversation
facebook-github-bot
added
the
CLA Signed
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| for (const [klass, m] of mutatedNodes) { | ||
| if ($isElementNode(klass.prototype)) { | ||
| for (const [nodeKey, value] of m) { | ||
| if (value === 'destroyed') { | ||
| dirtyElements.set(nodeKey, true); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This seems weird, in the case when we do $getRoot().clear() the key of the destroyed ParagraphNode appears in here even though it's not in the editor state. The same behavior doesn't happen for destroyed leaves. IMO we should probably have an efficient way to see which nodes were deleted between prevEditorState and editorState without any DOM dependency (without including ephemeral nodes that were created and then GC'd in the same reconciliation cycle).
There was a problem hiding this comment.
It looks like this behavior is intentional-ish but there's no way to know about deleted leaves https://github.com/facebook/lexical/blob/main/packages/lexical/src/LexicalGC.ts#L103-L107
There was a problem hiding this comment.
My preference would be to leave this alone for now. I don't really have a use case for dirtyElements or dirtyLeaves and you can't really do anything sound with dirtyElements+dirtyLeaves because they aren't coherent when setEditorState has happened (historic events).
Everything on my plate is better solved by using this new mutatedNodes prop. Just figured I'd point it out while I was in here writing the test coverage.
etrepum
requested review from
zurfyx,
fantactuka,
acywatson,
Fetz,
ivailop7,
Sahejkm and
potatowagon
as code owners
Description
In order to better support NodeState and other use cases (e.g. some sort of "createdDOM/updateDOM" middleware) where it's useful to know about all nodes that were mutated in the DOM this adds the mutatedNodes property to UpdateListenerPayload and adds some API docs.
With this change it would be possible to implement mutation listeners as an updateListener so we could reduce some surface area in the future with a reduced core.
Test plan
Includes unit tests for expected behavior of the payload.