From a0ec1da425414a502bb887ee45247e3e4dea309b Mon Sep 17 00:00:00 2001 From: YYClaw <197375+yyclaw@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:05:06 +0800 Subject: [PATCH] fix(settings): restore mounted flag on remount so auto-save can settle (#5613) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The auto-save mount effect only cleared `mountedRef` in its cleanup and never re-set it to true on setup. Under React StrictMode the initial setup/cleanup/setup cycle therefore leaves `mountedRef.current` pinned to false for the component's whole life. A successful save then never reaches the terminal branch (`succeeded && mountedRef.current`), so the status is stranded on "saving" and the success toast never fires — the settings save indicator spins forever even though the PATCH returned 200. Set `mountedRef.current = true` on every mount so the flag reflects the live component. Shared by all auto-saved settings (repositories, GitHub, etc.). Add a StrictMode regression test asserting onSuccess still fires. --- .../settings/components/use-auto-save.test.tsx | 18 ++++++++++++++++++ .../views/settings/components/use-auto-save.ts | 13 ++++++++----- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/views/settings/components/use-auto-save.test.tsx b/packages/views/settings/components/use-auto-save.test.tsx index 513bf8f6b..55ea0f16d 100644 --- a/packages/views/settings/components/use-auto-save.test.tsx +++ b/packages/views/settings/components/use-auto-save.test.tsx @@ -1,3 +1,4 @@ +import { StrictMode } from "react"; import { act, render, waitFor } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; import { useAutoSave } from "./use-auto-save"; @@ -53,6 +54,23 @@ describe("useAutoSave success feedback", () => { }); }); + it("still reports success under StrictMode's double-invoked mount", async () => { + // StrictMode runs setup/cleanup/setup on mount. A mount effect that only + // clears the mounted flag in cleanup leaves it false for the component's + // life, so a successful save is silently dropped: neither the "saved" + // status nor onSuccess ever fires and the indicator spins forever. + const onSave = vi.fn(async () => undefined); + const onSuccess = vi.fn(); + render( + + + , + ); + + await waitFor(() => expect(onSave).toHaveBeenCalledWith("saved")); + await waitFor(() => expect(onSuccess).toHaveBeenCalledWith("saved")); + }); + it("waits for the latest queued value before reporting success", async () => { vi.useFakeTimers({ shouldAdvanceTime: true }); const first = deferred(); diff --git a/packages/views/settings/components/use-auto-save.ts b/packages/views/settings/components/use-auto-save.ts index b0c1b0874..a8656213d 100644 --- a/packages/views/settings/components/use-auto-save.ts +++ b/packages/views/settings/components/use-auto-save.ts @@ -131,13 +131,16 @@ export function useAutoSave({ }; }, [delay, enabled, isEqual, runSave, value]); - useEffect( - () => () => { + useEffect(() => { + // Restore the invariant on every mount. Under React StrictMode the initial + // setup/cleanup/setup cycle would otherwise leave mountedRef pinned to false + // for the component's whole life, stranding status at "saving". + mountedRef.current = true; + return () => { mountedRef.current = false; if (timerRef.current) clearTimeout(timerRef.current); - }, - [], - ); + }; + }, []); return { status, flush, saveNow }; }