canCreateEdge in GraphView takes a (INode, INode), but it's being called with a nodeId
Describe the bug
Within GraphView.js, at line 709 there's a call to canCreateEdge(). This function should accept two INodes, but on this line it uses a nodeId. This breaks the typing, but also may not be correct code.
709: (canCreateEdge && canCreateEdge(nodeId)) ||
Expected behavior canCreateEdge should pass one or two nodes, not a nodeId
Investigate why canCreateEdge is being called with a nodeId on this line and change it, or change the types and document the purpose of this code. Also document the use cases for canCreateEdge (source node+target, only source, only target?, nodeId?).
I ran into this one today as I was trying to figure out how to implement something like canSwapEdge. This line has access to node (the node for the given nodeId) and so imo it should just call canCreateEdge(node) to be in line with other calls to canCreateEdge as well as the docs.
Use cases..
- Sending source and target: Check to see if the user wants an edge to be able to be created between two nodes.. Perhaps there are two nodes that shouldn't be connected. Perhaps they want to check for cycles among the entire graph and disallow such a thing.
- Sending just source: You never want an edge to come from a certain node.. Could be confusing when they go to make an edge and nothing happens, but it's up to the end-developer to implement such behavior.
- Sending just the target? Not sure you'd ever have just the target as currently edges start with a source and go either into nowhere-land as they are dragging around or to a target when they are finished dragging (in which case you obviously still have the source).
Is it possible to implement functionality like canSwapEdge using one of the prop methods? It seems like swap bypasses canCreateEdge, which is fine, but I'd like to limit the user's swapping ability. I have some logic in canCreateEdge that the user can just bypass by creating an edge within what I've limited them to do and then swapping it to a place I didn't want them to go. I tried implementing handleSwapEdge, and just not updating the edge state, but then the edge remains floating in the same position even when I move nodes around. Unsure if that was intended or not. Sorry, I don't know the correct way to ask such things other than direct message or opening up another issue.
I can also change the type of startNode to be required instead of optional if you want. I don't think it's possible to have an edge exist without a startNode, but I could be wrong?