diff --git a/packages/excalidraw/data/restore.ts b/packages/excalidraw/data/restore.ts index 31e3563dff..b78fa07f84 100644 --- a/packages/excalidraw/data/restore.ts +++ b/packages/excalidraw/data/restore.ts @@ -136,99 +136,103 @@ const repairBinding = ( existingElementsMap: Readonly | null | undefined, startOrEnd: "start" | "end", ): FixedPointBinding | null => { - if (!binding) { - return null; - } - - // --------------------------------------------------------------------------- - // elbow arrows - // --------------------------------------------------------------------------- - - if (isElbowArrow(element)) { - const fixedPointBinding: - | ExcalidrawElbowArrowElement["startBinding"] - | ExcalidrawElbowArrowElement["endBinding"] = { - ...binding, - fixedPoint: normalizeFixedPoint(binding.fixedPoint ?? [0, 0]), - mode: binding.mode || "orbit", - }; - - return fixedPointBinding; - } - - // --------------------------------------------------------------------------- - // simple arrows - // --------------------------------------------------------------------------- - - // binding schema v2 - // --------------------------------------------------------------------------- - - if (binding.mode) { - // if latest binding schema, don't check if binding.elementId exists - // (it's done in a separate pass) - if (binding.elementId) { - return { - elementId: binding.elementId, - mode: binding.mode, - fixedPoint: normalizeFixedPoint(binding.fixedPoint || [0.5, 0.5]), - } as FixedPointBinding | null; + try { + if (!binding) { + return null; } - return null; + + // --------------------------------------------------------------------------- + // elbow arrows + // --------------------------------------------------------------------------- + + if (isElbowArrow(element)) { + const fixedPointBinding: + | ExcalidrawElbowArrowElement["startBinding"] + | ExcalidrawElbowArrowElement["endBinding"] = { + ...binding, + fixedPoint: normalizeFixedPoint(binding.fixedPoint ?? [0, 0]), + mode: binding.mode || "orbit", + }; + + return fixedPointBinding; + } + + // --------------------------------------------------------------------------- + // simple arrows + // --------------------------------------------------------------------------- + + // binding schema v2 + // --------------------------------------------------------------------------- + + if (binding.mode) { + // if latest binding schema, don't check if binding.elementId exists + // (it's done in a separate pass) + if (binding.elementId) { + return { + elementId: binding.elementId, + mode: binding.mode, + fixedPoint: normalizeFixedPoint(binding.fixedPoint || [0.5, 0.5]), + } as FixedPointBinding | null; + } + return null; + } + + // binding schema v1 (legacy) -> attempt to migrate to v2 + // --------------------------------------------------------------------------- + + const targetBoundElement = + (targetElementsMap.get(binding.elementId) as ExcalidrawBindableElement) || + undefined; + const boundElement = + targetBoundElement || + (existingElementsMap?.get( + binding.elementId, + ) as ExcalidrawBindableElement) || + undefined; + const elementsMap = targetBoundElement + ? targetElementsMap + : existingElementsMap; + + // migrating legacy focus point bindings + if (boundElement && elementsMap) { + const p = LinearElementEditor.getPointAtIndexGlobalCoordinates( + element, + startOrEnd === "start" ? 0 : element.points.length - 1, + elementsMap, + ); + const mode = isPointInElement(p, boundElement, elementsMap) + ? "inside" + : "orbit"; + const focusPoint = + mode === "inside" + ? p + : projectFixedPointOntoDiagonal( + element, + p, + boundElement, + startOrEnd, + elementsMap, + ) || p; + const { fixedPoint } = calculateFixedPointForNonElbowArrowBinding( + element, + boundElement, + startOrEnd, + elementsMap, + focusPoint, + ); + + return { + mode, + elementId: binding.elementId, + fixedPoint, + }; + } + + console.error(`could not repair binding for element`); + } catch (error) { + console.error("Error repairing binding:", error); } - // binding schema v1 (legacy) -> attempt to migrate to v2 - // --------------------------------------------------------------------------- - - const targetBoundElement = - (targetElementsMap.get(binding.elementId) as ExcalidrawBindableElement) || - undefined; - const boundElement = - targetBoundElement || - (existingElementsMap?.get( - binding.elementId, - ) as ExcalidrawBindableElement) || - undefined; - const elementsMap = targetBoundElement - ? targetElementsMap - : existingElementsMap; - - // migrating legacy focus point bindings - if (boundElement && elementsMap) { - const p = LinearElementEditor.getPointAtIndexGlobalCoordinates( - element, - startOrEnd === "start" ? 0 : element.points.length - 1, - elementsMap, - ); - const mode = isPointInElement(p, boundElement, elementsMap) - ? "inside" - : "orbit"; - const focusPoint = - mode === "inside" - ? p - : projectFixedPointOntoDiagonal( - element, - p, - boundElement, - startOrEnd, - elementsMap, - ) || p; - const { fixedPoint } = calculateFixedPointForNonElbowArrowBinding( - element, - boundElement, - startOrEnd, - elementsMap, - focusPoint, - ); - - return { - mode, - elementId: binding.elementId, - fixedPoint, - }; - } - - console.error(`could not repair binding for element`); - return null; }; @@ -639,15 +643,20 @@ export const restoreElements = ( if (element.type === "selection") { return elements; } - - let migratedElement: ExcalidrawElement | null = restoreElement( - element, - targetElementsMap, - existingElementsMap, - { - deleteInvisibleElements: opts?.deleteInvisibleElements, - }, - ); + let migratedElement: ExcalidrawElement | null; + try { + migratedElement = restoreElement( + element, + targetElementsMap, + existingElementsMap, + { + deleteInvisibleElements: opts?.deleteInvisibleElements, + }, + ); + } catch (error) { + console.error("Error restoring element:", error); + migratedElement = null; + } if (migratedElement) { const localElement = existingElementsMap?.get(element.id); diff --git a/packages/excalidraw/tests/data/__snapshots__/restore.test.ts.snap b/packages/excalidraw/tests/data/__snapshots__/restore.test.ts.snap index 95826081f4..802edcc373 100644 --- a/packages/excalidraw/tests/data/__snapshots__/restore.test.ts.snap +++ b/packages/excalidraw/tests/data/__snapshots__/restore.test.ts.snap @@ -1,5 +1,52 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`repairing bindings > should strip arrow binding if repair throws 1`] = ` +{ + "angle": 0, + "backgroundColor": "transparent", + "boundElements": [], + "customData": undefined, + "elbowed": false, + "endArrowhead": null, + "endBinding": null, + "fillStyle": "solid", + "frameId": null, + "groupIds": [], + "height": 100, + "id": "id-arrow01", + "index": "a0", + "isDeleted": false, + "link": null, + "locked": false, + "opacity": 100, + "points": [ + [ + 0, + 0, + ], + [ + 100, + 100, + ], + ], + "roughness": 1, + "roundness": null, + "seed": Any, + "startArrowhead": null, + "startBinding": null, + "strokeColor": "#1e1e1e", + "strokeStyle": "solid", + "strokeWidth": 2, + "type": "arrow", + "updated": 1, + "version": 2, + "versionNonce": Any, + "width": 100, + "x": 0, + "y": 0, +} +`; + exports[`restoreElements > should restore arrow element correctly 1`] = ` { "angle": 0, @@ -381,3 +428,50 @@ exports[`restoreElements > should restore text element correctly with unknown fo "y": 0, } `; + +exports[`restoreElements > should strip arrow binding if repair throws 1`] = ` +{ + "angle": 0, + "backgroundColor": "transparent", + "boundElements": [], + "customData": undefined, + "elbowed": false, + "endArrowhead": null, + "endBinding": null, + "fillStyle": "solid", + "frameId": null, + "groupIds": [], + "height": 100, + "id": "id-arrow01", + "index": "a0", + "isDeleted": false, + "link": null, + "locked": false, + "opacity": 100, + "points": [ + [ + 0, + 0, + ], + [ + 100, + 100, + ], + ], + "roughness": 1, + "roundness": null, + "seed": Any, + "startArrowhead": null, + "startBinding": null, + "strokeColor": "#1e1e1e", + "strokeStyle": "solid", + "strokeWidth": 2, + "type": "arrow", + "updated": 1, + "version": 2, + "versionNonce": Any, + "width": 100, + "x": 0, + "y": 0, +} +`; diff --git a/packages/excalidraw/tests/data/restore.test.ts b/packages/excalidraw/tests/data/restore.test.ts index d666e48e11..4b06601bb2 100644 --- a/packages/excalidraw/tests/data/restore.test.ts +++ b/packages/excalidraw/tests/data/restore.test.ts @@ -200,6 +200,36 @@ describe("restoreElements", () => { }); }); + it("should strip element if restore fails", () => { + const rect1 = API.createElement({ + type: "rectangle", + boundElements: [], + }); + const rect2 = API.createElement({ + type: "rectangle", + boundElements: [], + }); + + // define getter for a property we access during restoreElement that throws + Object.defineProperty(rect2, "seed", { + get: () => { + throw new Error("FORBIDDEN!"); + }, + }); + + const restoredElements = restore.restoreElements([rect1, rect2], null); + + expect(restoredElements.length).toBe(1); + + expect(restoredElements).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: rect1.id, + }), + ]), + ); + }); + it("should remove imperceptibly small elements", () => { const arrowElement = API.createElement({ type: "arrow", @@ -717,6 +747,58 @@ describe("restoreAppState", () => { }); describe("repairing bindings", () => { + it("should strip arrow binding if repair throws", () => { + const container = API.createElement({ + type: "rectangle", + boundElements: [], + }); + const arrowElement = API.createElement({ + type: "arrow", + id: "id-arrow01", + endBinding: { + elementId: container.id, + fixedPoint: [0.5, 0.5], + mode: "inside", + }, + }); + + Object.assign(container, { + boundElements: [{ type: "arrow", id: arrowElement.id }], + }); + // make invalid binding -> throws during repair + Object.assign(arrowElement, { + endBinding: { + // invalid reference + elementId: 42, + }, + }); + + const restoredElements = restore.restoreElements( + [arrowElement, container], + null, + ); + + expect(restoredElements).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: arrowElement.id, + endBinding: null, + }), + expect.objectContaining({ + id: container.id, + }), + ]), + ); + + const restoredArrow = restoredElements[0] as ExcalidrawLinearElement; + + expect(restoredArrow).toMatchSnapshot({ + seed: expect.any(Number), + versionNonce: expect.any(Number), + // endBinding: expect.any(Object), + }); + }); + it("should repair container boundElements when repair is true", () => { const container = API.createElement({ type: "rectangle",