Compare commits

...

1 Commits

Author SHA1 Message Date
Marcel Mraz
e4c3744dc4 Normalize indices on init 2024-07-29 23:38:44 +02:00
8 changed files with 112 additions and 31 deletions

View File

@ -26,6 +26,7 @@ import {
TTDDialogTrigger, TTDDialogTrigger,
StoreAction, StoreAction,
reconcileElements, reconcileElements,
normalizeIndices,
} from "../packages/excalidraw"; } from "../packages/excalidraw";
import type { import type {
AppState, AppState,
@ -305,14 +306,21 @@ const initializeScene = async (opts: {
key: roomLinkData.roomKey, key: roomLinkData.roomKey,
}; };
} else if (scene) { } 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 return isExternalScene && jsonBackendMatch
? { ? {
scene, scene: normalizedScene,
isExternalScene, isExternalScene,
id: jsonBackendMatch[1], id: jsonBackendMatch[1],
key: jsonBackendMatch[2], key: jsonBackendMatch[2],
} }
: { scene, isExternalScene: false }; : { scene: normalizedScene, isExternalScene: false };
} }
return { scene: null, isExternalScene: false }; return { scene: null, isExternalScene: false };
}; };

View File

@ -18,6 +18,7 @@ import {
restoreElements, restoreElements,
zoomToFitBounds, zoomToFitBounds,
reconcileElements, reconcileElements,
normalizeIndices,
} from "../../packages/excalidraw"; } from "../../packages/excalidraw";
import type { Collaborator, Gesture } from "../../packages/excalidraw/types"; import type { Collaborator, Gesture } from "../../packages/excalidraw/types";
import { import {
@ -637,7 +638,16 @@ class Collab extends PureComponent<CollabProps, CollabState> {
fetchScene: true, fetchScene: true,
roomLinkData: existingRoomLinkData, 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( this.portal.socket.on(

View File

@ -254,7 +254,7 @@ export const loadScene = async (
await importFromBackend(id, privateKey), await importFromBackend(id, privateKey),
localDataState?.appState, localDataState?.appState,
localDataState?.elements, localDataState?.elements,
{ repairBindings: true, refreshDimensions: false }, { repairBindings: true },
); );
} else { } else {
data = restore(localDataState || null, null, null, { data = restore(localDataState || null, null, null, {

View File

@ -154,7 +154,11 @@ export const loadSceneOrLibraryFromBlob = async (
}, },
localAppState, localAppState,
localElements, localElements,
{ repairBindings: true, refreshDimensions: false }, {
repairBindings: true,
normalizeIndices: true,
refreshDimensions: false,
},
), ),
}; };
} else if (isValidLibrary(data)) { } else if (isValidLibrary(data)) {

View File

@ -46,9 +46,9 @@ import { arrayToMap } from "../utils";
import type { MarkOptional, Mutable } from "../utility-types"; import type { MarkOptional, Mutable } from "../utility-types";
import { detectLineHeight, getContainerElement } from "../element/textElement"; import { detectLineHeight, getContainerElement } from "../element/textElement";
import { normalizeLink } from "./url"; import { normalizeLink } from "./url";
import { syncInvalidIndices } from "../fractionalIndex";
import { getSizeFromPoints } from "../points"; import { getSizeFromPoints } from "../points";
import { getLineHeight } from "../fonts"; import { getLineHeight } from "../fonts";
import { normalizeIndices, syncInvalidIndices } from "../fractionalIndex";
type RestoredAppState = Omit< type RestoredAppState = Omit<
AppState, AppState,
@ -405,13 +405,18 @@ export const restoreElements = (
elements: ImportedDataState["elements"], elements: ImportedDataState["elements"],
/** NOTE doesn't serve for reconciliation */ /** NOTE doesn't serve for reconciliation */
localElements: readonly ExcalidrawElement[] | null | undefined, localElements: readonly ExcalidrawElement[] | null | undefined,
opts?: { refreshDimensions?: boolean; repairBindings?: boolean } | undefined, opts?:
| {
refreshDimensions?: boolean;
repairBindings?: boolean;
normalizeIndices?: boolean;
}
| undefined,
): OrderedExcalidrawElement[] => { ): OrderedExcalidrawElement[] => {
// used to detect duplicate top-level element ids // used to detect duplicate top-level element ids
const existingIds = new Set<string>(); const existingIds = new Set<string>();
const localElementsMap = localElements ? arrayToMap(localElements) : null; const localElementsMap = localElements ? arrayToMap(localElements) : null;
const restoredElements = syncInvalidIndices( const restoredElementsTemp = (elements || []).reduce((elements, element) => {
(elements || []).reduce((elements, element) => {
// filtering out selection, which is legacy, no longer kept in elements, // filtering out selection, which is legacy, no longer kept in elements,
// and causing issues if retained // and causing issues if retained
if (element.type !== "selection" && !isInvisiblySmallElement(element)) { if (element.type !== "selection" && !isInvisiblySmallElement(element)) {
@ -419,10 +424,7 @@ export const restoreElements = (
if (migratedElement) { if (migratedElement) {
const localElement = localElementsMap?.get(element.id); const localElement = localElementsMap?.get(element.id);
if (localElement && localElement.version > migratedElement.version) { if (localElement && localElement.version > migratedElement.version) {
migratedElement = bumpVersion( migratedElement = bumpVersion(migratedElement, localElement.version);
migratedElement,
localElement.version,
);
} }
if (existingIds.has(migratedElement.id)) { if (existingIds.has(migratedElement.id)) {
migratedElement = { ...migratedElement, id: randomId() }; migratedElement = { ...migratedElement, id: randomId() };
@ -433,8 +435,11 @@ export const restoreElements = (
} }
} }
return elements; return elements;
}, [] as ExcalidrawElement[]), }, [] as ExcalidrawElement[]);
);
const restoredElements = opts?.normalizeIndices
? normalizeIndices(restoredElementsTemp)
: syncInvalidIndices(restoredElementsTemp);
if (!opts?.repairBindings) { if (!opts?.repairBindings) {
return restoredElements; return restoredElements;
@ -601,7 +606,11 @@ export const restore = (
*/ */
localAppState: Partial<AppState> | null | undefined, localAppState: Partial<AppState> | null | undefined,
localElements: readonly ExcalidrawElement[] | null | undefined, localElements: readonly ExcalidrawElement[] | null | undefined,
elementsConfig?: { refreshDimensions?: boolean; repairBindings?: boolean }, elementsConfig?: {
refreshDimensions?: boolean;
repairBindings?: boolean;
normalizeIndices?: boolean;
},
): RestoredDataState => { ): RestoredDataState => {
return { return {
elements: restoreElements(data?.elements, localElements, elementsConfig), elements: restoreElements(data?.elements, localElements, elementsConfig),

View File

@ -6,6 +6,14 @@ import type {
OrderedExcalidrawElement, OrderedExcalidrawElement,
} from "./element/types"; } from "./element/types";
import { InvalidFractionalIndexError } from "./errors"; 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: * Envisioned relation between array order and fractional indices:

View File

@ -222,6 +222,8 @@ export {
restoreLibraryItems, restoreLibraryItems,
} from "./data/restore"; } from "./data/restore";
export { normalizeIndices } from "./fractionalIndex";
export { reconcileElements } from "./data/reconcile"; export { reconcileElements } from "./data/reconcile";
export { export {

View File

@ -4,6 +4,7 @@ import type {
ExcalidrawFreeDrawElement, ExcalidrawFreeDrawElement,
ExcalidrawLinearElement, ExcalidrawLinearElement,
ExcalidrawTextElement, ExcalidrawTextElement,
FractionalIndex,
} from "../../element/types"; } from "../../element/types";
import * as sizeHelpers from "../../element/sizeHelpers"; import * as sizeHelpers from "../../element/sizeHelpers";
import { API } from "../helpers/api"; 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", () => { describe("repairing bindings", () => {
it("should repair container boundElements when repair is true", () => { it("should repair container boundElements when repair is true", () => {
const container = API.createElement({ const container = API.createElement({