tweak API of duplicateElements to clarify usage

packages/excalidraw/actions/actionDuplicateSelection.tsx

@@ -53,7 +53,9 @@ export const actionDuplicateSelection = register({
     }
 
     let { newElements: duplicatedElements, elementsWithClones: nextElements } =
-      duplicateElements(elements, {
+      duplicateElements({
+        type: "in-place",
+        elements,
         idsOfElementsToDuplicate: arrayToMap(
           getSelectedElements(elements, appState, {
             includeBoundTextElement: true,
@@ -66,6 +68,7 @@ export const actionDuplicateSelection = register({
           x: element.x + DEFAULT_GRID_SIZE / 2,
           y: element.y + DEFAULT_GRID_SIZE / 2,
         }),
+        reverseOrder: false,
       });
 
     if (app.props.onDuplicate && nextElements) {

packages/excalidraw/components/App.tsx

@@ -3219,17 +3219,16 @@ class App extends React.Component<AppProps, AppState> {
 
     const [gridX, gridY] = getGridPoint(dx, dy, this.getEffectiveGridSize());
 
-    const { newElements } = duplicateElements(
-      elements.map((element) => {
+    const { newElements } = duplicateElements({
+      type: "everything",
+      elements: elements.map((element) => {
         return newElementWith(element, {
           x: element.x + gridX - minX,
           y: element.y + gridY - minY,
         });
       }),
-      {
       randomizeSeed: !opts.retainSeed,
-      },
-    );
+    });
 
     const prevElements = this.scene.getElementsIncludingDeleted();
     let nextElements = [...prevElements, ...newElements];
@@ -8434,7 +8433,9 @@ class App extends React.Component<AppProps, AppState> {
             }
 
             const { newElements: clonedElements, elementsWithClones } =
-              duplicateElements(elements, {
+              duplicateElements({
+                type: "in-place",
+                elements,
                 appState: this.state,
                 randomizeSeed: true,
                 idsOfElementsToDuplicate: new Map(

packages/excalidraw/components/LibraryMenuItems.tsx

@@ -162,8 +162,11 @@ export default function LibraryMenuItems({
           ...item,
           // duplicate each library item before inserting on canvas to confine
           // ids and bindings to each library item. See #6465
-          elements: duplicateElements(item.elements, { randomizeSeed: true })
-            .newElements,
+          elements: duplicateElements({
+            type: "everything",
+            elements: item.elements,
+            randomizeSeed: true,
+          }).newElements,
         };
       });
     },

packages/excalidraw/element/duplicate.test.tsx

@@ -7,7 +7,12 @@ import { FONT_FAMILY, ORIG_ID, ROUNDNESS } from "../constants";
 import { API } from "../tests/helpers/api";
 import { isPrimitive } from "../utils";
 
-import { act, assertElements, render } from "../tests/test-utils";
+import {
+  act,
+  assertElements,
+  getCloneByOrigId,
+  render,
+} from "../tests/test-utils";
 import { Excalidraw } from "..";
 import { actionDuplicateSelection } from "../actions";
 
@@ -162,7 +167,10 @@ describe("duplicating multiple elements", () => {
     // -------------------------------------------------------------------------
 
     const origElements = [rectangle1, text1, arrow1, arrow2, text2] as const;
-    const { newElements: clonedElements } = duplicateElements(origElements);
+    const { newElements: clonedElements } = duplicateElements({
+      type: "everything",
+      elements: origElements,
+    });
 
     // generic id in-equality checks
     // --------------------------------------------------------------------------
@@ -313,9 +321,10 @@ describe("duplicating multiple elements", () => {
     // -------------------------------------------------------------------------
 
     const origElements = [rectangle1, text1, arrow1, arrow2, arrow3] as const;
-    const { newElements: clonedElements } = duplicateElements(
-      origElements,
-    ) as any as { newElements: typeof origElements };
+    const { newElements: clonedElements } = duplicateElements({
+      type: "everything",
+      elements: origElements,
+    }) as any as { newElements: typeof origElements };
 
     const [
       clonedRectangle,
@@ -359,9 +368,10 @@ describe("duplicating multiple elements", () => {
       });
 
       const origElements = [rectangle1, rectangle2, rectangle3] as const;
-      const { newElements: clonedElements } = duplicateElements(
-        origElements,
-      ) as any as { newElements: typeof origElements };
+      const { newElements: clonedElements } = duplicateElements({
+        type: "everything",
+        elements: origElements,
+      }) as any as { newElements: typeof origElements };
       const [clonedRectangle1, clonedRectangle2, clonedRectangle3] =
         clonedElements;
 
@@ -384,7 +394,7 @@ describe("duplicating multiple elements", () => {
 
       const {
         newElements: [clonedRectangle1],
-      } = duplicateElements([rectangle1]);
+      } = duplicateElements({ type: "everything", elements: [rectangle1] });
 
       expect(typeof clonedRectangle1.groupIds[0]).toBe("string");
       expect(rectangle1.groupIds[0]).not.toBe(clonedRectangle1.groupIds[0]);

packages/excalidraw/element/duplicate.ts

@@ -96,20 +96,62 @@ export const duplicateElement = <TElement extends ExcalidrawElement>(
 };
 
 export const duplicateElements = (
-  elements: readonly ExcalidrawElement[],
-  opts?: {
-    idsOfElementsToDuplicate?: Map<ExcalidrawElement["id"], ExcalidrawElement>;
-    appState?: {
-      editingGroupId: AppState["editingGroupId"];
-      selectedGroupIds: AppState["selectedGroupIds"];
-    };
+  opts: {
+    elements: readonly ExcalidrawElement[];
+    randomizeSeed?: boolean;
     overrides?: (
       originalElement: ExcalidrawElement,
     ) => Partial<ExcalidrawElement>;
-    randomizeSeed?: boolean;
-    reverseOrder?: boolean;
-  },
+  } & (
+    | {
+        /**
+         * Duplicates all elements in array.
+         *
+         * Use this when programmaticaly duplicating elements, without direct
+         * user interaction.
+         */
+        type: "everything";
+      }
+    | {
+        /**
+         * Duplicates specified elements and inserts them back into the array
+         * in specified order.
+         *
+         * Use this when duplicating Scene elements, during user interaction
+         * such as alt-drag or on duplicate action.
+         */
+        type: "in-place";
+        idsOfElementsToDuplicate: Map<
+          ExcalidrawElement["id"],
+          ExcalidrawElement
+        >;
+        appState: {
+          editingGroupId: AppState["editingGroupId"];
+          selectedGroupIds: AppState["selectedGroupIds"];
+        };
+        /**
+         * If true, duplicated elements are inserted _before_ specified
+         * elements. Case: alt-dragging elements to duplicate them.
+         *
+         * TODO: remove this once (if) we stop replacing the original element
+         * with the duplicated one in the scene array.
+         */
+        reverseOrder: boolean;
+      }
+  ),
 ) => {
+  let { elements } = opts;
+
+  const appState =
+    "appState" in opts
+      ? opts.appState
+      : ({
+          editingGroupId: null,
+          selectedGroupIds: {},
+        } as const);
+
+  const reverseOrder = opts.type === "in-place" ? opts.reverseOrder : false;
+
   // Ids of elements that have already been processed so we don't push them
   // into the array twice if we end up backtracking when retrieving
   // discontiguous group of elements (can happen due to a bug, or in edge
@@ -128,11 +170,12 @@ export const duplicateElements = (
   const duplicatedElementsMap = new Map<string, ExcalidrawElement>();
   const elementsMap = arrayToMap(elements) as ElementsMap;
   const _idsOfElementsToDuplicate =
-    opts?.idsOfElementsToDuplicate ??
-    new Map(elements.map((el) => [el.id, el]));
+    opts.type === "in-place"
+      ? opts.idsOfElementsToDuplicate
+      : new Map(elements.map((el) => [el.id, el]));
 
   // For sanity
-  if (opts?.appState?.selectedGroupIds) {
+  if (opts.type === "in-place") {
     for (const groupId of Object.keys(opts.appState.selectedGroupIds)) {
       elements
         .filter((el) => el.groupIds?.includes(groupId))
@@ -165,11 +208,11 @@ export const duplicateElements = (
         processedIds.set(element.id, true);
 
         const newElement = duplicateElement(
-          opts?.appState?.editingGroupId ?? null,
+          appState.editingGroupId,
           groupIdMap,
           element,
-          opts?.overrides?.(element),
-          opts?.randomizeSeed,
+          opts.overrides?.(element),
+          opts.randomizeSeed,
         );
 
         processedIds.set(newElement.id, true);
@@ -204,18 +247,18 @@ export const duplicateElements = (
       return;
     }
 
-    if (opts?.reverseOrder && index < 1) {
+    if (reverseOrder && index < 1) {
       elementsWithClones.unshift(...castArray(elements));
       return;
     }
 
-    if (!opts?.reverseOrder && index > elementsWithClones.length - 1) {
+    if (!reverseOrder && index > elementsWithClones.length - 1) {
       elementsWithClones.push(...castArray(elements));
       return;
     }
 
     elementsWithClones.splice(
-      index + (opts?.reverseOrder ? 0 : 1),
+      index + (reverseOrder ? 0 : 1),
       0,
       ...castArray(elements),
     );
@@ -241,13 +284,7 @@ export const duplicateElements = (
     // groups
     // -------------------------------------------------------------------------
 
-    const groupId = getSelectedGroupForElement(
-      (opts?.appState ?? {
-        editingGroupId: null,
-        selectedGroupIds: {},
-      }) as AppState,
-      element,
-    );
+    const groupId = getSelectedGroupForElement(appState, element);
     if (groupId) {
       const groupElements = getElementsInGroup(elements, groupId).flatMap(
         (element) =>
@@ -256,7 +293,7 @@ export const duplicateElements = (
             : [element],
       );
 
-      const targetIndex = opts?.reverseOrder
+      const targetIndex = reverseOrder
         ? elementsWithClones.findIndex((el) => {
             return el.groupIds?.includes(groupId);
           })
@@ -306,7 +343,7 @@ export const duplicateElements = (
 
       if (boundTextElement) {
         insertBeforeOrAfterIndex(
-          targetIndex + (opts?.reverseOrder ? -1 : 0),
+          targetIndex + (reverseOrder ? -1 : 0),
           copyElements([element, boundTextElement]),
         );
       } else {

packages/excalidraw/groups.ts

@@ -214,7 +214,10 @@ export const isSelectedViaGroup = (
 ) => getSelectedGroupForElement(appState, element) != null;
 
 export const getSelectedGroupForElement = (
-  appState: InteractiveCanvasAppState,
+  appState: Pick<
+    InteractiveCanvasAppState,
+    "editingGroupId" | "selectedGroupIds"
+  >,
   element: ExcalidrawElement,
 ) =>
   element.groupIds