@lexical/utils -> registerNodesOfType

[zurfyx]

$nodesOfType is an expensive function, it iterates all the tree. It turns out that people are abusing this function as a handy shortcut (as we have no better utility) which can have performance implications on long documents.

There's two types of misuse:

1. For updates; people find it easier to use $nodesOfType than building more optimal tailored algorithm (via transform parent or making some hierarchy assumption).
2. For reads; $nodesOfType is much easier to use than registerMutationListener when tracking nodes as a whole as the mutation listener requires you to build your own data storage, do the first pass and handle creation and deletion.

This PR addresses the second, simplifies the whole mechanism and makes it easier to fall onto the pit of success

1. Forgetting the initial pass can cause a race condition
2. Slightly similar but this is a common race condition that has previously given us trouble with the contenteditable flag

const nodes = $nodesOfType(FooNode);
useEffect(() => {
  return editor.registerNodeMutation(..);

3. Tracking is verbose

Internal utilization reference https://fburl.com/diff/prwdmjw5

[vercel]

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

Name	Status	Preview	Comments	Updated
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Mar 10, 2023 at 11:43PM (UTC)
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Mar 10, 2023 at 11:43PM (UTC)

[github-actions]

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
packages/lexical/dist/Lexical.js	26.85 KB (0%)	538 ms (0%)	153 ms (-1.96% 🔽)	690 ms
packages/lexical-rich-text/dist/LexicalRichText.js	37.79 KB (+0.3% 🔺)	756 ms (+0.3% 🔺)	92 ms (+8.78% 🔺)	848 ms
packages/lexical-plain-text/dist/LexicalPlainText.js	37.77 KB (+0.31% 🔺)	756 ms (+0.31% 🔺)	135 ms (+62.84% 🔺)	891 ms

[acywatson]

Like the idea, not sure about the name. Perhaps we can think of something better - let me dwell on it a bit.

Other thing - it's a bit weird to have this be the only listener not accessed through a LexicalEditor instance. Not a blocker, just calling out the inconsistency. Maybe it's fine because this is, in a sense, a "higher order" listener.

[zurfyx]

Other thing - it's a bit weird to have this be the only listener not accessed through a LexicalEditor instance

That's a fair point. My thinking process was that the Core is already pretty bloated. IMO MutationListeners is an advanced topic and something that only power users need. But no strong opinions on this

[acywatson]

I'm good with this - can you add a doc block at the top of the function? See examples in the recent docs PRs

[fantactuka]

I'm not sure I understand why we reassign nodes here and whether it's needed at all. This var does not seem to be used as we only trigger listener with nodes of type that were added (newNodes)

Also, curious if users would expect this listener to be triggered with all nodes of type vs just newly added ones.

[zurfyx]

Hm, fair point, I guess part of the behavior you inherit from the nodesOfType name but it makes sense that this is not what you would often expect from an observer. I guess that alternatively we can have a callback that returns (added: Set<NodeKey>, removed: Set<NodeKey>, nodes: Set<NodeKey>)

[thegreatercurve]

@zurfyx Can this PR be merged? Or can you mark it as 0.12.0 if we want to introduce this as a breaking change?

[etrepum]

I think we might be able to abandon this, since #6357 basically covers the same use case and there's a cache for $nodesOfType such that the traversal of all nodes is only done once per read-only EditorState (via the internal getCachedTypeToNodeMap).

[github-actions]

Closing this PR due to staleness! If there are new updates, please reopen the PR.