Feature request - Improve `addNodeAndEdge` utility to also bind edges to ports

[Vadorequest]

At this time, the addNodeAndEdge utility helps us easily add a new node and link it to another node while creating the edge.

export declare function addNodeAndEdge(nodes: NodeData[], edges: EdgeData[], node: NodeData, toNode?: NodeData): {
    nodes: NodeData<any>[];
    edges: EdgeData<any>[];
};

Although, if ports are configured on both nodes, it should allow us to configure ports binding too.

At this time, if ports are defined on the nodes, they aren't being used, which generates something odd:

In a simple use-case, where both nodes have at most 1 port per side, it should be straightforward to automatically configure binding.

The toNode is the source and should be bound by its WEST port (when Canvas direction = 'RIGHT'), while the node is the destination and should be bound by its EAST port.

Things might get more complicated when a node has several ports in the same side, but I suggest having an auto strategy that binds to the first port it finds on the side the node should be bound.

We might have a 5th argument to addNodeAndEdge, such as:

addNodeAndEdge(nodes: NodeData[], edges: EdgeData[], node: NodeData, toNode?: NodeData, portsResolver: (node: NodeData, toNode?: NodeData) => {nodePorts: PortData[], toNodePorts: PortData[]}): {
    nodes: NodeData<any>[];
    edges: EdgeData<any>[];
};

If unspecified, the 5th argument would use the default implementation, which behaves as above mentioned (binding to the first port it finds on the expected side), providing a custom portsResolver would override this behaviour and allow for more customisation.

Does this feature request makes sense? Is it something that might be added natively? I believe it would ease usage of Ports by providing a sane default behavior, while allowing for extra customisation for non-covered use-cases.

[amcdnl]

The ports resolver might be a little much - what about just passing the port you want?

[Vadorequest]

I'm fine with it, I just thought it'd be a nice addition to have it natively supported.

[Vadorequest]

@amcdnl I've implemented my own methods, but it's sad because I had to do it for almost all utility functions provided by reaflow. (addNodeAndEdge, removeAndUpsertNodes and upsertNode)

Basically, the utilities provided aren't flexible enough to be comosable/overridden, so I had to duplicate the code and adapt it. I think those utilities might provide additional parameters to avoid duplicating the whole code and become more reusable.

/**
 * Add a node and optional edge, and automatically link their ports.
 *
 * Automatically connects the fromNode (left node) using its EAST port (right side) to the newNode (right node) using it's WEST port (left side).
 *
 * XXX Does not support adding a newNode as left node, it must be on the right side of the fromNode.
 *
 * Similar to reaflow.addNodeAndEdge utility.
 */
export function addNodeAndEdgeThroughPorts(
  nodes: BaseNodeData[],
  edges: BaseEdgeData[],
  newNode: BaseNodeData,
  fromNode?: BaseNodeData,
) {
  const fromPort: PortData | undefined = fromNode?.ports?.find((port: PortData) => port?.side === 'EAST');
  const toPort: PortData | undefined = newNode?.ports?.find((port: PortData) => port?.side === 'WEST');
  const newEdge: BaseEdgeData = {
    id: `${fromNode?.id || uuid()}-${newNode.id}`,
    from: fromNode?.id,
    to: newNode.id,
    parent: newNode.parent,
    fromPort: fromPort?.id,
    toPort: toPort?.id,
  };

  return {
    nodes: [...nodes, newNode],
    edges: [
      ...edges,
      ...(fromNode ?
        [
          newEdge,
        ]
        : []),
    ],
  };
}

/**
 * Helper function for upserting a node in a edge (split the edge in 2 and put the node in between), and automatically link their ports.
 *
 * Automatically connects the left edge to the newNode using it's WEST port (left side).
 * Automatically connects the right edge to the newNode using it's EAST port (right side).
 *
 * Similar to reaflow.upsertNode utility.
 */
export function upsertNodeThroughPorts(
  nodes: BaseNodeData[],
  edges: BaseEdgeData[],
  edge: BaseEdgeData,
  newNode: BaseNodeData
) {
  const oldEdgeIndex = edges.findIndex(e => e.id === edge.id);
  const edgeBeforeNewNode = {
    ...edge,
    id: `${edge.from}-${newNode.id}`,
    to: newNode.id
  };
  const edgeAfterNewNode = {
    ...edge,
    id: `${newNode.id}-${edge.to}`,
    from: newNode.id
  };

  if (edge.fromPort && edge.toPort) {
    const fromLeftNodeToWestPort: PortData | undefined = newNode?.ports?.find((port: PortData) => port?.side === 'WEST');
    const fromRightNodeToEastPort: PortData | undefined = newNode?.ports?.find((port: PortData) => port?.side === 'EAST');

    edgeBeforeNewNode.fromPort = edge.fromPort;
    edgeBeforeNewNode.toPort = fromLeftNodeToWestPort?.id || `${newNode.id}-to`;

    edgeAfterNewNode.fromPort = fromRightNodeToEastPort?.id || `${newNode.id}-from`;
    edgeAfterNewNode.toPort = edge.toPort;
  }

  edges.splice(oldEdgeIndex, 1, edgeBeforeNewNode, edgeAfterNewNode);

  return {
    nodes: [...nodes, newNode],
    edges: [...edges]
  };
}

/**
 * Removes a node between two edges and merges the two edges into one, and automatically link their ports.
 *
 * Similar to reaflow.removeAndUpsertNodes utility.
 */
export function removeAndUpsertNodesThroughPorts(
  nodes: BaseNodeData[],
  edges: BaseEdgeData[],
  removeNodes: BaseNodeData | BaseNodeData[],
  onNodeLinkCheck?: (
    newNodes: BaseNodeData[],
    newEdges: BaseEdgeData[],
    from: BaseNodeData,
    to: BaseNodeData,
    port?: PortData
  ) => undefined | boolean
) {
  if (!Array.isArray(removeNodes)) {
    removeNodes = [removeNodes];
  }

  const nodeIds = removeNodes.map((n) => n.id);
  const newNodes = nodes.filter((n) => !nodeIds.includes(n.id));
  const newEdges = edges.filter(
    (e: BaseEdgeData) => !nodeIds.includes(e?.from as string) && !nodeIds.includes(e?.to as string)
  );

  for (const nodeId of nodeIds) {
    const sourceEdges = edges.filter((e) => e.to === nodeId);
    const targetEdges = edges.filter((e) => e.from === nodeId);

    for (const sourceEdge of sourceEdges) {
      for (const targetEdge of targetEdges) {
        const sourceNode = nodes.find((n) => n.id === sourceEdge.from);
        const targetNode = nodes.find((n) => n.id === targetEdge.to);

        if (sourceNode && targetNode) {
          const canLink = onNodeLinkCheck?.(
            newNodes,
            newEdges,
            sourceNode,
            targetNode
          );

          if (canLink === undefined || canLink) {
            const fromPort: PortData | undefined = sourceNode?.ports?.find((port: PortData) => port?.side === 'EAST');
            const toPort: PortData | undefined = targetNode?.ports?.find((port: PortData) => port?.side === 'WEST');

            newEdges.push({
              id: `${sourceNode.id}-${targetNode.id}`,
              from: sourceNode.id,
              to: targetNode.id,
              parent: sourceNode?.parent,
              fromPort: fromPort?.id,
              toPort: toPort?.id,
            });
          }
        }
      }
    }
  }

  return {
    edges: newEdges,
    nodes: newNodes
  };
}

Essentially, there is no way to provide fromPort and toPort for the dynamically created edge in removeAndUpsertNodes and addNodeAndEdge. In upsertNode, it's the edgeBeforeNewNode.toPort and edgeAfterNewNode.fromPort that cannot be customised.

In the above implementations, I'm using the editor in direction='RIGHT' and ports are resolved with that configuration in mind. I believe the DX could be improved, I'm not sure how, though. I'm not sure if you'd want to increase the flexibility (and complexity) of the built-in helpers, but I thought I'd share my findings nevertheless. :smile:

[amcdnl]

I implemented the same in my app. It really depends on your requirements - for mine I had certain restrictions that were difficult to abstract in a way that would not make it more complicated to use. These helpers are really just starting places for common use cases - I would encourage you to make a PR to the docs in a seperate page like Advanced > Extended Helpers or something.

[Vadorequest]

I see. Is your goal to only provide a small set of helpers (as what's done currently), or would you like to provide more helpers for other use-cases?

If so, we could add those above-mentioned helpers, either by adding them as examples (if not in the core), or to improve the core helper (I think the above might be trivial to abstract).

I think, we should at the very least add those as examples for inspiration (in the StoryBook).

[Vadorequest]

Documentation updated in https://github.com/reaviz/reaflow/pull/72