`setNodes` + `fitView` in an uncontrolled flow is broken

[snelsi]

Describe the Bug

I think @xyflow/react: v12.0.0-next.9 introduced a regression.

I'm using a setup very similar to Elkjs Tree example from the docs: https://reactflow.dev/examples/layout/elkjs

Also, I've implemented Expand and Collapse similar to this example: https://reactflow.dev/examples/layout/expand-collapse

My flow looks something like this: elkjs => setNodes/setEdges => fitView for auto layout.

After upgrading to v12.0.0-next.9 I started to notice weird glitches with fitView function. Sometimes it only fits some of the nodes (ones that were updated). Sometimes it fits into random portion of the view. And sometimes it does nothing at all.

After some debugging, the only difference between v12.0.0-next.8 and v12.0.0-next.9 I've found is that initial nodes have computed.height and computed.width defined. But after calling setNodes method, new nodes have computed.height: undefined and computed.width: undefined

Steps to Reproduce the Bug or Issue

1) Update to 12.0.0-next.9 2) Use this example as a base 3) Call setNodes method a couple of times 4) New nodes will have undefined computed.height and computed.width 5) fitView method will be broken

Expected behavior

After calling fitView all nodes should be visible

Screenshots or Videos

Check this video: https://drive.google.com/file/d/1sCzIzVp6LicJ8_5ref2fA24tBPkLV7vR/view?usp=sharing

Platform

• OS: Windows 11
• Browser: Chrome
• Version: 122.0.6261.70

[snelsi]

I think the issue can be fixed from user side by modifying Elkjs Tree example to look like this:

  return elk
    .layout(graph)
    .then((layoutedGraph) => ({
      nodes: layoutedGraph.children.map((node) => ({
        ...node,
        position: { x: node.x, y: node.y },

        // Add this code to fix `fitView` 👇
        computed: {
          height: node.height,
          positionAbsolute: { x: node.x, y: node.y },
          width: node.width,
        },
      })),

      edges: layoutedGraph.edges,
    }));

[moklick]

I can't reproduce this issue. If you are destructing the nodes with the computed values, they should still be there. Does this example work for you https://codesandbox.io/p/sandbox/jovial-feistel-r5ywtz?file=/App.js:84,10 ?

[snelsi]

@moklick

[moklick]

👍 it should work now

[snelsi]

@moklick Can't reproduce the issue in your sandbox.

But here's a very simplified reproduction of how it looks in my app: https://codesandbox.io/p/sandbox/nostalgic-jepsen-ssslxn

I copy-pasted a sample ElkJs output from my app and saved it to mockedElkOutput.js file, so getLayoutedElements returns this mocked layouted data.

Initial view is rendered without problems. You can move nodes around and fitView will work fine. However, if you try to reset it by clicking a button in the top-right corner, fitView will be broken.

Maybe, it doesn't like that I'm using reactFlow.setNodes and reactFlow.setEdges directly? I see that you're using useNodesState and useEdgesState in your example

[moklick]

In your codesandbox fitView doesn't work, because it's not passed as a dependecy to the onLayout function.

[snelsi]

@moklick Looks like the previous link wasn't right, sorry. Please take a look again https://codesandbox.io/p/sandbox/nostalgic-jepsen-ssslxn

[moklick]

that sandbox is private :D csb is killing me..

[snelsi]

So true 😅 Should be public now

[moklick]

It's the same issue. You need to pass stuff that changes to the dependency arrays: https://codesandbox.io/p/sandbox/practical-cache-ms5hwv?file=/App.js:39,1-40,1

(I also decreased minZoom)

[snelsi]

Tried both adding reactFlow to dependency array and removing useCallback hook completely, no good. FitView is still broken when you click on the "Reset" button

[moklick]

My sandbox doesn't work for you?

[snelsi]

It's private...

[moklick]

hahaha omg :D should work now

[snelsi]

In your example nodes positions aren't actually updated when you click on the "Reset" button

[moklick]

Got it. It might be related with the new batching of setNodes. We will check it next week.

[moklick]

Could you check again with the latest version 12.0.0-next.21? We revised the batching.