Recursive "internals" prop causing nodes to keep growing

[d-vanhees-micoworks]

What platform were you using when you found the bug?

• React Flow / Svelte Flow version: React Flow version: 12.0.4
• Browser and version: Chrome 128.0.6613.114
• OS and version: OS X 14.3.1 (23D60)

Live code example

No response

Describe the Bug

First of all, thank you for creating and maintaining this awesome library and more importantly, keeping it open for anyone to use ❤️ Secondly, I am not sure if this is the right place or process to submit the found issue, so please guide me if I am mistaken.

Description

As for the issue, we noticed that a possible recursion can happen when the nodeLookup map is used and passed to setNodes. This recursion caused the internals field within the nodes to grow very big, very rapidly (as can be seen in screenshot below), eventually even leading to a recursion limit exceeded Prisma error when trying to persist the nodes as-is as JSON/bson data in a Postgresql DB.

Background

We took the Auto Layout and Persisting Data as a reference, where we keep state within our component using useNodesState and persist nodes as is in a DB. However, coupling this with useAutoLayout, this caused nodes to grow rapidly and eventually trigger the above mentioned DB exception.

Thoughts / some findings we would like to share

We have a suspicion that this has to do with shallow copying of the node in adoptUserNodes, which does not exclude internals when setting userNode as part of internals (line 115 below).

https://github.com/xyflow/xyflow/blob/ce51e02acf3d7702869c643488ac52a5384125d6/packages/system/src/utils/store.ts#L103-L117

Related to question 1 below, should custom hooks avoid using nodeLookup, or if non-avoidable, at least avoid calling setNodes with nodes of InternalNode type? If so, would it be possible to put a note in the documentation somewhere?

Question

1. Instead of using state.nodeLookup, are there downsides retrieving non-internal nodes using state.nodes to calculate the new layout?
2. As a general rule, are there any fields like internals we should never include when persisting nodes and edges in the DB?

Steps to reproduce the bug or issue

1. Download code auto-layout-pro-example from React Pro examples. => We used code sample downloaded on July 31st, but we are not sure if this is still reproducible with current version as our plan expired.
2. [for logging] Update ReactFlowAutoLayout and add a simple console log right after useEdgeState, like console.log("nodes", nodes);
3. Select dagre as layout algorithm
4. Add some nodes (as can be seen in screenshot below), which will cause nodes to grow after each invocation of the hook.

Expected behavior

As a user, I expected the nodes to not grow, instead I see it growing and getting more bloated after each re-render.

Screenshots or Videos

Additional context

No response

[moklick]

What is happening here? First of all the internal nodes should only be used internally. Every node that gets passed to nodes or defaultNode should be a regular node. If you are working with the nodeLookup, you can always access the user node (internalNode.internals.userNode) and pass that back to your nodes array. Does this solve your issue?

[d-vanhees-micoworks]

What is happening here? First of all the internal nodes should only be used internally. Every node that gets passed to nodes or defaultNode should be a regular node. If you are working with the nodeLookup, you can always access the user node (internalNode.internals.userNode) and pass that back to your nodes array. Does this solve your issue?

Hello @moklick, thank you for your swift reply.

We managed to reproduce the issue using the React Flow Pro example for Auto-layout as-is. This sample code uses nodeLookup in useAutoLayout to recalculate the new layout and then passes those nodes to setNodes. Instead of nodeLookup, would it be better to use nodes in said hook?

If our new-found understanding is correct, passing InternalNode type to setNodes is not advised, however it seems that TypeScript does not prevent this. To mitigate similar issues, is there a downside omitting internals and measured from internalNode.userNode in code below?

https://github.com/xyflow/xyflow/blob/5a6869fd40260d00b6c48d9eb7d27f8738f02802/packages/system/src/utils/store.ts#L103C7-L119

We look forward to your reply and hearing your thoughts, thank you.

[moklick]

We just fixed the pro example. It now uses nodes and edges instead of nodeLookup and edgeLookup. As you already suggested we will check if we can adjust the types or add a check in adoptUserNodes to prevent this issue.

[d-vanhees-micoworks]

Hello @moklick,

Thank you for the update on the example code and future steps, we appreciate it. One question, would it be possible to share the "patch"/changes in the example with us? We believe the most important change is to use nodes and edges instead of nodeLookup and edgeLookup as per your message, but we are not sure if there are other changes that we should be aware of.

Other than that, our initial question is addressed so the reported Issue can be treated as closed at your convenience.

Thank you and kind regards.

[peterkogo]

@d-vanhees-micoworks Unfortunately the pro-examples repo is private for obvious reasons. But you are correct. It's simply the usage of the lookups instead of nodes and edges that has changed.

[d-vanhees-micoworks]

Hi @peterkogo , first of all, apologies for the delayed response. Noted on not be able to share the latest version of the example, totally understandable (just had to ask haha). Thank you for confirming that our surmised changes are correct.

Keep up the great work team! ❤️

PS. Since there are no reasons from our side to keep this issue open, let us close it and keep things a bit tidy.