let two windows onto the same dashboards agree again
The dashboard-state cache had staleTime: Infinity and there is no invalidateQueries anywhere in the
repo, so it was fetched once per page load and never again: two windows diverged permanently and neither
was ever told. It now refetches on focus — with three non-default guards, because this cache is
optimistic and a refetch that started before an in-flight PATCH landed would overwrite the value we
already showed. Never on mount (splitting a panel mounts a fresh consumer, which is exactly when a write
is in flight), never on reconnect, and on focus only after a short quiet period with nothing in flight.
The PATCH stopped assembling a full state blob it then returned to nobody — three SELECTs per splitter
release, thrown away, and a caller that did read it would be reading state assembled before whatever
concurrent write it raced.
And the last three `.catch(() => {})` in this family are gone: dashboard create, rename and delete build
their own multi-key patches and so bypass the hook. They now go through persistDashboardState, which
keeps the in-flight bookkeeping honest and, on failure, invalidates rather than reverts — there is no
single previous value to swap back once the roster has been rewritten, and a refetch is the only thing
that makes the list agree with the server. A failed delete used to leave the dashboard gone from the list
and alive on the server, reappearing at the next reload with no hint why.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -121,6 +121,8 @@ dashboardsRouter.patch('/', async (ctx) => {
|
||||
return ctx.json({ error: `unknown dashboard-state key "${key}"` }, 400);
|
||||
}
|
||||
|
||||
const state = await getAllDashboardState(userId);
|
||||
return ctx.json(state);
|
||||
// Three SELECTs per splitter release, thrown away — no caller has ever read this response body, and one
|
||||
// that did would be reading a blob assembled *before* the concurrent write it raced. The client
|
||||
// converges by refetching on focus instead (`useDashboardState`).
|
||||
return ctx.json({ ok: true });
|
||||
});
|
||||
|
||||
@@ -6,7 +6,7 @@ import { useGlobal } from 'hooks/useGlobal';
|
||||
import { useClient } from 'hooks/useClient';
|
||||
import { generateSlug } from 'helpers/slug';
|
||||
import { useUserState } from 'state/useUserState';
|
||||
import { useDashboardState } from 'state/useDashboardState';
|
||||
import { useDashboardState, persistDashboardState } from 'state/useDashboardState';
|
||||
import type { DashboardDefinition } from '../../components/Workspace';
|
||||
import {
|
||||
AlertDialog,
|
||||
@@ -90,7 +90,7 @@ export const DashboardListApp = () => {
|
||||
const currentState = queryClient.getQueryData<Record<string, unknown>>(['DASHBOARD_STATE']) ?? {};
|
||||
const { [layoutKey]: _, ...rest } = currentState;
|
||||
queryClient.setQueryData(['DASHBOARD_STATE'], rest);
|
||||
client.patch('/dashboards', { [layoutKey]: null }).catch(() => {});
|
||||
persistDashboardState(client, queryClient, { [layoutKey]: null });
|
||||
setDeleting(null);
|
||||
};
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@ import { useQueryClient } from '@tanstack/react-query';
|
||||
import { useGlobal } from 'hooks/useGlobal';
|
||||
import { useClient } from 'hooks/useClient';
|
||||
import { useUserState } from 'state/useUserState';
|
||||
import { useDashboardState } from 'state/useDashboardState';
|
||||
import { useDashboardState, persistDashboardState } from 'state/useDashboardState';
|
||||
import { WorkspaceLayout, WorkspaceView, createDefaultLayout } from '../../components/Workspace';
|
||||
import type { LayoutNode, DashboardDefinition } from '../../components/Workspace';
|
||||
import { Button } from '@/components/ui/button';
|
||||
@@ -320,10 +320,10 @@ const CreatePanel = () => {
|
||||
|
||||
const patch: Record<string, unknown> = { [newLayoutKey]: wsLayout };
|
||||
for (const k of oldKeys) patch[k] = null;
|
||||
client.patch('/dashboards', patch).catch(() => {});
|
||||
persistDashboardState(client, queryClient, patch);
|
||||
} else {
|
||||
queryClient.setQueryData(['DASHBOARD_STATE'], { ...currentState, [newLayoutKey]: wsLayout });
|
||||
client.patch('/dashboards', { [newLayoutKey]: wsLayout }).catch(() => {});
|
||||
persistDashboardState(client, queryClient, { [newLayoutKey]: wsLayout });
|
||||
}
|
||||
|
||||
setEditingId(null);
|
||||
@@ -353,7 +353,7 @@ const CreatePanel = () => {
|
||||
setDashboards((prev) => [...prev, ws]);
|
||||
const currentState = queryClient.getQueryData<Record<string, unknown>>(['DASHBOARD_STATE']) ?? {};
|
||||
queryClient.setQueryData(['DASHBOARD_STATE'], { ...currentState, [layoutKey]: wsLayout });
|
||||
client.patch('/dashboards', { [layoutKey]: wsLayout }).catch(() => {});
|
||||
persistDashboardState(client, queryClient, { [layoutKey]: wsLayout });
|
||||
|
||||
setName('');
|
||||
setDescription('');
|
||||
|
||||
@@ -5,7 +5,9 @@ import { useClient } from 'hooks/useClient';
|
||||
import { useAuth } from 'hooks/useAuth';
|
||||
import type { UserState } from './useSettings';
|
||||
|
||||
const QUERY_KEY = ['DASHBOARD_STATE'];
|
||||
export const DASHBOARD_STATE_QUERY_KEY = ['DASHBOARD_STATE'];
|
||||
|
||||
const QUERY_KEY = DASHBOARD_STATE_QUERY_KEY;
|
||||
|
||||
export function useDashboardState<T>(key: string, defaultValue: T) {
|
||||
const client = useClient();
|
||||
@@ -18,7 +20,22 @@ export function useDashboardState<T>(key: string, defaultValue: T) {
|
||||
queryKey: QUERY_KEY,
|
||||
enabled: isAuthenticated,
|
||||
queryFn: () => client.get<UserState>('/dashboards'),
|
||||
staleTime: Infinity,
|
||||
// `Infinity` meant this was fetched once per page load and never again, with no `invalidateQueries`
|
||||
// anywhere in the repo — so two windows onto the same dashboards diverged permanently and neither was
|
||||
// ever told. Refetching on focus is the cheapest thing that converges them: you come back to the tab
|
||||
// and it catches up.
|
||||
//
|
||||
// The other three knobs are the safety around it, and they are not defaults. This cache is optimistic:
|
||||
// a write updates it immediately and the PATCH follows. A refetch that started before that PATCH
|
||||
// landed returns pre-write state and would overwrite the value we already showed — the same
|
||||
// lost-update shape the layout updaters exist to prevent, and self-healing only until the next
|
||||
// mutation composes on top of the stale tree. So: never on mount (splitting a panel mounts a fresh
|
||||
// consumer, which is precisely when a write is in flight), never on reconnect, and on focus only when
|
||||
// nothing has been written for a moment.
|
||||
staleTime: 30_000,
|
||||
refetchOnMount: false,
|
||||
refetchOnReconnect: false,
|
||||
refetchOnWindowFocus: () => inFlightWrites === 0 && Date.now() - lastWriteAt > 2_000,
|
||||
});
|
||||
|
||||
// Seed default to backend when key is missing after initial fetch
|
||||
@@ -30,9 +47,9 @@ export function useDashboardState<T>(key: string, defaultValue: T) {
|
||||
const currentState = queryClient.getQueryData<UserState>(QUERY_KEY) ?? {};
|
||||
if (!(key in currentState)) {
|
||||
queryClient.setQueryData(QUERY_KEY, { ...currentState, [key]: defaultValue });
|
||||
clientRef.current
|
||||
.patch('/dashboards', { [key]: defaultValue })
|
||||
.catch((err: unknown) => revert(queryClient, key, defaultValue, undefined, err));
|
||||
persist(clientRef.current, key, defaultValue).catch((err: unknown) =>
|
||||
revert(queryClient, key, defaultValue, undefined, err),
|
||||
);
|
||||
}
|
||||
}, [isSuccess, key, defaultValue, queryClient]);
|
||||
|
||||
@@ -46,11 +63,9 @@ export function useDashboardState<T>(key: string, defaultValue: T) {
|
||||
|
||||
queryClient.setQueryData(QUERY_KEY, { ...currentState, [key]: newValue });
|
||||
|
||||
clientRef.current
|
||||
.patch('/dashboards', { [key]: newValue })
|
||||
.catch((err: unknown) =>
|
||||
revert(queryClient, key, newValue, key in currentState ? currentValue : undefined, err),
|
||||
);
|
||||
persist(clientRef.current, key, newValue).catch((err: unknown) =>
|
||||
revert(queryClient, key, newValue, key in currentState ? currentValue : undefined, err),
|
||||
);
|
||||
},
|
||||
[key, defaultValue, queryClient],
|
||||
);
|
||||
@@ -58,6 +73,56 @@ export function useDashboardState<T>(key: string, defaultValue: T) {
|
||||
return { key, value, setValue, isLoaded: isSuccess };
|
||||
}
|
||||
|
||||
/**
|
||||
* Every instance of this hook addresses one query key, so the write-in-flight bookkeeping the focus
|
||||
* refetch reads is module-level rather than per-instance. `lastWriteAt` is stamped on both ends: the
|
||||
* quiet period that has to elapse is measured from when the last write *settled*, not when it started.
|
||||
*/
|
||||
let inFlightWrites = 0;
|
||||
let lastWriteAt = 0;
|
||||
|
||||
function persist(client: ReturnType<typeof useClient>, key: string, value: unknown): Promise<unknown> {
|
||||
return patchState(client, { [key]: value });
|
||||
}
|
||||
|
||||
function patchState(client: ReturnType<typeof useClient>, patch: Record<string, unknown>): Promise<unknown> {
|
||||
inFlightWrites += 1;
|
||||
lastWriteAt = Date.now();
|
||||
return client.patch('/dashboards', patch).finally(() => {
|
||||
inFlightWrites -= 1;
|
||||
lastWriteAt = Date.now();
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* The same write, for the three callers that cannot use the hook.
|
||||
*
|
||||
* Creating, renaming and deleting a dashboard each move several keys at once — a rename PATCHes the new
|
||||
* layout and nulls three old ones in one body — so they build the patch themselves and update the cache
|
||||
* by hand. They were the last `.catch(() => {})` in this family: a delete that failed left the dashboard
|
||||
* gone from the list and alive on the server, reappearing at the next reload with no hint why.
|
||||
*
|
||||
* Recovery here is an invalidate rather than the hook's compare-and-swap revert. There is no single
|
||||
* previous value to swap back — the caller has already rewritten the roster and dropped keys from the
|
||||
* blob — and refetching is the only thing that makes the list agree with the server again.
|
||||
*/
|
||||
export function persistDashboardState(
|
||||
client: ReturnType<typeof useClient>,
|
||||
queryClient: ReturnType<typeof useQueryClient>,
|
||||
patch: Record<string, unknown>,
|
||||
): void {
|
||||
patchState(client, patch).catch((err: unknown) => {
|
||||
const keys = Object.keys(patch)
|
||||
.map((k) => `"${k}"`)
|
||||
.join(', ');
|
||||
console.error(`[dashboard-state] failed to persist ${keys}`, err);
|
||||
toast.error('Could not save that change', {
|
||||
description: `${keys} was not written — reloading from the server. ${err instanceof Error ? err.message : String(err)}`,
|
||||
});
|
||||
void queryClient.invalidateQueries({ queryKey: QUERY_KEY });
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Read a stored value, or the caller's default when what is stored cannot be that value.
|
||||
*
|
||||
|
||||
Reference in New Issue
Block a user