fix(settings): restore mounted flag on remount so auto-save can settle (#5613)

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.
This commit is contained in:
YYClaw
2026-07-20 15:05:06 +08:00
committed by GitHub
parent 964b9269de
commit a0ec1da425
2 changed files with 26 additions and 5 deletions

View File

@@ -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(
<StrictMode>
<AutoSaveHarness value="saved" onSave={onSave} onSuccess={onSuccess} />
</StrictMode>,
);
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<void>();

View File

@@ -131,13 +131,16 @@ export function useAutoSave<T>({
};
}, [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 };
}