[lexical-utils] Feature: add reverse dfs iterator

[nigelgutzmann]

Description

I previously submitted #7094, which changed the behaviour of $dfsIterator to allow it to sometimes traverse the tree backwards. The feedback there was that changing the behaviour would not be the safest choice, and instead this should be implemented as a new function that developers can choose to use in their application code.

The use-case for this is to traverse backwards from a starting point, towards the beginning of the editor, until you meet some condition. For example:

let previousTextNode = null;
for (const node of $dfsIterator(startNode, $getRootNode())) {
    if ($isTextNode(node)) {
        previousTextNode = node;
        break
    }
}

The alternative is to call $dfs().reverse(), but that is slow for editors with many nodes (lots of overhead).

It is really useful that the traversal algorithm is exactly the same forwards and backwards, so that an application can have consistency.

Describe the changes in this pull request

• added $reverseDfsIterator to mirror dfsIterator
• added $reverseDfs to mirror $dfs
  Closes #

Test plan

Unit tests are added

[vercel]

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	Jan 28, 2025 7:09pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 28, 2025 7:09pm

[github-actions]

size-limit report 📦

Path	Size
lexical - cjs	29.07 KB (0%)
lexical - esm	28.86 KB (0%)
@lexical/rich-text - cjs	38.18 KB (+0.37% 🔺)
@lexical/rich-text - esm	30.92 KB (0%)
@lexical/plain-text - cjs	36.66 KB (+0.3% 🔺)
@lexical/plain-text - esm	28.22 KB (0%)
@lexical/react - cjs	39.91 KB (+0.16% 🔺)
@lexical/react - esm	32.28 KB (0%)

[etrepum]

Looks good to me!

[nigelgutzmann]

Awesome, thanks @etrepum!

This is my first PR to the project. I cannot merge it because I do not have write access. So I am not sure what I do now?

[etrepum]

It’ll get merged at some point soon, there are some additional checks that get run after approval so we don’t always add approved PRs to the merge queue immediately

[etrepum]

After looking more closely here (trying to re-implement in terms of #7046) I think this should be reverted or fixed because $dfs().reverse() is not the correct result for a version of $dfs() that operates from right to left instead of from left to right.

[etrepum]

PR to fix this is in #7112

[nigelgutzmann]

Interesting, I did intend it to be an exact mirror of $dfs, rather than a right-to-left depth-first-search. But I understand why that is confusing.

The reason why the mirror version is useful is because of the use-case that I wrote in the PR description (finding a node before the passed-in startNode).

For this use-case, if the passed-in startNode is an ElementNode, and the application has code like:

let previousTextNode = null;
for (const node of $reverseDfsIterator(startNode)) {
    if ($isTextNode(node)) {
        previousTextNode = node;
        break
    }
}

Then the result will be correct with my implementation (a mirror of $dfs), but incorrect with a true right-to-left depth-first-search.

The right-to-left depth-first-search will get the last text node inside the startNode, rather than the first textNode that exists before the startNode.

All that being said, I see now why the semantics are confusing/unclear, and why it might make sense to make the change in #7112

[etrepum]

This kind of awkwardness is why I don’t think the dfs abstraction and the available traversals are good. It should really work with points not nodes so you can start the traversal at an arbitrary location. The existing PointType is very unstable and inefficient for these sorts of use cases since it uses offsets into a linked list, which is why I’m working on #7046.

[etrepum]

For the specific traversal you're trying to do, since you don't need to know the depth, I think you should be able to implement it like this with the corrected reverseDfsIterator implementation:

function $findPreviousTextNode(startNode: LexicalNode): TextNode | null {
  const prevNode = $getNextRightPreorderNode(startNode);
  if (prevNode) {
    for (const {node} of $reverseDfsIterator(prevNode)) {
      if ($isTextNode(node)) {
        return node;
      }
    }
  }
  return null;
}

The trouble is that with a LexicalNode as a reference you can only specify "start at this node, inclusive" so you have to find a better node to start at.

It's not really any simpler with the iterator than just using the function by itself:

export function $findPreviousTextNode(startNode: LexicalNode): TextNode | null {
  let node: LexicalNode | null = startNode;
  while (node && !$isTextNode(node)) {
    node = $getNextRightPreorderNode(node);
  }
  return node;
}