fix: library init/import race conditions (#5101)

This commit is contained in:
David Luzar 2022-04-29 16:45:02 +02:00 committed by GitHub
parent 6a0f800716
commit d53ac2a61e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 248 additions and 133 deletions

View File

@ -25,9 +25,9 @@ export const actionAddToLibrary = register({
}
return app.library
.loadLibrary()
.getLatestLibrary()
.then((items) => {
return app.library.saveLibrary([
return app.library.setLibrary([
{
id: randomId(),
status: "unpublished",

View File

@ -257,6 +257,7 @@ import {
isPointHittingLinkIcon,
isLocalLink,
} from "../element/Hyperlink";
import { AbortError } from "../errors";
const defaultDeviceTypeContext: DeviceType = {
isMobile: false,
@ -703,21 +704,35 @@ class App extends React.Component<AppProps, AppState> {
window.history.replaceState({}, APP_NAME, `?${query.toString()}`);
}
const defaultStatus = "published";
this.setState({ isLibraryOpen: true });
try {
const request = await fetch(decodeURIComponent(url));
const blob = await request.blob();
const defaultStatus = "published";
const libraryItems = await loadLibraryFromBlob(blob, defaultStatus);
if (
token === this.id ||
window.confirm(
t("alerts.confirmAddLibrary", {
numShapes: libraryItems.length,
}),
)
) {
await this.library.importLibrary(libraryItems, defaultStatus);
}
await this.library.importLibrary(
new Promise<LibraryItems>(async (resolve, reject) => {
try {
const request = await fetch(decodeURIComponent(url));
const blob = await request.blob();
const libraryItems = await loadLibraryFromBlob(blob, defaultStatus);
if (
token === this.id ||
window.confirm(
t("alerts.confirmAddLibrary", {
numShapes: libraryItems.length,
}),
)
) {
resolve(libraryItems);
} else {
reject(new AbortError());
}
} catch (error: any) {
reject(error);
}
}),
);
} catch (error: any) {
console.error(error);
this.setState({ errorMessage: t("errors.importLibraryError") });
@ -1674,6 +1689,11 @@ class App extends React.Component<AppProps, AppState> {
collaborators?: SceneData["collaborators"];
commitToHistory?: SceneData["commitToHistory"];
libraryItems?:
| ((
currentLibraryItems: LibraryItems,
) =>
| Required<SceneData>["libraryItems"]
| Promise<Required<SceneData>["libraryItems"]>)
| Required<SceneData>["libraryItems"]
| Promise<Required<SceneData>["libraryItems"]>;
}) => {
@ -1694,20 +1714,20 @@ class App extends React.Component<AppProps, AppState> {
}
if (sceneData.libraryItems) {
this.library.saveLibrary(
new Promise<LibraryItems>(async (resolve, reject) => {
this.library.setLibrary((currentLibraryItems) => {
const nextItems =
typeof sceneData.libraryItems === "function"
? sceneData.libraryItems(currentLibraryItems)
: sceneData.libraryItems;
return new Promise<LibraryItems>(async (resolve, reject) => {
try {
resolve(
restoreLibraryItems(
await sceneData.libraryItems,
"unpublished",
),
);
} catch {
reject(new Error(t("errors.importLibraryError")));
resolve(restoreLibraryItems(await nextItems, "unpublished"));
} catch (error: any) {
reject(error);
}
}),
);
});
});
}
},
);
@ -5280,11 +5300,14 @@ class App extends React.Component<AppProps, AppState> {
file?.type === MIME_TYPES.excalidrawlib ||
file?.name?.endsWith(".excalidrawlib")
) {
this.library
.importLibrary(file)
.catch((error) =>
this.setState({ isLoading: false, errorMessage: error.message }),
);
this.setState({ isLibraryOpen: true });
this.library.importLibrary(file).catch((error) => {
console.error(error);
this.setState({
isLoading: false,
errorMessage: t("errors.importLibraryError"),
});
});
// default: assume an Excalidraw file regardless of extension/MimeType
} else if (file) {
this.setState({ isLoading: true });

View File

@ -13,6 +13,10 @@
width: 100%;
margin: 2px 0;
.Spinner {
margin-right: 1rem;
}
button {
// 2px from the left to account for focus border of left-most button
margin: 0 2px;

View File

@ -139,7 +139,7 @@ export const LibraryMenu = ({
const nextItems = libraryItems.filter(
(item) => !selectedItems.includes(item.id),
);
library.saveLibrary(nextItems).catch(() => {
library.setLibrary(nextItems).catch(() => {
setAppState({ errorMessage: t("alerts.errorRemovingFromLibrary") });
});
setSelectedItems([]);
@ -170,7 +170,7 @@ export const LibraryMenu = ({
...libraryItems,
];
onAddToLibrary();
library.saveLibrary(nextItems).catch(() => {
library.setLibrary(nextItems).catch(() => {
setAppState({ errorMessage: t("alerts.errorAddingToLibrary") });
});
},
@ -220,7 +220,7 @@ export const LibraryMenu = ({
libItem.status = "published";
}
});
library.saveLibrary(nextLibItems);
library.setLibrary(nextLibItems);
},
[setShowPublishLibraryDialog, setPublishLibSuccess, selectedItems, library],
);
@ -229,7 +229,10 @@ export const LibraryMenu = ({
LibraryItem["id"] | null
>(null);
if (libraryItemsData.status === "loading") {
if (
libraryItemsData.status === "loading" &&
!libraryItemsData.isInitialized
) {
return (
<LibraryMenuWrapper ref={ref}>
<div className="layer-ui__library-message">
@ -255,7 +258,7 @@ export const LibraryMenu = ({
}
onError={(error) => window.alert(error)}
updateItemsInStorage={() =>
library.saveLibrary(libraryItemsData.libraryItems)
library.setLibrary(libraryItemsData.libraryItems)
}
onRemove={(id: string) =>
setSelectedItems(selectedItems.filter((_id) => _id !== id))
@ -264,6 +267,7 @@ export const LibraryMenu = ({
)}
{publishLibSuccess && renderPublishSuccess()}
<LibraryMenuItems
isLoading={libraryItemsData.status === "loading"}
libraryItems={libraryItemsData.libraryItems}
onRemoveFromLibrary={() =>
removeFromLibrary(libraryItemsData.libraryItems)

View File

@ -22,8 +22,10 @@ import { Tooltip } from "./Tooltip";
import "./LibraryMenuItems.scss";
import { VERSIONS } from "../constants";
import Spinner from "./Spinner";
const LibraryMenuItems = ({
isLoading,
libraryItems,
onRemoveFromLibrary,
onAddToLibrary,
@ -40,6 +42,7 @@ const LibraryMenuItems = ({
onPublish,
resetLibrary,
}: {
isLoading: boolean;
libraryItems: LibraryItems;
pendingElements: LibraryItem["elements"];
onRemoveFromLibrary: () => void;
@ -108,7 +111,8 @@ const LibraryMenuItems = ({
importLibraryFromJSON(library)
.catch(muteFSAbortError)
.catch((error) => {
setAppState({ errorMessage: error.message });
console.error(error);
setAppState({ errorMessage: t("errors.importLibraryError") });
});
}}
className="library-actions--load"
@ -125,7 +129,7 @@ const LibraryMenuItems = ({
onClick={async () => {
const libraryItems = itemsSelected
? items
: await library.loadLibrary();
: await library.getLatestLibrary();
saveLibraryAsJSON(libraryItems)
.catch(muteFSAbortError)
.catch((error) => {
@ -284,16 +288,20 @@ const LibraryMenuItems = ({
{showRemoveLibAlert && renderRemoveLibAlert()}
<div className="layer-ui__library-header" key="library-header">
{renderLibraryActions()}
<a
href={`${process.env.REACT_APP_LIBRARY_URL}?target=${
window.name || "_blank"
}&referrer=${referrer}&useHash=true&token=${id}&theme=${theme}&version=${
VERSIONS.excalidrawLibrary
}`}
target="_excalidraw_libraries"
>
{t("labels.libraries")}
</a>
{isLoading ? (
<Spinner />
) : (
<a
href={`${process.env.REACT_APP_LIBRARY_URL}?target=${
window.name || "_blank"
}&referrer=${referrer}&useHash=true&token=${id}&theme=${theme}&version=${
VERSIONS.excalidrawLibrary
}`}
target="_excalidraw_libraries"
>
{t("labels.libraries")}
</a>
)}
</div>
<Stack.Col
className="library-menu-items-container__items"

View File

@ -5,13 +5,12 @@ import type App from "../components/App";
import { ImportedDataState } from "./types";
import { atom } from "jotai";
import { jotaiStore } from "../jotai";
import { isPromiseLike } from "../utils";
import { t } from "../i18n";
export const libraryItemsAtom = atom<
| { status: "loading"; libraryItems: null; promise: Promise<LibraryItems> }
| { status: "loaded"; libraryItems: LibraryItems }
>({ status: "loaded", libraryItems: [] });
export const libraryItemsAtom = atom<{
status: "loading" | "loaded";
isInitialized: boolean;
libraryItems: LibraryItems;
}>({ status: "loaded", isInitialized: true, libraryItems: [] });
const cloneLibraryItems = (libraryItems: LibraryItems): LibraryItems =>
JSON.parse(JSON.stringify(libraryItems));
@ -40,12 +39,28 @@ const isUniqueItem = (
});
};
/** Merges otherItems into localItems. Unique items in otherItems array are
sorted first. */
export const mergeLibraryItems = (
localItems: LibraryItems,
otherItems: LibraryItems,
): LibraryItems => {
const newItems = [];
for (const item of otherItems) {
if (isUniqueItem(localItems, item)) {
newItems.push(item);
}
}
return [...newItems, ...localItems];
};
class Library {
/** cache for currently active promise when initializing/updating libaries
asynchronously */
private libraryItemsPromise: Promise<LibraryItems> | null = null;
/** last resolved libraryItems */
/** latest libraryItems */
private lastLibraryItems: LibraryItems = [];
/** indicates whether library is initialized with library items (has gone
* though at least one update) */
private isInitialized = false;
private app: App;
@ -53,95 +68,138 @@ class Library {
this.app = app;
}
resetLibrary = async () => {
this.saveLibrary([]);
private updateQueue: Promise<LibraryItems>[] = [];
private getLastUpdateTask = (): Promise<LibraryItems> | undefined => {
return this.updateQueue[this.updateQueue.length - 1];
};
/** imports library (currently merges, removing duplicates) */
async importLibrary(
private notifyListeners = () => {
if (this.updateQueue.length > 0) {
jotaiStore.set(libraryItemsAtom, {
status: "loading",
libraryItems: this.lastLibraryItems,
isInitialized: this.isInitialized,
});
} else {
this.isInitialized = true;
jotaiStore.set(libraryItemsAtom, {
status: "loaded",
libraryItems: this.lastLibraryItems,
isInitialized: this.isInitialized,
});
try {
this.app.props.onLibraryChange?.(
cloneLibraryItems(this.lastLibraryItems),
);
} catch (error) {
console.error(error);
}
}
};
resetLibrary = () => {
return this.setLibrary([]);
};
/**
* imports library (from blob or libraryItems), merging with current library
* (attempting to remove duplicates)
*/
importLibrary(
library:
| Blob
| Required<ImportedDataState>["libraryItems"]
| Promise<Required<ImportedDataState>["libraryItems"]>,
defaultStatus: LibraryItem["status"] = "unpublished",
) {
return this.saveLibrary(
new Promise<LibraryItems>(async (resolve, reject) => {
try {
let libraryItems: LibraryItems;
if (library instanceof Blob) {
libraryItems = await loadLibraryFromBlob(library, defaultStatus);
} else {
libraryItems = restoreLibraryItems(await library, defaultStatus);
}
const existingLibraryItems = this.lastLibraryItems;
const filteredItems = [];
for (const item of libraryItems) {
if (isUniqueItem(existingLibraryItems, item)) {
filteredItems.push(item);
): Promise<LibraryItems> {
return this.setLibrary(
() =>
new Promise<LibraryItems>(async (resolve, reject) => {
try {
let libraryItems: LibraryItems;
if (library instanceof Blob) {
libraryItems = await loadLibraryFromBlob(library, defaultStatus);
} else {
libraryItems = restoreLibraryItems(await library, defaultStatus);
}
}
resolve([...filteredItems, ...existingLibraryItems]);
} catch (error) {
reject(new Error(t("errors.importLibraryError")));
}
}),
resolve(mergeLibraryItems(this.lastLibraryItems, libraryItems));
} catch (error) {
reject(error);
}
}),
);
}
loadLibrary = (): Promise<LibraryItems> => {
/**
* @returns latest cloned libraryItems. Awaits all in-progress updates first.
*/
getLatestLibrary = (): Promise<LibraryItems> => {
return new Promise(async (resolve) => {
try {
resolve(
cloneLibraryItems(
await (this.libraryItemsPromise || this.lastLibraryItems),
),
);
const libraryItems = await (this.getLastUpdateTask() ||
this.lastLibraryItems);
if (this.updateQueue.length > 0) {
resolve(this.getLatestLibrary());
} else {
resolve(cloneLibraryItems(libraryItems));
}
} catch (error) {
return resolve(this.lastLibraryItems);
}
});
};
saveLibrary = async (items: LibraryItems | Promise<LibraryItems>) => {
const prevLibraryItems = this.lastLibraryItems;
try {
let nextLibraryItems;
if (isPromiseLike(items)) {
const promise = items.then((items) => cloneLibraryItems(items));
this.libraryItemsPromise = promise;
jotaiStore.set(libraryItemsAtom, {
status: "loading",
promise,
libraryItems: null,
});
nextLibraryItems = await promise;
} else {
nextLibraryItems = cloneLibraryItems(items);
setLibrary = (
/**
* LibraryItems that will replace current items. Can be a function which
* will be invoked after all previous tasks are resolved
* (this is the prefered way to update the library to avoid race conditions,
* but you'll want to manually merge the library items in the callback
* - which is what we're doing in Library.importLibrary()).
*
* If supplied promise is rejected with AbortError, we swallow it and
* do not update the library.
*/
libraryItems:
| LibraryItems
| Promise<LibraryItems>
| ((
latestLibraryItems: LibraryItems,
) => LibraryItems | Promise<LibraryItems>),
): Promise<LibraryItems> => {
const task = new Promise<LibraryItems>(async (resolve, reject) => {
try {
await this.getLastUpdateTask();
if (typeof libraryItems === "function") {
libraryItems = libraryItems(this.lastLibraryItems);
}
this.lastLibraryItems = cloneLibraryItems(await libraryItems);
resolve(this.lastLibraryItems);
} catch (error: any) {
reject(error);
}
this.lastLibraryItems = nextLibraryItems;
this.libraryItemsPromise = null;
jotaiStore.set(libraryItemsAtom, {
status: "loaded",
libraryItems: nextLibraryItems,
})
.catch((error) => {
if (error.name === "AbortError") {
console.warn("Library update aborted by user");
return this.lastLibraryItems;
}
throw error;
})
.finally(() => {
this.updateQueue = this.updateQueue.filter((_task) => _task !== task);
this.notifyListeners();
});
await this.app.props.onLibraryChange?.(
cloneLibraryItems(nextLibraryItems),
);
} catch (error: any) {
this.lastLibraryItems = prevLibraryItems;
this.libraryItemsPromise = null;
jotaiStore.set(libraryItemsAtom, {
status: "loaded",
libraryItems: prevLibraryItems,
});
throw error;
}
this.updateQueue.push(task);
this.notifyListeners();
return task;
};
}

View File

@ -17,7 +17,9 @@ Please add the latest change on the top under the correct section.
#### Features
- Expose util `exportToClipboard`[https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#exportToClipboard] which allows to copy the scene contents to clipboard as `svg`, `png` or `json` [#5103](https://github.com/excalidraw/excalidraw/pull/5103).
- Support `libraryItems` argument in `initialData.libraryItems` and `updateScene({ libraryItems })` to be a Promise resolving to `LibraryItems`, and support functional update of `libraryItems` in [`updateScene({ libraryItems })`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#updateScene). [#5101](https://github.com/excalidraw/excalidraw/pull/5101).
- Expose util [`mergeLibraryItems`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#mergeLibraryItems) [#5101](https://github.com/excalidraw/excalidraw/pull/5101).
- Expose util [`exportToClipboard`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#exportToClipboard) which allows to copy the scene contents to clipboard as `svg`, `png` or `json` [#5103](https://github.com/excalidraw/excalidraw/pull/5103).
- Expose `window.EXCALIDRAW_EXPORT_SOURCE` which you can use to overwrite the `source` field in exported data [#5095](https://github.com/excalidraw/excalidraw/pull/5095).
- The `exportToBlob` utility now supports the `exportEmbedScene` option when generating a png image [#5047](https://github.com/excalidraw/excalidraw/pull/5047).
- Exported [`restoreLibraryItems`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#restoreLibraryItems) API [#4995](https://github.com/excalidraw/excalidraw/pull/4995).

View File

@ -436,7 +436,7 @@ This helps to load Excalidraw with `initialData`. It must be an object or a [pro
| `elements` | [ExcalidrawElement[]](https://github.com/excalidraw/excalidraw/blob/master/src/element/types.ts#L78) | The elements with which Excalidraw should be mounted. |
| `appState` | [AppState](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L42) | The App state with which Excalidraw should be mounted. |
| `scrollToContent` | boolean | This attribute implies whether to scroll to the nearest element to center once Excalidraw is mounted. By default, it will not scroll the nearest element to the center. Make sure you pass `initialData.appState.scrollX` and `initialData.appState.scrollY` when `scrollToContent` is false so that scroll positions are retained |
| `libraryItems` | [LibraryItems](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L151) | This library items with which Excalidraw should be mounted. |
| `libraryItems` | [LibraryItems](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L200) &#124; Promise<[LibraryItems](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L200)> | This library items with which Excalidraw should be mounted. |
| `files` | [BinaryFiles](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L64) | The files added to the scene. |
```json
@ -514,7 +514,7 @@ You can use this function to update the scene with the sceneData. It accepts the
| `appState` | [`ImportedDataState["appState"]`](https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L18) | The `appState` to be updated in the scene. |
| `collaborators` | <pre>Map<string, <a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L29">Collaborator></a></pre> | The list of collaborators to be updated in the scene. |
| `commitToHistory` | `boolean` | Implies if the `history (undo/redo)` should be recorded. Defaults to `false`. |
| `libraryItems` | [LibraryItems](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L258) | The `libraryItems` to be update in the scene. |
| `libraryItems` | [LibraryItems](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L200) &#124; Promise<[LibraryItems](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L200)> &#124; ((currentItems: [LibraryItems](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L200)>) => [LibraryItems](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L200) &#124; Promise<[LibraryItems](https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L200)>) | The `libraryItems` to be update in the scene. |
### `addFiles`
@ -960,7 +960,7 @@ If you want to overwrite the source field in the JSON string, you can set `windo
<pre>
serializeLibraryAsJSON({
libraryItems: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L191">LibraryItems[]</a>,
libraryItems: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L200">LibraryItems[]</a>,
</pre>
Takes the library items and returns a JSON string.
@ -1072,6 +1072,20 @@ getNonDeletedElements(elements: <a href="https://github.com/excalidraw/excalidra
This function returns an array of deleted elements.
#### `mergeLibraryItems`
```js
import { mergeLibraryItems } from "@excalidraw/excalidraw-next";
```
**_Signature_**
<pre>
mergeLibraryItems(localItems: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L200">LibraryItems</a>, otherItems: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L200">LibraryItems</a>) => <a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L200">LibraryItems</a>
</pre>
This function merges two `LibraryItems` arrays, where unique items from `otherItems` are sorted first in the returned array.
### Exported constants
#### `FONT_FAMILY`

View File

@ -198,6 +198,7 @@ export {
loadFromBlob,
getFreeDrawSvgPath,
exportToClipboard,
mergeLibraryItems,
} from "../../packages/utils";
export { isLinearElement } from "../../element/typeChecks";

View File

@ -194,3 +194,4 @@ export const exportToClipboard = async (
export { serializeAsJSON, serializeLibraryAsJSON } from "../data/json";
export { loadFromBlob, loadLibraryFromBlob } from "../data/blob";
export { getFreeDrawSvgPath } from "../renderer/renderElement";
export { mergeLibraryItems } from "../data/library";

View File

@ -14,12 +14,12 @@ describe("library", () => {
});
it("import library via drag&drop", async () => {
expect(await h.app.library.loadLibrary()).toEqual([]);
expect(await h.app.library.getLatestLibrary()).toEqual([]);
await API.drop(
await API.loadFile("./fixtures/fixture_library.excalidrawlib"),
);
await waitFor(async () => {
expect(await h.app.library.loadLibrary()).toEqual([
expect(await h.app.library.getLatestLibrary()).toEqual([
{
status: "unpublished",
elements: [expect.objectContaining({ id: "A" })],