react-flow-example incorrect

[alor]

looking at the example here: https://github.com/Infinium8/useUndoable/blob/main/react-flow-example/src/App.jsx

shouldn't the onEdgeChange method be something like:

  const onEdgesChange = useCallback(
    (changes) => {
      triggerUpdate("edges", applyEdgeChanges(changes, elements.edges));
    },
    [triggerUpdate, elements.edges]
  );

so the edge changes will be applied on every type of change. as of now it only "addEdge". and when you change that callback, the onConnect is not needed anymore.

[xplato]

shouldn't the onEdgeChange method be something like ...

Yes, correct. This was mere laziness on my part, as this example is not meant to be used verbatim. The reason why it is using the addEdge function is because I'm attaching the onEdgesChange function to both the onEdgesChange event and the onConnect event. For this reason, the more correct approach would be to create a new function, onConnect, that handles the creation of edges exclusively.

Thank you for pointing this out, @alor!

[alor]

since the onEdgesChange handles the "add" event, you don't actually need the onConnect handle to record the addition of a connection between nodes

[xplato]

True, yes, but I like your initial idea better. Separation of concerns is particularly important in this case due to the sensitive nature of ReactFlow v10 & useUndoable.

[alor]

wouldn't in this case the state be updated twice? once for the addEdge and one for the applyEdgeChanges? I will need to press "undo" twice to go back to the state where the edge is not present.

[xplato]

In my testing, this was not the case. For me, it appears that the applyEdgeChanges does not handle the creation of new edges. Does it do this for you?

If I simply replaced the addEdge function with applyEdgeChanges as you initially suggested, it would not let me create a new edge.

[alor]

you are correct. my fault. I looked at the react-flow code and saw the "add" event and thought that it will be handled inside the onEdgesChange. seems to be not the case.