diff --git a/panel/src/components/ValidParam.test.tsx b/panel/src/components/ValidParam.test.tsx new file mode 100644 index 0000000..11699eb --- /dev/null +++ b/panel/src/components/ValidParam.test.tsx @@ -0,0 +1,43 @@ +// @vitest-environment jsdom +import { describe, it, expect } from "vitest"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { Link, MemoryRouter, Route, Routes } from "react-router-dom"; +import { ValidParam } from "./ValidParam"; + +// A page with state of its own, as a console's half-typed command is. +function Page() { + return ( + <> + + beta + + ); +} + +function renderAt(path: string) { + return render( + + + }> + } /> + + + , + ); +} + +describe("ValidParam", () => { + it("starts the page afresh when the parameter changes", () => { + renderAt("/servers/alpha"); + fireEvent.change(screen.getByLabelText("command"), { target: { value: "stop" } }); + + fireEvent.click(screen.getByRole("link", { name: "beta" })); + expect((screen.getByLabelText("command") as HTMLInputElement).value).toBe(""); + }); + + it("says not found for a parameter of the wrong shape", () => { + renderAt("/servers/Alpha.."); + expect(screen.queryByLabelText("command")).toBeNull(); + expect(screen.getByText("Page not found")).toBeTruthy(); + }); +}); diff --git a/panel/src/components/ValidParam.tsx b/panel/src/components/ValidParam.tsx index 18c9b04..6118885 100644 --- a/panel/src/components/ValidParam.tsx +++ b/panel/src/components/ValidParam.tsx @@ -3,8 +3,10 @@ import { NotFound } from "@/components/States"; // ValidParam is a layout route that renders its children only when one URL // parameter has the expected shape (lib/params.ts), and "not found" otherwise. +// A new value remounts the page below, so /servers/a → /servers/b drops a's +// half-typed input, open dialog and log buffer along with its data. export function ValidParam({ param, pattern }: { param: string; pattern: RegExp }) { const value = useParams()[param]; if (value === undefined || !pattern.test(value)) return ; - return ; + return ; } diff --git a/panel/src/lib/hooks.test.tsx b/panel/src/lib/hooks.test.tsx new file mode 100644 index 0000000..98f3f1a --- /dev/null +++ b/panel/src/lib/hooks.test.tsx @@ -0,0 +1,102 @@ +// @vitest-environment jsdom +import { describe, it, expect } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import { useAsync, type AsyncOptions } from "./hooks"; + +// A request the test settles by hand, one per call, in call order. +function requests() { + const pending: { id: string; resolve: (v: string) => void; reject: (e: unknown) => void }[] = []; + const fetch = (id: string) => + new Promise((resolve, reject) => pending.push({ id, resolve, reject })); + const settle = async (i: number, how: { ok: string } | { fail: unknown }) => { + await act(async () => { + if ("ok" in how) pending[i].resolve(how.ok); + else pending[i].reject(how.fail); + }); + }; + return { pending, fetch, settle }; +} + +// Every render's (id, data, error, loading), so a stale value shown for a +// single render between new deps and the reload cannot slip past. +function mount(opts?: AsyncOptions) { + const req = requests(); + const seen: { id: string; data: string | null; error: unknown; loading: boolean }[] = []; + const hook = renderHook( + ({ id }: { id: string }) => { + const r = useAsync(() => req.fetch(id), [id], opts); + seen.push({ id, data: r.data, error: r.error, loading: r.loading }); + return r; + }, + { initialProps: { id: "a" } }, + ); + return { ...req, ...hook, seen }; +} + +describe("useAsync", () => { + it("never shows one id's result while another id loads", async () => { + const h = mount(); + await h.settle(0, { ok: "server a" }); + expect(h.result.current.data).toBe("server a"); + + h.rerender({ id: "b" }); + const forB = h.seen.filter((s) => s.id === "b"); + expect(forB.length).toBeGreaterThan(0); + expect(forB.filter((s) => s.data !== null || !s.loading)).toEqual([]); + + await h.settle(1, { ok: "server b" }); + expect(h.result.current).toMatchObject({ data: "server b", loading: false }); + }); + + it("never shows one id's error for another id", async () => { + const h = mount(); + await h.settle(0, { fail: new Error("a is gone") }); + expect(h.result.current.error).toBeInstanceOf(Error); + + h.rerender({ id: "b" }); + const forB = h.seen.filter((s) => s.id === "b"); + expect(forB.length).toBeGreaterThan(0); + expect(forB.filter((s) => s.error !== null)).toEqual([]); + }); + + it("keeps the data through a reload of the same id", async () => { + const h = mount(); + await h.settle(0, { ok: "first read" }); + + act(() => h.result.current.reload()); + expect(h.result.current).toMatchObject({ data: "first read", loading: true }); + + await h.settle(1, { ok: "second read" }); + expect(h.result.current).toMatchObject({ data: "second read", loading: false }); + }); + + it("keeps the data and reports the error when a reload of the same id fails", async () => { + const h = mount(); + await h.settle(0, { ok: "first read" }); + + act(() => h.result.current.reload()); + await h.settle(1, { fail: new Error("blip") }); + expect(h.result.current.data).toBe("first read"); + expect(h.result.current.error).toBeInstanceOf(Error); + expect(h.result.current.loading).toBe(false); + }); + + it("with keepPrevious keeps the last rows until the new ones arrive", async () => { + const h = mount({ keepPrevious: true }); + await h.settle(0, { ok: "page 1" }); + + h.rerender({ id: "b" }); + expect(h.result.current).toMatchObject({ data: "page 1", loading: true }); + + await h.settle(1, { ok: "page 2" }); + expect(h.result.current).toMatchObject({ data: "page 2", loading: false }); + }); + + it("drops a slow answer for an id it has moved past", async () => { + const h = mount(); + h.rerender({ id: "b" }); + await h.settle(1, { ok: "server b" }); + await h.settle(0, { ok: "server a" }); + expect(h.result.current.data).toBe("server b"); + }); +}); diff --git a/panel/src/lib/hooks.ts b/panel/src/lib/hooks.ts index 3fa4dbf..22448da 100644 --- a/panel/src/lib/hooks.ts +++ b/panel/src/lib/hooks.ts @@ -25,12 +25,33 @@ export interface AsyncState { reload: () => void; } +export interface AsyncOptions { + /** Keep showing the last result while new deps load, for a list whose deps + * are its filter or page: the old rows stay put (each still acts on its own + * item) instead of blanking to a spinner on every keystroke. */ + keepPrevious?: boolean; +} + +interface Settled { + /** The producer this result came from; a new one means new deps. */ + run: () => Promise; + data: T | null; + error: unknown; + loading: boolean; +} + /** useAsync runs an async producer on mount and on demand, guarding against - * setState-after-unmount and out-of-order responses. */ -export function useAsync(fn: () => Promise, deps: unknown[] = []): AsyncState { - const [data, setData] = useState(null); - const [error, setError] = useState(null); - const [loading, setLoading] = useState(true); + * setState-after-unmount and out-of-order responses. A reload with the same + * deps (polling, after a mutation) keeps the current data while it runs. New + * deps start from nothing: the result of /servers/a is never shown as + * /servers/b, whose buttons already act on b, not even for the one render + * before the new request starts. */ +export function useAsync( + fn: () => Promise, + deps: unknown[] = [], + { keepPrevious = false }: AsyncOptions = {}, +): AsyncState { + const [settled, setSettled] = useState | null>(null); const seq = useRef(0); // eslint-disable-next-line react-hooks/exhaustive-deps @@ -38,23 +59,23 @@ export function useAsync(fn: () => Promise, deps: unknown[] = []): AsyncSt const reload = useCallback(() => { const ticket = ++seq.current; - setLoading(true); - setError(null); + setSettled((s) => ({ + run, + data: s?.run === run || keepPrevious ? (s?.data ?? null) : null, + error: null, + loading: true, + })); run().then( (d) => { - if (ticket === seq.current) { - setData(d); - setLoading(false); - } + if (ticket === seq.current) setSettled({ run, data: d, error: null, loading: false }); }, (e) => { if (ticket === seq.current) { - setError(e); - setLoading(false); + setSettled((s) => ({ run, data: s?.data ?? null, error: e, loading: false })); } }, ); - }, [run]); + }, [run, keepPrevious]); useEffect(() => { reload(); @@ -65,7 +86,15 @@ export function useAsync(fn: () => Promise, deps: unknown[] = []): AsyncSt }; }, [reload]); - return { data, error, loading, reload }; + // Checked at render: between new deps and the effect that reloads them the + // state still holds the old producer's result. + const current = settled?.run === run; + return { + data: current || keepPrevious ? (settled?.data ?? null) : null, + error: current ? settled.error : null, + loading: current ? settled.loading : true, + reload, + }; } /** useUnsavedGuard asks the browser to confirm leaving the page (reload, tab diff --git a/panel/src/pages/admin/ImageBuildPage.tsx b/panel/src/pages/admin/ImageBuildPage.tsx index 90130db..b5dfd89 100644 --- a/panel/src/pages/admin/ImageBuildPage.tsx +++ b/panel/src/pages/admin/ImageBuildPage.tsx @@ -161,7 +161,7 @@ export function ImageBuildPage() { error: listError, loading: loadingBuilds, reload: reloadBuilds, - } = useAsync(listBuilds, [listBuilds]); + } = useAsync(listBuilds, [listBuilds], { keepPrevious: true }); const builds = useMemo(() => buildPage?.builds ?? [], [buildPage]); const total = buildPage?.total ?? 0; diff --git a/panel/src/pages/admin/UsersPage.tsx b/panel/src/pages/admin/UsersPage.tsx index 6f35fda..bcab8ab 100644 --- a/panel/src/pages/admin/UsersPage.tsx +++ b/panel/src/pages/admin/UsersPage.tsx @@ -53,7 +53,7 @@ export function UsersPage() { [query, roleFilter, disabledFilter, page], ); - const { data, error, loading, reload } = useAsync(fetchUsers, [fetchUsers]); + const { data, error, loading, reload } = useAsync(fetchUsers, [fetchUsers], { keepPrevious: true }); const handleSearch = (e: React.FormEvent) => { e.preventDefault();