fix(studio): a failed save never blocks undo, and a nudge burst always ends its pending edit

Author: miguel-heygen

packages/studio/src/App.tsx

@@ -256,7 +256,7 @@ export function StudioApp({ readOnlyPreview = false, readOnlyPreviewReason }: St
     showToast,
     syncHistoryPreviewAfterApply: previewPersistence.syncHistoryPreviewAfterApply,
     showHistoryRestoreNow: previewPersistence.showHistoryRestoreNow,
-    waitForPendingDomEditSaves: previewPersistence.waitForPendingDomEditSaves,
+    waitForPendingDomEditSaves: previewPersistence.settlePendingEdits,
     handleCopy,
     handlePaste,
     handleCut,

packages/studio/src/components/editor/useDomEditNudge.test.tsx

@@ -28,6 +28,7 @@ const REST_RECT: OverlayRect = {
 // Stable across renders on purpose: the test targets the `selection` identity
 // key specifically, so `groupSelections` must not itself be a source of churn.
 const EMPTY_GROUP_SELECTIONS: DomEditSelection[] = [];
+let flushNudge = () => {};
 
 function Harness({
   selection,
@@ -36,7 +37,7 @@ function Harness({
   selection: DomEditSelection | null;
   onPathOffsetCommit: UseDomEditNudgeParams["onPathOffsetCommitRef"]["current"];
 }) {
-  useDomEditNudge({
+  flushNudge = useDomEditNudge({
     selection,
     groupSelections: EMPTY_GROUP_SELECTIONS,
     allowCanvasMovement: true,
@@ -50,7 +51,7 @@ function Harness({
     onBlockedMoveRef: makeRef(() => {}),
     onPathOffsetCommitRef: makeRef(onPathOffsetCommit),
     onGroupPathOffsetCommitRef: makeRef(async () => {}),
-  });
+  }).flushNudge;
   return null;
 }
 
@@ -265,6 +266,7 @@ describe("useDomEditNudge carries the route its press chose", () => {
 describe("useDomEditNudge — undo right after a burst", () => {
   it("undo's drain commits a burst still inside its debounce and waits for its save", async () => {
     __resetForTests();
+    vi.useFakeTimers();
     const root = createRoot(document.body.appendChild(document.createElement("div")));
     const element = document.body.appendChild(document.createElement("div"));
     element.id = "dot-undo";
@@ -282,7 +284,9 @@ describe("useDomEditNudge — undo right after a burst", () => {
 
     let drained = false;
     const drain = flushStudioPendingEdits().then(() => (drained = true));
-    await vi.waitFor(() => expect(commit).toHaveBeenCalledTimes(1));
+    expect(commit).toHaveBeenCalledTimes(1);
+    vi.useRealTimers();
+    await new Promise((resolve) => setTimeout(resolve, 20));
     expect(drained).toBe(false);
     saved();
     await drain;
@@ -318,3 +322,28 @@ describe("useDomEditNudge — undo right after a burst", () => {
     act(() => root.unmount());
   });
 });
+
+describe("useDomEditNudge — a commit that throws", () => {
+  it("still ends the burst's pending edit, so undo and export never wait on it", async () => {
+    __resetForTests();
+    const root = createRoot(document.body.appendChild(document.createElement("div")));
+    const element = document.body.appendChild(document.createElement("div"));
+    element.id = "dot-throws";
+    const commit = vi.fn(() => {
+      throw new Error("The commit threw.");
+    });
+    act(() => {
+      root.render(
+        React.createElement(Harness, {
+          selection: makeSelection("Dot", element),
+          onPathOffsetCommit: commit,
+        }),
+      );
+    });
+    act(() => dispatchArrowRight());
+    expect(hasStudioPendingEdits()).toBe(true);
+    expect(() => flushNudge()).toThrow("The commit threw.");
+    await vi.waitFor(() => expect(hasStudioPendingEdits()).toBe(false));
+    act(() => root.unmount());
+  });
+});

packages/studio/src/components/editor/useDomEditNudge.ts

@@ -159,22 +159,26 @@ export function useDomEditNudge(params: UseDomEditNudgeParams): { flushNudge: ()
       plainTranslate: member.plainTranslate,
     }));
     const p = paramsRef.current;
-    const commit = session.isGroup
-      ? p.onGroupPathOffsetCommitRef.current(updates)
-      : p.onPathOffsetCommitRef.current(updates[0].selection, updates[0].next, {
-          plainTranslate: updates[0].plainTranslate,
-        });
-    const saved = Promise.resolve(commit)
-      .catch(() => {
-        for (const member of session.members) {
-          if (isStudioManualEditGestureCurrent(member.element, member.gestureToken)) {
-            restoreStudioPathOffset(member.element, member.initialPathOffset);
+    let saved: Promise<unknown> | undefined;
+    try {
+      const commit = session.isGroup
+        ? p.onGroupPathOffsetCommitRef.current(updates)
+        : p.onPathOffsetCommitRef.current(updates[0].selection, updates[0].next, {
+            plainTranslate: updates[0].plainTranslate,
+          });
+      saved = Promise.resolve(commit)
+        .catch(() => {
+          for (const member of session.members) {
+            if (isStudioManualEditGestureCurrent(member.element, member.gestureToken)) {
+              restoreStudioPathOffset(member.element, member.initialPathOffset);
+            }
           }
-        }
-      })
-      .finally(() => endManualOffsetDragMembers(session.members));
-    session.endPendingEdit(saved);
-    return saved;
+        })
+        .finally(() => endManualOffsetDragMembers(session.members));
+      return saved;
+    } finally {
+      session.endPendingEdit(saved);
+    }
   };
   const commitSessionRef = useRef(commitSession);
   commitSessionRef.current = commitSession;

packages/studio/src/hooks/useEditHistoryActions.paint.test.tsx

@@ -83,7 +83,7 @@ async function studio() {
       showToast: () => {},
       syncHistoryPreviewAfterApply: persistence.syncHistoryPreviewAfterApply,
       showHistoryRestoreNow: persistence.showHistoryRestoreNow,
-      waitForPendingDomEditSaves: persistence.waitForPendingDomEditSaves,
+      waitForPendingDomEditSaves: persistence.settlePendingEdits,
     });
     return null;
   }
@@ -225,16 +225,22 @@ it("an undo pressed while a nudge waits for more keys never shows the move befor
   expect(s.box()).toBe("50px");
 });
 
-it("an undo pressed while an edit's save fails undoes the edit before it, file and box alike", async () => {
+it("an undo pressed while a queued save fails undoes the edit before it, file and box alike", async () => {
   const s = await studio();
   await s.edit();
   const box = s.element("box");
   let fail!: () => void;
   const handleDomStyleCommit = vi.fn(async () => {
     box.style.left = "70px";
-    await new Promise<void>((resolve) => (fail = resolve));
-    box.style.left = "50px";
-    throw new Error("The save failed.");
+    try {
+      await s.persistence().queueDomEditSave(async () => {
+        await new Promise<void>((resolve) => (fail = resolve));
+        throw new Error("The save failed.");
+      });
+    } catch (error) {
+      box.style.left = "50px";
+      throw error;
+    }
   });
   let actions!: ReturnType<typeof useDomEditActionsContext>;
   function Canvas() {
@@ -252,6 +258,7 @@ it("an undo pressed while an edit's save fails undoes the edit before it, file a
   const failed = actions.handleDomStyleCommit("left", "70px");
 
   const undone = s.actions().undo();
+  await vi.waitFor(() => expect(fail).toBeTypeOf("function"));
   fail();
   await expect(failed).rejects.toThrow("The save failed.");
   await act(() => undone);

packages/studio/src/hooks/usePreviewPersistence.ts

@@ -159,6 +159,11 @@ export function usePreviewPersistence({
     if (result.status !== "clean") throw result.error;
   }, [drainPendingDomEditSaves]);
 
+  const settlePendingEdits = useCallback(async (): Promise<void> => {
+    await flushStudioPendingEdits();
+    await domEditSaveQueueRef.current?.waitForIdle();
+  }, []);
+
   const resetDomEditSaveQueueBreaker = useCallback(() => {
     domEditSaveQueueRef.current?.reset();
     setDomEditSaveQueuePaused(null);
@@ -256,6 +261,7 @@ export function usePreviewPersistence({
     queueDomEditSave,
     drainPendingDomEditSaves,
     waitForPendingDomEditSaves,
+    settlePendingEdits,
     domEditSaveQueuePaused,
     resetDomEditSaveQueueBreaker,
     applyCurrentStudioManualEditsToPreview,