fix: Race conditions when adding many library items (#10013)

* Fix for race condition when adding many library items

* Remove unused import

* Replace any with LibraryItem type

* Fix comments on pr

* Fix build errors

* Fix hoisted variable

* new mime type

* duplicate before passing down to be sure

* lint

* fix tests

* Remove unused import

---------

Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com>
This commit is contained in:
ericvannunen
2025-09-23 23:47:03 +02:00
committed by GitHub
parent f55ecb96cc
commit 06c5ea94d3
6 changed files with 147 additions and 75 deletions

View File

@@ -266,7 +266,10 @@ export const STRING_MIME_TYPES = {
json: "application/json",
// excalidraw data
excalidraw: "application/vnd.excalidraw+json",
// LEGACY: fully-qualified library JSON data
excalidrawlib: "application/vnd.excalidrawlib+json",
// list of excalidraw library item ids
excalidrawlibIds: "application/vnd.excalidrawlib.ids+json",
} as const;
export const MIME_TYPES = {

View File

@@ -433,6 +433,8 @@ import { findShapeByKey } from "./shapes";
import UnlockPopup from "./UnlockPopup";
import type { ExcalidrawLibraryIds } from "../data/types";
import type {
RenderInteractiveSceneCallback,
ScrollBars,
@@ -10545,16 +10547,44 @@ class App extends React.Component<AppProps, AppState> {
if (imageFiles.length > 0 && this.isToolSupported("image")) {
return this.insertImages(imageFiles, sceneX, sceneY);
}
const libraryJSON = dataTransferList.getData(MIME_TYPES.excalidrawlib);
if (libraryJSON && typeof libraryJSON === "string") {
const excalidrawLibrary_ids = dataTransferList.getData(
MIME_TYPES.excalidrawlibIds,
);
const excalidrawLibrary_data = dataTransferList.getData(
MIME_TYPES.excalidrawlib,
);
if (excalidrawLibrary_ids || excalidrawLibrary_data) {
try {
const libraryItems = parseLibraryJSON(libraryJSON);
this.addElementsFromPasteOrLibrary({
elements: distributeLibraryItemsOnSquareGrid(libraryItems),
position: event,
files: null,
});
let libraryItems: LibraryItems | null = null;
if (excalidrawLibrary_ids) {
const { itemIds } = JSON.parse(
excalidrawLibrary_ids,
) as ExcalidrawLibraryIds;
const allLibraryItems = await this.library.getLatestLibrary();
libraryItems = allLibraryItems.filter((item) =>
itemIds.includes(item.id),
);
// legacy library dataTransfer format
} else if (excalidrawLibrary_data) {
libraryItems = parseLibraryJSON(excalidrawLibrary_data);
}
if (libraryItems?.length) {
libraryItems = libraryItems.map((item) => ({
...item,
// #6465
elements: duplicateElements({
type: "everything",
elements: item.elements,
randomizeSeed: true,
}).duplicatedElements,
}));
this.addElementsFromPasteOrLibrary({
elements: distributeLibraryItemsOnSquareGrid(libraryItems),
position: event,
files: null,
});
}
} catch (error: any) {
this.setState({ errorMessage: error.message });
}

View File

@@ -10,7 +10,6 @@ import { MIME_TYPES, arrayToMap } from "@excalidraw/common";
import { duplicateElements } from "@excalidraw/element";
import { serializeLibraryAsJSON } from "../data/json";
import { useLibraryCache } from "../hooks/useLibraryItemSvg";
import { useScrollPosition } from "../hooks/useScrollPosition";
import { t } from "../i18n";
@@ -27,6 +26,8 @@ import Stack from "./Stack";
import "./LibraryMenuItems.scss";
import type { ExcalidrawLibraryIds } from "../data/types";
import type {
ExcalidrawProps,
LibraryItem,
@@ -175,12 +176,17 @@ export default function LibraryMenuItems({
const onItemDrag = useCallback(
(id: LibraryItem["id"], event: React.DragEvent) => {
// we want to serialize just the ids so the operation is fast and there's
// no race condition if people drop the library items on canvas too fast
const data: ExcalidrawLibraryIds = {
itemIds: selectedItems.includes(id) ? selectedItems : [id],
};
event.dataTransfer.setData(
MIME_TYPES.excalidrawlib,
serializeLibraryAsJSON(getInsertedElements(id)),
MIME_TYPES.excalidrawlibIds,
JSON.stringify(data),
);
},
[getInsertedElements],
[selectedItems],
);
const isItemSelected = useCallback(

View File

@@ -192,6 +192,7 @@ const createLibraryUpdate = (
class Library {
/** latest libraryItems */
private currLibraryItems: LibraryItems = [];
/** snapshot of library items since last onLibraryChange call */
private prevLibraryItems = cloneLibraryItems(this.currLibraryItems);

View File

@@ -6,6 +6,7 @@ import type { cleanAppStateForExport } from "../appState";
import type {
AppState,
BinaryFiles,
LibraryItem,
LibraryItems,
LibraryItems_anyVersion,
} from "../types";
@@ -59,3 +60,7 @@ export interface ImportedLibraryData extends Partial<ExportedLibraryData> {
/** @deprecated v1 */
library?: LibraryItems;
}
export type ExcalidrawLibraryIds = {
itemIds: LibraryItem["id"][];
};

View File

@@ -15,7 +15,7 @@ import { Excalidraw } from "../index";
import { API } from "./helpers/api";
import { UI } from "./helpers/ui";
import { fireEvent, getCloneByOrigId, render, waitFor } from "./test-utils";
import { fireEvent, render, waitFor } from "./test-utils";
import type { LibraryItem, LibraryItems } from "../types";
@@ -46,52 +46,8 @@ vi.mock("../data/filesystem.ts", async (importOriginal) => {
};
});
describe("library", () => {
describe("library items inserting", () => {
beforeEach(async () => {
await render(<Excalidraw />);
await act(() => {
return h.app.library.resetLibrary();
});
});
it("import library via drag&drop", async () => {
expect(await h.app.library.getLatestLibrary()).toEqual([]);
await API.drop([
{
kind: "file",
type: MIME_TYPES.excalidrawlib,
file: await API.loadFile("./fixtures/fixture_library.excalidrawlib"),
},
]);
await waitFor(async () => {
expect(await h.app.library.getLatestLibrary()).toEqual([
{
status: "unpublished",
elements: [expect.objectContaining({ id: "A" })],
id: "id0",
created: expect.any(Number),
},
]);
});
});
// NOTE: mocked to test logic, not actual drag&drop via UI
it("drop library item onto canvas", async () => {
expect(h.elements).toEqual([]);
const libraryItems = parseLibraryJSON(await libraryJSONPromise);
await API.drop([
{
kind: "string",
value: serializeLibraryAsJSON(libraryItems),
type: MIME_TYPES.excalidrawlib,
},
]);
await waitFor(() => {
expect(h.elements).toEqual([expect.objectContaining({ [ORIG_ID]: "A" })]);
});
});
it("should regenerate ids but retain bindings on library insert", async () => {
const rectangle = API.createElement({
id: "rectangle1",
type: "rectangle",
@@ -117,45 +73,116 @@ describe("library", () => {
},
});
const libraryItems: LibraryItems = [
{
id: "libraryItem_id0",
status: "unpublished",
elements: [rectangle, text, arrow],
created: 0,
name: "test",
},
];
await render(<Excalidraw initialData={{ libraryItems }} />);
});
afterEach(async () => {
await act(() => {
return h.app.library.resetLibrary();
});
});
it("should regenerate ids but retain bindings on library insert", async () => {
const libraryItems = await h.app.library.getLatestLibrary();
expect(libraryItems.length).toBe(1);
await API.drop([
{
kind: "string",
value: serializeLibraryAsJSON([
{
id: "item1",
status: "published",
elements: [rectangle, text, arrow],
created: 1,
},
]),
type: MIME_TYPES.excalidrawlib,
value: JSON.stringify({
itemIds: [libraryItems[0].id],
}),
type: MIME_TYPES.excalidrawlibIds,
},
]);
await waitFor(() => {
const rectangle = h.elements.find((e) => e.type === "rectangle")!;
const text = h.elements.find((e) => e.type === "text")!;
const arrow = h.elements.find((e) => e.type === "arrow")!;
expect(h.elements).toEqual(
expect.arrayContaining([
expect.objectContaining({
[ORIG_ID]: "rectangle1",
type: "rectangle",
id: expect.not.stringMatching("rectangle1"),
boundElements: expect.arrayContaining([
{ type: "text", id: getCloneByOrigId("text1").id },
{ type: "arrow", id: getCloneByOrigId("arrow1").id },
{ type: "text", id: text.id },
{ type: "arrow", id: arrow.id },
]),
}),
expect.objectContaining({
[ORIG_ID]: "text1",
containerId: getCloneByOrigId("rectangle1").id,
type: "text",
id: expect.not.stringMatching("text1"),
containerId: rectangle.id,
}),
expect.objectContaining({
[ORIG_ID]: "arrow1",
type: "arrow",
id: expect.not.stringMatching("arrow1"),
endBinding: expect.objectContaining({
elementId: getCloneByOrigId("rectangle1").id,
elementId: rectangle.id,
}),
}),
]),
);
});
});
});
describe("library", () => {
beforeEach(async () => {
await render(<Excalidraw />);
await act(() => {
return h.app.library.resetLibrary();
});
});
it("import library via drag&drop", async () => {
expect(await h.app.library.getLatestLibrary()).toEqual([]);
await API.drop([
{
kind: "file",
type: MIME_TYPES.excalidrawlib,
file: await API.loadFile("./fixtures/fixture_library.excalidrawlib"),
},
]);
await waitFor(async () => {
expect(await h.app.library.getLatestLibrary()).toEqual([
{
status: "unpublished",
elements: [expect.objectContaining({ id: "A" })],
id: expect.any(String),
created: expect.any(Number),
},
]);
});
});
// NOTE: mocked to test logic, not actual drag&drop via UI
it("drop library item onto canvas", async () => {
expect(h.elements).toEqual([]);
const libraryItems = parseLibraryJSON(await libraryJSONPromise);
await API.drop([
{
kind: "string",
value: serializeLibraryAsJSON(libraryItems),
type: MIME_TYPES.excalidrawlib,
},
]);
await waitFor(() => {
expect(h.elements).toEqual([expect.objectContaining({ [ORIG_ID]: "A" })]);
});
});
it("should fix duplicate ids between items on insert", async () => {
// note, we're not testing for duplicate group ids and such because