From a6392ef51c1110dbe5f585c99041ecbc7ef91973 Mon Sep 17 00:00:00 2001 From: Rishit Sharma Date: Sat, 22 Aug 2026 01:02:43 +0530 Subject: [PATCH 1/2] fix(dashboard): keep service rows during refetch (#666) The services tab collapsed into its skeleton pulse-cards on every action and every tab switch. Two loading flags gated the whole tab and both flipped to true on every refetch: - refreshServices() set isLoading: true unconditionally, and its error path wiped the list to [] on transient failures. - ServicesTab OR-ed containersLoading (which flips on every fetchData call and every remount of the tab) straight into its skeleton gate. Fix: stale-while-revalidate semantics, matching what useEndpoint/ beginFetchState already do for project info. - refreshServices reports loading only when the list is empty and keeps the previous list when a refetch fails. - The tab renders its skeleton only when there is nothing to show; service rows render without container data (status falls back per service) while the containers read is in flight. Consumers of servicesData.isLoading (LogsSettings, OverviewTab, DomainSettings) inherit the calmer semantics: stale data stays on screen during revalidation instead of flashing placeholders. Fixes #666 --- .../projects/[id]/components/ServicesTab.tsx | 7 ++++++- .../src/context/ProjectSettingsContext.tsx | 18 ++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/apps/dashboard/src/app/(dashboard)/projects/[id]/components/ServicesTab.tsx b/apps/dashboard/src/app/(dashboard)/projects/[id]/components/ServicesTab.tsx index cbcbf9f95..c49f36f02 100644 --- a/apps/dashboard/src/app/(dashboard)/projects/[id]/components/ServicesTab.tsx +++ b/apps/dashboard/src/app/(dashboard)/projects/[id]/components/ServicesTab.tsx @@ -59,7 +59,12 @@ export const ServicesTab = () => { () => sortServicesByPublicFirst(servicesData.services), [servicesData.services], ); - const loading = servicesData.isLoading || containersLoading; + // Skeleton only when there is nothing to show. containersLoading flips on + // every refetch (and every remount of this tab), so OR-ing it raw flashed + // the full-tab skeleton on every action and tab switch (#666) — rows render + // fine without container data (status falls back per service). + const loading = + servicesData.isLoading || (containersLoading && services.length === 0); const projectSlugBase = projectData.slug || projectData.name || "project"; const selectedId = slug?.[1] ?? null; const hasProjectId = Boolean(id && id !== "undefined"); diff --git a/apps/dashboard/src/context/ProjectSettingsContext.tsx b/apps/dashboard/src/context/ProjectSettingsContext.tsx index 0489e6990..386008931 100644 --- a/apps/dashboard/src/context/ProjectSettingsContext.tsx +++ b/apps/dashboard/src/context/ProjectSettingsContext.tsx @@ -748,7 +748,14 @@ export const ProjectSettingsProvider: React.FC = ({ let promise!: Promise; promise = (async () => { - setServicesData((prev) => ({ ...prev, isLoading: true, error: null })); + // Stale-while-revalidate: report loading only when there is nothing on + // screen to keep. Flipping isLoading on every refetch collapsed the + // services tab into its skeleton on every action (#666). + setServicesData((prev) => ({ + ...prev, + isLoading: prev.services.length === 0, + error: null, + })); try { const response = await servicesApi.list(id); @@ -764,11 +771,14 @@ export const ProjectSettingsProvider: React.FC = ({ } catch (error) { console.error("Failed to fetch project services:", error); if (servicesRequestIdRef.current === requestId) { - setServicesData({ - services: [], + // Keep the previous list on a failed refetch — wiping it blanked + // the tab on transient errors. A first-load failure still surfaces + // the error state because there was nothing to keep. + setServicesData((prev) => ({ + ...prev, isLoading: false, error: "Failed to load services", - }); + })); } return []; } finally { From 987c7061e9b687ad4e5c7334625d30f9e29d9bae Mon Sep 17 00:00:00 2001 From: Rishit Sharma Date: Sat, 22 Aug 2026 01:17:05 +0530 Subject: [PATCH 2/2] cleanup(dashboard): services fetch state (#666) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on #684 — port the documented beginFetchState rule instead of sniffing loading off the data, and pair the tab's error gate so kept rows are actually visible. - New pure module src/context/services-fetch-state.ts: loading is loadedId !== id, not a property of the list. Same id is a REFRESH (keep rows, no skeleton); different id is a NAVIGATION (nothing of this project's to show — calling it loaded would render project A's services under project B). - refreshServices tracks servicesLoadedIdRef; both failure paths (success:false and throw) collapse into one symmetric fail() that keeps rows only for a same-id refresh. - ServicesTab: full-tab error card only when nothing is showable; failed refetch with rows keeps them and reports inline with retry. Fixes empty-project flash too: creating the first service no longer re-skeletons, since loadedId already matches. - Tests: 7 cases pinning refresh/navigation/first-load/failure — fails if anyone re-reads loading off the data again. --- .../projects/[id]/components/ServicesTab.tsx | 20 ++++++- .../src/context/ProjectSettingsContext.tsx | 44 +++++++-------- .../src/context/services-fetch-state.test.ts | 53 +++++++++++++++++++ .../src/context/services-fetch-state.ts | 40 ++++++++++++++ 4 files changed, 132 insertions(+), 25 deletions(-) create mode 100644 apps/dashboard/src/context/services-fetch-state.test.ts create mode 100644 apps/dashboard/src/context/services-fetch-state.ts diff --git a/apps/dashboard/src/app/(dashboard)/projects/[id]/components/ServicesTab.tsx b/apps/dashboard/src/app/(dashboard)/projects/[id]/components/ServicesTab.tsx index c49f36f02..fb08d0f37 100644 --- a/apps/dashboard/src/app/(dashboard)/projects/[id]/components/ServicesTab.tsx +++ b/apps/dashboard/src/app/(dashboard)/projects/[id]/components/ServicesTab.tsx @@ -231,7 +231,12 @@ export const ServicesTab = () => { } /* ── Error state ───────────────────────────────────────────────── */ - if (error || servicesData.error) { + // Full-tab error only when there is nothing to show. A failed refetch with + // rows on screen keeps them and reports the failure inline — blanking a + // working list on a transient 5xx was the same complaint as the skeleton + // flash (#666). + const failure = error || servicesData.error; + if (failure && services.length === 0) { return (
@@ -529,6 +534,19 @@ export const ServicesTab = () => {
)} + {failure && ( +
+ + {failure} + +
+ )} +
{services.map((svc) => { const ct = containerFor(svc.id); diff --git a/apps/dashboard/src/context/ProjectSettingsContext.tsx b/apps/dashboard/src/context/ProjectSettingsContext.tsx index 386008931..293b3fea8 100644 --- a/apps/dashboard/src/context/ProjectSettingsContext.tsx +++ b/apps/dashboard/src/context/ProjectSettingsContext.tsx @@ -18,6 +18,7 @@ import { projectsApi, servicesApi, type Service } from "@/lib/api"; import { PROJECT_INFO_NOT_FOUND, useProjectInfo } from "@/hooks/useProjectEndpoints"; import type { ActiveMigration } from "@/utils/project-status"; import { dedupeServerLogs } from "./server-log-dedup"; +import { beginServicesFetch, failServicesFetch } from "./services-fetch-state"; interface ProjectDomain { domain: string; @@ -730,11 +731,14 @@ export const ProjectSettingsProvider: React.FC = ({ null, ); const servicesRequestIdRef = useRef(0); + /** Which project the list in `servicesData` belongs to. null = holds nothing. */ + const servicesLoadedIdRef = useRef(null); const refreshServices = useCallback(async () => { if (!id || id === "undefined") { servicesRequestIdRef.current += 1; servicesRequestRef.current = null; + servicesLoadedIdRef.current = null; setServicesData({ services: [], isLoading: false, error: null }); return []; } @@ -748,38 +752,30 @@ export const ProjectSettingsProvider: React.FC = ({ let promise!: Promise; promise = (async () => { - // Stale-while-revalidate: report loading only when there is nothing on - // screen to keep. Flipping isLoading on every refetch collapsed the - // services tab into its skeleton on every action (#666). - setServicesData((prev) => ({ - ...prev, - isLoading: prev.services.length === 0, - error: null, - })); + const loadedId = servicesLoadedIdRef.current; + setServicesData((prev) => beginServicesFetch(prev, loadedId, id)); + + const fail = () => { + if (servicesRequestIdRef.current !== requestId) return; + setServicesData((prev) => failServicesFetch(prev, loadedId, id)); + if (loadedId !== id) servicesLoadedIdRef.current = null; + }; try { const response = await servicesApi.list(id); - const services = response.success ? (response.services ?? []) : []; + if (!response.success) { + fail(); + return []; + } + const services = response.services ?? []; if (servicesRequestIdRef.current === requestId) { - setServicesData({ - services, - isLoading: false, - error: response.success ? null : "Failed to load services", - }); + servicesLoadedIdRef.current = id; + setServicesData({ services, isLoading: false, error: null }); } return services; } catch (error) { console.error("Failed to fetch project services:", error); - if (servicesRequestIdRef.current === requestId) { - // Keep the previous list on a failed refetch — wiping it blanked - // the tab on transient errors. A first-load failure still surfaces - // the error state because there was nothing to keep. - setServicesData((prev) => ({ - ...prev, - isLoading: false, - error: "Failed to load services", - })); - } + fail(); return []; } finally { if (servicesRequestRef.current?.promise === promise) { diff --git a/apps/dashboard/src/context/services-fetch-state.test.ts b/apps/dashboard/src/context/services-fetch-state.test.ts new file mode 100644 index 000000000..96b23aa28 --- /dev/null +++ b/apps/dashboard/src/context/services-fetch-state.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect } from "vitest"; +import type { Service } from "@/lib/api"; +import { beginServicesFetch, failServicesFetch } from "./services-fetch-state"; + +/** + * The #666 regression and its inverse both live here: a refresh that reports + * loading (skeleton flash on every action), and a navigation that doesn't + * (project A's rows rendered under project B). The fix must satisfy both. + */ + +const rows = [{ id: "s1" }] as unknown as Service[]; +const prev = { services: rows, isLoading: false, error: null }; + +describe("beginServicesFetch", () => { + it("refresh keeps the rows on screen and reports no loading", () => { + expect(beginServicesFetch(prev, "A", "A")).toMatchObject({ + isLoading: false, + services: rows, + }); + }); + + it("navigation reports loading even though the held list is non-empty", () => { + // A's rows must never pass for B's — the skeleton is correct here. + expect(beginServicesFetch(prev, "A", "B").isLoading).toBe(true); + }); + + it("first load with nothing held reports loading", () => { + expect( + beginServicesFetch({ services: [], isLoading: false, error: null }, null, "A").isLoading, + ).toBe(true); + }); + + it("clears a previous error once a fetch starts", () => { + const failing = { ...prev, error: "Failed to load services" }; + expect(beginServicesFetch(failing, "A", "A").error).toBeNull(); + }); +}); + +describe("failServicesFetch", () => { + it("failed refresh keeps the rows that were on screen", () => { + expect(failServicesFetch(prev, "A", "A").services).toBe(rows); + }); + + it("failed navigation drops the previous project's list", () => { + expect(failServicesFetch(prev, "A", "B").services).toEqual([]); + }); + + it("always clears loading and surfaces the failure", () => { + const out = failServicesFetch({ ...prev, isLoading: true }, "A", "A"); + expect(out.isLoading).toBe(false); + expect(out.error).toBe("Failed to load services"); + }); +}); diff --git a/apps/dashboard/src/context/services-fetch-state.ts b/apps/dashboard/src/context/services-fetch-state.ts new file mode 100644 index 000000000..a607466e4 --- /dev/null +++ b/apps/dashboard/src/context/services-fetch-state.ts @@ -0,0 +1,40 @@ +import type { Service } from "@/lib/api"; + +export interface ServicesFetchState { + services: Service[]; + isLoading: boolean; + error: string | null; +} + +/** + * Whether a services read has anything on screen worth keeping cannot be read off + * the list — it depends on whether that list belongs to the id being fetched. + * `beginFetchState` owns the full reasoning (and the infinite-loop regression it + * exists to prevent); this is the same rule for the services slice, whose shape is + * `services`, not `data`. + * + * Same id is a REFRESH: keep the rows, report no loading — flipping it collapsed + * the tab into its skeleton on every action and tab switch (#666). + * Different id is a NAVIGATION: there is nothing of this project's to show, and + * calling it loaded would render project A's services under project B. + */ +export function beginServicesFetch( + prev: ServicesFetchState, + loadedId: string | null, + id: string, +): ServicesFetchState { + return { ...prev, isLoading: loadedId !== id, error: null }; +} + +/** A failed REFRESH keeps its rows; a failed NAVIGATION must not leave the previous project's. */ +export function failServicesFetch( + prev: ServicesFetchState, + loadedId: string | null, + id: string, +): ServicesFetchState { + return { + services: loadedId === id ? prev.services : [], + isLoading: false, + error: "Failed to load services", + }; +}