Separate mutation of elbow arrows from mutateElement

packages/element/src/binding.ts

@@ -66,7 +66,7 @@ import {
 } from "./typeChecks";
 
 import { aabbForElement, getElementShape, pointInsideBounds } from "./shapes";
-import { updateElbowArrowPoints } from "./elbowArrow";
+import { mutateElbowArrow, updateElbowArrowPoints } from "./elbowArrow";
 
 import type { Bounds } from "./bounds";
 import type { ElementUpdate } from "./mutateElement";
@@ -796,7 +796,20 @@ export const updateBoundElements = (
 
     // `linearElement` is being moved/scaled already, just update the binding
     if (simultaneouslyUpdatedElementIds.has(element.id)) {
+      if (isElbowArrow(element)) {
+        mutateElbowArrow(
+          element,
+          bindings as {
+            startBinding: FixedPointBinding;
+            endBinding: FixedPointBinding;
+          },
+          true,
+          elementsMap,
+        );
+      } else {
         mutateElement(element, bindings, true);
+      }
+
       return;
     }
 

packages/element/src/elbowArrow.ts

@@ -22,6 +22,8 @@ import {
   isDevEnv,
 } from "@excalidraw/common";
 
+import type { Radians } from "@excalidraw/math";
+
 import type { AppState } from "@excalidraw/excalidraw/types";
 
 import {
@@ -45,8 +47,8 @@ import {
   vectorToHeading,
   headingForPoint,
 } from "./heading";
-import { type ElementUpdate } from "./mutateElement";
+import { mutateElement, type ElementUpdate } from "./mutateElement";
-import { isBindableElement } from "./typeChecks";
+import { isBindableElement, isElbowArrow } from "./typeChecks";
 import {
   type ExcalidrawElbowArrowElement,
   type NonDeletedSceneElementsMap,
@@ -61,6 +63,7 @@ import type {
   Arrowhead,
   ElementsMap,
   ExcalidrawBindableElement,
+  ExcalidrawElement,
   FixedPointBinding,
   FixedSegment,
   NonDeletedExcalidrawElement,
@@ -879,6 +882,64 @@ const handleEndpointDrag = (
 
 const MAX_POS = 1e6;
 
+export const elbowArrowNeedsToGetNormalized = (
+  element: Readonly<ExcalidrawElement>,
+  updates: {
+    points?: readonly LocalPoint[];
+    fixedSegments?: readonly FixedSegment[] | null;
+    startBinding?: FixedPointBinding | null;
+    endBinding?: FixedPointBinding | null;
+  },
+) => {
+  const { points, fixedSegments, startBinding, endBinding } = updates;
+
+  return (
+    isElbowArrow(element) &&
+    (Object.keys(updates).length === 0 || // normalization case
+      typeof points !== "undefined" || // repositioning
+      typeof fixedSegments !== "undefined" || // segment fixing
+      typeof startBinding !== "undefined" ||
+      typeof endBinding !== "undefined") // manual binding to element
+  );
+};
+
+/**
+ * Mutates an elbow arrow element and renormalizes it's properties if necessary.
+ */
+export const mutateElbowArrow = (
+  element: Readonly<ExcalidrawElbowArrowElement>,
+  updates: ElementUpdate<ExcalidrawElbowArrowElement>,
+  informMutation: boolean = true,
+  elementsMap: NonDeletedSceneElementsMap | SceneElementsMap | ElementsMap,
+  options?: {
+    isDragging?: boolean;
+  },
+): ElementUpdate<ExcalidrawElbowArrowElement> => {
+  invariant(
+    !isElbowArrow(element),
+    `Element "${element.type}" is not an elbow arrow! Use \`mutateElement\` instead`,
+  );
+
+  if (!elbowArrowNeedsToGetNormalized(element, updates)) {
+    return mutateElement(element, updates, informMutation);
+  }
+
+  return mutateElement(
+    element,
+    {
+      ...updates,
+      angle: 0 as Radians,
+      ...updateElbowArrowPoints(
+        element,
+        elementsMap as NonDeletedSceneElementsMap,
+        updates,
+        options,
+      ),
+    },
+    informMutation,
+  );
+};
+
 /**
  *
  */
@@ -887,7 +948,7 @@ export const updateElbowArrowPoints = (
   elementsMap: NonDeletedSceneElementsMap,
   updates: {
     points?: readonly LocalPoint[];
-    fixedSegments?: FixedSegment[] | null;
+    fixedSegments?: readonly FixedSegment[] | null;
     startBinding?: FixedPointBinding | null;
     endBinding?: FixedPointBinding | null;
   },

packages/element/src/linearElementEditor.ts

@@ -22,7 +22,7 @@ import {
 
 // TODO: remove direct dependency on the scene, should be passed in or injected instead
 // eslint-disable-next-line @typescript-eslint/no-restricted-imports
-import Scene from "@excalidraw/excalidraw/scene/Scene";
+import type Scene from "@excalidraw/excalidraw/scene/Scene";
 
 import type { Store } from "@excalidraw/excalidraw/store";
 
@@ -50,7 +50,7 @@ import {
   getMinMaxXYFromCurvePathOps,
 } from "./bounds";
 
-import { updateElbowArrowPoints } from "./elbowArrow";
+import { mutateElbowArrow, updateElbowArrowPoints } from "./elbowArrow";
 
 import { headingIsHorizontal, vectorToHeading } from "./heading";
 import { bumpVersion, mutateElement } from "./mutateElement";
@@ -794,7 +794,10 @@ export class LinearElementEditor {
         elementsMap,
       );
     } else if (event.altKey && appState.editingLinearElement) {
-      if (linearElementEditor.lastUncommittedPoint == null) {
+      if (
+        linearElementEditor.lastUncommittedPoint == null &&
+        !isElbowArrow(element)
+      ) {
         mutateElement(element, {
           points: [
             ...element.points,
@@ -1219,7 +1222,12 @@ export class LinearElementEditor {
       return acc;
     }, []);
 
-    mutateElement(element, { points: nextPoints });
+    const updates = { points: nextPoints };
+    if (isElbowArrow(element)) {
+      mutateElbowArrow(element, updates, true, elementsMap);
+    } else {
+      mutateElement(element, updates);
+    }
 
     // temp hack to ensure the line doesn't move when adding point to the end,
     // potentially expanding the bounding box
@@ -1425,9 +1433,12 @@ export class LinearElementEditor {
       ...element.points.slice(segmentMidpoint.index!),
     ];
 
-    mutateElement(element, {
+    const updates = { points };
-      points,
+    if (isElbowArrow(element)) {
-    });
+      mutateElbowArrow(element, updates, true, elementsMap);
+    } else {
+      mutateElement(element, updates);
+    }
 
     ret.pointerDownState = {
       ...linearElementEditor.pointerDownState,
@@ -1479,8 +1490,8 @@ export class LinearElementEditor {
 
       updates.points = Array.from(nextPoints);
 
-      if (!options?.sceneElementsMap || Scene.getScene(element)) {
+      if (!options?.sceneElementsMap) {
-        mutateElement(element, updates, true, {
+        mutateElbowArrow(element, updates, true, options?.sceneElementsMap!, {
           isDragging: options?.isDragging,
         });
       } else {
@@ -1825,9 +1836,14 @@ export class LinearElementEditor {
         .map((segment) => segment.index)
         .reduce((count, idx) => (idx < index ? count + 1 : count), 0);
 
-      mutateElement(element, {
+      mutateElbowArrow(
+        element,
+        {
           fixedSegments: nextFixedSegments,
-      });
+        },
+        true,
+        elementsMap,
+      );
 
       const point = pointFrom<GlobalPoint>(
         element.x +
@@ -1859,14 +1875,19 @@ export class LinearElementEditor {
 
   static deleteFixedSegment(
     element: ExcalidrawElbowArrowElement,
+    elementsMap: NonDeletedSceneElementsMap,
     index: number,
   ): void {
-    mutateElement(element, {
+    mutateElbowArrow(
+      element,
+      {
         fixedSegments: element.fixedSegments?.filter(
           (segment) => segment.index !== index,
         ),
-    });
+      },
-    mutateElement(element, {}, true);
+      true,
+      elementsMap,
+    );
   }
 }
 

packages/element/src/mutateElement.ts

@@ -2,23 +2,20 @@ import {
   getSizeFromPoints,
   randomInteger,
   getUpdatedTimestamp,
-  toBrandedType,
+  invariant,
 } from "@excalidraw/common";
 
 // TODO: remove direct dependency on the scene, should be passed in or injected instead
 // eslint-disable-next-line @typescript-eslint/no-restricted-imports
 import Scene from "@excalidraw/excalidraw/scene/Scene";
 
-import type { Radians } from "@excalidraw/math";
-
 import type { Mutable } from "@excalidraw/common/utility-types";
 
 import { ShapeCache } from "./ShapeCache";
 
-import { updateElbowArrowPoints } from "./elbowArrow";
+import { elbowArrowNeedsToGetNormalized } from "./elbowArrow";
-import { isElbowArrow } from "./typeChecks";
 
-import type { ExcalidrawElement, NonDeletedSceneElementsMap } from "./types";
+import type { ExcalidrawElement } from "./types";
 
 export type ElementUpdate<TElement extends ExcalidrawElement> = Omit<
   Partial<TElement>,
@@ -33,54 +30,25 @@ export const mutateElement = <TElement extends Mutable<ExcalidrawElement>>(
   element: TElement,
   updates: ElementUpdate<TElement>,
   informMutation = true,
-  options?: {
-    // Currently only for elbow arrows.
-    // If true, the elbow arrow tries to bind to the nearest element. If false
-    // it tries to keep the same bound element, if any.
-    isDragging?: boolean;
-  },
 ): TElement => {
   let didChange = false;
 
   // casting to any because can't use `in` operator
   // (see https://github.com/microsoft/TypeScript/issues/21732)
-  const { points, fixedSegments, fileId, startBinding, endBinding } =
+  const { points, fileId, fixedSegments, startBinding, endBinding } =
     updates as any;
 
-  if (
+  invariant(
-    isElbowArrow(element) &&
+    elbowArrowNeedsToGetNormalized(element, {
-    (Object.keys(updates).length === 0 || // normalization case
-      typeof points !== "undefined" || // repositioning
-      typeof fixedSegments !== "undefined" || // segment fixing
-      typeof startBinding !== "undefined" ||
-      typeof endBinding !== "undefined") // manual binding to element
-  ) {
-    const elementsMap = toBrandedType<NonDeletedSceneElementsMap>(
-      Scene.getScene(element)?.getNonDeletedElementsMap() ?? new Map(),
-    );
-
-    updates = {
-      ...updates,
-      angle: 0 as Radians,
-      ...updateElbowArrowPoints(
-        {
-          ...element,
-          x: updates.x || element.x,
-          y: updates.y || element.y,
-        },
-        elementsMap,
-        {
-          fixedSegments,
       points,
+      fixedSegments,
       startBinding,
       endBinding,
-        },
+    }),
-        {
+    "Elbow arrow should get normalized! Use `mutateElbowArrow` instead.",
-          isDragging: options?.isDragging,
+  );
-        },
+
-      ),
+  if (typeof points !== "undefined") {
-    };
-  } else if (typeof points !== "undefined") {
     updates = { ...getSizeFromPoints(points), ...updates };
   }
 

packages/element/src/resizeElements.ts

@@ -60,6 +60,8 @@ import {
 
 import { isInGroup } from "./groups";
 
+import { mutateElbowArrow } from "./elbowArrow";
+
 import type { BoundingBox } from "./bounds";
 import type {
   MaybeTransformHandleType,
@@ -545,9 +547,14 @@ const rotateMultipleElements = (
 
       if (isElbowArrow(element)) {
         // Needed to re-route the arrow
-        mutateElement(element, {
+        mutateElbowArrow(
+          element,
+          {
             points: getArrowLocalFixedPoints(element, elementsMap),
-        });
+          },
+          false,
+          elementsMap,
+        );
       } else {
         mutateElement(
           element,
@@ -1527,10 +1534,14 @@ export const resizeMultipleElements = (
     } of elementsAndUpdates) {
       const { width, height, angle } = update;
 
-      mutateElement(element, update, false, {
+      if (isElbowArrow(element)) {
+        mutateElbowArrow(element, update, false, elementsMap, {
           // needed for the fixed binding point udpate to take effect
           isDragging: true,
         });
+      } else {
+        mutateElement(element, update, false);
+      }
 
       updateBoundElements(element, elementsMap as SceneElementsMap, {
         simultaneouslyUpdated: elementsToUpdate,

packages/excalidraw/actions/actionBoundText.tsx

@@ -21,6 +21,7 @@ import {
 
 import {
   hasBoundTextElement,
+  isElbowArrow,
   isTextBindableContainer,
   isTextElement,
   isUsingAdaptiveRadius,
@@ -33,11 +34,14 @@ import { syncMovedIndices } from "@excalidraw/element/fractionalIndex";
 
 import { newElement } from "@excalidraw/element/newElement";
 
+import { mutateElbowArrow } from "@excalidraw/element/elbowArrow";
+
 import type {
   ExcalidrawElement,
   ExcalidrawLinearElement,
   ExcalidrawTextContainer,
   ExcalidrawTextElement,
+  FixedPointBinding,
 } from "@excalidraw/element/types";
 
 import type { Mutable } from "@excalidraw/common/utility-types";
@@ -297,7 +301,21 @@ export const actionWrapTextInContainer = register({
             }
 
             if (startBinding || endBinding) {
-              mutateElement(ele, { startBinding, endBinding }, false);
+              const updates = { startBinding, endBinding };
+
+              if (isElbowArrow(ele)) {
+                mutateElbowArrow(
+                  ele,
+                  updates as {
+                    startBinding: FixedPointBinding;
+                    endBinding: FixedPointBinding;
+                  },
+                  false,
+                  app.scene.getNonDeletedElementsMap(),
+                );
+              } else {
+                mutateElement(ele, updates, false);
+              }
             }
           });
         }

packages/excalidraw/actions/actionDeleteSelected.tsx

@@ -3,10 +3,7 @@ import { KEYS, updateActiveTool } from "@excalidraw/common";
 import { getNonDeletedElements } from "@excalidraw/element";
 import { fixBindingsAfterDeletion } from "@excalidraw/element/binding";
 import { LinearElementEditor } from "@excalidraw/element/linearElementEditor";
-import {
+import { newElementWith } from "@excalidraw/element/mutateElement";
-  mutateElement,
-  newElementWith,
-} from "@excalidraw/element/mutateElement";
 import { getContainerElement } from "@excalidraw/element/textElement";
 import {
   isBoundToContainer,
@@ -20,6 +17,8 @@ import {
   selectGroupsForSelectedElements,
 } from "@excalidraw/element/groups";
 
+import { mutateElbowArrow } from "@excalidraw/element/elbowArrow";
+
 import type { ExcalidrawElement } from "@excalidraw/element/types";
 
 import { t } from "../i18n";
@@ -94,15 +93,21 @@ const deleteSelectedElements = (
         el.boundElements.forEach((candidate) => {
           const bound = app.scene.getNonDeletedElementsMap().get(candidate.id);
           if (bound && isElbowArrow(bound)) {
-            mutateElement(bound, {
+            mutateElbowArrow(
+              bound,
+              {
                 startBinding:
                   el.id === bound.startBinding?.elementId
                     ? null
                     : bound.startBinding,
                 endBinding:
-                el.id === bound.endBinding?.elementId ? null : bound.endBinding,
+                  el.id === bound.endBinding?.elementId
-            });
+                    ? null
-            mutateElement(bound, { points: bound.points });
+                    : bound.endBinding,
+              },
+              true,
+              app.scene.getNonDeletedElementsMap(),
+            );
           }
         });
       }

packages/excalidraw/actions/actionFinalize.tsx

@@ -5,9 +5,12 @@ import {
   bindOrUnbindLinearElement,
 } from "@excalidraw/element/binding";
 import { LinearElementEditor } from "@excalidraw/element/linearElementEditor";
+
 import { mutateElement } from "@excalidraw/element/mutateElement";
+import { mutateElbowArrow } from "@excalidraw/element/elbowArrow";
 import {
   isBindingElement,
+  isElbowArrow,
   isLinearElement,
 } from "@excalidraw/element/typeChecks";
 
@@ -16,6 +19,13 @@ import { isPathALoop } from "@excalidraw/element/shapes";
 
 import { isInvisiblySmallElement } from "@excalidraw/element/sizeHelpers";
 
+import type {
+  ExcalidrawElbowArrowElement,
+  ExcalidrawElement,
+} from "@excalidraw/element/types";
+
+import type { ElementUpdate } from "@excalidraw/element/mutateElement";
+
 import { t } from "../i18n";
 import { resetCursor } from "../cursor";
 import { done } from "../components/icons";
@@ -85,6 +95,16 @@ export const actionFinalize = register({
       ? appState.newElement
       : null;
 
+    const mutate = (updates: ElementUpdate<ExcalidrawElbowArrowElement>) =>
+      isElbowArrow(multiPointElement as ExcalidrawElbowArrowElement)
+        ? mutateElbowArrow(
+            multiPointElement as ExcalidrawElbowArrowElement,
+            updates,
+            true,
+            elementsMap,
+          )
+        : mutateElement(multiPointElement as ExcalidrawElement, updates);
+
     if (multiPointElement) {
       // pen and mouse have hover
       if (
@@ -96,7 +116,7 @@ export const actionFinalize = register({
           !lastCommittedPoint ||
           points[points.length - 1] !== lastCommittedPoint
         ) {
-          mutateElement(multiPointElement, {
+          mutate({
             points: multiPointElement.points.slice(0, -1),
           });
         }
@@ -120,7 +140,7 @@ export const actionFinalize = register({
         if (isLoop) {
           const linePoints = multiPointElement.points;
           const firstPoint = linePoints[0];
-          mutateElement(multiPointElement, {
+          mutate({
             points: linePoints.map((p, index) =>
               index === linePoints.length - 1
                 ? pointFrom(firstPoint[0], firstPoint[1])

packages/excalidraw/components/App.tsx

@@ -302,6 +302,10 @@ import {
 
 import { isNonDeletedElement } from "@excalidraw/element";
 
+import { mutateElbowArrow } from "@excalidraw/element/elbowArrow";
+
+import type { ElementUpdate } from "@excalidraw/element/mutateElement";
+
 import type { LocalPoint, Radians } from "@excalidraw/math";
 
 import type {
@@ -327,6 +331,7 @@ import type {
   MagicGenerationData,
   ExcalidrawNonSelectionElement,
   ExcalidrawArrowElement,
+  ExcalidrawElbowArrowElement,
 } from "@excalidraw/element/types";
 
 import type { ValueOf } from "@excalidraw/common/utility-types";
@@ -5485,7 +5490,11 @@ class App extends React.Component<AppProps, AppState> {
 
         if (midPoint && midPoint > -1) {
           this.store.shouldCaptureIncrement();
-          LinearElementEditor.deleteFixedSegment(selectedElements[0], midPoint);
+          LinearElementEditor.deleteFixedSegment(
+            selectedElements[0],
+            this.scene.getNonDeletedElementsMap(),
+            midPoint,
+          );
 
           const nextCoords = LinearElementEditor.getSegmentMidpointHitCoords(
             {
@@ -5991,10 +6000,7 @@ class App extends React.Component<AppProps, AppState> {
         if (isPathALoop(points, this.state.zoom.value)) {
           setCursor(this.interactiveCanvas, CURSOR_TYPE.POINTER);
         }
-        // update last uncommitted point
+        const updates = {
-        mutateElement(
-          multiElement,
-          {
           points: [
             ...points.slice(0, -1),
             pointFrom<LocalPoint>(
@@ -6002,12 +6008,22 @@ class App extends React.Component<AppProps, AppState> {
               lastCommittedY + dyFromLastCommitted,
             ),
           ],
-          },
+        };
+
+        if (isElbowArrow(multiElement)) {
+          mutateElbowArrow(
+            multiElement,
+            updates,
             false,
+            this.scene.getNonDeletedElementsMap(),
             {
               isDragging: true,
             },
           );
+        } else {
+          // update last uncommitted point
+          mutateElement(multiElement, updates, false);
+        }
 
         // in this path, we're mutating multiElement to reflect
         // how it will be after adding pointer position as the next point
@@ -8672,25 +8688,33 @@ class App extends React.Component<AppProps, AppState> {
             ));
           }
 
-          if (points.length === 1) {
+          const mutate = (
-            mutateElement(
+            updates: ElementUpdate<ExcalidrawLinearElement>,
+            options: { isDragging?: boolean } = {},
+          ) =>
+            isElbowArrow(newElement)
+              ? mutateElbowArrow(
                   newElement,
-              {
+                  updates as ElementUpdate<ExcalidrawElbowArrowElement>,
-                points: [...points, pointFrom<LocalPoint>(dx, dy)],
-              },
                   false,
-            );
+                  this.scene.getNonDeletedElementsMap(),
+                  options,
+                )
+              : mutateElement(newElement, updates, false);
+
+          if (points.length === 1) {
+            mutate({
+              points: [...points, pointFrom<LocalPoint>(dx, dy)],
+            });
           } else if (
             points.length === 2 ||
             (points.length > 1 && isElbowArrow(newElement))
           ) {
-            mutateElement(
+            mutate(
-              newElement,
               {
                 points: [...points.slice(0, -1), pointFrom<LocalPoint>(dx, dy)],
               },
-              false,
+              { isDragging: true }
-              { isDragging: true },
             );
           }
 
@@ -8937,7 +8961,12 @@ class App extends React.Component<AppProps, AppState> {
             this.scene.getNonDeletedElementsMap(),
           );
           if (element) {
-            mutateElement(element, {}, true);
+            mutateElbowArrow(
+              element as ExcalidrawElbowArrowElement,
+              {},
+              true,
+              this.scene.getNonDeletedElementsMap(),
+            );
           }
         }
 
@@ -9080,7 +9109,7 @@ class App extends React.Component<AppProps, AppState> {
         );
 
         if (!pointerDownState.drag.hasOccurred && newElement && !multiElement) {
-          mutateElement(newElement, {
+          const updates = {
             points: [
               ...newElement.points,
               pointFrom<LocalPoint>(
@@ -9088,7 +9117,19 @@ class App extends React.Component<AppProps, AppState> {
                 pointerCoords.y - newElement.y,
               ),
             ],
-          });
+          };
+
+          if (isElbowArrow(newElement)) {
+            mutateElbowArrow(
+              newElement,
+              updates,
+              false,
+              this.scene.getNonDeletedElementsMap(),
+            );
+          } else {
+            mutateElement(newElement, updates, false);
+          }
+
           this.setState({
             multiElement: newElement,
             newElement,

packages/excalidraw/components/Stats/MultiDimension.tsx

@@ -13,10 +13,12 @@ import {
   handleBindTextResize,
 } from "@excalidraw/element/textElement";
 
-import { isTextElement } from "@excalidraw/element/typeChecks";
+import { isElbowArrow, isTextElement } from "@excalidraw/element/typeChecks";
 
 import { getCommonBounds } from "@excalidraw/utils";
 
+import { mutateElbowArrow } from "@excalidraw/element/elbowArrow";
+
 import type {
   ElementsMap,
   ExcalidrawElement,
@@ -80,7 +82,12 @@ const resizeElementInGroup = (
 ) => {
   const updates = getResizedUpdates(anchorX, anchorY, scale, origElement);
 
+  if (isElbowArrow(latestElement)) {
+    mutateElbowArrow(latestElement, updates, false, elementsMap);
+  } else {
     mutateElement(latestElement, updates, false);
+  }
+
   const boundTextElement = getBoundTextElement(
     origElement,
     originalElementsMap,

packages/excalidraw/index.tsx

@@ -264,6 +264,8 @@ export {
   bumpVersion,
 } from "@excalidraw/element/mutateElement";
 
+export { mutateElbowArrow } from "@excalidraw/element/elbowArrow";
+
 export { CaptureUpdateAction } from "./store";
 
 export { parseLibraryTokensFromUrl, useHandleLibrary } from "./data/library";