mirror of
https://github.com/AmruthPillai/Reactive-Resume.git
synced 2026-10-03 10:13:47 +10:00
fix(builder): save pending drafts before navigation (#3453)
* fix(builder): save pending drafts before navigation * fix(builder): bound navigation waits for slow saves
This commit is contained in:
@@ -4794,6 +4794,10 @@ msgstr "Save the file with every page upright."
|
||||
msgid "Saved"
|
||||
msgstr "Saved"
|
||||
|
||||
#: src/features/resume/builder/draft.ts
|
||||
msgid "Saving is taking longer than expected. Your changes are still open."
|
||||
msgstr "Saving is taking longer than expected. Your changes are still open."
|
||||
|
||||
#: src/routes/builder/$resumeId/-components/header.tsx
|
||||
msgid "Saving…"
|
||||
msgstr "Saving…"
|
||||
|
||||
@@ -9,6 +9,7 @@ import { defaultResumeData } from "@reactive-resume/schema/resume/default";
|
||||
import {
|
||||
isEditableElementFocused,
|
||||
useBuilderResumeUpdateSubscription,
|
||||
useResumeCleanup,
|
||||
useResumeStore,
|
||||
useResumeUpdateSubscription,
|
||||
} from "./draft";
|
||||
@@ -20,6 +21,8 @@ const orpcMocks = vi.hoisted(() => ({
|
||||
updateResume: vi.fn(),
|
||||
}));
|
||||
|
||||
const useBlockerMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
const consumeEventIteratorMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
const queryClientMock = vi.hoisted(() => ({
|
||||
@@ -45,6 +48,7 @@ vi.mock("@tanstack/react-query", () => ({
|
||||
|
||||
vi.mock("@tanstack/react-router", () => ({
|
||||
useParams: () => routerParamsMock.value,
|
||||
useBlocker: useBlockerMock,
|
||||
}));
|
||||
|
||||
vi.mock("@/libs/orpc/client", () => ({
|
||||
@@ -115,9 +119,139 @@ async function flushMicrotasks() {
|
||||
}
|
||||
|
||||
describe("builder resume autosave", () => {
|
||||
it("waits for the latest draft to save before allowing navigation", async () => {
|
||||
const initial = makeResume("navigation-debounce");
|
||||
useResumeStore.getState().initialize(initial);
|
||||
routerParamsMock.value = { resumeId: initial.id };
|
||||
const hook = renderHook(() => useResumeCleanup());
|
||||
let complete!: (resume: Resume) => void;
|
||||
orpcMocks.updateResume.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise<Resume>((resolve) => {
|
||||
complete = resolve;
|
||||
}),
|
||||
);
|
||||
useResumeStore.getState().updateResumeData((draft) => {
|
||||
draft.basics.name = "Navigate safely";
|
||||
});
|
||||
const blocker = useBlockerMock.mock.lastCall?.[0];
|
||||
expect(blocker).toBeDefined();
|
||||
let settled = false;
|
||||
const result = blocker.shouldBlockFn({ next: { params: {} } }).then((blocked: boolean) => {
|
||||
settled = true;
|
||||
return blocked;
|
||||
});
|
||||
await flushMicrotasks();
|
||||
expect(settled).toBe(false);
|
||||
expect(orpcMocks.updateResume.mock.lastCall?.[1].signal.aborted).toBe(false);
|
||||
complete(withBasicsName(initial, "Navigate safely"));
|
||||
expect(await result).toBe(false);
|
||||
expect(useResumeStore.getState().saveStatus).toBe("saved");
|
||||
hook.unmount();
|
||||
});
|
||||
|
||||
it("waits for a queued edit after an in-flight save before navigating", async () => {
|
||||
const initial = makeResume("navigation-queued");
|
||||
useResumeStore.getState().initialize(initial);
|
||||
routerParamsMock.value = { resumeId: initial.id };
|
||||
const hook = renderHook(() => useResumeCleanup());
|
||||
const completions: Array<(resume: Resume) => void> = [];
|
||||
orpcMocks.updateResume.mockImplementation(
|
||||
() =>
|
||||
new Promise<Resume>((resolve) => {
|
||||
completions.push(resolve);
|
||||
}),
|
||||
);
|
||||
useResumeStore.getState().updateResumeData((draft) => {
|
||||
draft.basics.name = "First";
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(500);
|
||||
useResumeStore.getState().updateResumeData((draft) => {
|
||||
draft.basics.name = "Latest";
|
||||
});
|
||||
const blocker = useBlockerMock.mock.lastCall?.[0];
|
||||
let settled = false;
|
||||
const result = blocker.shouldBlockFn({ next: { params: {} } }).then((blocked: boolean) => {
|
||||
settled = true;
|
||||
return blocked;
|
||||
});
|
||||
completions[0](withBasicsName(initial, "First"));
|
||||
await flushMicrotasks();
|
||||
expect(settled).toBe(false);
|
||||
expect(orpcMocks.updateResume.mock.lastCall?.[0].data.basics.name).toBe("Latest");
|
||||
completions[1](withBasicsName(initial, "Latest"));
|
||||
expect(await result).toBe(false);
|
||||
expect(useResumeStore.getState().resume?.data.basics.name).toBe("Latest");
|
||||
hook.unmount();
|
||||
});
|
||||
|
||||
it("ends a stalled navigation wait without aborting or discarding the pending save", async () => {
|
||||
const initial = makeResume("navigation-timeout");
|
||||
useResumeStore.getState().initialize(initial);
|
||||
routerParamsMock.value = { resumeId: initial.id };
|
||||
const hook = renderHook(() => useResumeCleanup());
|
||||
let complete!: (resume: Resume) => void;
|
||||
orpcMocks.updateResume.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise<Resume>((resolve) => {
|
||||
complete = resolve;
|
||||
}),
|
||||
);
|
||||
useResumeStore.getState().updateResumeData((draft) => {
|
||||
draft.basics.name = "Pending name";
|
||||
});
|
||||
const blocker = useBlockerMock.mock.lastCall?.[0];
|
||||
let settled = false;
|
||||
const result = blocker.shouldBlockFn({ next: { params: {} } }).then((blocked: boolean) => {
|
||||
settled = true;
|
||||
return blocked;
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(10000);
|
||||
expect(settled).toBe(true);
|
||||
expect(await result).toBe(true);
|
||||
expect(useResumeStore.getState().saveStatus).toBe("saving");
|
||||
expect(useResumeStore.getState().resume?.data.basics.name).toBe("Pending name");
|
||||
expect(orpcMocks.updateResume.mock.lastCall?.[1].signal.aborted).toBe(false);
|
||||
|
||||
orpcMocks.updateResume.mockResolvedValueOnce(withBasicsName(initial, "Latest name"));
|
||||
useResumeStore.getState().updateResumeData((draft) => {
|
||||
draft.basics.name = "Latest name";
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(500);
|
||||
expect(orpcMocks.updateResume).toHaveBeenCalledTimes(1);
|
||||
complete(withBasicsName(initial, "Pending name"));
|
||||
await flushMicrotasks();
|
||||
expect(useResumeStore.getState().saveStatus).toBe("saved");
|
||||
expect(useResumeStore.getState().resume?.data.basics.name).toBe("Latest name");
|
||||
expect(await blocker.shouldBlockFn({ next: { params: {} } })).toBe(false);
|
||||
expect(orpcMocks.updateResume).toHaveBeenCalledTimes(2);
|
||||
hook.unmount();
|
||||
});
|
||||
|
||||
it("keeps a failed draft in the builder and retries on the next navigation", async () => {
|
||||
const initial = makeResume("navigation-error");
|
||||
useResumeStore.getState().initialize(initial);
|
||||
routerParamsMock.value = { resumeId: initial.id };
|
||||
const hook = renderHook(() => useResumeCleanup());
|
||||
orpcMocks.updateResume.mockRejectedValueOnce(new Error("Offline"));
|
||||
useResumeStore.getState().updateResumeData((draft) => {
|
||||
draft.basics.name = "Keep this draft";
|
||||
});
|
||||
const blocker = useBlockerMock.mock.lastCall?.[0];
|
||||
expect(blocker).toBeDefined();
|
||||
expect(await blocker.shouldBlockFn({ next: { params: {} } })).toBe(true);
|
||||
expect(useResumeStore.getState().resume?.data.basics.name).toBe("Keep this draft");
|
||||
expect(blocker.enableBeforeUnload()).toBe(true);
|
||||
orpcMocks.updateResume.mockResolvedValueOnce(withBasicsName(initial, "Keep this draft"));
|
||||
expect(await blocker.shouldBlockFn({ next: { params: {} } })).toBe(false);
|
||||
expect(blocker.enableBeforeUnload()).toBe(false);
|
||||
hook.unmount();
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
orpcMocks.getResumeById.mockReset();
|
||||
useBlockerMock.mockReset();
|
||||
orpcMocks.patchResume.mockReset();
|
||||
orpcMocks.streamSubscribe.mockReset();
|
||||
orpcMocks.updateResume.mockReset();
|
||||
|
||||
@@ -4,7 +4,7 @@ import type { WritableDraft } from "immer";
|
||||
import { t } from "@lingui/core/macro";
|
||||
import { consumeEventIterator } from "@orpc/client";
|
||||
import { useQueryClient } from "@tanstack/react-query";
|
||||
import { useParams } from "@tanstack/react-router";
|
||||
import { useBlocker, useParams } from "@tanstack/react-router";
|
||||
import { debounce, isEqual } from "es-toolkit";
|
||||
import { useCallback, useEffect, useState } from "react";
|
||||
import { immer } from "zustand/middleware/immer";
|
||||
@@ -65,6 +65,7 @@ type Runtime = {
|
||||
isSaving: boolean;
|
||||
pendingResume?: Resume;
|
||||
syncErrorToastId?: string;
|
||||
slowSaveToastId?: string;
|
||||
syncResume: ReturnType<typeof debounce<(resume: Resume) => void>>;
|
||||
beforeUnloadHandler?: () => void;
|
||||
deferredRemoteResume?: Resume;
|
||||
@@ -78,6 +79,7 @@ type ResumeUpdateSubscriptionOptions = {
|
||||
};
|
||||
|
||||
const SAVE_DEBOUNCE_MS = 500;
|
||||
const NAVIGATION_SAVE_WAIT_MS = 10_000;
|
||||
// Rapid edits within this window coalesce into a single undo step (e.g. typing a word / dragging).
|
||||
const HISTORY_COALESCE_MS = 500;
|
||||
// Bounded stacks: keep undo/redo memory (whole-resume snapshots) predictable during a long session.
|
||||
@@ -231,6 +233,10 @@ async function flushResumeSave(id: string) {
|
||||
timeout: 0,
|
||||
});
|
||||
} finally {
|
||||
if (runtime.slowSaveToastId !== undefined) {
|
||||
toast.close(runtime.slowSaveToastId);
|
||||
runtime.slowSaveToastId = undefined;
|
||||
}
|
||||
runtime.isSaving = false;
|
||||
if (runtime.pendingResume && runtime.syncErrorToastId === undefined) void flushResumeSave(id);
|
||||
}
|
||||
@@ -664,11 +670,56 @@ export function useBuilderResumeUpdateSubscription() {
|
||||
useResumeUpdateSubscription({ resumeId, onUpdate, onError });
|
||||
}
|
||||
|
||||
// Route transitions can await a save; unmount cleanup and browser unload cannot.
|
||||
function saveResumeBeforeLeaving(id: string): boolean | Promise<boolean> {
|
||||
const runtime = runtimes.get(id);
|
||||
const current = useResumeStore.getState().resume;
|
||||
if (!runtime?.hasPendingLocalChanges || current?.id !== id) return true;
|
||||
|
||||
runtime.syncResume.cancel();
|
||||
runtime.pendingResume = cloneResume(current);
|
||||
useResumeStore.getState().setSaveStatus("saving");
|
||||
|
||||
return new Promise<boolean>((resolve) => {
|
||||
const finish = (saved: boolean) => {
|
||||
clearTimeout(timeout);
|
||||
unsubscribe();
|
||||
resolve(saved);
|
||||
};
|
||||
const unsubscribe = useResumeStore.subscribe((state) => {
|
||||
if (state.resume?.id !== id || state.saveStatus === "error") {
|
||||
finish(false);
|
||||
} else if (state.saveStatus === "saved" && !runtime.hasPendingLocalChanges) {
|
||||
finish(true);
|
||||
}
|
||||
});
|
||||
const timeout = setTimeout(() => {
|
||||
finish(false);
|
||||
// Keep the write in flight: it may already have reached the server.
|
||||
runtime.slowSaveToastId = toast.add({
|
||||
type: "info",
|
||||
description: t`Saving is taking longer than expected. Your changes are still open.`,
|
||||
id: runtime.slowSaveToastId,
|
||||
timeout: 0,
|
||||
});
|
||||
}, NAVIGATION_SAVE_WAIT_MS);
|
||||
void flushResumeSave(id);
|
||||
});
|
||||
}
|
||||
|
||||
export function useResumeCleanup() {
|
||||
const params = useParams({ strict: false }) as { resumeId?: string };
|
||||
const resumeId = params.resumeId;
|
||||
const reset = useResumeStore((state) => state.reset);
|
||||
|
||||
useBlocker({
|
||||
shouldBlockFn: async ({ next }) => {
|
||||
if (!resumeId || ("resumeId" in next.params && next.params.resumeId === resumeId)) return false;
|
||||
return !(await saveResumeBeforeLeaving(resumeId));
|
||||
},
|
||||
enableBeforeUnload: () => !!resumeId && (runtimes.get(resumeId)?.hasPendingLocalChanges ?? false),
|
||||
});
|
||||
|
||||
useEffect(() => {
|
||||
if (!resumeId) return;
|
||||
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
import type { Page, TestInfo } from "@playwright/test";
|
||||
import { createSampleResumeFromDashboard } from "../fixtures/resume";
|
||||
import { expect, test } from "../fixtures/test";
|
||||
|
||||
const updateUrl = "**/api/rpc/resume/update";
|
||||
function barrier() {
|
||||
let resolve!: () => void;
|
||||
const promise = new Promise<void>((done) => {
|
||||
resolve = done;
|
||||
});
|
||||
return { promise, resolve };
|
||||
}
|
||||
function waitSave(page: Page) {
|
||||
return page.waitForResponse(
|
||||
(response) => new URL(response.url()).pathname === "/api/rpc/resume/update" && response.ok(),
|
||||
);
|
||||
}
|
||||
|
||||
async function prepareNavigationTest(page: Page, testInfo: TestInfo) {
|
||||
await createSampleResumeFromDashboard(page, testInfo);
|
||||
await page.reload();
|
||||
const warmup = waitSave(page);
|
||||
await page.getByLabel("Headline", { exact: true }).fill("Navigation fixture ready");
|
||||
await warmup;
|
||||
return page.url();
|
||||
}
|
||||
|
||||
test("retries a failed autosave before leaving the builder", async ({ authPage: page }, testInfo) => {
|
||||
const url = await prepareNavigationTest(page, testInfo);
|
||||
|
||||
await page.route(updateUrl, async (route) => {
|
||||
await route.abort("failed");
|
||||
});
|
||||
await page.getByLabel("Name", { exact: true }).fill("Draft recovered before leaving");
|
||||
await expect(page.getByText("Your latest changes could not be saved.", { exact: true })).toBeVisible();
|
||||
await page.unroute(updateUrl);
|
||||
const arrived = barrier();
|
||||
const release = barrier();
|
||||
await page.route(updateUrl, async (route) => {
|
||||
arrived.resolve();
|
||||
await release.promise;
|
||||
await route.continue();
|
||||
});
|
||||
await page.getByRole("button", { name: "Go to resumes dashboard", exact: true }).click();
|
||||
await arrived.promise;
|
||||
expect(page.url()).toBe(url);
|
||||
await expect(page.getByLabel("Name", { exact: true })).toHaveValue("Draft recovered before leaving");
|
||||
release.resolve();
|
||||
await page.waitForURL(/\/dashboard/);
|
||||
await page.goto(url);
|
||||
await expect(page.getByLabel("Name", { exact: true })).toHaveValue("Draft recovered before leaving");
|
||||
});
|
||||
|
||||
test("retains the current draft when saving during navigation fails", async ({ authPage: page }, testInfo) => {
|
||||
const url = await prepareNavigationTest(page, testInfo);
|
||||
let attempts = 0;
|
||||
await page.route(updateUrl, async (route) => {
|
||||
attempts++;
|
||||
await route.abort("failed");
|
||||
});
|
||||
await page.getByLabel("Name", { exact: true }).fill("Keep unsaved draft");
|
||||
await expect(page.getByText("Your latest changes could not be saved.", { exact: true })).toBeVisible();
|
||||
await page.getByRole("button", { name: "Go to resumes dashboard", exact: true }).click();
|
||||
await expect.poll(() => attempts).toBe(2);
|
||||
await expect(page.getByRole("status").filter({ hasText: "Couldn't save" })).toBeVisible();
|
||||
expect(page.url()).toBe(url);
|
||||
await expect(page.getByLabel("Name", { exact: true })).toHaveValue("Keep unsaved draft");
|
||||
await page.unroute(updateUrl);
|
||||
await page.getByRole("button", { name: "Go to resumes dashboard", exact: true }).click();
|
||||
await page.waitForURL(/\/dashboard/);
|
||||
await page.goto(url);
|
||||
await expect(page.getByLabel("Name", { exact: true })).toHaveValue("Keep unsaved draft");
|
||||
});
|
||||
|
||||
test("stops waiting for a slow save while preserving late acknowledgements and queued edits", async ({
|
||||
authPage: page,
|
||||
}, testInfo) => {
|
||||
const url = await prepareNavigationTest(page, testInfo);
|
||||
await page.clock.install();
|
||||
const arrived = barrier();
|
||||
const release = barrier();
|
||||
let attempts = 0;
|
||||
await page.route(updateUrl, async (route) => {
|
||||
attempts++;
|
||||
if (attempts === 1) {
|
||||
arrived.resolve();
|
||||
await release.promise;
|
||||
}
|
||||
await route.continue();
|
||||
});
|
||||
await page.getByLabel("Name", { exact: true }).fill("Slow save draft");
|
||||
await page.clock.fastForward(600);
|
||||
await arrived.promise;
|
||||
await page.getByRole("button", { name: "Go to resumes dashboard", exact: true }).click();
|
||||
await page.clock.fastForward(10000);
|
||||
const slowNotice = page.getByText("Saving is taking longer than expected. Your changes are still open.", {
|
||||
exact: true,
|
||||
});
|
||||
await expect(slowNotice).toBeVisible();
|
||||
await expect(page.getByRole("status").filter({ hasText: "Saving" })).toBeVisible();
|
||||
expect(page.url()).toBe(url);
|
||||
expect(attempts).toBe(1);
|
||||
|
||||
await page.getByLabel("Headline", { exact: true }).fill("Latest edit during slow save");
|
||||
await page.clock.fastForward(600);
|
||||
expect(attempts).toBe(1);
|
||||
const latestSaved = page.waitForResponse(
|
||||
(response) =>
|
||||
new URL(response.url()).pathname === "/api/rpc/resume/update" &&
|
||||
response.ok() &&
|
||||
(response.request().postData() ?? "").includes("Latest edit during slow save"),
|
||||
);
|
||||
release.resolve();
|
||||
await latestSaved;
|
||||
await expect(page.getByRole("status").filter({ hasText: "Saved" })).toBeVisible();
|
||||
await expect(slowNotice).toBeHidden();
|
||||
await page.getByRole("button", { name: "Go to resumes dashboard", exact: true }).click();
|
||||
await page.waitForURL(/\/dashboard/);
|
||||
await page.goto(url);
|
||||
await expect(page.getByLabel("Name", { exact: true })).toHaveValue("Slow save draft");
|
||||
await expect(page.getByLabel("Headline", { exact: true })).toHaveValue("Latest edit during slow save");
|
||||
expect(attempts).toBe(2);
|
||||
});
|
||||
Reference in New Issue
Block a user