[discussion] new hybrid approach to manage react-flow v10 with useUndoable

[alor]

I've tried many approaches to correctly handle all the situations (layouting, positioning of the nodes, selections, nodes & edges deletion, etc) and the best user experience I've come up is the one in this example: https://codesandbox.io/s/react-flow-v10-undo-hybrid-ysp9lw?file=/src/App.js

it uses an hybrid state management. it lets the react-flow manage the two separate states for nodes and edges as per their documentation but it "hooks" the key actions to create an history that can be undone. I've hooked the addition and removal of edges and nodes to create a new point in time "snapshot" of the states. the node positioning is saved using the onNodeDragStop callback to save only the final position of a node and not all the pixel-by-pixel updates. when you undo() or redo(), the state managed by useUndoable is synced back to the nodes and edges managed by react-flow

the only "issue" I still don't like is: when you delete a connected node, you have to undo() twice to go back in time (one for the node and one for the removed edge) in all other cases it behaves consistently. btw this is a consequence of how react-flow fires the events, it first deletes the edges and then the nodes in two different updates of the states.

what do you think about this approach? I know that in this way the current state is not managed by useUndoable (which is desirable) but I've not found a consistent way to sync the nodes and edges attributes in a single state

any suggestion is more than welcome.

[xplato]

what do you think about this approach?

For v10, I think this is likely the most sensible approach. Although it can get a bit sticky in some cases, it looks like you've handled it well.

As issue #12 (which I will probably close, as we are discussing this here) points out, there is still the issue of the nodes and edges changes being made one after another, leading to this strange behavior.

So, it looks like this problem only goes one way. If you delete just an edge, it works fine, as expected. The issue is when you delete a node that has edges connected to it, as that counts as two updates. What if you could intercept the node changes and manually update the state all at once, manually?

In your onNodesChange function (line 61), try something like this (you'll have to modify this a bit to work with your hybrid state):

if (changes[0].type === "remove") {
    const nodeIDs = changes.map(change => change.id);

    // Let ReactFlow handle the deletion of nodes normally with applyNodeChanges
    setElements({
        nodes: applyNodeChanges(changes, nodes),

        // Manually filter the edges and exclude all who point to or from any deleted nodes
        edges: edges.filter(edge => !nodeIDs.includes(edge.source) || !nodeIDs.incudes(edge.target))
    });
}

(You may even want to filter nodes manually as well, that just depends)

Doing it this way, you converge the two separate updates into one state mutation, allowing you to undo() and redo() at will, and without delay.

Of course, in the onEdgesChange function, you'll also have to handle this by conditionally ignoring updates if the onNodesChange function handled the deletion already. For this, you could maintain a new state specifically for tracking changes made to the state, but in an explicit manner. That way, you could check and see if the deletion was already handled by the onNodesChange function with something like: if (changes.includes(edge.id) && changes.type === 'remove') { ignoreThis() }. This way, you can ignore the update if the edge was already modified by the onNodesChange function.

This is largely conceptual, as I don't really have the time to test all of this out (and because this isn't an issue with useUndoable, just an unfortunate side-effect of its behavior with ReactFlow v10).

If this doesn't work, try getting rid of the generic onNodesChange and onEdgesChange functions, and try using the more specific event handlers. I'm not sure if this will work entirely either, but you can give it a shot.

If all else fails—and as I've mentioned before—ReactFlow v9 "just works" with useUndoable, more or less. If your project doesn't need something that v10 offers, save yourself a little time and go with v9, which has been proven to work nicely. This is, of course, up to you.

[xplato]

Since this isn't really an issue with the source code or behavior of useUndoable itself, I'm going to close this. We can continue discussing this if need be.