Browse Source

Abstract away or eliminate most of the mutation of the Elements array (#955)

Pete Hunt 5 years ago
parent
commit
e1e2249f57

+ 1 - 2
src/actions/actionFinalize.tsx

@@ -6,7 +6,6 @@ import { ToolButton } from "../components/ToolButton";
 import { done } from "../components/icons";
 import { t } from "../i18n";
 import { register } from "./register";
-import { invalidateShapeForElement } from "../renderer/renderElement";
 import { mutateElement } from "../element/mutateElement";
 
 export const actionFinalize = register({
@@ -29,7 +28,7 @@ export const actionFinalize = register({
       if (isInvisiblySmallElement(appState.multiElement)) {
         newElements = newElements.slice(0, -1);
       }
-      invalidateShapeForElement(appState.multiElement);
+
       if (!appState.elementLocked) {
         appState.selectedElementIds[appState.multiElement.id] = true;
       }

+ 3 - 3
src/actions/actionProperties.tsx

@@ -11,7 +11,7 @@ import { AppState } from "../../src/types";
 import { t } from "../i18n";
 import { DEFAULT_FONT } from "../appState";
 import { register } from "./register";
-import { newElementWith, newTextElementWith } from "../element/mutateElement";
+import { newElementWith } from "../element/mutateElement";
 
 const changeProperty = (
   elements: readonly ExcalidrawElement[],
@@ -266,7 +266,7 @@ export const actionChangeFontSize = register({
     return {
       elements: changeProperty(elements, appState, el => {
         if (isTextElement(el)) {
-          const element: ExcalidrawTextElement = newTextElementWith(el, {
+          const element: ExcalidrawTextElement = newElementWith(el, {
             font: `${value}px ${el.font.split("px ")[1]}`,
           });
           redrawTextBoundingBox(element);
@@ -313,7 +313,7 @@ export const actionChangeFontFamily = register({
     return {
       elements: changeProperty(elements, appState, el => {
         if (isTextElement(el)) {
-          const element: ExcalidrawTextElement = newTextElementWith(el, {
+          const element: ExcalidrawTextElement = newElementWith(el, {
             font: `${el.font.split("px ")[0]}px ${value}`,
           });
           redrawTextBoundingBox(element);

+ 2 - 2
src/actions/actionStyles.ts

@@ -6,7 +6,7 @@ import {
 import { KEYS } from "../keys";
 import { DEFAULT_FONT } from "../appState";
 import { register } from "./register";
-import { mutateTextElement, newElementWith } from "../element/mutateElement";
+import { mutateElement, newElementWith } from "../element/mutateElement";
 
 let copiedStyles: string = "{}";
 
@@ -44,7 +44,7 @@ export const actionPasteStyles = register({
             roughness: pastedElement?.roughness,
           });
           if (isTextElement(newElement)) {
-            mutateTextElement(newElement, {
+            mutateElement(newElement, {
               font: pastedElement?.font || DEFAULT_FONT,
             });
             redrawTextBoundingBox(newElement);

+ 228 - 207
src/components/App.tsx

@@ -3,7 +3,6 @@ import React from "react";
 import socketIOClient from "socket.io-client";
 import rough from "roughjs/bin/rough";
 import { RoughCanvas } from "roughjs/bin/canvas";
-import { Point } from "roughjs/bin/geometry";
 
 import {
   newElement,
@@ -95,9 +94,9 @@ import {
 } from "../constants";
 import { LayerUI } from "./LayerUI";
 import { ScrollBars } from "../scene/types";
-import { invalidateShapeForElement } from "../renderer/renderElement";
 import { generateCollaborationLink, getCollaborationLinkData } from "../data";
 import { mutateElement, newElementWith } from "../element/mutateElement";
+import { invalidateShapeForElement } from "../renderer/renderElement";
 
 // -----------------------------------------------------------------------------
 // TEST HOOKS
@@ -106,27 +105,24 @@ import { mutateElement, newElementWith } from "../element/mutateElement";
 declare global {
   interface Window {
     __TEST__: {
-      elements: typeof elements;
+      elements: readonly ExcalidrawElement[];
       appState: AppState;
     };
-    // TEMPORARY until we have a UI to support this
-    generateCollaborationLink: () => Promise<string>;
   }
 }
 
 if (process.env.NODE_ENV === "test") {
   window.__TEST__ = {} as Window["__TEST__"];
 }
-window.generateCollaborationLink = generateCollaborationLink;
 
 // -----------------------------------------------------------------------------
 
-let { elements } = createScene();
+const scene = createScene();
 
 if (process.env.NODE_ENV === "test") {
   Object.defineProperty(window.__TEST__, "elements", {
     get() {
-      return elements;
+      return scene.getAllElements();
     },
   });
 }
@@ -173,7 +169,7 @@ export class App extends React.Component<any, AppState> {
     this.actionManager = new ActionManager(
       this.syncActionResult,
       () => this.state,
-      () => elements,
+      () => scene.getAllElements(),
     );
     this.actionManager.registerAll(actions);
 
@@ -181,6 +177,11 @@ export class App extends React.Component<any, AppState> {
     this.actionManager.registerAction(createRedoAction(history));
   }
 
+  private replaceElements = (nextElements: readonly ExcalidrawElement[]) => {
+    scene.replaceAllElements(nextElements);
+    this.setState({});
+  };
+
   private syncActionResult = (
     res: ActionResult,
     commitToHistory: boolean = true,
@@ -189,11 +190,10 @@ export class App extends React.Component<any, AppState> {
       return;
     }
     if (res.elements) {
-      elements = res.elements;
+      this.replaceElements(res.elements);
       if (commitToHistory) {
         history.resumeRecording();
       }
-      this.setState({});
     }
 
     if (res.appState) {
@@ -212,12 +212,12 @@ export class App extends React.Component<any, AppState> {
     if (isWritableElement(event.target)) {
       return;
     }
-    copyToAppClipboard(elements, this.state);
+    copyToAppClipboard(scene.getAllElements(), this.state);
     const { elements: nextElements, appState } = deleteSelectedElements(
-      elements,
+      scene.getAllElements(),
       this.state,
     );
-    elements = nextElements;
+    this.replaceElements(nextElements);
     history.resumeRecording();
     this.setState({ ...appState });
     event.preventDefault();
@@ -226,7 +226,7 @@ export class App extends React.Component<any, AppState> {
     if (isWritableElement(event.target)) {
       return;
     }
-    copyToAppClipboard(elements, this.state);
+    copyToAppClipboard(scene.getAllElements(), this.state);
     event.preventDefault();
   };
 
@@ -291,70 +291,75 @@ export class App extends React.Component<any, AppState> {
               // Perform reconciliation - in collaboration, if we encounter
               // elements with more staler versions than ours, ignore them
               // and keep ours.
-              if (elements == null || elements.length === 0) {
-                elements = restoredState.elements;
+              if (
+                scene.getAllElements() == null ||
+                scene.getAllElements().length === 0
+              ) {
+                this.replaceElements(restoredState.elements);
               } else {
                 // create a map of ids so we don't have to iterate
                 // over the array more than once.
-                const localElementMap = getElementMap(elements);
+                const localElementMap = getElementMap(scene.getAllElements());
 
                 // Reconcile
-                elements = restoredState.elements
-                  .reduce((elements, element) => {
-                    // if the remote element references one that's currently
-                    //  edited on local, skip it (it'll be added in the next
-                    //  step)
-                    if (
-                      element.id === this.state.editingElement?.id ||
-                      element.id === this.state.resizingElement?.id ||
-                      element.id === this.state.draggingElement?.id
-                    ) {
-                      return elements;
-                    }
-
-                    if (
-                      localElementMap.hasOwnProperty(element.id) &&
-                      localElementMap[element.id].version > element.version
-                    ) {
-                      elements.push(localElementMap[element.id]);
-                      delete localElementMap[element.id];
-                    } else if (
-                      localElementMap.hasOwnProperty(element.id) &&
-                      localElementMap[element.id].version === element.version &&
-                      localElementMap[element.id].versionNonce !==
-                        element.versionNonce
-                    ) {
-                      // resolve conflicting edits deterministically by taking the one with the lowest versionNonce
+                this.replaceElements(
+                  restoredState.elements
+                    .reduce((elements, element) => {
+                      // if the remote element references one that's currently
+                      //  edited on local, skip it (it'll be added in the next
+                      //  step)
                       if (
-                        localElementMap[element.id].versionNonce <
-                        element.versionNonce
+                        element.id === this.state.editingElement?.id ||
+                        element.id === this.state.resizingElement?.id ||
+                        element.id === this.state.draggingElement?.id
+                      ) {
+                        return elements;
+                      }
+
+                      if (
+                        localElementMap.hasOwnProperty(element.id) &&
+                        localElementMap[element.id].version > element.version
                       ) {
                         elements.push(localElementMap[element.id]);
+                        delete localElementMap[element.id];
+                      } else if (
+                        localElementMap.hasOwnProperty(element.id) &&
+                        localElementMap[element.id].version ===
+                          element.version &&
+                        localElementMap[element.id].versionNonce !==
+                          element.versionNonce
+                      ) {
+                        // resolve conflicting edits deterministically by taking the one with the lowest versionNonce
+                        if (
+                          localElementMap[element.id].versionNonce <
+                          element.versionNonce
+                        ) {
+                          elements.push(localElementMap[element.id]);
+                        } else {
+                          // it should be highly unlikely that the two versionNonces are the same. if we are
+                          // really worried about this, we can replace the versionNonce with the socket id.
+                          elements.push(element);
+                        }
+                        delete localElementMap[element.id];
                       } else {
-                        // it should be highly unlikely that the two versionNonces are the same. if we are
-                        // really worried about this, we can replace the versionNonce with the socket id.
                         elements.push(element);
+                        delete localElementMap[element.id];
                       }
-                      delete localElementMap[element.id];
-                    } else {
-                      elements.push(element);
-                      delete localElementMap[element.id];
-                    }
-
-                    return elements;
-                  }, [] as any)
-                  // add local elements that weren't deleted or on remote
-                  .concat(...Object.values(localElementMap));
+
+                      return elements;
+                    }, [] as any)
+                    // add local elements that weren't deleted or on remote
+                    .concat(...Object.values(localElementMap)),
+                );
               }
               this.lastBroadcastedOrReceivedSceneVersion = getDrawingVersion(
-                elements,
+                scene.getAllElements(),
               );
               // We haven't yet implemented multiplayer undo functionality, so we clear the undo stack
               // when we receive any messages from another peer. This UX can be pretty rough -- if you
               // undo, a user makes a change, and then try to redo, your element(s) will be lost. However,
               // right now we think this is the right tradeoff.
               history.clear();
-              this.setState({});
               if (this.socketInitialized === false) {
                 this.socketInitialized = true;
               }
@@ -423,12 +428,12 @@ export class App extends React.Component<any, AppState> {
     const data: SocketUpdateDataSource["SCENE_UPDATE"] = {
       type: "SCENE_UPDATE",
       payload: {
-        elements: getSyncableElements(elements),
+        elements: getSyncableElements(scene.getAllElements()),
       },
     };
     this.lastBroadcastedOrReceivedSceneVersion = Math.max(
       this.lastBroadcastedOrReceivedSceneVersion,
-      getDrawingVersion(elements),
+      getDrawingVersion(scene.getAllElements()),
     );
     return this._broadcastSocketData(
       data as typeof data & { _brand: "socketUpdateData" },
@@ -553,7 +558,9 @@ export class App extends React.Component<any, AppState> {
   public state: AppState = getDefaultAppState();
 
   private onResize = () => {
-    elements.forEach(element => invalidateShapeForElement(element));
+    scene
+      .getAllElements()
+      .forEach(element => invalidateShapeForElement(element));
     this.setState({});
   };
 
@@ -587,23 +594,24 @@ export class App extends React.Component<any, AppState> {
       const step = event.shiftKey
         ? ELEMENT_SHIFT_TRANSLATE_AMOUNT
         : ELEMENT_TRANSLATE_AMOUNT;
-      elements = elements.map(el => {
-        if (this.state.selectedElementIds[el.id]) {
-          const update: { x?: number; y?: number } = {};
-          if (event.key === KEYS.ARROW_LEFT) {
-            update.x = el.x - step;
-          } else if (event.key === KEYS.ARROW_RIGHT) {
-            update.x = el.x + step;
-          } else if (event.key === KEYS.ARROW_UP) {
-            update.y = el.y - step;
-          } else if (event.key === KEYS.ARROW_DOWN) {
-            update.y = el.y + step;
+      this.replaceElements(
+        scene.getAllElements().map(el => {
+          if (this.state.selectedElementIds[el.id]) {
+            const update: { x?: number; y?: number } = {};
+            if (event.key === KEYS.ARROW_LEFT) {
+              update.x = el.x - step;
+            } else if (event.key === KEYS.ARROW_RIGHT) {
+              update.x = el.x + step;
+            } else if (event.key === KEYS.ARROW_UP) {
+              update.y = el.y - step;
+            } else if (event.key === KEYS.ARROW_DOWN) {
+              update.y = el.y + step;
+            }
+            return newElementWith(el, update);
           }
-          return newElementWith(el, update);
-        }
-        return el;
-      });
-      this.setState({});
+          return el;
+        }),
+      );
       event.preventDefault();
     } else if (
       shapesShortcutKeys.includes(event.key.toLowerCase()) &&
@@ -635,14 +643,17 @@ export class App extends React.Component<any, AppState> {
   };
 
   private copyToAppClipboard = () => {
-    copyToAppClipboard(elements, this.state);
+    copyToAppClipboard(scene.getAllElements(), this.state);
   };
 
   private copyToClipboardAsPng = () => {
-    const selectedElements = getSelectedElements(elements, this.state);
+    const selectedElements = getSelectedElements(
+      scene.getAllElements(),
+      this.state,
+    );
     exportCanvas(
       "clipboard",
-      selectedElements.length ? selectedElements : elements,
+      selectedElements.length ? selectedElements : scene.getAllElements(),
       this.state,
       this.canvas!,
       this.state,
@@ -686,7 +697,7 @@ export class App extends React.Component<any, AppState> {
           this.state.currentItemFont,
         );
 
-        elements = [...elements, element];
+        this.replaceElements([...scene.getAllElements(), element]);
         this.setState({ selectedElementIds: { [element.id]: true } });
         history.resumeRecording();
       }
@@ -729,11 +740,6 @@ export class App extends React.Component<any, AppState> {
     this.setState(obj);
   };
 
-  setElements = (elements_: readonly ExcalidrawElement[]) => {
-    elements = elements_;
-    this.setState({});
-  };
-
   removePointer = (event: React.PointerEvent<HTMLElement>) => {
     gesture.pointers.delete(event.pointerId);
   };
@@ -768,8 +774,8 @@ export class App extends React.Component<any, AppState> {
           appState={this.state}
           setAppState={this.setAppState}
           actionManager={this.actionManager}
-          elements={elements}
-          setElements={this.setElements}
+          elements={scene.getAllElements()}
+          setElements={this.replaceElements}
           language={getLanguage()}
           onRoomCreate={this.createRoom}
           onRoomDestroy={this.destroyRoom}
@@ -810,7 +816,7 @@ export class App extends React.Component<any, AppState> {
               );
 
               const element = getElementAtPosition(
-                elements,
+                scene.getAllElements(),
                 this.state,
                 x,
                 y,
@@ -824,7 +830,7 @@ export class App extends React.Component<any, AppState> {
                       action: () => this.pasteFromClipboard(null),
                     },
                     probablySupportsClipboardBlob &&
-                      hasNonDeletedElements(elements) && {
+                      hasNonDeletedElements(scene.getAllElements()) && {
                         label: t("labels.copyAsPng"),
                         action: this.copyToClipboardAsPng,
                       },
@@ -908,7 +914,7 @@ export class App extends React.Component<any, AppState> {
     );
 
     const elementAtPosition = getElementAtPosition(
-      elements,
+      scene.getAllElements(),
       this.state,
       x,
       y,
@@ -940,10 +946,11 @@ export class App extends React.Component<any, AppState> {
     let textY = event.clientY;
 
     if (elementAtPosition && isTextElement(elementAtPosition)) {
-      elements = elements.filter(
-        element => element.id !== elementAtPosition.id,
+      this.replaceElements(
+        scene
+          .getAllElements()
+          .filter(element => element.id !== elementAtPosition.id),
       );
-      this.setState({});
 
       const centerElementX = elementAtPosition.x + elementAtPosition.width / 2;
       const centerElementY = elementAtPosition.y + elementAtPosition.height / 2;
@@ -998,14 +1005,14 @@ export class App extends React.Component<any, AppState> {
       zoom: this.state.zoom,
       onSubmit: text => {
         if (text) {
-          elements = [
-            ...elements,
+          this.replaceElements([
+            ...scene.getAllElements(),
             {
               // we need to recreate the element to update dimensions &
               //  position
               ...newTextElement(element, text, element.font),
             },
-          ];
+          ]);
         }
         this.setState(prevState => ({
           selectedElementIds: {
@@ -1083,11 +1090,11 @@ export class App extends React.Component<any, AppState> {
       const originX = multiElement.x;
       const originY = multiElement.y;
       const points = multiElement.points;
-      const pnt = points[points.length - 1];
-      pnt[0] = x - originX;
-      pnt[1] = y - originY;
-      mutateElement(multiElement);
-      invalidateShapeForElement(multiElement);
+
+      mutateElement(multiElement, {
+        points: [...points.slice(0, -1), [x - originX, y - originY]],
+      });
+
       this.setState({});
       return;
     }
@@ -1097,10 +1104,13 @@ export class App extends React.Component<any, AppState> {
       return;
     }
 
-    const selectedElements = getSelectedElements(elements, this.state);
+    const selectedElements = getSelectedElements(
+      scene.getAllElements(),
+      this.state,
+    );
     if (selectedElements.length === 1 && !isOverScrollBar) {
       const resizeElement = getElementWithResizeHandler(
-        elements,
+        scene.getAllElements(),
         this.state,
         { x, y },
         this.state.zoom,
@@ -1114,7 +1124,7 @@ export class App extends React.Component<any, AppState> {
       }
     }
     const hitElement = getElementAtPosition(
-      elements,
+      scene.getAllElements(),
       this.state,
       x,
       y,
@@ -1306,14 +1316,17 @@ export class App extends React.Component<any, AppState> {
     let elementIsAddedToSelection = false;
     if (this.state.elementType === "selection") {
       const resizeElement = getElementWithResizeHandler(
-        elements,
+        scene.getAllElements(),
         this.state,
         { x, y },
         this.state.zoom,
         event.pointerType,
       );
 
-      const selectedElements = getSelectedElements(elements, this.state);
+      const selectedElements = getSelectedElements(
+        scene.getAllElements(),
+        this.state,
+      );
       if (selectedElements.length === 1 && resizeElement) {
         this.setState({
           resizingElement: resizeElement ? resizeElement.element : null,
@@ -1326,7 +1339,7 @@ export class App extends React.Component<any, AppState> {
         isResizingElements = true;
       } else {
         hitElement = getElementAtPosition(
-          elements,
+          scene.getAllElements(),
           this.state,
           x,
           y,
@@ -1353,7 +1366,7 @@ export class App extends React.Component<any, AppState> {
                 [hitElement!.id]: true,
               },
             }));
-            elements = elements.slice();
+            this.replaceElements(scene.getAllElements());
             elementIsAddedToSelection = true;
           }
 
@@ -1363,7 +1376,7 @@ export class App extends React.Component<any, AppState> {
             // put the duplicates where the selected elements used to be.
             const nextElements = [];
             const elementsToAppend = [];
-            for (const element of elements) {
+            for (const element of scene.getAllElements()) {
               if (this.state.selectedElementIds[element.id]) {
                 nextElements.push(duplicateElement(element));
                 elementsToAppend.push(element);
@@ -1371,7 +1384,7 @@ export class App extends React.Component<any, AppState> {
                 nextElements.push(element);
               }
             }
-            elements = [...nextElements, ...elementsToAppend];
+            this.replaceElements([...nextElements, ...elementsToAppend]);
           }
         }
       }
@@ -1421,12 +1434,12 @@ export class App extends React.Component<any, AppState> {
         zoom: this.state.zoom,
         onSubmit: text => {
           if (text) {
-            elements = [
-              ...elements,
+            this.replaceElements([
+              ...scene.getAllElements(),
               {
                 ...newTextElement(element, text, this.state.currentItemFont),
               },
-            ];
+            ]);
           }
           this.setState(prevState => ({
             selectedElementIds: {
@@ -1469,9 +1482,9 @@ export class App extends React.Component<any, AppState> {
             [multiElement.id]: true,
           },
         }));
-        multiElement.points.push([x - rx, y - ry]);
-        mutateElement(multiElement);
-        invalidateShapeForElement(multiElement);
+        mutateElement(multiElement, {
+          points: [...multiElement.points, [x - rx, y - ry]],
+        });
       } else {
         this.setState(prevState => ({
           selectedElementIds: {
@@ -1479,10 +1492,10 @@ export class App extends React.Component<any, AppState> {
             [element.id]: false,
           },
         }));
-        element.points.push([0, 0]);
-        mutateElement(element);
-        invalidateShapeForElement(element);
-        elements = [...elements, element];
+        mutateElement(element, {
+          points: [...element.points, [0, 0]],
+        });
+        this.replaceElements([...scene.getAllElements(), element]);
         this.setState({
           draggingElement: element,
           editingElement: element,
@@ -1494,7 +1507,7 @@ export class App extends React.Component<any, AppState> {
         draggingElement: element,
       });
     } else {
-      elements = [...elements, element];
+      this.replaceElements([...scene.getAllElements(), element]);
       this.setState({
         multiElement: null,
         draggingElement: element,
@@ -1505,7 +1518,7 @@ export class App extends React.Component<any, AppState> {
     let resizeArrowFn:
       | ((
           element: ExcalidrawElement,
-          p1: Point,
+          pointIndex: number,
           deltaX: number,
           deltaY: number,
           pointerX: number,
@@ -1516,13 +1529,14 @@ export class App extends React.Component<any, AppState> {
 
     const arrowResizeOrigin = (
       element: ExcalidrawElement,
-      p1: Point,
+      pointIndex: number,
       deltaX: number,
       deltaY: number,
       pointerX: number,
       pointerY: number,
       perfect: boolean,
     ) => {
+      const p1 = element.points[pointIndex];
       if (perfect) {
         const absPx = p1[0] + element.x;
         const absPy = p1[1] + element.y;
@@ -1535,44 +1549,52 @@ export class App extends React.Component<any, AppState> {
 
         const dx = element.x + width + p1[0];
         const dy = element.y + height + p1[1];
-        p1[0] = absPx - element.x;
-        p1[1] = absPy - element.y;
         mutateElement(element, {
           x: dx,
           y: dy,
+          points: element.points.map((point, i) =>
+            i === pointIndex ? [absPx - element.x, absPy - element.y] : point,
+          ),
         });
       } else {
-        p1[0] -= deltaX;
-        p1[1] -= deltaY;
         mutateElement(element, {
           x: element.x + deltaX,
           y: element.y + deltaY,
+          points: element.points.map((point, i) =>
+            i === pointIndex ? [p1[0] - deltaX, p1[1] - deltaY] : point,
+          ),
         });
       }
     };
 
     const arrowResizeEnd = (
       element: ExcalidrawElement,
-      p1: Point,
+      pointIndex: number,
       deltaX: number,
       deltaY: number,
       pointerX: number,
       pointerY: number,
       perfect: boolean,
     ) => {
+      const p1 = element.points[pointIndex];
       if (perfect) {
         const { width, height } = getPerfectElementSize(
           element.type,
           pointerX - element.x,
           pointerY - element.y,
         );
-        p1[0] = width;
-        p1[1] = height;
+        mutateElement(element, {
+          points: element.points.map((point, i) =>
+            i === pointIndex ? [width, height] : point,
+          ),
+        });
       } else {
-        p1[0] += deltaX;
-        p1[1] += deltaY;
+        mutateElement(element, {
+          points: element.points.map((point, i) =>
+            i === pointIndex ? [p1[0] + deltaX, p1[1] + deltaY] : point,
+          ),
+        });
       }
-      mutateElement(element);
     };
 
     const onPointerMove = (event: PointerEvent) => {
@@ -1623,7 +1645,10 @@ export class App extends React.Component<any, AppState> {
       if (isResizingElements && this.state.resizingElement) {
         this.setState({ isResizing: true });
         const el = this.state.resizingElement;
-        const selectedElements = getSelectedElements(elements, this.state);
+        const selectedElements = getSelectedElements(
+          scene.getAllElements(),
+          this.state,
+        );
         if (selectedElements.length === 1) {
           const { x, y } = viewportCoordsToSceneCoords(
             event,
@@ -1646,15 +1671,7 @@ export class App extends React.Component<any, AppState> {
                     resizeArrowFn = arrowResizeOrigin;
                   }
                 }
-                resizeArrowFn(
-                  element,
-                  p1,
-                  deltaX,
-                  deltaY,
-                  x,
-                  y,
-                  event.shiftKey,
-                );
+                resizeArrowFn(element, 1, deltaX, deltaY, x, y, event.shiftKey);
               } else {
                 mutateElement(element, {
                   x: element.x + deltaX,
@@ -1678,15 +1695,7 @@ export class App extends React.Component<any, AppState> {
                     resizeArrowFn = arrowResizeOrigin;
                   }
                 }
-                resizeArrowFn(
-                  element,
-                  p1,
-                  deltaX,
-                  deltaY,
-                  x,
-                  y,
-                  event.shiftKey,
-                );
+                resizeArrowFn(element, 1, deltaX, deltaY, x, y, event.shiftKey);
               } else {
                 const nextWidth = element.width + deltaX;
                 mutateElement(element, {
@@ -1708,15 +1717,7 @@ export class App extends React.Component<any, AppState> {
                     resizeArrowFn = arrowResizeOrigin;
                   }
                 }
-                resizeArrowFn(
-                  element,
-                  p1,
-                  deltaX,
-                  deltaY,
-                  x,
-                  y,
-                  event.shiftKey,
-                );
+                resizeArrowFn(element, 1, deltaX, deltaY, x, y, event.shiftKey);
               } else {
                 mutateElement(element, {
                   x: element.x + deltaX,
@@ -1737,15 +1738,7 @@ export class App extends React.Component<any, AppState> {
                     resizeArrowFn = arrowResizeOrigin;
                   }
                 }
-                resizeArrowFn(
-                  element,
-                  p1,
-                  deltaX,
-                  deltaY,
-                  x,
-                  y,
-                  event.shiftKey,
-                );
+                resizeArrowFn(element, 1, deltaX, deltaY, x, y, event.shiftKey);
               } else {
                 mutateElement(element, {
                   width: element.width + deltaX,
@@ -1756,9 +1749,13 @@ export class App extends React.Component<any, AppState> {
               }
               break;
             case "n": {
+              let points;
               if (element.points.length > 0) {
                 const len = element.points.length;
-                const points = [...element.points].sort((a, b) => a[1] - b[1]);
+                points = [...element.points].sort((a, b) => a[1] - b[1]) as [
+                  number,
+                  number,
+                ][];
 
                 for (let i = 1; i < points.length; ++i) {
                   const pnt = points[i];
@@ -1769,13 +1766,18 @@ export class App extends React.Component<any, AppState> {
               mutateElement(element, {
                 height: element.height - deltaY,
                 y: element.y + deltaY,
+                points,
               });
               break;
             }
             case "w": {
+              let points;
               if (element.points.length > 0) {
                 const len = element.points.length;
-                const points = [...element.points].sort((a, b) => a[0] - b[0]);
+                points = [...element.points].sort((a, b) => a[0] - b[0]) as [
+                  number,
+                  number,
+                ][];
 
                 for (let i = 0; i < points.length; ++i) {
                   const pnt = points[i];
@@ -1786,13 +1788,19 @@ export class App extends React.Component<any, AppState> {
               mutateElement(element, {
                 width: element.width - deltaX,
                 x: element.x + deltaX,
+                points,
               });
               break;
             }
             case "s": {
+              let points;
+
               if (element.points.length > 0) {
                 const len = element.points.length;
-                const points = [...element.points].sort((a, b) => a[1] - b[1]);
+                points = [...element.points].sort((a, b) => a[1] - b[1]) as [
+                  number,
+                  number,
+                ][];
 
                 for (let i = 1; i < points.length; ++i) {
                   const pnt = points[i];
@@ -1802,14 +1810,18 @@ export class App extends React.Component<any, AppState> {
 
               mutateElement(element, {
                 height: element.height + deltaY,
-                points: element.points, // no-op, but signifies that we mutated points in-place above
+                points,
               });
               break;
             }
             case "e": {
+              let points;
               if (element.points.length > 0) {
                 const len = element.points.length;
-                const points = [...element.points].sort((a, b) => a[0] - b[0]);
+                points = [...element.points].sort((a, b) => a[0] - b[0]) as [
+                  number,
+                  number,
+                ][];
 
                 for (let i = 1; i < points.length; ++i) {
                   const pnt = points[i];
@@ -1819,7 +1831,7 @@ export class App extends React.Component<any, AppState> {
 
               mutateElement(element, {
                 width: element.width + deltaX,
-                points: element.points, // no-op, but signifies that we mutated points in-place above
+                points,
               });
               break;
             }
@@ -1838,7 +1850,6 @@ export class App extends React.Component<any, AppState> {
             x: element.x,
             y: element.y,
           });
-          invalidateShapeForElement(el);
 
           lastX = x;
           lastY = y;
@@ -1851,7 +1862,10 @@ export class App extends React.Component<any, AppState> {
         // Marking that click was used for dragging to check
         // if elements should be deselected on pointerup
         draggingOccurred = true;
-        const selectedElements = getSelectedElements(elements, this.state);
+        const selectedElements = getSelectedElements(
+          scene.getAllElements(),
+          this.state,
+        );
         if (selectedElements.length > 0) {
           const { x, y } = viewportCoordsToSceneCoords(
             event,
@@ -1906,14 +1920,12 @@ export class App extends React.Component<any, AppState> {
         }
 
         if (points.length === 1) {
-          points.push([dx, dy]);
+          mutateElement(draggingElement, { points: [...points, [dx, dy]] });
         } else if (points.length > 1) {
-          const pnt = points[points.length - 1];
-          pnt[0] = dx;
-          pnt[1] = dy;
+          mutateElement(draggingElement, {
+            points: [...points.slice(0, -1), [dx, dy]],
+          });
         }
-
-        mutateElement(draggingElement, { points });
       } else {
         if (event.shiftKey) {
           ({ width, height } = getPerfectElementSize(
@@ -1935,14 +1947,15 @@ export class App extends React.Component<any, AppState> {
         });
       }
 
-      invalidateShapeForElement(draggingElement);
-
       if (this.state.elementType === "selection") {
-        if (!event.shiftKey && isSomeElementSelected(elements, this.state)) {
+        if (
+          !event.shiftKey &&
+          isSomeElementSelected(scene.getAllElements(), this.state)
+        ) {
           this.setState({ selectedElementIds: {} });
         }
         const elementsWithinSelection = getElementsWithinSelection(
-          elements,
+          scene.getAllElements(),
           draggingElement,
         );
         this.setState(prevState => ({
@@ -1990,12 +2003,12 @@ export class App extends React.Component<any, AppState> {
             this.state,
             this.canvas,
           );
-          draggingElement.points.push([
-            x - draggingElement.x,
-            y - draggingElement.y,
-          ]);
-          mutateElement(draggingElement);
-          invalidateShapeForElement(draggingElement);
+          mutateElement(draggingElement, {
+            points: [
+              ...draggingElement.points,
+              [x - draggingElement.x, y - draggingElement.y],
+            ],
+          });
           this.setState({
             multiElement: this.state.draggingElement,
             editingElement: this.state.draggingElement,
@@ -2030,7 +2043,7 @@ export class App extends React.Component<any, AppState> {
         isInvisiblySmallElement(draggingElement)
       ) {
         // remove invisible element which was added in onPointerDown
-        elements = elements.slice(0, -1);
+        this.replaceElements(scene.getAllElements().slice(0, -1));
         this.setState({
           draggingElement: null,
         });
@@ -2047,7 +2060,9 @@ export class App extends React.Component<any, AppState> {
       }
 
       if (resizingElement && isInvisiblySmallElement(resizingElement)) {
-        elements = elements.filter(el => el.id !== resizingElement.id);
+        this.replaceElements(
+          scene.getAllElements().filter(el => el.id !== resizingElement.id),
+        );
       }
 
       // If click occurred on already selected element
@@ -2090,7 +2105,7 @@ export class App extends React.Component<any, AppState> {
 
       if (
         elementType !== "selection" ||
-        isSomeElementSelected(elements, this.state)
+        isSomeElementSelected(scene.getAllElements(), this.state)
       ) {
         history.resumeRecording();
       }
@@ -2163,7 +2178,7 @@ export class App extends React.Component<any, AppState> {
       return duplicate;
     });
 
-    elements = [...elements, ...newElements];
+    this.replaceElements([...scene.getAllElements(), ...newElements]);
     history.resumeRecording();
     this.setState({
       selectedElementIds: newElements.reduce((map, element) => {
@@ -2174,7 +2189,11 @@ export class App extends React.Component<any, AppState> {
   };
 
   private getTextWysiwygSnappedToCenterPosition(x: number, y: number) {
-    const elementClickedInside = getElementContainingPosition(elements, x, y);
+    const elementClickedInside = getElementContainingPosition(
+      scene.getAllElements(),
+      x,
+      y,
+    );
     if (elementClickedInside) {
       const elementCenterX =
         elementClickedInside.x + elementClickedInside.width / 2;
@@ -2209,7 +2228,7 @@ export class App extends React.Component<any, AppState> {
   };
 
   private saveDebounced = debounce(() => {
-    saveToLocalStorage(elements, this.state);
+    saveToLocalStorage(scene.getAllElements(), this.state);
   }, 300);
 
   componentDidUpdate() {
@@ -2233,7 +2252,7 @@ export class App extends React.Component<any, AppState> {
       );
     });
     const { atLeastOneVisibleElement, scrollBars } = renderScene(
-      elements,
+      scene.getAllElements(),
       this.state,
       this.state.selectionElement,
       window.devicePixelRatio,
@@ -2254,20 +2273,22 @@ export class App extends React.Component<any, AppState> {
       currentScrollBars = scrollBars;
     }
     const scrolledOutside =
-      !atLeastOneVisibleElement && hasNonDeletedElements(elements);
+      !atLeastOneVisibleElement &&
+      hasNonDeletedElements(scene.getAllElements());
     if (this.state.scrolledOutside !== scrolledOutside) {
       this.setState({ scrolledOutside: scrolledOutside });
     }
     this.saveDebounced();
 
     if (
-      getDrawingVersion(elements) > this.lastBroadcastedOrReceivedSceneVersion
+      getDrawingVersion(scene.getAllElements()) >
+      this.lastBroadcastedOrReceivedSceneVersion
     ) {
       this.broadcastSceneUpdate();
     }
 
     if (history.isRecording()) {
-      history.pushEntry(this.state, elements);
+      history.pushEntry(this.state, scene.getAllElements());
       history.skipRecording();
     }
   }

+ 1 - 1
src/data/json.ts

@@ -14,7 +14,7 @@ export function serializeAsJSON(
       type: "excalidraw",
       version: 1,
       source: window.location.origin,
-      elements,
+      elements: elements.filter(element => !element.isDeleted),
       appState: cleanAppStateForExport(appState),
     },
     null,

+ 4 - 1
src/data/localStorage.ts

@@ -10,7 +10,10 @@ export function saveToLocalStorage(
   elements: readonly ExcalidrawElement[],
   appState: AppState,
 ) {
-  localStorage.setItem(LOCAL_STORAGE_KEY, JSON.stringify(elements));
+  localStorage.setItem(
+    LOCAL_STORAGE_KEY,
+    JSON.stringify(elements.filter(element => !element.isDeleted)),
+  );
   localStorage.setItem(
     LOCAL_STORAGE_KEY_STATE,
     JSON.stringify(clearAppStateForLocalStorage(appState)),

+ 1 - 1
src/data/restore.ts

@@ -1,4 +1,4 @@
-import { Point } from "roughjs/bin/geometry";
+import { Point } from "../types";
 
 import { ExcalidrawElement } from "../element/types";
 import { AppState } from "../types";

+ 3 - 3
src/element/bounds.ts

@@ -1,7 +1,7 @@
 import { ExcalidrawElement } from "./types";
 import { rotate } from "../math";
 import { Drawable } from "roughjs/bin/core";
-import { Point } from "roughjs/bin/geometry";
+import { Point } from "../types";
 import { getShapeForElement } from "../renderer/renderElement";
 
 // If the element is created from right to left, the width is going to be negative
@@ -68,7 +68,7 @@ export function getLinearElementAbsoluteBounds(element: ExcalidrawElement) {
       // move, bcurveTo, lineTo, and curveTo
       if (op === "move") {
         // change starting point
-        currentP = data as Point;
+        currentP = (data as unknown) as Point;
         // move operation does not draw anything; so, it always
         // returns false
       } else if (op === "bcurveTo") {
@@ -133,7 +133,7 @@ export function getArrowPoints(element: ExcalidrawElement, shape: Drawable[]) {
   const prevOp = ops[ops.length - 2];
   let p0: Point = [0, 0];
   if (prevOp.op === "move") {
-    p0 = prevOp.data as Point;
+    p0 = (prevOp.data as unknown) as Point;
   } else if (prevOp.op === "bcurveTo") {
     p0 = [prevOp.data[4], prevOp.data[5]];
   }

+ 2 - 2
src/element/collision.ts

@@ -7,7 +7,7 @@ import {
   getElementAbsoluteCoords,
   getLinearElementAbsoluteBounds,
 } from "./bounds";
-import { Point } from "roughjs/bin/geometry";
+import { Point } from "../types";
 import { Drawable, OpSet } from "roughjs/bin/core";
 import { AppState } from "../types";
 import { getShapeForElement } from "../renderer/renderElement";
@@ -231,7 +231,7 @@ const hitTestRoughShape = (opSet: OpSet[], x: number, y: number) => {
     // move, bcurveTo, lineTo, and curveTo
     if (op === "move") {
       // change starting point
-      currentP = data as Point;
+      currentP = (data as unknown) as Point;
       // move operation does not draw anything; so, it always
       // returns false
     } else if (op === "bcurveTo") {

+ 25 - 34
src/element/mutateElement.ts

@@ -1,5 +1,6 @@
-import { ExcalidrawElement, ExcalidrawTextElement } from "./types";
+import { ExcalidrawElement } from "./types";
 import { randomSeed } from "roughjs/bin/math";
+import { invalidateShapeForElement } from "../renderer/renderElement";
 
 type ElementUpdate<TElement extends ExcalidrawElement> = Omit<
   Partial<TElement>,
@@ -9,45 +10,35 @@ type ElementUpdate<TElement extends ExcalidrawElement> = Omit<
 // This function tracks updates of text elements for the purposes for collaboration.
 // The version is used to compare updates when more than one user is working in
 // the same drawing.
-export function mutateElement(
-  element: ExcalidrawElement,
-  updates?: ElementUpdate<ExcalidrawElement>,
+export function mutateElement<TElement extends ExcalidrawElement>(
+  element: TElement,
+  updates: ElementUpdate<TElement>,
 ) {
-  if (updates) {
-    Object.assign(element, updates);
+  const mutableElement = element as any;
+
+  for (const key in updates) {
+    const value = (updates as any)[key];
+    if (typeof value !== "undefined") {
+      mutableElement[key] = value;
+    }
   }
-  (element as any).version++;
-  (element as any).versionNonce = randomSeed();
-}
 
-export function newElementWith(
-  element: ExcalidrawElement,
-  updates: ElementUpdate<ExcalidrawElement>,
-): ExcalidrawElement {
-  return {
-    ...element,
-    ...updates,
-    version: element.version + 1,
-    versionNonce: randomSeed(),
-  };
-}
+  if (
+    typeof updates.height !== "undefined" ||
+    typeof updates.width !== "undefined" ||
+    typeof updates.points !== "undefined"
+  ) {
+    invalidateShapeForElement(element);
+  }
 
-// This function tracks updates of text elements for the purposes for collaboration.
-// The version is used to compare updates when more than one user is working in
-// the same document.
-export function mutateTextElement(
-  element: ExcalidrawTextElement,
-  updates: ElementUpdate<ExcalidrawTextElement>,
-): void {
-  Object.assign(element, updates);
-  (element as any).version++;
-  (element as any).versionNonce = randomSeed();
+  mutableElement.version++;
+  mutableElement.versionNonce = randomSeed();
 }
 
-export function newTextElementWith(
-  element: ExcalidrawTextElement,
-  updates: ElementUpdate<ExcalidrawTextElement>,
-): ExcalidrawTextElement {
+export function newElementWith<TElement extends ExcalidrawElement>(
+  element: TElement,
+  updates: ElementUpdate<TElement>,
+): TElement {
   return {
     ...element,
     ...updates,

+ 2 - 2
src/element/newElement.ts

@@ -1,6 +1,6 @@
 import { randomSeed } from "roughjs/bin/math";
 import nanoid from "nanoid";
-import { Point } from "roughjs/bin/geometry";
+import { Point } from "../types";
 
 import { ExcalidrawElement, ExcalidrawTextElement } from "../element/types";
 import { measureText } from "../utils";
@@ -32,7 +32,7 @@ export function newElement(
     roughness,
     opacity,
     seed: randomSeed(),
-    points: [] as Point[],
+    points: [] as readonly Point[],
     version: 1,
     versionNonce: 0,
     isDeleted: false,

+ 0 - 3
src/element/sizeHelpers.ts

@@ -1,5 +1,4 @@
 import { ExcalidrawElement } from "./types";
-import { invalidateShapeForElement } from "../renderer/renderElement";
 import { mutateElement } from "./mutateElement";
 
 export function isInvisiblySmallElement(element: ExcalidrawElement): boolean {
@@ -101,7 +100,5 @@ export function normalizeDimensions(
     });
   }
 
-  invalidateShapeForElement(element);
-
   return true;
 }

+ 2 - 2
src/element/textElement.ts

@@ -1,10 +1,10 @@
 import { measureText } from "../utils";
 import { ExcalidrawTextElement } from "./types";
-import { mutateTextElement } from "./mutateElement";
+import { mutateElement } from "./mutateElement";
 
 export const redrawTextBoundingBox = (element: ExcalidrawTextElement) => {
   const metrics = measureText(element.text, element.font);
-  mutateTextElement(element, {
+  mutateElement(element, {
     width: metrics.width,
     height: metrics.height,
     baseline: metrics.baseline,

+ 1 - 1
src/math.ts

@@ -1,4 +1,4 @@
-import { Point } from "roughjs/bin/geometry";
+import { Point } from "./types";
 
 // https://stackoverflow.com/a/6853926/232122
 export function distanceBetweenPointAndSegment(

+ 2 - 5
src/renderer/renderElement.ts

@@ -7,7 +7,6 @@ import {
 } from "../element/bounds";
 import { RoughCanvas } from "roughjs/bin/canvas";
 import { Drawable } from "roughjs/bin/core";
-import { Point } from "roughjs/bin/geometry";
 import { RoughSVG } from "roughjs/bin/svg";
 import { RoughGenerator } from "roughjs/bin/generator";
 import { SceneState } from "../scene/types";
@@ -214,13 +213,11 @@ function generateElement(
         };
         // points array can be empty in the beginning, so it is important to add
         // initial position to it
-        const points: Point[] = element.points.length
-          ? element.points
-          : [[0, 0]];
+        const points = element.points.length ? element.points : [[0, 0]];
 
         // curve is always the first element
         // this simplifies finding the curve for an element
-        shape = [generator.curve(points, options)];
+        shape = [generator.curve(points as [number, number][], options)];
 
         // add lines only in arrow
         if (element.type === "arrow") {

+ 13 - 2
src/scene/createScene.ts

@@ -1,6 +1,17 @@
 import { ExcalidrawElement } from "../element/types";
 
+class SceneState {
+  constructor(private _elements: readonly ExcalidrawElement[] = []) {}
+
+  getAllElements() {
+    return this._elements;
+  }
+
+  replaceAllElements(nextElements: readonly ExcalidrawElement[]) {
+    this._elements = nextElements;
+  }
+}
+
 export const createScene = () => {
-  const elements: readonly ExcalidrawElement[] = [];
-  return { elements };
+  return new SceneState();
 };

+ 2 - 0
src/types.ts

@@ -1,7 +1,9 @@
 import { ExcalidrawElement, PointerType } from "./element/types";
 import { SHAPES } from "./shapes";
+import { Point as RoughPoint } from "roughjs/bin/geometry";
 
 export type FlooredNumber = number & { _brand: "FlooredNumber" };
+export type Point = Readonly<RoughPoint>;
 
 export type AppState = {
   draggingElement: ExcalidrawElement | null;