sambruk/excalidraw
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: pass localElements to restore and restoreElement API's and bump versions of duplicate elements on import (#3797)

Browse Source
This commit is contained in:
David Luzar 2021-07-04 22:23:35 +02:00 committed by GitHub
parent 038e9c13dd
commit 097362662d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 164 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/actions/actionExport.tsx
View File

@ -201,7 +201,7 @@ export const actionLoadScene = register({
const {
elements: loadedElements,
appState: loadedAppState,
} = await loadFromJSON(appState);
} = await loadFromJSON(appState, elements);
return {
elements: loadedElements,
appState: loadedAppState,

16
src/components/App.tsx
View File

@ -654,7 +654,7 @@ class App extends React.Component<AppProps, AppState> {
const fileHandle = launchParams.files[0];
const blob: Blob = await fileHandle.getFile();
blob.handle = fileHandle;
loadFromBlob(blob, this.state)
loadFromBlob(blob, this.state, this.scene.getElements())
.then(({ elements, appState }) =>
this.syncActionResult({
elements,
@ -692,7 +692,7 @@ class App extends React.Component<AppProps, AppState> {
};
}
const scene = restore(initialData, null);
const scene = restore(initialData, null, null);
scene.appState = {
...scene.appState,
isLoading: false,
@ -1201,7 +1201,7 @@ class App extends React.Component<AppProps, AppState> {
});
} else if (data.elements) {
this.addElementsFromPasteOrLibrary({
elements: restoreElements(data.elements),
elements: data.elements,
position: "cursor",
});
} else if (data.text) {
@ -1216,7 +1216,7 @@ class App extends React.Component<AppProps, AppState> {
elements: readonly ExcalidrawElement[];
position: { clientX: number; clientY: number } | "cursor" | "center";
}) => {
const elements = restoreElements(opts.elements);
const elements = restoreElements(opts.elements, null);
const [minX, minY, maxX, maxY] = getCommonBounds(elements);
const elementsCenterX = distance(minX, maxX) / 2;
@ -3805,7 +3805,11 @@ class App extends React.Component<AppProps, AppState> {
try {
const file = event.dataTransfer.files[0];
if (file?.type === "image/png" || file?.type === "image/svg+xml") {
const { elements, appState } = await loadFromBlob(file, this.state);
const { elements, appState } = await loadFromBlob(
file,
this.state,
this.scene.getElements(),
);
this.syncActionResult({
elements,
appState: {
@ -3865,7 +3869,7 @@ class App extends React.Component<AppProps, AppState> {
};
loadFileToCanvas = (file: Blob) => {
loadFromBlob(file, this.state)
loadFromBlob(file, this.state, this.scene.getElements())
.then(({ elements, appState }) =>
this.syncActionResult({
elements,

3
src/data/blob.ts
View File

@ -1,6 +1,7 @@
import { cleanAppStateForExport } from "../appState";
import { EXPORT_DATA_TYPES } from "../constants";
import { clearElementsForExport } from "../element";
import { ExcalidrawElement } from "../element/types";
import { CanvasError } from "../errors";
import { t } from "../i18n";
import { calculateScrollCenter } from "../scene";
@ -83,6 +84,7 @@ export const loadFromBlob = async (
blob: Blob,
/** @see restore.localAppState */
localAppState: AppState | null,
localElements: readonly ExcalidrawElement[] | null,
) => {
const contents = await parseFileContents(blob);
try {
@ -103,6 +105,7 @@ export const loadFromBlob = async (
},
},
localAppState,
localElements,
);
return result;

7
src/data/json.ts
View File

@ -49,7 +49,10 @@ export const saveAsJSON = async (
return { fileHandle };
};
export const loadFromJSON = async (localAppState: AppState) => {
export const loadFromJSON = async (
localAppState: AppState,
localElements: readonly ExcalidrawElement[] | null,
) => {
const blob = await fileOpen({
description: "Excalidraw files",
// ToDo: Be over-permissive until https://bugs.webkit.org/show_bug.cgi?id=34442
@ -64,7 +67,7 @@ export const loadFromJSON = async (localAppState: AppState) => {
],
*/
});
return loadFromBlob(blob, localAppState);
return loadFromBlob(blob, localAppState, localElements);
};
export const isValidExcalidrawData = (data?: {

2
src/data/library.ts
View File

@ -18,7 +18,7 @@ class Library {
};
restoreLibraryItem = (libraryItem: LibraryItem): LibraryItem | null => {
const elements = getNonDeletedElements(restoreElements(libraryItem));
const elements = getNonDeletedElements(restoreElements(libraryItem, null));
return elements.length ? elements : null;
};

31
src/data/restore.ts
View File

@ -5,7 +5,11 @@ import {
} from "../element/types";
import { AppState, NormalizedZoomValue } from "../types";
import { ImportedDataState } from "./types";
import { getNormalizedDimensions, isInvisiblySmallElement } from "../element";
import {
getElementMap,
getNormalizedDimensions,
isInvisiblySmallElement,
} from "../element";
import { isLinearElementType } from "../element/typeChecks";
import { randomId } from "../random";
import {
@ -16,6 +20,7 @@ import {
} from "../constants";
import { getDefaultAppState } from "../appState";
import { LinearElementEditor } from "../element/linearElementEditor";
import { bumpVersion } from "../element/mutateElement";
type RestoredAppState = Omit<
AppState,
@ -181,13 +186,20 @@ const restoreElement = (
export const restoreElements = (
elements: ImportedDataState["elements"],
/** NOTE doesn't serve for reconciliation */
localElements: readonly ExcalidrawElement[] | null | undefined,
): ExcalidrawElement[] => {
const localElementsMap = localElements ? getElementMap(localElements) : null;
return (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)) {
const migratedElement = restoreElement(element);
let migratedElement: ExcalidrawElement = restoreElement(element);
if (migratedElement) {
const localElement = localElementsMap?.[element.id];
if (localElement && localElement.version > migratedElement.version) {
migratedElement = bumpVersion(migratedElement, localElement.version);
}
elements.push(migratedElement);
}
}
@ -197,25 +209,25 @@ export const restoreElements = (
export const restoreAppState = (
appState: ImportedDataState["appState"],
localAppState: Partial<AppState> | null,
localAppState: Partial<AppState> | null | undefined,
): RestoredAppState => {
appState = appState || {};
const defaultAppState = getDefaultAppState();
const nextAppState = {} as typeof defaultAppState;
for (const [key, val] of Object.entries(defaultAppState) as [
for (const [key, defaultValue] of Object.entries(defaultAppState) as [
keyof typeof defaultAppState,
any,
][]) {
const restoredValue = appState[key];
const suppliedValue = appState[key];
const localValue = localAppState ? localAppState[key] : undefined;
(nextAppState as any)[key] =
restoredValue !== undefined
? restoredValue
suppliedValue !== undefined
? suppliedValue
: localValue !== undefined
? localValue
: val;
: defaultValue;
}
return {
@ -243,9 +255,10 @@ export const restore = (
* Supply `null` if you can't get access to it.
*/
localAppState: Partial<AppState> | null | undefined,
localElements: readonly ExcalidrawElement[] | null | undefined,
): RestoredDataState => {
return {
elements: restoreElements(data?.elements),
elements: restoreElements(data?.elements, localElements),
appState: restoreAppState(data?.appState, localAppState || null),
};
};

7
src/element/mutateElement.ts
View File

@ -120,8 +120,11 @@ export const newElementWith = <TElement extends ExcalidrawElement>(
*
* NOTE: does not trigger re-render.
*/
export const bumpVersion = (element: Mutable<ExcalidrawElement>) => {
element.version = element.version + 1;
export const bumpVersion = (
element: Mutable<ExcalidrawElement>,
version?: ExcalidrawElement["version"],
) => {
element.version = (version ?? element.version) + 1;
element.versionNonce = randomInteger();
return element;
};

2
src/excalidraw-app/data/firebase.ts
View File

@ -196,5 +196,5 @@ export const loadFromFirebase = async (
firebaseSceneVersionCache.set(socket, getSceneVersion(elements));
}
return restoreElements(elements);
return restoreElements(elements, null);
};

3
src/excalidraw-app/data/index.ts
View File

@ -257,9 +257,10 @@ export const loadScene = async (
data = restore(
await importFromBackend(id, privateKey),
localDataState?.appState,
localDataState?.elements,
);
} else {
data = restore(localDataState || null, null);
data = restore(localDataState || null, null, null);
}
return {

2
src/excalidraw-app/index.tsx
View File

@ -141,7 +141,7 @@ const initializeScene = async (opts: {
const url = externalUrlMatch[1];
try {
const request = await fetch(window.decodeURIComponent(url));
const data = await loadFromBlob(await request.blob(), null);
const data = await loadFromBlob(await request.blob(), null, null);
if (
!scene.elements.length ||
window.confirm(t("alerts.loadSceneOverridePrompt"))

6
src/packages/excalidraw/CHANGELOG.md
View File

@ -19,6 +19,12 @@ Please add the latest change on the top under the correct section.
### Features
- [`restore(data, localAppState, localElements)`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#restore) and [`restoreElements(elements, localElements)`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#restoreElements) now take `localElements` argument which will be used to ensure existing elements' versions are used and incremented. This fixes an issue where importing the same file would resolve to elements with older versions, potentially causing issues when reconciling [#3797](https://github.com/excalidraw/excalidraw/pull/3797).
#### BREAKING CHANGE
- `localElements` argument is mandatory (can be `null`/`undefined`) if using TypeScript.
- Support `appState.exportEmbedScene` attribute in [`exportToSvg`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#exportToSvg) which allows to embed the scene data.
#### BREAKING CHANGE

16
src/packages/excalidraw/README_NEXT.md
View File

@ -701,7 +701,7 @@ This function returns an object where each element is mapped to its id.
**_Signature_**
<pre>
restoreAppState(appState: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L17">ImportedDataState["appState"]</a>, localAppState: Partial<<a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L42">AppState</a>> | null): <a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L42">AppState</a>
restoreAppState(appState: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L17">ImportedDataState["appState"]</a>, localAppState: Partial<<a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L42">AppState</a>> | null): <a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L42">AppState</a>
</pre>
**_How to use_**
@ -710,14 +710,16 @@ restoreAppState(appState: <a href="https://github.com/excalidraw/excalidraw/blo
import { restoreAppState } from "@excalidraw/excalidraw-next";
```
This function will make sure all the keys have appropriate values in [appState](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L42) and if any key is missing, it will be set to default value. If you pass `localAppState`, `localAppState` value will be preferred over the `appState` passed in params.
This function will make sure all the keys have appropriate values in [appState](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L42) and if any key is missing, it will be set to default value.
When `localAppState` is supplied, it's used in place of values that are missing (`undefined`) in `appState` instead of defaults. Use this as a way to not override user's defaults if you persist them. Required: supply `null`/`undefined` if not applicable.
#### `restoreElements`
**_Signature_**
<pre>
restoreElements(elements: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L16">ImportedDataState["elements"]</a>): <a href="https://github.com/excalidraw/excalidraw/blob/master/src/element/types.ts#L78">ExcalidrawElement[]</a>
restoreElements(elements: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L16">ImportedDataState["elements"]</a>, localElements: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L16">ExcalidrawElement[]</a> | null | undefined): <a href="https://github.com/excalidraw/excalidraw/blob/master/src/element/types.ts#L78">ExcalidrawElement[]</a>
</pre>
**_How to use_**
@ -728,21 +730,25 @@ import { restoreElements } from "@excalidraw/excalidraw-next";
This function will make sure all properties of element is correctly set and if any attribute is missing, it will be set to default value.
When `localElements` are supplied, they are used to ensure that existing restored elements reuse `version` (and increment it), and regenerate `versionNonce`. Use this when you import elements which may already be present in the scene to ensure that you do not disregard the newly imported elements if you're using element version to detect the updates.
#### `restore`
**_Signature_**
<pre>
restoreElements(data: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L12">ImportedDataState</a>): <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L4">DataState</a>
restoreElements(data: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L12">ImportedDataState</a>, localAppState: Partial<<a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L42">AppState</a>> | null | undefined, localElements: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L16">ExcalidrawElement[]</a> | null | undefined): <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L4">DataState</a>
</pre>
See [`restoreAppState()`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#restoreAppState) about `localAppState`, and [`restoreElements()`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#restoreElements) about `localElements`.
**_How to use_**
```js
import { restore } from "@excalidraw/excalidraw-next";
```
This function makes sure elements and state is set to appropriate values and set to default value if not present. It is combination of [restoreElements](#restoreElements) and [restoreAppState](#restoreAppState)
This function makes sure elements and state is set to appropriate values and set to default value if not present. It is a combination of [restoreElements](#restoreElements) and [restoreAppState](#restoreAppState).
#### `serializeAsJSON`

2
src/packages/utils.ts
View File

@ -25,6 +25,7 @@ export const exportToCanvas = ({
const { elements: restoredElements, appState: restoredAppState } = restore(
{ elements, appState },
null,
null,
);
const { exportBackground, viewBackgroundColor } = restoredAppState;
return _exportToCanvas(
@ -84,6 +85,7 @@ export const exportToSvg = async ({
const { elements: restoredElements, appState: restoredAppState } = restore(
{ elements, appState },
null,
null,
);
return _exportToSvg(getNonDeletedElements(restoredElements), {
...restoredAppState,

126
src/tests/data/restore.test.ts
View File

@ -11,6 +11,7 @@ import { getDefaultAppState } from "../../appState";
import { ImportedDataState } from "../../data/types";
import { NormalizedZoomValue } from "../../types";
import { FONT_FAMILY } from "../../constants";
import { newElementWith } from "../../element/mutateElement";
const mockSizeHelper = jest.spyOn(sizeHelpers, "isInvisiblySmallElement");
@ -20,12 +21,12 @@ beforeEach(() => {
describe("restoreElements", () => {
it("should return empty array when element is null", () => {
expect(restore.restoreElements(null)).toStrictEqual([]);
expect(restore.restoreElements(null, null)).toStrictEqual([]);
});
it("should not call isInvisiblySmallElement when element is a selection element", () => {
const selectionEl = { type: "selection" } as ExcalidrawElement;
const restoreElements = restore.restoreElements([selectionEl]);
const restoreElements = restore.restoreElements([selectionEl], null);
expect(restoreElements.length).toBe(0);
expect(sizeHelpers.isInvisiblySmallElement).toBeCalledTimes(0);
});
@ -36,14 +37,16 @@ describe("restoreElements", () => {
});
dummyNotSupportedElement.type = "not supported";
expect(restore.restoreElements([dummyNotSupportedElement]).length).toBe(0);
expect(
restore.restoreElements([dummyNotSupportedElement], null).length,
).toBe(0);
});
it("should return empty array when isInvisiblySmallElement is true", () => {
const rectElement = API.createElement({ type: "rectangle" });
mockSizeHelper.mockImplementation(() => true);
expect(restore.restoreElements([rectElement]).length).toBe(0);
expect(restore.restoreElements([rectElement], null).length).toBe(0);
});
it("should restore text element correctly passing value for each attribute", () => {
@ -57,9 +60,10 @@ describe("restoreElements", () => {
id: "id-text01",
});
const restoredText = restore.restoreElements([
textElement,
])[0] as ExcalidrawTextElement;
const restoredText = restore.restoreElements(
[textElement],
null,
)[0] as ExcalidrawTextElement;
expect(restoredText).toMatchSnapshot({
seed: expect.any(Number),
@ -77,9 +81,10 @@ describe("restoreElements", () => {
textElement.text = null;
textElement.font = "10 unknown";
const restoredText = restore.restoreElements([
textElement,
])[0] as ExcalidrawTextElement;
const restoredText = restore.restoreElements(
[textElement],
null,
)[0] as ExcalidrawTextElement;
expect(restoredText).toMatchSnapshot({
seed: expect.any(Number),
});
@ -91,9 +96,10 @@ describe("restoreElements", () => {
id: "id-freedraw01",
});
const restoredFreedraw = restore.restoreElements([
freedrawElement,
])[0] as ExcalidrawFreeDrawElement;
const restoredFreedraw = restore.restoreElements(
[freedrawElement],
null,
)[0] as ExcalidrawFreeDrawElement;
expect(restoredFreedraw).toMatchSnapshot({ seed: expect.any(Number) });
});
@ -107,10 +113,10 @@ describe("restoreElements", () => {
});
drawElement.type = "draw";
const restoredElements = restore.restoreElements([
lineElement,
drawElement,
]);
const restoredElements = restore.restoreElements(
[lineElement, drawElement],
null,
);
const restoredLine = restoredElements[0] as ExcalidrawLinearElement;
const restoredDraw = restoredElements[1] as ExcalidrawLinearElement;
@ -122,7 +128,7 @@ describe("restoreElements", () => {
it("should restore arrow element correctly", () => {
const arrowElement = API.createElement({ type: "arrow", id: "id-arrow01" });
const restoredElements = restore.restoreElements([arrowElement]);
const restoredElements = restore.restoreElements([arrowElement], null);
const restoredArrow = restoredElements[0] as ExcalidrawLinearElement;
@ -132,7 +138,7 @@ describe("restoreElements", () => {
it("when arrow element has defined endArrowHead", () => {
const arrowElement = API.createElement({ type: "arrow" });
const restoredElements = restore.restoreElements([arrowElement]);
const restoredElements = restore.restoreElements([arrowElement], null);
const restoredArrow = restoredElements[0] as ExcalidrawLinearElement;
@ -145,7 +151,7 @@ describe("restoreElements", () => {
get: jest.fn(() => undefined),
});
const restoredElements = restore.restoreElements([arrowElement]);
const restoredElements = restore.restoreElements([arrowElement], null);
const restoredArrow = restoredElements[0] as ExcalidrawLinearElement;
@ -166,9 +172,10 @@ describe("restoreElements", () => {
[lineElement.width, lineElement.height],
];
const restoredLine = restore.restoreElements([
lineElement,
])[0] as ExcalidrawLinearElement;
const restoredLine = restore.restoreElements(
[lineElement],
null,
)[0] as ExcalidrawLinearElement;
expect(restoredLine.points).toMatchObject(expectedLinePoints);
});
@ -205,10 +212,10 @@ describe("restoreElements", () => {
get: jest.fn(() => pointsEl_1),
});
const restoredElements = restore.restoreElements([
lineElement_0,
lineElement_1,
]);
const restoredElements = restore.restoreElements(
[lineElement_0, lineElement_1],
null,
);
const restoredLine_0 = restoredElements[0] as ExcalidrawLinearElement;
const restoredLine_1 = restoredElements[1] as ExcalidrawLinearElement;
@ -254,12 +261,37 @@ describe("restoreElements", () => {
elements.push(element);
});
const restoredElements = restore.restoreElements(elements);
const restoredElements = restore.restoreElements(elements, null);
expect(restoredElements[0]).toMatchSnapshot({ seed: expect.any(Number) });
expect(restoredElements[1]).toMatchSnapshot({ seed: expect.any(Number) });
expect(restoredElements[2]).toMatchSnapshot({ seed: expect.any(Number) });
});
it("bump versions of local duplicate elements when supplied", () => {
const rectangle = API.createElement({ type: "rectangle" });
const ellipse = API.createElement({ type: "ellipse" });
const rectangle_modified = newElementWith(rectangle, { isDeleted: true });
const restoredElements = restore.restoreElements(
[rectangle, ellipse],
[rectangle_modified],
);
expect(restoredElements[0].id).toBe(rectangle.id);
expect(restoredElements[0].versionNonce).not.toBe(rectangle.versionNonce);
expect(restoredElements).toEqual([
expect.objectContaining({
id: rectangle.id,
version: rectangle_modified.version + 1,
}),
expect.objectContaining({
id: ellipse.id,
version: ellipse.version,
versionNonce: ellipse.versionNonce,
}),
]);
});
});
describe("restoreAppState", () => {
@ -429,7 +461,7 @@ describe("restore", () => {
it("when imported data state is null it should return an empty array of elements", () => {
const stubLocalAppState = getDefaultAppState();
const restoredData = restore.restore(null, stubLocalAppState);
const restoredData = restore.restore(null, stubLocalAppState, null);
expect(restoredData.elements.length).toBe(0);
});
@ -438,7 +470,7 @@ describe("restore", () => {
stubLocalAppState.cursorButton = "down";
stubLocalAppState.name = "local app state";
const restoredData = restore.restore(null, stubLocalAppState);
const restoredData = restore.restore(null, stubLocalAppState, null);
expect(restoredData.appState.cursorButton).toBe(
stubLocalAppState.cursorButton,
);
@ -455,7 +487,11 @@ describe("restore", () => {
const importedDataState = {} as ImportedDataState;
importedDataState.elements = elements;
const restoredData = restore.restore(importedDataState, stubLocalAppState);
const restoredData = restore.restore(
importedDataState,
stubLocalAppState,
null,
);
expect(restoredData.elements.length).toBe(elements.length);
});
@ -467,10 +503,36 @@ describe("restore", () => {
const importedDataState = {} as ImportedDataState;
importedDataState.appState = stubImportedAppState;
const restoredData = restore.restore(importedDataState, null);
const restoredData = restore.restore(importedDataState, null, null);
expect(restoredData.appState.cursorButton).toBe(
stubImportedAppState.cursorButton,
);
expect(restoredData.appState.name).toBe(stubImportedAppState.name);
});
it("bump versions of local duplicate elements when supplied", () => {
const rectangle = API.createElement({ type: "rectangle" });
const ellipse = API.createElement({ type: "ellipse" });
const rectangle_modified = newElementWith(rectangle, { isDeleted: true });
const restoredData = restore.restore(
{ elements: [rectangle, ellipse] },
null,
[rectangle_modified],
);
expect(restoredData.elements[0].id).toBe(rectangle.id);
expect(restoredData.elements[0].versionNonce).not.toBe(
rectangle.versionNonce,
);
expect(restoredData.elements).toEqual([
expect.objectContaining({ version: rectangle_modified.version + 1 }),
expect.objectContaining({
id: ellipse.id,
version: ellipse.version,
versionNonce: ellipse.versionNonce,
}),
]);
});
});