Fix various issues, fix tests

* Fix linear to elbow conversion

* Add invariant check

* Add dev invariant fix

* Add arrowhead

.env.development

@@ -1,3 +1,5 @@
+MODE="development"
+
 VITE_APP_BACKEND_V2_GET_URL=https://json-dev.excalidraw.com/api/v2/
 VITE_APP_BACKEND_V2_POST_URL=https://json-dev.excalidraw.com/api/v2/post/
 

.env.production

@@ -1,5 +1,7 @@
-VITE_APP_BACKEND_V2_GET_URL=https://ex.dylanbanta.com/api/v2/scenes/
+MODE="production"
-VITE_APP_BACKEND_V2_POST_URL=https://ex.dylanbanta.com/api/v2/scenes/
+
+VITE_APP_BACKEND_V2_GET_URL=https://json.excalidraw.com/api/v2/
+VITE_APP_BACKEND_V2_POST_URL=https://json.excalidraw.com/api/v2/post/
 
 VITE_APP_LIBRARY_URL=https://libraries.excalidraw.com
 VITE_APP_LIBRARY_BACKEND=https://us-central1-excalidraw-room-persistence.cloudfunctions.net/libraries

README.md

@@ -34,6 +34,9 @@
   <a href="https://discord.gg/UexuTaE">
     <img alt="Chat on Discord" src="https://img.shields.io/discord/723672430744174682?color=738ad6&label=Chat%20on%20Discord&logo=discord&logoColor=ffffff&widge=false"/>
   </a>
+  <a href="https://deepwiki.com/excalidraw/excalidraw">
+    <img alt="Ask DeepWiki" src="https://deepwiki.com/badge.svg" />
+  </a>
   <a href="https://twitter.com/excalidraw">
     <img alt="Follow Excalidraw on Twitter" src="https://img.shields.io/twitter/follow/excalidraw.svg?label=follow+@excalidraw&style=social&logo=twitter"/>
   </a>

docker-compose.yml

@@ -19,7 +19,7 @@ services:
       - ./:/opt/node_app/app:delegated
       - ./package.json:/opt/node_app/package.json
       - ./yarn.lock:/opt/node_app/yarn.lock
-#       - notused:/opt/node_app/app/node_modules
+      - notused:/opt/node_app/app/node_modules
 
-# volumes:
+volumes:
-#   notused:
+  notused:

excalidraw-app/App.tsx

@@ -926,21 +926,16 @@ const ExcalidrawWrapper = () => {
         <ShareDialog
           collabAPI={collabAPI}
           onExportToBackend={async () => {
-            if (!excalidrawAPI) {
+            if (excalidrawAPI) {
-              return;
+              try {
-            }
+                await onExportToBackend(
-            try {
+                  excalidrawAPI.getSceneElements(),
-              const { url, errorMessage } = await exportToBackend(
+                  excalidrawAPI.getAppState(),
-                excalidrawAPI.getSceneElements(),
+                  excalidrawAPI.getFiles(),
-                excalidrawAPI.getAppState(),
+                );
-                excalidrawAPI.getFiles(),
+              } catch (error: any) {
-              );
+                setErrorMessage(error.message);
-              if (errorMessage) {
-                throw new Error(errorMessage);
               }
-              setLatestShareableLink(url);
-            } catch (error: any) {
-              setErrorMessage(error.message);
             }
           }}
         />

excalidraw-app/package.json

@@ -41,8 +41,8 @@
   "prettier": "@excalidraw/prettier-config",
   "scripts": {
     "build-node": "node ./scripts/build-node.js",
-    "build:app:docker": "vite build",
+    "build:app:docker": "cross-env VITE_APP_DISABLE_SENTRY=true vite build",
-    "build:app": "vite build",
+    "build:app": "cross-env VITE_APP_GIT_SHA=$VERCEL_GIT_COMMIT_SHA cross-env VITE_APP_ENABLE_TRACKING=true vite build",
     "build:version": "node ../scripts/build-version.js",
     "build": "yarn build:app && yarn build:version",
     "start": "yarn && vite",

excalidraw-app/tests/collab.test.tsx

@@ -205,6 +205,7 @@ describe("collaboration", () => {
     // with explicit undo (as addition) we expect our item to be restored from the snapshot!
     await waitFor(() => {
       expect(API.getUndoStack().length).toBe(1);
+      expect(API.getRedoStack().length).toBe(1);
       expect(API.getSnapshot()).toEqual([
         expect.objectContaining(rect1Props),
         expect.objectContaining({ ...rect2Props, isDeleted: false }),
@@ -236,7 +237,7 @@ describe("collaboration", () => {
 
     // with explicit redo (as removal) we again restore the element from the snapshot!
     await waitFor(() => {
-      expect(API.getUndoStack().length).toBe(2);
+      expect(API.getUndoStack().length).toBe(1);
       expect(API.getRedoStack().length).toBe(0);
       expect(API.getSnapshot()).toEqual([
         expect.objectContaining(rect1Props),
@@ -248,13 +249,11 @@ describe("collaboration", () => {
       ]);
     });
 
-    act(() => h.app.actionManager.executeAction(undoAction));
-
     // simulate local update
     API.updateScene({
       elements: syncInvalidIndices([
         h.elements[0],
-        newElementWith(h.elements[1], { x: 100 }),
+        newElementWith(h.elements[1], { x: 100, isDeleted: false }),
       ]),
       captureUpdate: CaptureUpdateAction.IMMEDIATELY,
     });
@@ -271,55 +270,5 @@ describe("collaboration", () => {
         expect.objectContaining({ ...rect2Props, isDeleted: false, x: 100 }),
       ]);
     });
-
-    act(() => h.app.actionManager.executeAction(undoAction));
-
-    // we expect to iterate the stack to the first visible change
-    await waitFor(() => {
-      expect(API.getUndoStack().length).toBe(1);
-      expect(API.getRedoStack().length).toBe(1);
-      expect(API.getSnapshot()).toEqual([
-        expect.objectContaining(rect1Props),
-        expect.objectContaining({ ...rect2Props, isDeleted: false, x: 0 }),
-      ]);
-      expect(h.elements).toEqual([
-        expect.objectContaining(rect1Props),
-        expect.objectContaining({ ...rect2Props, isDeleted: false, x: 0 }),
-      ]);
-    });
-
-    // simulate force deleting the element remotely
-    API.updateScene({
-      elements: syncInvalidIndices([rect1]),
-      captureUpdate: CaptureUpdateAction.NEVER,
-    });
-
-    // snapshot was correctly updated and marked the element as deleted
-    await waitFor(() => {
-      expect(API.getUndoStack().length).toBe(1);
-      expect(API.getRedoStack().length).toBe(1);
-      expect(API.getSnapshot()).toEqual([
-        expect.objectContaining(rect1Props),
-        expect.objectContaining({ ...rect2Props, isDeleted: true, x: 0 }),
-      ]);
-      expect(h.elements).toEqual([expect.objectContaining(rect1Props)]);
-    });
-
-    act(() => h.app.actionManager.executeAction(redoAction));
-
-    // with explicit redo (as update) we again restored the element from the snapshot!
-    await waitFor(() => {
-      expect(API.getUndoStack().length).toBe(2);
-      expect(API.getRedoStack().length).toBe(0);
-      expect(API.getSnapshot()).toEqual([
-        expect.objectContaining({ id: "A", isDeleted: false }),
-        expect.objectContaining({ id: "B", isDeleted: true, x: 100 }),
-      ]);
-      expect(h.history.isRedoStackEmpty).toBeTruthy();
-      expect(h.elements).toEqual([
-        expect.objectContaining({ id: "A", isDeleted: false }),
-        expect.objectContaining({ id: "B", isDeleted: true, x: 100 }),
-      ]);
-    });
   });
 });

packages/common/src/utils.ts

@@ -712,8 +712,8 @@ export const arrayToObject = <T>(
   array: readonly T[],
   groupBy?: (value: T) => string | number,
 ) =>
-  array.reduce((acc, value) => {
+  array.reduce((acc, value, idx) => {
-    acc[groupBy ? groupBy(value) : String(value)] = value;
+    acc[groupBy ? groupBy(value) : idx] = value;
     return acc;
   }, {} as { [key: string]: T });
 

packages/element/src/delta.ts

@@ -9,7 +9,7 @@ import {
 
 import type {
   ExcalidrawElement,
-  ExcalidrawImageElement,
+  ExcalidrawFreeDrawElement,
   ExcalidrawLinearElement,
   ExcalidrawTextElement,
   NonDeleted,
@@ -18,7 +18,12 @@ import type {
   SceneElementsMap,
 } from "@excalidraw/element/types";
 
-import type { DTO, SubtypeOf, ValueOf } from "@excalidraw/common/utility-types";
+import type {
+  DTO,
+  Mutable,
+  SubtypeOf,
+  ValueOf,
+} from "@excalidraw/common/utility-types";
 
 import type {
   AppState,
@@ -51,6 +56,8 @@ import { orderByFractionalIndex, syncMovedIndices } from "./fractionalIndex";
 
 import { Scene } from "./Scene";
 
+import { StoreSnapshot } from "./store";
+
 import type { BindableProp, BindingProp } from "./binding";
 
 import type { ElementUpdate } from "./mutateElement";
@@ -256,12 +263,14 @@ export class Delta<T> {
           arrayToObject(deletedArray, groupBy),
           arrayToObject(insertedArray, groupBy),
         ),
+        (x) => x,
       );
       const insertedDifferences = arrayToObject(
         Delta.getRightDifferences(
           arrayToObject(deletedArray, groupBy),
           arrayToObject(insertedArray, groupBy),
         ),
+        (x) => x,
       );
 
       if (
@@ -858,10 +867,17 @@ export class AppStateDelta implements DeltaContainer<AppState> {
   }
 }
 
-type ElementPartial<T extends ExcalidrawElement = ExcalidrawElement> = Omit<
+type ElementPartial<TElement extends ExcalidrawElement = ExcalidrawElement> =
-  ElementUpdate<Ordered<T>>,
+  Omit<Partial<Ordered<TElement>>, "id" | "updated" | "seed">;
-  "seed"
+
->;
+export type ApplyToOptions = {
+  excludedProperties: Set<keyof ElementPartial>;
+};
+
+type ApplyToFlags = {
+  containsVisibleDifference: boolean;
+  containsZindexDifference: boolean;
+};
 
 /**
  * Elements change is a low level primitive to capture a change between two sets of elements.
@@ -1150,12 +1166,15 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
 
   public applyTo(
     elements: SceneElementsMap,
-    elementsSnapshot: Map<string, OrderedExcalidrawElement>,
+    snapshot: StoreSnapshot["elements"] = StoreSnapshot.empty().elements,
+    options: ApplyToOptions = {
+      excludedProperties: new Set(),
+    },
   ): [SceneElementsMap, boolean] {
     let nextElements = new Map(elements) as SceneElementsMap;
     let changedElements: Map<string, OrderedExcalidrawElement>;
 
-    const flags = {
+    const flags: ApplyToFlags = {
       containsVisibleDifference: false,
       containsZindexDifference: false,
     };
@@ -1164,13 +1183,14 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
     try {
       const applyDeltas = ElementsDelta.createApplier(
         nextElements,
-        elementsSnapshot,
+        snapshot,
+        options,
         flags,
       );
 
-      const addedElements = applyDeltas("added", this.added);
+      const addedElements = applyDeltas(this.added);
-      const removedElements = applyDeltas("removed", this.removed);
+      const removedElements = applyDeltas(this.removed);
-      const updatedElements = applyDeltas("updated", this.updated);
+      const updatedElements = applyDeltas(this.updated);
 
       const affectedElements = this.resolveConflicts(elements, nextElements);
 
@@ -1229,18 +1249,12 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
   private static createApplier =
     (
       nextElements: SceneElementsMap,
-      snapshot: Map<string, OrderedExcalidrawElement>,
+      snapshot: StoreSnapshot["elements"],
-      flags: {
+      options: ApplyToOptions,
-        containsVisibleDifference: boolean;
+      flags: ApplyToFlags,
-        containsZindexDifference: boolean;
-      },
     ) =>
-    (
+    (deltas: Record<string, Delta<ElementPartial>>) => {
-      type: "added" | "removed" | "updated",
-      deltas: Record<string, Delta<ElementPartial>>,
-    ) => {
       const getElement = ElementsDelta.createGetter(
-        type,
         nextElements,
         snapshot,
         flags,
@@ -1250,7 +1264,13 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
         const element = getElement(id, delta.inserted);
 
         if (element) {
-          const newElement = ElementsDelta.applyDelta(element, delta, flags);
+          const newElement = ElementsDelta.applyDelta(
+            element,
+            delta,
+            options,
+            flags,
+          );
+
           nextElements.set(newElement.id, newElement);
           acc.set(newElement.id, newElement);
         }
@@ -1261,13 +1281,9 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
 
   private static createGetter =
     (
-      type: "added" | "removed" | "updated",
       elements: SceneElementsMap,
-      snapshot: Map<string, OrderedExcalidrawElement>,
+      snapshot: StoreSnapshot["elements"],
-      flags: {
+      flags: ApplyToFlags,
-        containsVisibleDifference: boolean;
-        containsZindexDifference: boolean;
-      },
     ) =>
     (id: string, partial: ElementPartial) => {
       let element = elements.get(id);
@@ -1281,10 +1297,7 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
           flags.containsZindexDifference = true;
 
           // as the element was force deleted, we need to check if adding it back results in a visible change
-          if (
+          if (!partial.isDeleted || (partial.isDeleted && !element.isDeleted)) {
-            partial.isDeleted === false ||
-            (partial.isDeleted !== true && element.isDeleted === false)
-          ) {
             flags.containsVisibleDifference = true;
           }
         } else {
@@ -1304,16 +1317,28 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
   private static applyDelta(
     element: OrderedExcalidrawElement,
     delta: Delta<ElementPartial>,
-    flags: {
+    options: ApplyToOptions,
-      containsVisibleDifference: boolean;
+    flags: ApplyToFlags,
-      containsZindexDifference: boolean;
-    } = {
-      // by default we don't care about about the flags
-      containsVisibleDifference: true,
-      containsZindexDifference: true,
-    },
   ) {
-    const { boundElements, ...directlyApplicablePartial } = delta.inserted;
+    const directlyApplicablePartial: Mutable<ElementPartial> = {};
+
+    // some properties are not directly applicable, such as:
+    // - boundElements which contains only diff)
+    // - version & versionNonce, if we don't want to return to previous versions
+    for (const key of Object.keys(delta.inserted) as Array<
+      keyof typeof delta.inserted
+    >) {
+      if (key === "boundElements") {
+        continue;
+      }
+
+      if (options.excludedProperties.has(key)) {
+        continue;
+      }
+
+      const value = delta.inserted[key];
+      Reflect.set(directlyApplicablePartial, key, value);
+    }
 
     if (
       delta.deleted.boundElements?.length ||
@@ -1331,19 +1356,6 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
       });
     }
 
-    // TODO: this looks wrong, shouldn't be here
-    if (element.type === "image") {
-      const _delta = delta as Delta<ElementPartial<ExcalidrawImageElement>>;
-      // we want to override `crop` only if modified so that we don't reset
-      // when undoing/redoing unrelated change
-      if (_delta.deleted.crop || _delta.inserted.crop) {
-        Object.assign(directlyApplicablePartial, {
-          // apply change verbatim
-          crop: _delta.inserted.crop ?? null,
-        });
-      }
-    }
-
     if (!flags.containsVisibleDifference) {
       // strip away fractional index, as even if it would be different, it doesn't have to result in visible change
       const { index, ...rest } = directlyApplicablePartial;
@@ -1650,6 +1662,32 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
   ): [ElementPartial, ElementPartial] {
     try {
       Delta.diffArrays(deleted, inserted, "boundElements", (x) => x.id);
+
+      // don't diff the points as:
+      // - we can't ensure the multiplayer order consistency without fractional index on each point
+      // - we prefer to not merge the points, as it might just lead to unexpected / incosistent results
+      const deletedPoints =
+        (
+          deleted as ElementPartial<
+            ExcalidrawFreeDrawElement | ExcalidrawLinearElement
+          >
+        ).points ?? [];
+
+      const insertedPoints =
+        (
+          inserted as ElementPartial<
+            ExcalidrawFreeDrawElement | ExcalidrawLinearElement
+          >
+        ).points ?? [];
+
+      if (
+        !Delta.isLeftDifferent(deletedPoints, insertedPoints) &&
+        !Delta.isRightDifferent(deletedPoints, insertedPoints)
+      ) {
+        // delete the points from delta if there is no difference, otherwise leave them as they were captured due to consistency
+        Reflect.deleteProperty(deleted, "points");
+        Reflect.deleteProperty(inserted, "points");
+      }
     } catch (e) {
       // if postprocessing fails, it does not make sense to bubble up, but let's make sure we know about it
       console.error(`Couldn't postprocess elements delta.`);
@@ -1665,7 +1703,7 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
   private static stripIrrelevantProps(
     partial: Partial<OrderedExcalidrawElement>,
   ): ElementPartial {
-    const { id, updated, version, versionNonce, ...strippedPartial } = partial;
+    const { id, updated, ...strippedPartial } = partial;
 
     return strippedPartial;
   }

packages/element/src/elbowArrow.ts

@@ -974,6 +974,25 @@ export const updateElbowArrowPoints = (
         ),
       "Elbow arrow segments must be either horizontal or vertical",
     );
+
+    invariant(
+      updates.fixedSegments?.find(
+        (segment) =>
+          segment.index === 1 &&
+          pointsEqual(segment.start, (updates.points ?? arrow.points)[0]),
+      ) == null &&
+        updates.fixedSegments?.find(
+          (segment) =>
+            segment.index === (updates.points ?? arrow.points).length - 1 &&
+            pointsEqual(
+              segment.end,
+              (updates.points ?? arrow.points)[
+                (updates.points ?? arrow.points).length - 1
+              ],
+            ),
+        ) == null,
+      "The first and last segments cannot be fixed",
+    );
   }
 
   const fixedSegments = updates.fixedSegments ?? arrow.fixedSegments ?? [];

packages/element/src/fractionalIndex.ts

@@ -2,7 +2,9 @@ import { generateNKeysBetween } from "fractional-indexing";
 
 import { arrayToMap } from "@excalidraw/common";
 
-import { mutateElement } from "./mutateElement";
+import type { Mutable } from "@excalidraw/common/utility-types";
+
+import { mutateElement, newElementWith } from "./mutateElement";
 import { getBoundTextElement } from "./textElement";
 import { hasBoundTextElement } from "./typeChecks";
 
@@ -161,9 +163,15 @@ export const syncMovedIndices = (
 
     // try generatating indices, throws on invalid movedElements
     const elementsUpdates = generateIndices(elements, indicesGroups);
-    const elementsCandidates = elements.map((x) =>
+    const elementsCandidates = elements.map((x) => {
-      elementsUpdates.has(x) ? { ...x, ...elementsUpdates.get(x) } : x,
+      const elementUpdates = elementsUpdates.get(x);
-    );
+
+      if (elementUpdates) {
+        return { ...x, index: elementUpdates.index };
+      }
+
+      return x;
+    });
 
     // ensure next indices are valid before mutation, throws on invalid ones
     validateFractionalIndices(
@@ -177,8 +185,8 @@ export const syncMovedIndices = (
     );
 
     // split mutation so we don't end up in an incosistent state
-    for (const [element, update] of elementsUpdates) {
+    for (const [element, { index }] of elementsUpdates) {
-      mutateElement(element, elementsMap, update);
+      mutateElement(element, elementsMap, { index });
     }
   } catch (e) {
     // fallback to default sync
@@ -189,19 +197,33 @@ export const syncMovedIndices = (
 };
 
 /**
- * Synchronizes all invalid fractional indices with the array order by mutating passed elements.
+ * Synchronizes all invalid fractional indices with the array order by mutating passed elements array.
+ *
+ * When `shouldCreateNewInstances` is true, it creates new instances of the elements, instead of mutating the existing ones.
  *
  * WARN: in edge cases it could modify the elements which were not moved, as it's impossible to guess the actually moved elements from the elements array itself.
  */
 export const syncInvalidIndices = (
   elements: readonly ExcalidrawElement[],
+  {
+    shouldCreateNewInstances = false,
+  }: {
+    shouldCreateNewInstances?: boolean;
+  } = {},
 ): OrderedExcalidrawElement[] => {
-  const elementsMap = arrayToMap(elements);
   const indicesGroups = getInvalidIndicesGroups(elements);
   const elementsUpdates = generateIndices(elements, indicesGroups);
 
-  for (const [element, update] of elementsUpdates) {
+  for (const [element, { index, arrayIndex }] of elementsUpdates) {
-    mutateElement(element, elementsMap, update);
+    if (shouldCreateNewInstances) {
+      const updatedElement = newElementWith(element, { index });
+
+      // mutate the element in the array with the new updated instance
+      (elements as Mutable<typeof elements>)[arrayIndex] = updatedElement;
+    } else {
+      const elementsMap = arrayToMap(elements);
+      mutateElement(element, elementsMap, { index });
+    }
   }
 
   return elements as OrderedExcalidrawElement[];
@@ -380,7 +402,7 @@ const generateIndices = (
 ) => {
   const elementsUpdates = new Map<
     ExcalidrawElement,
-    { index: FractionalIndex }
+    { index: FractionalIndex; arrayIndex: number }
   >();
 
   for (const indices of indicesGroups) {
@@ -398,6 +420,7 @@ const generateIndices = (
 
       elementsUpdates.set(element, {
         index: fractionalIndices[i],
+        arrayIndex: indices[i],
       });
     }
   }

packages/element/src/store.ts

@@ -213,16 +213,7 @@ export class Store {
       // using the same instance, since in history we have a check against `HistoryEntry`, so that we don't re-record the same delta again
       storeDelta = delta;
     } else {
-      // calculate the deltas based on the previous and next snapshot
+      storeDelta = StoreDelta.calculate(prevSnapshot, snapshot);
-      const elementsDelta = snapshot.metadata.didElementsChange
-        ? ElementsDelta.calculate(prevSnapshot.elements, snapshot.elements)
-        : ElementsDelta.empty();
-
-      const appStateDelta = snapshot.metadata.didAppStateChange
-        ? AppStateDelta.calculate(prevSnapshot.appState, snapshot.appState)
-        : AppStateDelta.empty();
-
-      storeDelta = StoreDelta.create(elementsDelta, appStateDelta);
     }
 
     if (!storeDelta.isEmpty()) {
@@ -505,6 +496,24 @@ export class StoreDelta {
     return new this(opts.id, elements, appState);
   }
 
+  /**
+   * Calculate the delta between the previous and next snapshot.
+   */
+  public static calculate(
+    prevSnapshot: StoreSnapshot,
+    nextSnapshot: StoreSnapshot,
+  ) {
+    const elementsDelta = nextSnapshot.metadata.didElementsChange
+      ? ElementsDelta.calculate(prevSnapshot.elements, nextSnapshot.elements)
+      : ElementsDelta.empty();
+
+    const appStateDelta = nextSnapshot.metadata.didAppStateChange
+      ? AppStateDelta.calculate(prevSnapshot.appState, nextSnapshot.appState)
+      : AppStateDelta.empty();
+
+    return this.create(elementsDelta, appStateDelta);
+  }
+
   /**
    * Restore a store delta instance from a DTO.
    */
@@ -524,9 +533,7 @@ export class StoreDelta {
     id,
     elements: { added, removed, updated },
   }: DTO<StoreDelta>) {
-    const elements = ElementsDelta.create(added, removed, updated, {
+    const elements = ElementsDelta.create(added, removed, updated);
-      shouldRedistribute: false,
-    });
 
     return new this(id, elements, AppStateDelta.empty());
   }
@@ -534,27 +541,10 @@ export class StoreDelta {
   /**
    * Inverse store delta, creates new instance of `StoreDelta`.
    */
-  public static inverse(delta: StoreDelta): StoreDelta {
+  public static inverse(delta: StoreDelta) {
     return this.create(delta.elements.inverse(), delta.appState.inverse());
   }
 
-  /**
-   * Apply latest (remote) changes to the delta, creates new instance of `StoreDelta`.
-   */
-  public static applyLatestChanges(
-    delta: StoreDelta,
-    elements: SceneElementsMap,
-    modifierOptions: "deleted" | "inserted",
-  ): StoreDelta {
-    return this.create(
-      delta.elements.applyLatestChanges(elements, modifierOptions),
-      delta.appState,
-      {
-        id: delta.id,
-      },
-    );
-  }
-
   /**
    * Apply the delta to the passed elements and appState, does not modify the snapshot.
    */
@@ -562,12 +552,9 @@ export class StoreDelta {
     delta: StoreDelta,
     elements: SceneElementsMap,
     appState: AppState,
-    prevSnapshot: StoreSnapshot = StoreSnapshot.empty(),
   ): [SceneElementsMap, AppState, boolean] {
-    const [nextElements, elementsContainVisibleChange] = delta.elements.applyTo(
+    const [nextElements, elementsContainVisibleChange] =
-      elements,
+      delta.elements.applyTo(elements);
-      prevSnapshot.elements,
-    );
 
     const [nextAppState, appStateContainsVisibleChange] =
       delta.appState.applyTo(appState, nextElements);
@@ -687,11 +674,10 @@ export class StoreSnapshot {
       nextElements.set(id, changedElement);
     }
 
-    const nextAppState = Object.assign(
+    const nextAppState = getObservedAppState({
-      {},
+      ...this.appState,
-      this.appState,
+      ...change.appState,
-      change.appState,
+    });
-    ) as ObservedAppState;
 
     return StoreSnapshot.create(nextElements, nextAppState, {
       // by default we assume that change is different from what we have in the snapshot
@@ -944,18 +930,26 @@ const getDefaultObservedAppState = (): ObservedAppState => {
   };
 };
 
-export const getObservedAppState = (appState: AppState): ObservedAppState => {
+export const getObservedAppState = (
+  appState: AppState | ObservedAppState,
+): ObservedAppState => {
   const observedAppState = {
     name: appState.name,
     editingGroupId: appState.editingGroupId,
     viewBackgroundColor: appState.viewBackgroundColor,
     selectedElementIds: appState.selectedElementIds,
     selectedGroupIds: appState.selectedGroupIds,
-    editingLinearElementId: appState.editingLinearElement?.elementId || null,
-    selectedLinearElementId: appState.selectedLinearElement?.elementId || null,
     croppingElementId: appState.croppingElementId,
     activeLockedId: appState.activeLockedId,
     lockedMultiSelections: appState.lockedMultiSelections,
+    editingLinearElementId:
+      (appState as AppState).editingLinearElement?.elementId ?? // prefer app state, as it's likely newer
+      (appState as ObservedAppState).editingLinearElementId ?? // fallback to observed app state, as it's likely older coming from a previous snapshot
+      null,
+    selectedLinearElementId:
+      (appState as AppState).selectedLinearElement?.elementId ??
+      (appState as ObservedAppState).selectedLinearElementId ??
+      null,
   };
 
   Reflect.defineProperty(observedAppState, hiddenObservedAppStateProp, {

packages/element/tests/duplicate.test.tsx

@@ -505,8 +505,6 @@ describe("group-related duplication", () => {
       mouse.up(frame.x + frame.width + 50, frame.y + frame.height + 50);
     });
 
-    // console.log(h.elements);
-
     assertElements(h.elements, [
       { id: frame.id },
       { id: rectangle1.id, frameId: frame.id },

packages/excalidraw/actions/actionProperties.tsx

@@ -1483,13 +1483,13 @@ const getArrowheadOptions = (flip: boolean) => {
       value: "crowfoot_one",
       text: t("labels.arrowhead_crowfoot_one"),
       icon: <ArrowheadCrowfootOneIcon flip={flip} />,
-      keyBinding: "c",
+      keyBinding: "x",
     },
     {
       value: "crowfoot_many",
       text: t("labels.arrowhead_crowfoot_many"),
       icon: <ArrowheadCrowfootIcon flip={flip} />,
-      keyBinding: "x",
+      keyBinding: "c",
     },
     {
       value: "crowfoot_one_or_many",

packages/excalidraw/components/App.tsx

@@ -104,7 +104,11 @@ import {
   Emitter,
 } from "@excalidraw/common";
 
-import { getCommonBounds, getElementAbsoluteCoords } from "@excalidraw/element";
+import {
+  getCommonBounds,
+  getElementAbsoluteCoords,
+  getObservedAppState,
+} from "@excalidraw/element";
 
 import {
   bindOrUnbindLinearElement,
@@ -3993,22 +3997,30 @@ class App extends React.Component<AppProps, AppState> {
     }) => {
       const { elements, appState, collaborators, captureUpdate } = sceneData;
 
-      const nextElements = elements ? syncInvalidIndices(elements) : undefined;
+      const nextElements = elements
+        ? syncInvalidIndices(elements, {
+            // we have to create new instances here, otherwise scheduled micro action below won't be able to
+            // detect the fractional index change and won't update the store snapshot
+            shouldCreateNewInstances: true,
+          })
+        : undefined;
 
       if (captureUpdate) {
         const nextElementsMap = elements
           ? (arrayToMap(nextElements ?? []) as SceneElementsMap)
           : undefined;
 
-        const nextAppState = appState
+        const nextObservedAppState = appState
-          ? // new instance, with partial appstate applied to previously captured one, including hidden prop inside `prevCommittedAppState`
+          ? getObservedAppState({
-            Object.assign({}, this.store.snapshot.appState, appState)
+              ...this.store.snapshot.appState,
+              ...appState,
+            })
           : undefined;
 
         this.store.scheduleMicroAction({
           action: captureUpdate,
           elements: nextElementsMap,
-          appState: nextAppState,
+          appState: nextObservedAppState,
         });
       }
 

packages/excalidraw/components/ConvertElementTypePopup.tsx

@@ -564,7 +564,7 @@ export const convertElementTypes = (
               continue;
             }
             const fixedSegments: FixedSegment[] = [];
-            for (let i = 0; i < nextPoints.length - 1; i++) {
+            for (let i = 1; i < nextPoints.length - 2; i++) {
               fixedSegments.push({
                 start: nextPoints[i],
                 end: nextPoints[i + 1],
@@ -581,6 +581,7 @@ export const convertElementTypes = (
             );
             mutateElement(element, app.scene.getNonDeletedElementsMap(), {
               ...updates,
+              endArrowhead: "arrow",
             });
           } else {
             // if we're converting to non-elbow linear element, check if

packages/excalidraw/history.ts

@@ -7,11 +7,62 @@ import {
   type Store,
 } from "@excalidraw/element";
 
+import type { StoreSnapshot } from "@excalidraw/element";
+
 import type { SceneElementsMap } from "@excalidraw/element/types";
 
 import type { AppState } from "./types";
 
-class HistoryEntry extends StoreDelta {}
+export class HistoryDelta extends StoreDelta {
+  /**
+   * Apply the delta to the passed elements and appState, does not modify the snapshot.
+   */
+  public applyTo(
+    elements: SceneElementsMap,
+    appState: AppState,
+    snapshot: StoreSnapshot,
+  ): [SceneElementsMap, AppState, boolean] {
+    const [nextElements, elementsContainVisibleChange] = this.elements.applyTo(
+      elements,
+      // used to fallback into local snapshot in case we couldn't apply the delta
+      // due to a missing elements in the scene (force deleted)
+      snapshot.elements,
+      // we don't want to apply the `version` and `versionNonce` properties for history
+      // as we always need to end up with a new version due to collaboration,
+      // approaching each undo / redo as a new user operation
+      {
+        excludedProperties: new Set(["version", "versionNonce"]),
+      },
+    );
+
+    const [nextAppState, appStateContainsVisibleChange] = this.appState.applyTo(
+      appState,
+      nextElements,
+    );
+
+    const appliedVisibleChanges =
+      elementsContainVisibleChange || appStateContainsVisibleChange;
+
+    return [nextElements, nextAppState, appliedVisibleChanges];
+  }
+
+  /**
+   * Overriding once to avoid type casting everywhere.
+   */
+  public static override calculate(
+    prevSnapshot: StoreSnapshot,
+    nextSnapshot: StoreSnapshot,
+  ) {
+    return super.calculate(prevSnapshot, nextSnapshot) as HistoryDelta;
+  }
+
+  /**
+   * Overriding once to avoid type casting everywhere.
+   */
+  public static override inverse(delta: StoreDelta): HistoryDelta {
+    return super.inverse(delta) as HistoryDelta;
+  }
+}
 
 export class HistoryChangedEvent {
   constructor(
@@ -25,8 +76,8 @@ export class History {
     [HistoryChangedEvent]
   >();
 
-  public readonly undoStack: HistoryEntry[] = [];
+  public readonly undoStack: HistoryDelta[] = [];
-  public readonly redoStack: HistoryEntry[] = [];
+  public readonly redoStack: HistoryDelta[] = [];
 
   public get isUndoStackEmpty() {
     return this.undoStack.length === 0;
@@ -48,16 +99,16 @@ export class History {
    * Do not re-record history entries, which were already pushed to undo / redo stack, as part of history action.
    */
   public record(delta: StoreDelta) {
-    if (delta.isEmpty() || delta instanceof HistoryEntry) {
+    if (delta.isEmpty() || delta instanceof HistoryDelta) {
       return;
     }
 
     // construct history entry, so once it's emitted, it's not recorded again
-    const entry = HistoryEntry.inverse(delta);
+    const historyDelta = HistoryDelta.inverse(delta);
 
-    this.undoStack.push(entry);
+    this.undoStack.push(historyDelta);
 
-    if (!entry.elements.isEmpty()) {
+    if (!historyDelta.elements.isEmpty()) {
       // don't reset redo stack on local appState changes,
       // as a simple click (unselect) could lead to losing all the redo entries
       // only reset on non empty elements changes!
@@ -74,7 +125,7 @@ export class History {
       elements,
       appState,
       () => History.pop(this.undoStack),
-      (entry: HistoryEntry) => History.push(this.redoStack, entry, elements),
+      (entry: HistoryDelta) => History.push(this.redoStack, entry),
     );
   }
 
@@ -83,20 +134,20 @@ export class History {
       elements,
       appState,
       () => History.pop(this.redoStack),
-      (entry: HistoryEntry) => History.push(this.undoStack, entry, elements),
+      (entry: HistoryDelta) => History.push(this.undoStack, entry),
     );
   }
 
   private perform(
     elements: SceneElementsMap,
     appState: AppState,
-    pop: () => HistoryEntry | null,
+    pop: () => HistoryDelta | null,
-    push: (entry: HistoryEntry) => void,
+    push: (entry: HistoryDelta) => void,
   ): [SceneElementsMap, AppState] | void {
     try {
-      let historyEntry = pop();
+      let historyDelta = pop();
 
-      if (historyEntry === null) {
+      if (historyDelta === null) {
         return;
       }
 
@@ -109,15 +160,10 @@ export class History {
       let containsVisibleChange = false;
 
       // iterate through the history entries in case they result in no visible changes
-      while (historyEntry) {
+      while (historyDelta) {
         try {
           [nextElements, nextAppState, containsVisibleChange] =
-            StoreDelta.applyTo(
+            historyDelta.applyTo(nextElements, nextAppState, prevSnapshot);
-              historyEntry,
-              nextElements,
-              nextAppState,
-              prevSnapshot,
-            );
 
           const nextSnapshot = prevSnapshot.maybeClone(
             action,
@@ -125,24 +171,31 @@ export class History {
             nextAppState,
           );
 
+          const change = StoreChange.create(prevSnapshot, nextSnapshot);
+
+          // update the history entry, so that it's the a new history entry instance
+          // and so that it contains the latest changes in both inserted and deleted partials
+          // including version and versionNonce or properties which were in the meantime updated by a remote client
+          historyDelta = HistoryDelta.calculate(prevSnapshot, nextSnapshot);
+
           // schedule immediate capture, so that it's emitted for the sync purposes
           this.store.scheduleMicroAction({
             action,
-            change: StoreChange.create(prevSnapshot, nextSnapshot),
+            change,
-            delta: historyEntry,
+            delta: historyDelta,
           });
 
           prevSnapshot = nextSnapshot;
         } finally {
           // make sure to always push, even if the delta is corrupted
-          push(historyEntry);
+          push(historyDelta);
         }
 
         if (containsVisibleChange) {
           break;
         }
 
-        historyEntry = pop();
+        historyDelta = pop();
       }
 
       return [nextElements, nextAppState];
@@ -155,7 +208,7 @@ export class History {
     }
   }
 
-  private static pop(stack: HistoryEntry[]): HistoryEntry | null {
+  private static pop(stack: HistoryDelta[]): HistoryDelta | null {
     if (!stack.length) {
       return null;
     }
@@ -169,18 +222,12 @@ export class History {
     return null;
   }
 
-  private static push(
+  private static push(stack: HistoryDelta[], entry: HistoryDelta) {
-    stack: HistoryEntry[],
+    if (entry.isEmpty()) {
-    entry: HistoryEntry,
+      return;
-    prevElements: SceneElementsMap,
+    }
-  ) {
-    const inversedEntry = HistoryEntry.inverse(entry);
-    const updatedEntry = HistoryEntry.applyLatestChanges(
-      inversedEntry,
-      prevElements,
-      "inserted",
-    );
 
-    return stack.push(updatedEntry);
+    const inversedEntry = HistoryDelta.inverse(entry);
+    return stack.push(inversedEntry);
   }
 }

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

@@ -1277,6 +1277,7 @@ exports[`contextMenu element > selecting 'Add to library' in context menu adds e
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,
@@ -1525,6 +1526,7 @@ exports[`contextMenu element > selecting 'Bring forward' in context menu brings
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,
@@ -1579,6 +1581,7 @@ exports[`contextMenu element > selecting 'Bring forward' in context menu brings
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": 20,
             "y": 30,
@@ -1614,9 +1617,11 @@ exports[`contextMenu element > selecting 'Bring forward' in context menu brings
         "id0": {
           "deleted": {
             "index": "a2",
+            "version": 4,
           },
           "inserted": {
             "index": "a0",
+            "version": 3,
           },
         },
       },
@@ -1858,6 +1863,7 @@ exports[`contextMenu element > selecting 'Bring to front' in context menu brings
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,
@@ -1912,6 +1918,7 @@ exports[`contextMenu element > selecting 'Bring to front' in context menu brings
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": 20,
             "y": 30,
@@ -1947,9 +1954,11 @@ exports[`contextMenu element > selecting 'Bring to front' in context menu brings
         "id0": {
           "deleted": {
             "index": "a2",
+            "version": 4,
           },
           "inserted": {
             "index": "a0",
+            "version": 3,
           },
         },
       },
@@ -2159,6 +2168,7 @@ exports[`contextMenu element > selecting 'Copy styles' in context menu copies st
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,
@@ -2371,6 +2381,7 @@ exports[`contextMenu element > selecting 'Delete' in context menu deletes elemen
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,
@@ -2402,9 +2413,11 @@ exports[`contextMenu element > selecting 'Delete' in context menu deletes elemen
         "id0": {
           "deleted": {
             "isDeleted": true,
+            "version": 4,
           },
           "inserted": {
             "isDeleted": false,
+            "version": 3,
           },
         },
       },
@@ -2648,6 +2661,7 @@ exports[`contextMenu element > selecting 'Duplicate' in context menu duplicates
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,
@@ -2702,6 +2716,7 @@ exports[`contextMenu element > selecting 'Duplicate' in context menu duplicates
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 5,
             "width": 20,
             "x": 0,
             "y": 10,
@@ -2959,6 +2974,7 @@ exports[`contextMenu element > selecting 'Group selection' in context menu group
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,
@@ -3013,6 +3029,7 @@ exports[`contextMenu element > selecting 'Group selection' in context menu group
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": 20,
             "y": 30,
@@ -3068,9 +3085,11 @@ exports[`contextMenu element > selecting 'Group selection' in context menu group
             "groupIds": [
               "id9",
             ],
+            "version": 4,
           },
           "inserted": {
             "groupIds": [],
+            "version": 3,
           },
         },
         "id3": {
@@ -3078,9 +3097,11 @@ exports[`contextMenu element > selecting 'Group selection' in context menu group
             "groupIds": [
               "id9",
             ],
+            "version": 4,
           },
           "inserted": {
             "groupIds": [],
+            "version": 3,
           },
         },
       },
@@ -3324,6 +3345,7 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,
@@ -3378,6 +3400,7 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": 20,
             "y": 30,
@@ -3405,9 +3428,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
         "id3": {
           "deleted": {
             "strokeColor": "#e03131",
+            "version": 4,
           },
           "inserted": {
             "strokeColor": "#1e1e1e",
+            "version": 3,
           },
         },
       },
@@ -3428,9 +3453,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
         "id3": {
           "deleted": {
             "backgroundColor": "#a5d8ff",
+            "version": 5,
           },
           "inserted": {
             "backgroundColor": "transparent",
+            "version": 4,
           },
         },
       },
@@ -3451,9 +3478,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
         "id3": {
           "deleted": {
             "fillStyle": "cross-hatch",
+            "version": 6,
           },
           "inserted": {
             "fillStyle": "solid",
+            "version": 5,
           },
         },
       },
@@ -3474,9 +3503,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
         "id3": {
           "deleted": {
             "strokeStyle": "dotted",
+            "version": 7,
           },
           "inserted": {
             "strokeStyle": "solid",
+            "version": 6,
           },
         },
       },
@@ -3497,9 +3528,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
         "id3": {
           "deleted": {
             "roughness": 2,
+            "version": 8,
           },
           "inserted": {
             "roughness": 1,
+            "version": 7,
           },
         },
       },
@@ -3520,9 +3553,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
         "id3": {
           "deleted": {
             "opacity": 60,
+            "version": 9,
           },
           "inserted": {
             "opacity": 100,
+            "version": 8,
           },
         },
       },
@@ -3556,6 +3591,7 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
             "roughness": 2,
             "strokeColor": "#e03131",
             "strokeStyle": "dotted",
+            "version": 4,
           },
           "inserted": {
             "backgroundColor": "transparent",
@@ -3564,6 +3600,7 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
             "roughness": 1,
             "strokeColor": "#1e1e1e",
             "strokeStyle": "solid",
+            "version": 3,
           },
         },
       },
@@ -3805,6 +3842,7 @@ exports[`contextMenu element > selecting 'Send backward' in context menu sends e
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,
@@ -3859,6 +3897,7 @@ exports[`contextMenu element > selecting 'Send backward' in context menu sends e
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": 20,
             "y": 30,
@@ -3886,9 +3925,11 @@ exports[`contextMenu element > selecting 'Send backward' in context menu sends e
         "id3": {
           "deleted": {
             "index": "Zz",
+            "version": 4,
           },
           "inserted": {
             "index": "a1",
+            "version": 3,
           },
         },
       },
@@ -4130,6 +4171,7 @@ exports[`contextMenu element > selecting 'Send to back' in context menu sends el
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,
@@ -4184,6 +4226,7 @@ exports[`contextMenu element > selecting 'Send to back' in context menu sends el
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": 20,
             "y": 30,
@@ -4211,9 +4254,11 @@ exports[`contextMenu element > selecting 'Send to back' in context menu sends el
         "id3": {
           "deleted": {
             "index": "Zz",
+            "version": 4,
           },
           "inserted": {
             "index": "a1",
+            "version": 3,
           },
         },
       },
@@ -4458,6 +4503,7 @@ exports[`contextMenu element > selecting 'Ungroup selection' in context menu ung
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,
@@ -4512,6 +4558,7 @@ exports[`contextMenu element > selecting 'Ungroup selection' in context menu ung
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": 20,
             "y": 30,
@@ -4567,9 +4614,11 @@ exports[`contextMenu element > selecting 'Ungroup selection' in context menu ung
             "groupIds": [
               "id9",
             ],
+            "version": 4,
           },
           "inserted": {
             "groupIds": [],
+            "version": 3,
           },
         },
         "id3": {
@@ -4577,9 +4626,11 @@ exports[`contextMenu element > selecting 'Ungroup selection' in context menu ung
             "groupIds": [
               "id9",
             ],
+            "version": 4,
           },
           "inserted": {
             "groupIds": [],
+            "version": 3,
           },
         },
       },
@@ -4606,21 +4657,25 @@ exports[`contextMenu element > selecting 'Ungroup selection' in context menu ung
         "id0": {
           "deleted": {
             "groupIds": [],
+            "version": 5,
           },
           "inserted": {
             "groupIds": [
               "id9",
             ],
+            "version": 4,
           },
         },
         "id3": {
           "deleted": {
             "groupIds": [],
+            "version": 5,
           },
           "inserted": {
             "groupIds": [
               "id9",
             ],
+            "version": 4,
           },
         },
       },
@@ -5739,6 +5794,7 @@ exports[`contextMenu element > shows 'Group selection' in context menu for multi
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 10,
             "x": -10,
             "y": 0,
@@ -5793,6 +5849,7 @@ exports[`contextMenu element > shows 'Group selection' in context menu for multi
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 10,
             "x": 12,
             "y": 0,
@@ -6966,6 +7023,7 @@ exports[`contextMenu element > shows 'Ungroup selection' in context menu for gro
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 10,
             "x": -10,
             "y": 0,
@@ -7020,6 +7078,7 @@ exports[`contextMenu element > shows 'Ungroup selection' in context menu for gro
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 10,
             "x": 12,
             "y": 0,
@@ -7097,9 +7156,11 @@ exports[`contextMenu element > shows 'Ungroup selection' in context menu for gro
             "groupIds": [
               "id12",
             ],
+            "version": 4,
           },
           "inserted": {
             "groupIds": [],
+            "version": 3,
           },
         },
         "id3": {
@@ -7107,9 +7168,11 @@ exports[`contextMenu element > shows 'Ungroup selection' in context menu for gro
             "groupIds": [
               "id12",
             ],
+            "version": 4,
           },
           "inserted": {
             "groupIds": [],
+            "version": 3,
           },
         },
       },
@@ -9926,6 +9989,7 @@ exports[`contextMenu element > shows context menu for element > [end of test] un
             "strokeStyle": "solid",
             "strokeWidth": 2,
             "type": "rectangle",
+            "version": 3,
             "width": 20,
             "x": -10,
             "y": 0,

packages/excalidraw/tests/history.test.tsx

@@ -25,7 +25,7 @@ import "@excalidraw/utils/test-utils";
 
 import { ElementsDelta, AppStateDelta } from "@excalidraw/element";
 
-import { CaptureUpdateAction, StoreDelta } from "@excalidraw/element";
+import { CaptureUpdateAction } from "@excalidraw/element";
 
 import type { LocalPoint, Radians } from "@excalidraw/math";
 
@@ -34,6 +34,7 @@ import type {
   ExcalidrawFrameElement,
   ExcalidrawGenericElement,
   ExcalidrawLinearElement,
+  ExcalidrawRectangleElement,
   ExcalidrawTextElement,
   FixedPointBinding,
   FractionalIndex,
@@ -53,6 +54,8 @@ import { getDefaultAppState } from "../appState";
 import { Excalidraw } from "../index";
 import * as StaticScene from "../renderer/staticScene";
 
+import { HistoryDelta } from "../history";
+
 import { API } from "./helpers/api";
 import { Keyboard, Pointer, UI } from "./helpers/ui";
 import {
@@ -120,9 +123,19 @@ describe("history", () => {
 
       API.setElements([rect]);
 
-      const corrupedEntry = StoreDelta.create(
+      const corrupedEntry = HistoryDelta.create(
         ElementsDelta.empty(),
-        AppStateDelta.empty(),
+        // delta can't be empty, otherwise it won't be pushed into the undo stack
+        AppStateDelta.restore({
+          delta: {
+            inserted: {
+              selectedElementIds: {},
+            },
+            deleted: {
+              selectedElementIds: { [rect.id]: true },
+            },
+          },
+        }),
       );
 
       vi.spyOn(corrupedEntry.elements, "applyTo").mockImplementation(() => {
@@ -389,7 +402,7 @@ describe("history", () => {
       ]);
     });
 
-    it("should iterate through the history when selection changes do not produce visible change", async () => {
+    it("should not push into redo stack when selection changes dooes not produce a visible change", async () => {
       await render(<Excalidraw handleKeyboardGlobally={true} />);
 
       const rect = UI.createElement("rectangle", { x: 10 });
@@ -420,18 +433,18 @@ describe("history", () => {
       expect(API.getSelectedElements().length).toBe(0);
 
       Keyboard.redo(); // acceptable empty redo
-      expect(API.getUndoStack().length).toBe(3);
+      expect(API.getUndoStack().length).toBe(2); // empty change, nothing goes into undo stack
       expect(API.getRedoStack().length).toBe(0);
       expect(API.getSelectedElements().length).toBe(0);
 
       Keyboard.undo();
-      expect(API.getUndoStack().length).toBe(2);
+      expect(API.getUndoStack().length).toBe(1);
       expect(API.getRedoStack().length).toBe(1);
       assertSelectedElements(rect);
 
       Keyboard.undo();
-      expect(API.getUndoStack().length).toBe(0); // now we iterated through the same undos!
+      expect(API.getUndoStack().length).toBe(0);
-      expect(API.getRedoStack().length).toBe(3);
+      expect(API.getRedoStack().length).toBe(2);
       expect(API.getSelectedElements().length).toBe(0);
       expect(h.elements).toEqual([
         expect.objectContaining({ id: rect.id, isDeleted: true }),
@@ -1313,6 +1326,10 @@ describe("history", () => {
           captureUpdate: CaptureUpdateAction.IMMEDIATELY,
         });
 
+        rect1 = h.elements[0] as ExcalidrawRectangleElement;
+        text = h.elements[1] as ExcalidrawTextElement;
+        rect2 = h.elements[2] as ExcalidrawRectangleElement;
+
         // bind text1 to rect1
         mouse.select([rect1, text]);
         fireEvent.contextMenu(GlobalTestState.interactiveCanvas);
@@ -2362,10 +2379,10 @@ describe("history", () => {
         }),
       ]);
 
-      // We reached the bottom, again we iterate through invisible changes and reach the top
       Keyboard.redo();
       assertSelectedElements();
-      expect(API.getUndoStack().length).toBe(2);
+      // We reached the bottom, now there is only one non-empty history delta, which will be pushed to the undo stack
+      expect(API.getUndoStack().length).toBe(1);
       expect(API.getRedoStack().length).toBe(0);
       expect(h.elements).toEqual([
         expect.objectContaining({
@@ -2430,7 +2447,31 @@ describe("history", () => {
       ]);
 
       Keyboard.redo();
-      expect(API.getUndoStack().length).toBe(5);
+      expect(API.getUndoStack().length).toBe(2);
+      expect(API.getRedoStack().length).toBe(3);
+      // visible change detected, so we don't iterate up again
+      expect(API.getSelectedElements()).toEqual([]);
+      expect(h.elements).toEqual([
+        expect.objectContaining({
+          id: rect1.id,
+          isDeleted: false,
+        }),
+        expect.objectContaining({
+          id: rect2.id,
+          isDeleted: true,
+          backgroundColor: transparent,
+        }),
+        expect.objectContaining({
+          id: rect3.id,
+          isDeleted: true,
+          x: 30,
+          y: 30,
+        }),
+      ]);
+
+      Keyboard.redo();
+      // we iterate all the way up, as there are no visible changes
+      expect(API.getUndoStack().length).toBe(4);
       expect(API.getRedoStack().length).toBe(0);
       expect(API.getSelectedElements()).toEqual([]);
       expect(h.elements).toEqual([
@@ -2489,8 +2530,8 @@ describe("history", () => {
       ]);
 
       Keyboard.redo();
-      expect(API.getUndoStack().length).toBe(3);
+      expect(API.getUndoStack().length).toBe(2);
-      expect(API.getRedoStack().length).toBe(0);
+      expect(API.getRedoStack().length).toBe(1);
       // do not expect any selectedElementIds, as all relate to deleted elements
       expect(API.getSelectedElements()).toEqual([]);
       expect(h.elements).toEqual([
@@ -2506,7 +2547,6 @@ describe("history", () => {
         expect.objectContaining({ id: rect1.id, isDeleted: false }),
       ]);
 
-      // Simulate remote update
       API.updateScene({
         elements: [
           h.elements[0],
@@ -2523,14 +2563,13 @@ describe("history", () => {
       Keyboard.redo();
       expect(API.getUndoStack().length).toBe(2);
       expect(API.getRedoStack().length).toBe(1);
-      expect(API.getSelectedElements()).toEqual([
+      // redo entry was calculated again with the latest undo, which goes back to nothing being selected
-        expect.objectContaining({ id: rect2.id, isDeleted: false }),
+      expect(API.getSelectedElements()).toEqual([]);
-      ]);
 
       Keyboard.redo();
       expect(API.getUndoStack().length).toBe(3);
       expect(API.getRedoStack().length).toBe(0);
-      // now we again expect these as selected, as they got restored remotely
+      // now we again expect these as selected, as they got restored remotely and this redo entry was calculated in the beginning
       expect(API.getSelectedElements()).toEqual([
         expect.objectContaining({ id: rect2.id }),
         expect.objectContaining({ id: rect3.id }),
@@ -2599,16 +2638,14 @@ describe("history", () => {
 
       Keyboard.undo();
       expect(API.getUndoStack().length).toBe(0);
-      expect(API.getRedoStack().length).toBe(2); // iterated two steps back!
+      expect(API.getRedoStack().length).toBe(1); // iterated two steps back and reduce two empty entries into one!
       expect(h.state.selectedGroupIds).toEqual({});
 
       Keyboard.redo();
-      expect(API.getUndoStack().length).toBe(2); // iterated two steps forward!
+      expect(API.getUndoStack().length).toBe(0); // no changes applied, so there is no new entry
       expect(API.getRedoStack().length).toBe(0);
       expect(h.state.selectedGroupIds).toEqual({});
 
-      Keyboard.undo();
-
       // Simulate remote update
       API.updateScene({
         elements: [
@@ -2621,22 +2658,6 @@ describe("history", () => {
         ],
         captureUpdate: CaptureUpdateAction.NEVER,
       });
-
-      Keyboard.redo();
-      expect(API.getUndoStack().length).toBe(1);
-      expect(API.getRedoStack().length).toBe(1);
-      expect(h.state.selectedGroupIds).toEqual({ A: true });
-
-      // Simulate remote update
-      API.updateScene({
-        elements: [h.elements[0], h.elements[1], rect3, rect4],
-        captureUpdate: CaptureUpdateAction.NEVER,
-      });
-
-      Keyboard.redo();
-      expect(API.getUndoStack().length).toBe(2);
-      expect(API.getRedoStack().length).toBe(0);
-      expect(h.state.selectedGroupIds).toEqual({ A: true, B: true });
     });
 
     it("should iterate through the history when editing group contains only remotely deleted elements", async () => {
@@ -2681,33 +2702,7 @@ describe("history", () => {
 
       Keyboard.undo();
       expect(API.getUndoStack().length).toBe(0);
-      expect(API.getRedoStack().length).toBe(3);
+      expect(API.getRedoStack().length).toBe(0); // all changes relate to remotely deleted elements, there is no change and thus nothing to redo
-      expect(h.state.editingGroupId).toBeNull();
-
-      Keyboard.redo();
-      expect(API.getUndoStack().length).toBe(3);
-      expect(API.getRedoStack().length).toBe(0);
-      expect(h.state.editingGroupId).toBeNull();
-
-      // Simulate remote update
-      API.updateScene({
-        elements: [
-          newElementWith(h.elements[0], {
-            isDeleted: false,
-          }),
-          h.elements[1],
-        ],
-        captureUpdate: CaptureUpdateAction.NEVER,
-      });
-
-      Keyboard.undo();
-      expect(API.getUndoStack().length).toBe(2);
-      expect(API.getRedoStack().length).toBe(1);
-      expect(h.state.editingGroupId).toBe("A");
-
-      Keyboard.redo();
-      expect(API.getUndoStack().length).toBe(3);
-      expect(API.getRedoStack().length).toBe(0);
       expect(h.state.editingGroupId).toBeNull();
     });
 
@@ -2744,13 +2739,13 @@ describe("history", () => {
       });
 
       Keyboard.undo();
-      expect(API.getUndoStack().length).toBe(1);
+      expect(API.getUndoStack().length).toBe(1); // iterated few entries back
-      expect(API.getRedoStack().length).toBe(3);
+      expect(API.getRedoStack().length).toBe(1); // added just one non-empty entry into redo stack
       expect(h.state.editingLinearElement).toBeNull();
       expect(h.state.selectedLinearElement).toBeNull();
 
       Keyboard.redo();
-      expect(API.getUndoStack().length).toBe(4);
+      expect(API.getUndoStack().length).toBe(1);
       expect(API.getRedoStack().length).toBe(0);
       expect(h.state.editingLinearElement).toBeNull();
       expect(h.state.selectedLinearElement).toBeNull();
@@ -2838,7 +2833,7 @@ describe("history", () => {
       // We iterated two steps as there was no change in order!
       expect(API.getUndoStack().length).toBe(0);
       expect(API.getRedoStack().length).toBe(2);
-      expect(API.getSelectedElements().length).toBe(0);
+      assertSelectedElements([]);
       expect(h.elements).toEqual([
         expect.objectContaining({ id: rect1.id }), // a "Zx"
         expect.objectContaining({ id: rect3.id }), // c "Zy"
@@ -2846,7 +2841,7 @@ describe("history", () => {
       ]);
     });
 
-    it("should iterate through the history when z-index changes do not produce visible change and we synced all indices", async () => {
+    it("should tolerate remote z-index changes with incorrect fractional indices", async () => {
       const rect1 = API.createElement({ type: "rectangle", x: 10, y: 10 });
       const rect2 = API.createElement({ type: "rectangle", x: 20, y: 20 });
       const rect3 = API.createElement({ type: "rectangle", x: 30, y: 30 });
@@ -2899,22 +2894,32 @@ describe("history", () => {
       // Simulate remote update
       API.updateScene({
         elements: [
-          h.elements[1], // rect2
           h.elements[0], // rect3
           h.elements[2], // rect1
+          h.elements[1], // rect2
         ],
         captureUpdate: CaptureUpdateAction.NEVER,
       });
 
       Keyboard.undo();
-      expect(API.getUndoStack().length).toBe(0);
+      expect(API.getUndoStack().length).toBe(1);
-      expect(API.getRedoStack().length).toBe(2); // now we iterated two steps back!
+      expect(API.getRedoStack().length).toBe(1);
-      assertSelectedElements([]);
+      assertSelectedElements([rect2]);
       expect(h.elements).toEqual([
         expect.objectContaining({ id: rect2.id }),
         expect.objectContaining({ id: rect3.id }),
         expect.objectContaining({ id: rect1.id }),
       ]);
+
+      Keyboard.redo();
+      expect(API.getUndoStack().length).toBe(2);
+      expect(API.getRedoStack().length).toBe(0);
+      assertSelectedElements([rect2]);
+      expect(h.elements).toEqual([
+        expect.objectContaining({ id: rect3.id }),
+        expect.objectContaining({ id: rect1.id }),
+        expect.objectContaining({ id: rect2.id }),
+      ]);
     });
 
     it("should not let remote changes to interfere with in progress freedraw", async () => {
@@ -3724,54 +3729,52 @@ describe("history", () => {
           captureUpdate: CaptureUpdateAction.NEVER,
         });
 
-        runTwice(() => {
+        Keyboard.redo();
-          Keyboard.redo();
+        expect(API.getUndoStack().length).toBe(1);
-          expect(API.getUndoStack().length).toBe(1);
+        expect(API.getRedoStack().length).toBe(0);
-          expect(API.getRedoStack().length).toBe(0);
+        expect(h.elements).toEqual([
-          expect(h.elements).toEqual([
+          expect.objectContaining({
-            expect.objectContaining({
+            id: container.id,
-              id: container.id,
+            // previously bound text is preserved
-              // previously bound text is preserved
+            // text bindings are not duplicated
-              // text bindings are not duplicated
+            boundElements: [{ id: remoteText.id, type: "text" }],
-              boundElements: [{ id: remoteText.id, type: "text" }],
+            isDeleted: false,
-              isDeleted: false,
+          }),
-            }),
+          expect.objectContaining({
-            expect.objectContaining({
+            id: text.id,
-              id: text.id,
+            // unbound
-              // unbound
+            containerId: null,
-              containerId: null,
+            isDeleted: false,
-              isDeleted: false,
+          }),
-            }),
+          expect.objectContaining({
-            expect.objectContaining({
+            id: remoteText.id,
-              id: remoteText.id,
+            // preserved existing binding!
-              // preserved existing binding!
+            containerId: container.id,
-              containerId: container.id,
+            isDeleted: false,
-              isDeleted: false,
+          }),
-            }),
+        ]);
-          ]);
 
-          Keyboard.undo();
+        Keyboard.undo();
-          expect(API.getUndoStack().length).toBe(0);
+        expect(API.getUndoStack().length).toBe(0);
-          expect(API.getRedoStack().length).toBe(1);
+        expect(API.getRedoStack().length).toBe(1);
-          expect(h.elements).toEqual([
+        expect(h.elements).toEqual([
-            expect.objectContaining({
+          expect.objectContaining({
-              id: container.id,
+            id: container.id,
-              boundElements: [{ id: remoteText.id, type: "text" }],
+            boundElements: [{ id: remoteText.id, type: "text" }],
-              isDeleted: false, // isDeleted got remotely updated to false
+            isDeleted: false, // isDeleted got remotely updated to false
-            }),
+          }),
-            expect.objectContaining({
+          expect.objectContaining({
-              id: text.id,
+            id: text.id,
-              containerId: null,
+            containerId: null,
-              isDeleted: false,
+            isDeleted: false,
-            }),
+          }),
-            expect.objectContaining({
+          expect.objectContaining({
-              id: remoteText.id,
+            id: remoteText.id,
-              // unbound
+            // unbound
-              containerId: container.id,
+            containerId: container.id,
-              isDeleted: false,
+            isDeleted: false,
-            }),
+          }),
-          ]);
+        ]);
-        });
       });
 
       it("should preserve latest remotely added binding and unbind previous one when the text is added through history", async () => {

packages/excalidraw/tests/test-utils.ts

@@ -435,12 +435,17 @@ export const assertElements = <T extends AllPossibleKeys<ExcalidrawElement>>(
   expect(h.state.selectedElementIds).toEqual(selectedElementIds);
 };
 
-const stripSeed = (deltas: Record<string, { deleted: any; inserted: any }>) =>
+const stripProps = (
+  deltas: Record<string, { deleted: any; inserted: any }>,
+  props: string[],
+) =>
   Object.entries(deltas).reduce((acc, curr) => {
     const { inserted, deleted, ...rest } = curr[1];
 
-    delete inserted.seed;
+    for (const prop of props) {
-    delete deleted.seed;
+      delete inserted[prop];
+      delete deleted[prop];
+    }
 
     acc[curr[0]] = {
       inserted,
@@ -457,9 +462,9 @@ export const checkpointHistory = (history: History, name: string) => {
       ...x,
       elements: {
         ...x.elements,
-        added: stripSeed(x.elements.added),
+        added: stripProps(x.elements.added, ["seed", "versionNonce"]),
-        removed: stripSeed(x.elements.removed),
+        removed: stripProps(x.elements.removed, ["seed", "versionNonce"]),
-        updated: stripSeed(x.elements.updated),
+        updated: stripProps(x.elements.updated, ["seed", "versionNonce"]),
       },
     })),
   ).toMatchSnapshot(`[${name}] undo stack`);
@@ -469,9 +474,9 @@ export const checkpointHistory = (history: History, name: string) => {
       ...x,
       elements: {
         ...x.elements,
-        added: stripSeed(x.elements.added),
+        added: stripProps(x.elements.added, ["seed", "versionNonce"]),
-        removed: stripSeed(x.elements.removed),
+        removed: stripProps(x.elements.removed, ["seed", "versionNonce"]),
-        updated: stripSeed(x.elements.updated),
+        updated: stripProps(x.elements.updated, ["seed", "versionNonce"]),
       },
     })),
   ).toMatchSnapshot(`[${name}] redo stack`);