diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index cdecbf34..71d6ea82 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -268,6 +268,7 @@ import { muteFSAbortError, isTestEnv, easeOut, + updateStable, } from "../utils"; import { createSrcDoc, @@ -4736,13 +4737,31 @@ class App extends React.Component { event, ); - this.setState({ - snapLines, - originSnapOffset: originOffset, + this.setState((prevState) => { + const nextSnapLines = updateStable(prevState.snapLines, snapLines); + const nextOriginOffset = prevState.originSnapOffset + ? updateStable(prevState.originSnapOffset, originOffset) + : originOffset; + + if ( + prevState.snapLines === nextSnapLines && + prevState.originSnapOffset === nextOriginOffset + ) { + return null; + } + return { + snapLines: nextSnapLines, + originSnapOffset: nextOriginOffset, + }; }); } else if (!this.state.draggingElement) { - this.setState({ - snapLines: [], + this.setState((prevState) => { + if (prevState.snapLines.length) { + return { + snapLines: [], + }; + } + return null; }); } @@ -7227,7 +7246,7 @@ class App extends React.Component { isRotating, } = this.state; - this.setState({ + this.setState((prevState) => ({ isResizing: false, isRotating: false, resizingElement: null, @@ -7241,10 +7260,10 @@ class App extends React.Component { multiElement || isTextElement(this.state.editingElement) ? this.state.editingElement : null, - snapLines: [], + snapLines: updateStable(prevState.snapLines, []), originSnapOffset: null, - }); + })); SnapCache.setReferenceSnapPoints(null); SnapCache.setVisibleGaps(null); diff --git a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap index 015ff7c4..f348d050 100644 --- a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap @@ -1,6 +1,6 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`duplicate element on move when ALT is clicked > rectangle 1`] = ` +exports[`duplicate element on move when ALT is clicked > rectangle 5`] = ` { "angle": 0, "backgroundColor": "transparent", @@ -32,7 +32,7 @@ exports[`duplicate element on move when ALT is clicked > rectangle 1`] = ` } `; -exports[`duplicate element on move when ALT is clicked > rectangle 2`] = ` +exports[`duplicate element on move when ALT is clicked > rectangle 6`] = ` { "angle": 0, "backgroundColor": "transparent", @@ -64,7 +64,7 @@ exports[`duplicate element on move when ALT is clicked > rectangle 2`] = ` } `; -exports[`move element > rectangle 1`] = ` +exports[`move element > rectangle 5`] = ` { "angle": 0, "backgroundColor": "transparent", @@ -96,7 +96,7 @@ exports[`move element > rectangle 1`] = ` } `; -exports[`move element > rectangles with binding arrow 1`] = ` +exports[`move element > rectangles with binding arrow 5`] = ` { "angle": 0, "backgroundColor": "transparent", @@ -133,7 +133,7 @@ exports[`move element > rectangles with binding arrow 1`] = ` } `; -exports[`move element > rectangles with binding arrow 2`] = ` +exports[`move element > rectangles with binding arrow 6`] = ` { "angle": 0, "backgroundColor": "transparent", @@ -170,7 +170,7 @@ exports[`move element > rectangles with binding arrow 2`] = ` } `; -exports[`move element > rectangles with binding arrow 3`] = ` +exports[`move element > rectangles with binding arrow 7`] = ` { "angle": 0, "backgroundColor": "transparent", diff --git a/packages/excalidraw/tests/__snapshots__/multiPointCreate.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/multiPointCreate.test.tsx.snap index 48ae1f64..12e7e61e 100644 --- a/packages/excalidraw/tests/__snapshots__/multiPointCreate.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/multiPointCreate.test.tsx.snap @@ -1,6 +1,6 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`multi point mode in linear elements > arrow 1`] = ` +exports[`multi point mode in linear elements > arrow 3`] = ` { "angle": 0, "backgroundColor": "transparent", @@ -54,7 +54,7 @@ exports[`multi point mode in linear elements > arrow 1`] = ` } `; -exports[`multi point mode in linear elements > line 1`] = ` +exports[`multi point mode in linear elements > line 3`] = ` { "angle": 0, "backgroundColor": "transparent", diff --git a/packages/excalidraw/tests/linearElementEditor.test.tsx b/packages/excalidraw/tests/linearElementEditor.test.tsx index 6c44c6cd..f4ddeafd 100644 --- a/packages/excalidraw/tests/linearElementEditor.test.tsx +++ b/packages/excalidraw/tests/linearElementEditor.test.tsx @@ -173,14 +173,14 @@ describe("Test Linear Elements", () => { createTwoPointerLinearElement("line"); const line = h.elements[0] as ExcalidrawLinearElement; - expect(renderInteractiveScene).toHaveBeenCalledTimes(5); - expect(renderStaticScene).toHaveBeenCalledTimes(5); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot(`5`); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`5`); expect((h.elements[0] as ExcalidrawLinearElement).points.length).toEqual(2); // drag line from midpoint drag(midpoint, [midpoint[0] + delta, midpoint[1] + delta]); - expect(renderInteractiveScene).toHaveBeenCalledTimes(9); - expect(renderStaticScene).toHaveBeenCalledTimes(7); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot(`9`); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`7`); expect(line.points.length).toEqual(3); expect(line.points).toMatchInlineSnapshot(` [ @@ -273,8 +273,10 @@ describe("Test Linear Elements", () => { // drag line from midpoint drag(midpoint, [midpoint[0] + delta, midpoint[1] + delta]); - expect(renderInteractiveScene).toHaveBeenCalledTimes(14); - expect(renderStaticScene).toHaveBeenCalledTimes(6); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `12`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`6`); expect(line.points.length).toEqual(3); expect(line.points).toMatchInlineSnapshot(` @@ -311,8 +313,10 @@ describe("Test Linear Elements", () => { // update roundness fireEvent.click(screen.getByTitle("Round")); - expect(renderInteractiveScene).toHaveBeenCalledTimes(10); - expect(renderStaticScene).toHaveBeenCalledTimes(8); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `10`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`8`); const midPointsWithRoundEdge = LinearElementEditor.getEditorMidPoints( h.elements[0] as ExcalidrawLinearElement, @@ -357,8 +361,10 @@ describe("Test Linear Elements", () => { // Move the element drag(startPoint, endPoint); - expect(renderInteractiveScene).toHaveBeenCalledTimes(14); - expect(renderStaticScene).toHaveBeenCalledTimes(9); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `13`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`9`); expect([line.x, line.y]).toEqual([ points[0][0] + deltaX, @@ -416,8 +422,10 @@ describe("Test Linear Elements", () => { lastSegmentMidpoint[1] + delta, ]); - expect(renderInteractiveScene).toHaveBeenCalledTimes(21); - expect(renderStaticScene).toHaveBeenCalledTimes(9); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `17`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`9`); expect(line.points.length).toEqual(5); @@ -457,8 +465,10 @@ describe("Test Linear Elements", () => { // Drag from first point drag(hitCoords, [hitCoords[0] - delta, hitCoords[1] - delta]); - expect(renderInteractiveScene).toHaveBeenCalledTimes(14); - expect(renderStaticScene).toHaveBeenCalledTimes(8); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `13`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`8`); const newPoints = LinearElementEditor.getPointsGlobalCoordinates(line); expect([newPoints[0][0], newPoints[0][1]]).toEqual([ @@ -484,8 +494,10 @@ describe("Test Linear Elements", () => { // Drag from first point drag(hitCoords, [hitCoords[0] + delta, hitCoords[1] + delta]); - expect(renderInteractiveScene).toHaveBeenCalledTimes(14); - expect(renderStaticScene).toHaveBeenCalledTimes(8); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `13`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`8`); const newPoints = LinearElementEditor.getPointsGlobalCoordinates(line); expect([newPoints[0][0], newPoints[0][1]]).toEqual([ @@ -519,8 +531,10 @@ describe("Test Linear Elements", () => { // delete 3rd point deletePoint(points[2]); expect(line.points.length).toEqual(3); - expect(renderInteractiveScene).toHaveBeenCalledTimes(21); - expect(renderStaticScene).toHaveBeenCalledTimes(9); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `19`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`9`); const newMidPoints = LinearElementEditor.getEditorMidPoints( line, @@ -566,8 +580,10 @@ describe("Test Linear Elements", () => { lastSegmentMidpoint[0] + delta, lastSegmentMidpoint[1] + delta, ]); - expect(renderInteractiveScene).toHaveBeenCalledTimes(21); - expect(renderStaticScene).toHaveBeenCalledTimes(9); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `17`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`9`); expect(line.points.length).toEqual(5); expect((h.elements[0] as ExcalidrawLinearElement).points) @@ -642,8 +658,10 @@ describe("Test Linear Elements", () => { // Drag from first point drag(hitCoords, [hitCoords[0] + delta, hitCoords[1] + delta]); - expect(renderInteractiveScene).toHaveBeenCalledTimes(14); - expect(renderStaticScene).toHaveBeenCalledTimes(8); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `13`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`8`); const newPoints = LinearElementEditor.getPointsGlobalCoordinates(line); expect([newPoints[0][0], newPoints[0][1]]).toEqual([ diff --git a/packages/excalidraw/tests/move.test.tsx b/packages/excalidraw/tests/move.test.tsx index 248f43d7..22d828ee 100644 --- a/packages/excalidraw/tests/move.test.tsx +++ b/packages/excalidraw/tests/move.test.tsx @@ -42,8 +42,10 @@ describe("move element", () => { fireEvent.pointerMove(canvas, { clientX: 60, clientY: 70 }); fireEvent.pointerUp(canvas); - expect(renderInteractiveScene).toHaveBeenCalledTimes(6); - expect(renderStaticScene).toHaveBeenCalledTimes(6); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `6`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`6`); expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(1); expect(h.state.selectedElementIds[h.elements[0].id]).toBeTruthy(); @@ -57,8 +59,8 @@ describe("move element", () => { fireEvent.pointerMove(canvas, { clientX: 20, clientY: 40 }); fireEvent.pointerUp(canvas); - expect(renderInteractiveScene).toHaveBeenCalledTimes(3); - expect(renderStaticScene).toHaveBeenCalledTimes(2); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot(`3`); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`2`); expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(1); expect([h.elements[0].x, h.elements[0].y]).toEqual([0, 40]); @@ -84,8 +86,10 @@ describe("move element", () => { // select the second rectangles new Pointer("mouse").clickOn(rectB); - expect(renderInteractiveScene).toHaveBeenCalledTimes(24); - expect(renderStaticScene).toHaveBeenCalledTimes(19); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `21`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`19`); expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(3); expect(h.state.selectedElementIds[rectB.id]).toBeTruthy(); @@ -103,8 +107,8 @@ describe("move element", () => { Keyboard.keyDown(KEYS.ARROW_DOWN); // Check that the arrow size has been changed according to moving the rectangle - expect(renderInteractiveScene).toHaveBeenCalledTimes(3); - expect(renderStaticScene).toHaveBeenCalledTimes(3); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot(`3`); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`3`); expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(3); expect(h.state.selectedElementIds[rectB.id]).toBeTruthy(); @@ -130,8 +134,10 @@ describe("duplicate element on move when ALT is clicked", () => { fireEvent.pointerMove(canvas, { clientX: 60, clientY: 70 }); fireEvent.pointerUp(canvas); - expect(renderInteractiveScene).toHaveBeenCalledTimes(6); - expect(renderStaticScene).toHaveBeenCalledTimes(6); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot( + `6`, + ); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`6`); expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(1); expect(h.state.selectedElementIds[h.elements[0].id]).toBeTruthy(); @@ -152,8 +158,8 @@ describe("duplicate element on move when ALT is clicked", () => { // TODO: This used to be 4, but binding made it go up to 5. Do we need // that additional render? - expect(renderInteractiveScene).toHaveBeenCalledTimes(5); - expect(renderStaticScene).toHaveBeenCalledTimes(3); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot(`4`); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`3`); expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(2); diff --git a/packages/excalidraw/tests/multiPointCreate.test.tsx b/packages/excalidraw/tests/multiPointCreate.test.tsx index 93256d07..f462cfac 100644 --- a/packages/excalidraw/tests/multiPointCreate.test.tsx +++ b/packages/excalidraw/tests/multiPointCreate.test.tsx @@ -47,8 +47,8 @@ describe("remove shape in non linear elements", () => { fireEvent.pointerDown(canvas, { clientX: 30, clientY: 20 }); fireEvent.pointerUp(canvas, { clientX: 30, clientY: 30 }); - expect(renderInteractiveScene).toHaveBeenCalledTimes(5); - expect(renderStaticScene).toHaveBeenCalledTimes(5); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot(`5`); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`5`); expect(h.elements.length).toEqual(0); }); @@ -62,8 +62,8 @@ describe("remove shape in non linear elements", () => { fireEvent.pointerDown(canvas, { clientX: 30, clientY: 20 }); fireEvent.pointerUp(canvas, { clientX: 30, clientY: 30 }); - expect(renderInteractiveScene).toHaveBeenCalledTimes(5); - expect(renderStaticScene).toHaveBeenCalledTimes(5); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot(`5`); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`5`); expect(h.elements.length).toEqual(0); }); @@ -77,8 +77,8 @@ describe("remove shape in non linear elements", () => { fireEvent.pointerDown(canvas, { clientX: 30, clientY: 20 }); fireEvent.pointerUp(canvas, { clientX: 30, clientY: 30 }); - expect(renderInteractiveScene).toHaveBeenCalledTimes(5); - expect(renderStaticScene).toHaveBeenCalledTimes(5); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot(`5`); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`5`); expect(h.elements.length).toEqual(0); }); }); @@ -110,8 +110,8 @@ describe("multi point mode in linear elements", () => { key: KEYS.ENTER, }); - expect(renderInteractiveScene).toHaveBeenCalledTimes(11); - expect(renderStaticScene).toHaveBeenCalledTimes(9); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot(`9`); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`9`); expect(h.elements.length).toEqual(1); const element = h.elements[0] as ExcalidrawLinearElement; @@ -153,8 +153,8 @@ describe("multi point mode in linear elements", () => { fireEvent.keyDown(document, { key: KEYS.ENTER, }); - expect(renderInteractiveScene).toHaveBeenCalledTimes(11); - expect(renderStaticScene).toHaveBeenCalledTimes(9); + expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot(`9`); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`9`); expect(h.elements.length).toEqual(1); const element = h.elements[0] as ExcalidrawLinearElement; diff --git a/packages/excalidraw/utils.ts b/packages/excalidraw/utils.ts index 69b3426e..4278f36f 100644 --- a/packages/excalidraw/utils.ts +++ b/packages/excalidraw/utils.ts @@ -769,6 +769,24 @@ export const queryFocusableElements = (container: HTMLElement | null) => { : []; }; +/** use as a fallback after identity check (for perf reasons) */ +const _defaultIsShallowComparatorFallback = (a: any, b: any): boolean => { + // consider two empty arrays equal + if ( + Array.isArray(a) && + Array.isArray(b) && + a.length === 0 && + b.length === 0 + ) { + return true; + } + return a === b; +}; + +/** + * Returns whether object/array is shallow equal. + * Considers empty object/arrays as equal (whether top-level or second-level). + */ export const isShallowEqual = < T extends Record, K extends readonly unknown[], @@ -796,10 +814,12 @@ export const isShallowEqual = < if (comparators && Array.isArray(comparators)) { for (const key of comparators) { - const ret = objA[key] === objB[key]; + const ret = + objA[key] === objB[key] || + _defaultIsShallowComparatorFallback(objA[key], objB[key]); if (!ret) { if (debug) { - console.info( + console.warn( `%cisShallowEqual: ${key} not equal ->`, "color: #8B4000", objA[key], @@ -818,9 +838,11 @@ export const isShallowEqual = < )?.[key as keyof T]; const ret = comparator ? comparator(objA[key], objB[key]) - : objA[key] === objB[key]; + : objA[key] === objB[key] || + _defaultIsShallowComparatorFallback(objA[key], objB[key]); + if (!ret && debug) { - console.info( + console.warn( `%cisShallowEqual: ${key} not equal ->`, "color: #8B4000", objA[key], @@ -960,3 +982,13 @@ export const cloneJSON = (obj: T): T => JSON.parse(JSON.stringify(obj)); export const isFiniteNumber = (value: any): value is number => { return typeof value === "number" && Number.isFinite(value); }; + +export const updateStable = >( + prevValue: T, + nextValue: T, +) => { + if (isShallowEqual(prevValue, nextValue)) { + return prevValue; + } + return nextValue; +};