How does changing the node without calling SKIP cause performance issues?

[lubiah]

I have been reading the docs over at https://unifiedjs.com/learn/recipe/remove-node/#replacing-a-node-with-its-children. It says if you remove a node or replace it's content, it would cause performance issues because you are still transversing the tree. I don't understand how it causes performance issues. I tried replace a node directly with parent[index] = new node contents and it works perfectly.
Can you elaborate on the performance issues it causes and help me understand what I'm missing.

From what I understand, each plugin takes a tree modifies it and returns it to the next plugin so without calling the SKIP, won't the next plugin realise there's nothing there???

[ChristianMurphy]

Welcome @lubiah 👋
The potential issue is within the plugin where the transformation is happening.

The visitor does a depth first traversal.
https://github.com/syntax-tree/unist-util-visit-parents#visitparentstree-test-visitor-reverse
https://en.m.wikipedia.org/wiki/Tree_traversal

Meaning if you modify the current node, that will effect the traversal of its children.

Some example of how this can cause bugs or performance problems (there are many more)

1. Doing async replacement, the visitor is sync, if the children or node are modified async, the tree traversal will work on the old tree and will never see the new nodes or know the old node was removed.
2. Modifying the node accidentally with a back reference to a parent. Visitors only work on trees, if a self reference turns the tree into a graph, the visitor could get stuck in an infinite loop.

The recommendations in the recipe are there as defensive measures, to raise awareness, a reminder to be thoughtful in how you modify the tree, and considerate of how that could impact performance.

[lubiah]

Hi @ChristianMurphy , thank's for the explanation. Also, sorry for the delay in replying. I needed to read more about tree transversals before I could understand what you wrote😂.

If I understand you correctly for the problem 1, this is what you mean,
Let's say you are transversing a node A. and you replace the node

import { visit } from "unist-util-visit";

visit(tree, node=>{
node.children = [...newchildren]
})

When I make the above changes, the tree doesn't know about these changes and it might continue to transverse the old tree which I have already removed, as a result, it could lead to the formation of wrong nodes. Is that what you mean???
So when I use the"SKIP" from the visitor, it will inform the visitor not to transverse the children of the tree which I've modified.

For the second point, how will modifying a node create a back reference to it's parent if only you modify it's children.

[ChristianMurphy]

When I make the above changes, the tree doesn't know about these changes and it might continue to transverse the old tree which I have already removed, as a result, it could lead to the formation of wrong nodes. Is that what you mean???

If the modification to the tree is done asynchronously, yes, this could happen.
In your sample code, it is updated synchronously, which would not cause the outdated tree issue.

But could have its own issue.
The visitor will continue on to the newly added children and transform those as well, you may not (and likely do not) want the visitor to process your programmatically added content as well.

For the second point, how will modifying a node create a back reference to it's parent if only you modify it's children.

Because that's how references work.
If you make the current node's children property include its own parent.
The visitor doesn't know that, the visitor pattern assumes you have made a valid tree, not a cyclic graph.
The visitor will keep going to the children property, bouncing back and forth between the parent and child node ad infinitum.

[lubiah]

Thanks @ChristianMurphy