From e4c3744dc467b7ed06ef33a2ed85a8fd51737644 Mon Sep 17 00:00:00 2001 From: Marcel Mraz Date: Tue, 21 May 2024 12:58:31 +0100 Subject: [PATCH] Normalize indices on init --- excalidraw-app/App.tsx | 12 +++- excalidraw-app/collab/Collab.tsx | 12 +++- excalidraw-app/data/index.ts | 2 +- packages/excalidraw/data/blob.ts | 6 +- packages/excalidraw/data/restore.ts | 61 +++++++++++-------- packages/excalidraw/fractionalIndex.ts | 8 +++ packages/excalidraw/index.tsx | 2 + .../excalidraw/tests/data/restore.test.ts | 40 ++++++++++++ 8 files changed, 112 insertions(+), 31 deletions(-) diff --git a/excalidraw-app/App.tsx b/excalidraw-app/App.tsx index a11ea59b3..f1d0dd38d 100644 --- a/excalidraw-app/App.tsx +++ b/excalidraw-app/App.tsx @@ -26,6 +26,7 @@ import { TTDDialogTrigger, StoreAction, reconcileElements, + normalizeIndices, } from "../packages/excalidraw"; import type { AppState, @@ -305,14 +306,21 @@ const initializeScene = async (opts: { key: roomLinkData.roomKey, }; } else if (scene) { + const normalizedScene = { + ...scene, + // non-collab scenes are always always normalized on init + // collab scenes are normalized only on "first-in-room" as part of collabAPI + elements: normalizeIndices(scene.elements), + }; + return isExternalScene && jsonBackendMatch ? { - scene, + scene: normalizedScene, isExternalScene, id: jsonBackendMatch[1], key: jsonBackendMatch[2], } - : { scene, isExternalScene: false }; + : { scene: normalizedScene, isExternalScene: false }; } return { scene: null, isExternalScene: false }; }; diff --git a/excalidraw-app/collab/Collab.tsx b/excalidraw-app/collab/Collab.tsx index 7059a67c5..e6a492849 100644 --- a/excalidraw-app/collab/Collab.tsx +++ b/excalidraw-app/collab/Collab.tsx @@ -18,6 +18,7 @@ import { restoreElements, zoomToFitBounds, reconcileElements, + normalizeIndices, } from "../../packages/excalidraw"; import type { Collaborator, Gesture } from "../../packages/excalidraw/types"; import { @@ -637,7 +638,16 @@ class Collab extends PureComponent { fetchScene: true, roomLinkData: existingRoomLinkData, }); - scenePromise.resolve(sceneData); + + if (sceneData) { + scenePromise.resolve({ + ...sceneData, + // normalize fractional indices on init for shared scenes, while making sure there are no other collaborators + elements: normalizeIndices([...sceneData.elements]), + }); + } else { + scenePromise.resolve(null); + } }); this.portal.socket.on( diff --git a/excalidraw-app/data/index.ts b/excalidraw-app/data/index.ts index ba7df82b2..70264e84b 100644 --- a/excalidraw-app/data/index.ts +++ b/excalidraw-app/data/index.ts @@ -254,7 +254,7 @@ export const loadScene = async ( await importFromBackend(id, privateKey), localDataState?.appState, localDataState?.elements, - { repairBindings: true, refreshDimensions: false }, + { repairBindings: true }, ); } else { data = restore(localDataState || null, null, null, { diff --git a/packages/excalidraw/data/blob.ts b/packages/excalidraw/data/blob.ts index 1eb1e1bed..424ac8fd1 100644 --- a/packages/excalidraw/data/blob.ts +++ b/packages/excalidraw/data/blob.ts @@ -154,7 +154,11 @@ export const loadSceneOrLibraryFromBlob = async ( }, localAppState, localElements, - { repairBindings: true, refreshDimensions: false }, + { + repairBindings: true, + normalizeIndices: true, + refreshDimensions: false, + }, ), }; } else if (isValidLibrary(data)) { diff --git a/packages/excalidraw/data/restore.ts b/packages/excalidraw/data/restore.ts index c256e4e02..87e2fea26 100644 --- a/packages/excalidraw/data/restore.ts +++ b/packages/excalidraw/data/restore.ts @@ -46,9 +46,9 @@ import { arrayToMap } from "../utils"; import type { MarkOptional, Mutable } from "../utility-types"; import { detectLineHeight, getContainerElement } from "../element/textElement"; import { normalizeLink } from "./url"; -import { syncInvalidIndices } from "../fractionalIndex"; import { getSizeFromPoints } from "../points"; import { getLineHeight } from "../fonts"; +import { normalizeIndices, syncInvalidIndices } from "../fractionalIndex"; type RestoredAppState = Omit< AppState, @@ -405,36 +405,41 @@ export const restoreElements = ( elements: ImportedDataState["elements"], /** NOTE doesn't serve for reconciliation */ localElements: readonly ExcalidrawElement[] | null | undefined, - opts?: { refreshDimensions?: boolean; repairBindings?: boolean } | undefined, + opts?: + | { + refreshDimensions?: boolean; + repairBindings?: boolean; + normalizeIndices?: boolean; + } + | undefined, ): OrderedExcalidrawElement[] => { // used to detect duplicate top-level element ids const existingIds = new Set(); const localElementsMap = localElements ? arrayToMap(localElements) : null; - const restoredElements = syncInvalidIndices( - (elements || []).reduce((elements, element) => { - // filtering out selection, which is legacy, no longer kept in elements, - // and causing issues if retained - if (element.type !== "selection" && !isInvisiblySmallElement(element)) { - let migratedElement: ExcalidrawElement | null = restoreElement(element); - if (migratedElement) { - const localElement = localElementsMap?.get(element.id); - if (localElement && localElement.version > migratedElement.version) { - migratedElement = bumpVersion( - migratedElement, - localElement.version, - ); - } - if (existingIds.has(migratedElement.id)) { - migratedElement = { ...migratedElement, id: randomId() }; - } - existingIds.add(migratedElement.id); - - elements.push(migratedElement); + const restoredElementsTemp = (elements || []).reduce((elements, element) => { + // filtering out selection, which is legacy, no longer kept in elements, + // and causing issues if retained + if (element.type !== "selection" && !isInvisiblySmallElement(element)) { + let migratedElement: ExcalidrawElement | null = restoreElement(element); + if (migratedElement) { + const localElement = localElementsMap?.get(element.id); + if (localElement && localElement.version > migratedElement.version) { + migratedElement = bumpVersion(migratedElement, localElement.version); } + if (existingIds.has(migratedElement.id)) { + migratedElement = { ...migratedElement, id: randomId() }; + } + existingIds.add(migratedElement.id); + + elements.push(migratedElement); } - return elements; - }, [] as ExcalidrawElement[]), - ); + } + return elements; + }, [] as ExcalidrawElement[]); + + const restoredElements = opts?.normalizeIndices + ? normalizeIndices(restoredElementsTemp) + : syncInvalidIndices(restoredElementsTemp); if (!opts?.repairBindings) { return restoredElements; @@ -601,7 +606,11 @@ export const restore = ( */ localAppState: Partial | null | undefined, localElements: readonly ExcalidrawElement[] | null | undefined, - elementsConfig?: { refreshDimensions?: boolean; repairBindings?: boolean }, + elementsConfig?: { + refreshDimensions?: boolean; + repairBindings?: boolean; + normalizeIndices?: boolean; + }, ): RestoredDataState => { return { elements: restoreElements(data?.elements, localElements, elementsConfig), diff --git a/packages/excalidraw/fractionalIndex.ts b/packages/excalidraw/fractionalIndex.ts index 01b6d7015..acef46206 100644 --- a/packages/excalidraw/fractionalIndex.ts +++ b/packages/excalidraw/fractionalIndex.ts @@ -6,6 +6,14 @@ import type { OrderedExcalidrawElement, } from "./element/types"; import { InvalidFractionalIndexError } from "./errors"; +import { arrayToMap } from "./utils"; + +/** + * Normalizes indices for all elements, to prevent possible issues caused by using stale (too old, too long) indices. + */ +export const normalizeIndices = (elements: ExcalidrawElement[]) => { + return syncMovedIndices(elements, arrayToMap(elements)); +}; /** * Envisioned relation between array order and fractional indices: diff --git a/packages/excalidraw/index.tsx b/packages/excalidraw/index.tsx index 645e5ea8d..010148a39 100644 --- a/packages/excalidraw/index.tsx +++ b/packages/excalidraw/index.tsx @@ -222,6 +222,8 @@ export { restoreLibraryItems, } from "./data/restore"; +export { normalizeIndices } from "./fractionalIndex"; + export { reconcileElements } from "./data/reconcile"; export { diff --git a/packages/excalidraw/tests/data/restore.test.ts b/packages/excalidraw/tests/data/restore.test.ts index ef1e3198b..0b6af8bb6 100644 --- a/packages/excalidraw/tests/data/restore.test.ts +++ b/packages/excalidraw/tests/data/restore.test.ts @@ -4,6 +4,7 @@ import type { ExcalidrawFreeDrawElement, ExcalidrawLinearElement, ExcalidrawTextElement, + FractionalIndex, } from "../../element/types"; import * as sizeHelpers from "../../element/sizeHelpers"; import { API } from "../helpers/api"; @@ -579,6 +580,45 @@ describe("restore", () => { }); }); +describe("normalize indices", () => { + it("shoudl normalize indices of all elements when normalize is true", () => { + const ellipse = API.createElement({ + type: "ellipse", + index: "Zz" as FractionalIndex, + }); + const container = API.createElement({ + type: "rectangle", + index: undefined, + }); + const boundElement = API.createElement({ + type: "text", + containerId: container.id, + index: "a0000000000000000000000" as FractionalIndex, + }); + + const restoredElements = restore.restoreElements( + [ellipse, container, boundElement], + null, + { normalizeIndices: true }, + ); + + expect(restoredElements).toEqual([ + expect.objectContaining({ + id: ellipse.id, + index: "a0", + }), + expect.objectContaining({ + id: container.id, + index: "a1", + }), + expect.objectContaining({ + id: boundElement.id, + index: "a2", + }), + ]); + }); +}); + describe("repairing bindings", () => { it("should repair container boundElements when repair is true", () => { const container = API.createElement({