refactor(system): revise calcNextPosition

examples/react/src/examples/Subflow/index.tsx

@@ -13,6 +13,7 @@ import {
   MiniMap,
   Background,
   Panel,
+  NodeOrigin,
 } from '@xyflow/react';
 
 import DebugNode from './DebugNode';
@@ -119,6 +120,7 @@ const initialNodes: Node[] = [
     data: { label: 'Node 3' },
     position: { x: 400, y: 100 },
     className: 'light',
+    extent: 'parent',
   },
 ];
 

packages/react/src/hooks/useUpdateNodePositions.ts

@@ -1,5 +1,5 @@
 import { useCallback } from 'react';
-import { calcNextPosition, snapPosition } from '@xyflow/system';
+import { calculateNodePosition, snapPosition } from '@xyflow/system';
 
 import { Node } from '../types';
 import { useStoreApi } from '../hooks/useStore';
@@ -17,7 +17,17 @@ export function useUpdateNodePositions() {
   const store = useStoreApi();
 
   const updatePositions = useCallback((params: { x: number; y: number; isShiftPressed: boolean }) => {
-    const { nodeExtent, nodes, snapToGrid, snapGrid, nodesDraggable, onError, updateNodePositions } = store.getState();
+    const {
+      nodeExtent,
+      nodes,
+      snapToGrid,
+      snapGrid,
+      nodesDraggable,
+      onError,
+      updateNodePositions,
+      nodeLookup,
+      nodeOrigin,
+    } = store.getState();
     const selectedNodes = nodes.filter(selectedAndDraggable(nodesDraggable));
     // by default a node moves 5px on each key press, or 20px if shift is pressed
     // if snap grid is enabled, we use that for the velocity.
@@ -31,27 +41,24 @@ export function useUpdateNodePositions() {
     const nodeUpdates = selectedNodes.map((node) => {
       if (node.computed?.positionAbsolute) {
         let nextPosition = {
-          x: node.computed?.positionAbsolute.x + xDiff,
-          y: node.computed?.positionAbsolute.y + yDiff,
+          x: node.computed.positionAbsolute.x + xDiff,
+          y: node.computed.positionAbsolute.y + yDiff,
         };
 
         if (snapToGrid) {
           nextPosition = snapPosition(nextPosition, snapGrid);
         }
 
-        const { positionAbsolute, position } = calcNextPosition(
-          node,
+        const { position, positionAbsolute } = calculateNodePosition({
+          nodeId: node.id,
           nextPosition,
-          nodes,
+          nodeLookup,
           nodeExtent,
-          undefined,
-          onError
-        );
+          nodeOrigin,
+          onError,
+        });
 
         node.position = position;
-        if (!node.computed) {
-          node.computed = {};
-        }
         node.computed.positionAbsolute = positionAbsolute;
       }
 

packages/react/src/store/initialState.ts

@@ -6,6 +6,7 @@ import {
   getViewportForBounds,
   Transform,
   updateConnectionLookup,
+  devWarn,
 } from '@xyflow/system';
 
 import type { Edge, Node, ReactFlowStore } from '../types';
@@ -100,7 +101,7 @@ const getInitialState = ({
     autoPanOnConnect: true,
     autoPanOnNodeDrag: true,
     connectionRadius: 20,
-    onError: () => null,
+    onError: devWarn,
     isValidConnection: undefined,
     onSelectionChangeHandlers: [],
 

packages/system/src/utils/general.ts

@@ -198,3 +198,7 @@ export const getViewportForBounds = (
 };
 
 export const isMacOs = () => typeof navigator !== 'undefined' && navigator?.userAgent?.indexOf('Mac') >= 0;
+
+export function isCoordinateExtent(extent?: CoordinateExtent | 'parent'): extent is CoordinateExtent {
+  return extent !== undefined && extent !== 'parent';
+}

packages/system/src/utils/graph.ts

@@ -4,11 +4,11 @@ import {
   clampPosition,
   getBoundsOfBoxes,
   getOverlappingArea,
-  isNumeric,
   rectToBox,
   nodeToRect,
   pointToRendererPoint,
   getViewportForBounds,
+  isCoordinateExtent,
 } from './general';
 import {
   type Transform,
@@ -19,10 +19,10 @@ import {
   type EdgeBase,
   type FitViewParamsBase,
   type FitViewOptionsBase,
-  NodeDragItem,
   CoordinateExtent,
   OnError,
   OnBeforeDeleteBase,
+  NodeLookup,
 } from '../types';
 import { errorMessages } from '../constants';
 
@@ -258,69 +258,87 @@ export function fitView<Params extends FitViewParamsBase<NodeBase>, Options exte
   return false;
 }
 
-function clampNodeExtent(node: NodeDragItem | NodeBase, extent?: CoordinateExtent | 'parent') {
+/**
+ * This function clamps the passed extend by the node's width and height.
+ * This is needed to prevent the node from being dragged outside of its extent.
+ *
+ * @param node
+ * @param extent
+ * @returns
+ */
+function clampNodeExtent<NodeType extends NodeBase>(
+  node: NodeType,
+  extent?: CoordinateExtent | 'parent'
+): CoordinateExtent | 'parent' | undefined {
   if (!extent || extent === 'parent') {
     return extent;
   }
   return [extent[0], [extent[1][0] - (node.computed?.width ?? 0), extent[1][1] - (node.computed?.height ?? 0)]];
 }
 
-export function calcNextPosition<NodeType extends NodeBase>(
-  node: NodeDragItem | NodeType,
-  nextPosition: XYPosition,
-  nodes: NodeType[],
-  nodeExtent?: CoordinateExtent,
-  nodeOrigin: NodeOrigin = [0, 0],
-  onError?: OnError
-): { position: XYPosition; positionAbsolute: XYPosition } {
-  const clampedNodeExtent = clampNodeExtent(node, node.extent || nodeExtent);
-  let currentExtent = clampedNodeExtent;
-  let parentNode: NodeType | null = null;
-  let parentPos = { x: 0, y: 0 };
-
-  if (node.parentNode) {
-    parentNode = nodes.find((n) => n.id === node.parentNode) || null;
-    parentPos = parentNode
-      ? getNodePositionWithOrigin(parentNode, parentNode.origin || nodeOrigin).positionAbsolute
-      : parentPos;
-  }
+/**
+ * This function calculates the next position of a node, taking into account the node's extent, parent node, and origin.
+ *
+ * @internal
+ * @returns position, positionAbsolute
+ */
+export function calculateNodePosition<NodeType extends NodeBase>({
+  nodeId,
+  nextPosition,
+  nodeLookup,
+  nodeOrigin = [0, 0],
+  nodeExtent,
+  onError,
+}: {
+  nodeId: string;
+  nextPosition: XYPosition;
+  nodeLookup: NodeLookup<NodeType>;
+  nodeOrigin?: NodeOrigin;
+  nodeExtent?: CoordinateExtent;
+  onError?: OnError;
+}): { position: XYPosition; positionAbsolute: XYPosition } {
+  const node = nodeLookup.get(nodeId)!;
+  const parentNode = node.parentNode ? nodeLookup.get(node.parentNode) : undefined;
+  const { x: parentX, y: parentY } = parentNode
+    ? getNodePositionWithOrigin(parentNode, parentNode.origin || nodeOrigin).positionAbsolute
+    : { x: 0, y: 0 };
+  let currentExtent = clampNodeExtent(node, node.extent || nodeExtent);
 
   if (node.extent === 'parent' && !node.expandParent) {
-    const nodeWidth = node.computed?.width;
-    const nodeHeight = node.computed?.height;
-    if (node.parentNode && nodeWidth && nodeHeight) {
-      const currNodeOrigin = node.origin || nodeOrigin;
-
-      currentExtent =
-        parentNode && isNumeric(parentNode.computed?.width) && isNumeric(parentNode.computed?.height)
-          ? [
-              [parentPos.x + nodeWidth * currNodeOrigin[0], parentPos.y + nodeHeight * currNodeOrigin[1]],
-              [
-                parentPos.x + (parentNode.computed?.width ?? 0) - nodeWidth + nodeWidth * currNodeOrigin[0],
-                parentPos.y + (parentNode.computed?.height ?? 0) - nodeHeight + nodeHeight * currNodeOrigin[1],
-              ],
-            ]
-          : currentExtent;
-    } else {
+    if (!parentNode) {
       onError?.('005', errorMessages['error005']());
-      currentExtent = clampedNodeExtent;
+    } else {
+      const nodeWidth = node.computed?.width;
+      const nodeHeight = node.computed?.height;
+      const parentWidth = parentNode?.computed?.width;
+      const parentHeight = parentNode?.computed?.height;
+
+      if (nodeWidth && nodeHeight && parentWidth && parentHeight) {
+        const currNodeOrigin = node.origin || nodeOrigin;
+        const extentX = parentX + nodeWidth * currNodeOrigin[0];
+        const extentY = parentY + nodeHeight * currNodeOrigin[1];
+
+        currentExtent = [
+          [extentX, extentY],
+          [extentX + parentWidth - nodeWidth, extentY + parentHeight - nodeHeight],
+        ];
+      }
     }
-  } else if (node.extent && node.parentNode && node.extent !== 'parent') {
+  } else if (parentNode && isCoordinateExtent(node.extent)) {
     currentExtent = [
-      [node.extent[0][0] + parentPos.x, node.extent[0][1] + parentPos.y],
-      [node.extent[1][0] + parentPos.x, node.extent[1][1] + parentPos.y],
+      [node.extent[0][0] + parentX, node.extent[0][1] + parentY],
+      [node.extent[1][0] + parentX, node.extent[1][1] + parentY],
     ];
   }
 
-  const positionAbsolute =
-    currentExtent && currentExtent !== 'parent'
-      ? clampPosition(nextPosition, currentExtent as CoordinateExtent)
-      : nextPosition;
+  const positionAbsolute = isCoordinateExtent(currentExtent)
+    ? clampPosition(nextPosition, currentExtent)
+    : nextPosition;
 
   return {
     position: {
-      x: positionAbsolute.x - parentPos.x,
-      y: positionAbsolute.y - parentPos.y,
+      x: positionAbsolute.x - parentX,
+      y: positionAbsolute.y - parentY,
     },
     positionAbsolute,
   };

packages/system/src/xydrag/XYDrag.ts

@@ -5,7 +5,7 @@ import {
   calcAutoPan,
   getEventPosition,
   getPointerPosition,
-  calcNextPosition,
+  calculateNodePosition,
   snapPosition,
   getNodesBounds,
   rectToBox,
@@ -103,7 +103,6 @@ export function XYDrag<OnNodeDrag extends (e: any, nodes: any, node: any) => voi
   function update({ noDragClassName, handleSelector, domNode, isSelectable, nodeId }: DragUpdateParams) {
     function updateNodes({ x, y }: XYPosition) {
       const {
-        nodes,
         nodeLookup,
         nodeExtent,
         snapGrid,
@@ -149,13 +148,20 @@ export function XYDrag<OnNodeDrag extends (e: any, nodes: any, node: any) => voi
             n.computed.positionAbsolute.y + (n.computed?.height ?? 0) - nodesBox.y2 + nodeExtent[1][1];
         }
 
-        const updatedPos = calcNextPosition(n, nextPosition, nodes, adjustedNodeExtent, nodeOrigin, onError);
+        const { position, positionAbsolute } = calculateNodePosition({
+          nodeId: n.id,
+          nextPosition,
+          nodeLookup,
+          nodeExtent: adjustedNodeExtent,
+          nodeOrigin,
+          onError,
+        });
 
         // we want to make sure that we only fire a change event when there is a change
-        hasChange = hasChange || n.position.x !== updatedPos.position.x || n.position.y !== updatedPos.position.y;
+        hasChange = hasChange || n.position.x !== position.x || n.position.y !== position.y;
 
-        n.position = updatedPos.position;
-        n.computed.positionAbsolute = updatedPos.positionAbsolute;
+        n.position = position;
+        n.computed.positionAbsolute = positionAbsolute;
 
         return n;
       });