Duplicate fixes

packages/excalidraw/actions/actionDuplicateSelection.tsx

@@ -52,7 +52,8 @@ export const actionDuplicateSelection = register({
       }
     }
 
-    const duplicatedElements = duplicateElements(elements, {
+    let { newElements: duplicatedElements, elementsWithClones: nextElements } =
+      duplicateElements(elements, {
         idsOfElementsToDuplicate: arrayToMap(
           getSelectedElements(elements, appState, {
             includeBoundTextElement: true,
@@ -67,10 +68,6 @@ export const actionDuplicateSelection = register({
         }),
       });
 
-    let nextElements = (elements as ExcalidrawElement[])
-      .slice()
-      .concat(duplicatedElements);
-
     if (app.props.onDuplicate && nextElements) {
       const mappedElements = app.props.onDuplicate(nextElements, elements);
       if (mappedElements) {

packages/excalidraw/components/App.tsx

@@ -3219,7 +3219,7 @@ class App extends React.Component<AppProps, AppState> {
 
     const [gridX, gridY] = getGridPoint(dx, dy, this.getEffectiveGridSize());
 
-    const newElements = duplicateElements(
+    const { newElements } = duplicateElements(
       elements.map((element) => {
         return newElementWith(element, {
           x: element.x + gridX - minX,
@@ -8419,6 +8419,7 @@ class App extends React.Component<AppProps, AppState> {
 
             pointerDownState.hit.hasBeenDuplicated = true;
 
+            const elements = this.scene.getElementsIncludingDeleted();
             const hitElement = pointerDownState.hit.element;
             const selectedElements = this.scene.getSelectedElements({
               selectedElementIds: this.state.selectedElementIds,
@@ -8431,13 +8432,18 @@ class App extends React.Component<AppProps, AppState> {
             ) {
               selectedElements.push(hitElement);
             }
-            const clonedElements = duplicateElements(selectedElements, {
+
+            const { newElements: clonedElements, elementsWithClones } =
+              duplicateElements(elements, {
                 appState: this.state,
                 randomizeSeed: true,
+                idsOfElementsToDuplicate: new Map(
+                  selectedElements.map((el) => [el.id, el]),
+                ),
               });
 
             const nextSceneElements = syncMovedIndices(
-              [...clonedElements, ...this.scene.getElementsIncludingDeleted()],
+              elementsWithClones,
               arrayToMap(clonedElements),
             );
 

packages/excalidraw/components/LibraryMenuItems.tsx

@@ -162,7 +162,8 @@ 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 }),
+          elements: duplicateElements(item.elements, { randomizeSeed: true })
+            .newElements,
         };
       });
     },

packages/excalidraw/element/duplicate.test.ts

@@ -46,7 +46,7 @@ describe("duplicating single elements", () => {
       points: [pointFrom<LocalPoint>(1, 2), pointFrom<LocalPoint>(3, 4)],
     });
 
-    const copy = duplicateElement(null, new Map(), element);
+    const copy = duplicateElement(null, new Map(), element, undefined, true);
 
     assertCloneObjects(element, copy);
 
@@ -65,6 +65,8 @@ describe("duplicating single elements", () => {
       ...element,
       id: copy.id,
       seed: copy.seed,
+      version: copy.version,
+      versionNonce: copy.versionNonce,
     });
   });
 
@@ -150,7 +152,7 @@ describe("duplicating multiple elements", () => {
     // -------------------------------------------------------------------------
 
     const origElements = [rectangle1, text1, arrow1, arrow2, text2] as const;
-    const clonedElements = duplicateElements(origElements);
+    const { newElements: clonedElements } = duplicateElements(origElements);
 
     // generic id in-equality checks
     // --------------------------------------------------------------------------
@@ -369,7 +371,9 @@ describe("duplicating multiple elements", () => {
         groupIds: ["g1"],
       });
 
-      const [clonedRectangle1] = duplicateElements([rectangle1]);
+      const {
+        newElements: [clonedRectangle1],
+      } = duplicateElements([rectangle1]);
 
       expect(typeof clonedRectangle1.groupIds[0]).toBe("string");
       expect(rectangle1.groupIds[0]).not.toBe(clonedRectangle1.groupIds[0]);

packages/excalidraw/element/duplicate.ts

@@ -130,7 +130,8 @@ export const duplicateElements = (
   const duplicatedElementsMap = new Map<string, ExcalidrawElement>();
   const elementsMap = arrayToMap(elements) as ElementsMap;
   const _idsOfElementsToDuplicate =
-    opts?.idsOfElementsToDuplicate ?? new Set(elements.map((el) => el.id));
+    opts?.idsOfElementsToDuplicate ??
+    new Map(elements.map((el) => [el.id, el]));
 
   elements = normalizeElementOrder(elements);
 
@@ -397,138 +398,12 @@ export const duplicateElements = (
     oldIdToDuplicatedId,
   );
 
-  return newElements;
+  return {
+    newElements,
+    elementsWithClones,
+  };
 };
 
-/**
- * Clones elements, regenerating their ids (including bindings) and group ids.
- *
- * If bindings don't exist in the elements array, they are removed. Therefore,
- * it's advised to supply the whole elements array, or sets of elements that
- * are encapsulated (such as library items), if the purpose is to retain
- * bindings to the cloned elements intact.
- *
- * NOTE by default does not randomize or regenerate anything except the id.
- */
-// export const duplicateElements = (
-//   elements: readonly ExcalidrawElement[],
-//   opts?: {
-//     /** NOTE also updates version flags and `updated` */
-//     randomizeSeed?: boolean;
-//     overrides?: (element: ExcalidrawElement) => Partial<ExcalidrawElement>;
-//   },
-// ) => {
-//   const clonedElements: ExcalidrawElement[] = [];
-
-//   const origElementsMap = arrayToMap(elements);
-
-//   // used for for migrating old ids to new ids
-//   const elementNewIdsMap = new Map<
-//     /* orig */ ExcalidrawElement["id"],
-//     /* new */ ExcalidrawElement["id"]
-//   >();
-
-//   const maybeGetNewIdFor = (id: ExcalidrawElement["id"]) => {
-//     // if we've already migrated the element id, return the new one directly
-//     if (elementNewIdsMap.has(id)) {
-//       return elementNewIdsMap.get(id)!;
-//     }
-//     // if we haven't migrated the element id, but an old element with the same
-//     // id exists, generate a new id for it and return it
-//     if (origElementsMap.has(id)) {
-//       const newId = randomId();
-//       elementNewIdsMap.set(id, newId);
-//       return newId;
-//     }
-//     // if old element doesn't exist, return null to mark it for removal
-//     return null;
-//   };
-
-//   const groupNewIdsMap = new Map</* orig */ GroupId, /* new */ GroupId>();
-
-//   for (const element of elements) {
-//     let clonedElement: Mutable<ExcalidrawElement> = _deepCopyElement(element);
-
-//     if (opts?.overrides) {
-//       clonedElement = Object.assign(
-//         clonedElement,
-//         opts.overrides(clonedElement),
-//       );
-//     }
-
-//     clonedElement.id = maybeGetNewIdFor(element.id)!;
-//     if (isTestEnv()) {
-//       __test__defineOrigId(clonedElement, element.id);
-//     }
-
-//     if (opts?.randomizeSeed) {
-//       clonedElement.seed = randomInteger();
-//       bumpVersion(clonedElement);
-//     }
-
-//     if (clonedElement.groupIds) {
-//       clonedElement.groupIds = clonedElement.groupIds.map((groupId) => {
-//         if (!groupNewIdsMap.has(groupId)) {
-//           groupNewIdsMap.set(groupId, randomId());
-//         }
-//         return groupNewIdsMap.get(groupId)!;
-//       });
-//     }
-
-//     if ("containerId" in clonedElement && clonedElement.containerId) {
-//       const newContainerId = maybeGetNewIdFor(clonedElement.containerId);
-//       clonedElement.containerId = newContainerId;
-//     }
-
-//     if ("boundElements" in clonedElement && clonedElement.boundElements) {
-//       clonedElement.boundElements = clonedElement.boundElements.reduce(
-//         (
-//           acc: Mutable<NonNullable<ExcalidrawElement["boundElements"]>>,
-//           binding,
-//         ) => {
-//           const newBindingId = maybeGetNewIdFor(binding.id);
-//           if (newBindingId) {
-//             acc.push({ ...binding, id: newBindingId });
-//           }
-//           return acc;
-//         },
-//         [],
-//       );
-//     }
-
-//     if ("endBinding" in clonedElement && clonedElement.endBinding) {
-//       const newEndBindingId = maybeGetNewIdFor(
-//         clonedElement.endBinding.elementId,
-//       );
-//       clonedElement.endBinding = newEndBindingId
-//         ? {
-//             ...clonedElement.endBinding,
-//             elementId: newEndBindingId,
-//           }
-//         : null;
-//     }
-//     if ("startBinding" in clonedElement && clonedElement.startBinding) {
-//       const newEndBindingId = maybeGetNewIdFor(
-//         clonedElement.startBinding.elementId,
-//       );
-//       clonedElement.startBinding = newEndBindingId
-//         ? {
-//             ...clonedElement.startBinding,
-//             elementId: newEndBindingId,
-//           }
-//         : null;
-//     }
-
-//     if (clonedElement.frameId) {
-//       clonedElement.frameId = maybeGetNewIdFor(clonedElement.frameId);
-//     }
-
-//     clonedElements.push(clonedElement);
-//   }
-
-//   return clonedElements;
-// };
-
 // Simplified deep clone for the purpose of cloning ExcalidrawElement.
 //
 // Only clones plain objects and arrays. Doesn't clone Date, RegExp, Map, Set,

packages/excalidraw/tests/__snapshots__/contextmenu.test.tsx.snap

@@ -2606,8 +2606,8 @@ exports[`contextMenu element > selecting 'Duplicate' in context menu duplicates
   "strokeWidth": 2,
   "type": "rectangle",
   "updated": 1,
-  "version": 4,
+  "version": 5,
-  "versionNonce": 238820263,
+  "versionNonce": 400692809,
   "width": 20,
   "x": 0,
   "y": 10,

packages/excalidraw/tests/__snapshots__/history.test.tsx.snap

@@ -15172,9 +15172,11 @@ History {
             "selectedElementIds": {
               "id61": true,
             },
+            "selectedLinearElementId": "id61",
           },
           "inserted": {
             "selectedElementIds": {},
+            "selectedLinearElementId": null,
           },
         },
       },
@@ -18946,7 +18948,7 @@ exports[`history > singleplayer undo/redo > should support duplication of groups
   "strokeWidth": 2,
   "type": "rectangle",
   "updated": 1,
-  "version": 3,
+  "version": 4,
   "width": 100,
   "x": 10,
   "y": 10,
@@ -18980,7 +18982,7 @@ exports[`history > singleplayer undo/redo > should support duplication of groups
   "strokeWidth": 2,
   "type": "rectangle",
   "updated": 1,
-  "version": 3,
+  "version": 4,
   "width": 100,
   "x": 110,
   "y": 110,
@@ -19014,7 +19016,7 @@ exports[`history > singleplayer undo/redo > should support duplication of groups
   "strokeWidth": 2,
   "type": "rectangle",
   "updated": 1,
-  "version": 6,
+  "version": 7,
   "width": 100,
   "x": 10,
   "y": 10,
@@ -19048,7 +19050,7 @@ exports[`history > singleplayer undo/redo > should support duplication of groups
   "strokeWidth": 2,
   "type": "rectangle",
   "updated": 1,
-  "version": 6,
+  "version": 7,
   "width": 100,
   "x": 110,
   "y": 110,

packages/excalidraw/tests/__snapshots__/move.test.tsx.snap

@@ -1,73 +1,5 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
-exports[`duplicate element on move when ALT is clicked > rectangle 5`] = `
-{
-  "angle": 0,
-  "backgroundColor": "transparent",
-  "boundElements": null,
-  "customData": undefined,
-  "fillStyle": "solid",
-  "frameId": null,
-  "groupIds": [],
-  "height": 50,
-  "id": "id2",
-  "index": "a0",
-  "isDeleted": false,
-  "link": null,
-  "locked": false,
-  "opacity": 100,
-  "roughness": 1,
-  "roundness": {
-    "type": 3,
-  },
-  "seed": 238820263,
-  "strokeColor": "#1e1e1e",
-  "strokeStyle": "solid",
-  "strokeWidth": 2,
-  "type": "rectangle",
-  "updated": 1,
-  "version": 5,
-  "versionNonce": 400692809,
-  "width": 30,
-  "x": 30,
-  "y": 20,
-}
-`;
-
-exports[`duplicate element on move when ALT is clicked > rectangle 6`] = `
-{
-  "angle": 0,
-  "backgroundColor": "transparent",
-  "boundElements": null,
-  "customData": undefined,
-  "fillStyle": "solid",
-  "frameId": null,
-  "groupIds": [],
-  "height": 50,
-  "id": "id0",
-  "index": "a1",
-  "isDeleted": false,
-  "link": null,
-  "locked": false,
-  "opacity": 100,
-  "roughness": 1,
-  "roundness": {
-    "type": 3,
-  },
-  "seed": 1278240551,
-  "strokeColor": "#1e1e1e",
-  "strokeStyle": "solid",
-  "strokeWidth": 2,
-  "type": "rectangle",
-  "updated": 1,
-  "version": 6,
-  "versionNonce": 23633383,
-  "width": 30,
-  "x": -10,
-  "y": 60,
-}
-`;
-
 exports[`move element > rectangle 5`] = `
 {
   "angle": 0,

packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap

@@ -2139,7 +2139,7 @@ History {
               "frameId": null,
               "groupIds": [],
               "height": 10,
-              "index": "a0",
+              "index": "a1",
               "isDeleted": false,
               "link": null,
               "locked": false,
@@ -2153,8 +2153,8 @@ History {
               "strokeWidth": 2,
               "type": "rectangle",
               "width": 10,
-              "x": 10,
+              "x": 20,
-              "y": 10,
+              "y": 20,
             },
             "inserted": {
               "isDeleted": true,
@@ -2164,12 +2164,10 @@ History {
         "updated": Map {
           "id0" => Delta {
             "deleted": {
-              "index": "a1",
               "x": 20,
               "y": 20,
             },
             "inserted": {
-              "index": "a0",
               "x": 10,
               "y": 10,
             },
@@ -10631,7 +10629,7 @@ History {
                 "id7",
               ],
               "height": 10,
-              "index": "a0",
+              "index": "a3",
               "isDeleted": false,
               "link": null,
               "locked": false,
@@ -10645,8 +10643,8 @@ History {
               "strokeWidth": 2,
               "type": "rectangle",
               "width": 10,
-              "x": 10,
+              "x": 20,
-              "y": 10,
+              "y": 20,
             },
             "inserted": {
               "isDeleted": true,
@@ -10664,7 +10662,7 @@ History {
                 "id7",
               ],
               "height": 10,
-              "index": "a1",
+              "index": "a4",
               "isDeleted": false,
               "link": null,
               "locked": false,
@@ -10678,8 +10676,8 @@ History {
               "strokeWidth": 2,
               "type": "rectangle",
               "width": 10,
-              "x": 30,
+              "x": 40,
-              "y": 10,
+              "y": 20,
             },
             "inserted": {
               "isDeleted": true,
@@ -10697,7 +10695,7 @@ History {
                 "id7",
               ],
               "height": 10,
-              "index": "a2",
+              "index": "a5",
               "isDeleted": false,
               "link": null,
               "locked": false,
@@ -10711,8 +10709,8 @@ History {
               "strokeWidth": 2,
               "type": "rectangle",
               "width": 10,
-              "x": 50,
+              "x": 60,
-              "y": 10,
+              "y": 20,
             },
             "inserted": {
               "isDeleted": true,
@@ -10722,36 +10720,30 @@ History {
         "updated": Map {
           "id0" => Delta {
             "deleted": {
-              "index": "a3",
               "x": 20,
               "y": 20,
             },
             "inserted": {
-              "index": "a0",
               "x": 10,
               "y": 10,
             },
           },
           "id1" => Delta {
             "deleted": {
-              "index": "a4",
               "x": 40,
               "y": 20,
             },
             "inserted": {
-              "index": "a1",
               "x": 30,
               "y": 10,
             },
           },
           "id2" => Delta {
             "deleted": {
-              "index": "a5",
               "x": 60,
               "y": 20,
             },
             "inserted": {
-              "index": "a2",
               "x": 50,
               "y": 10,
             },