diff --git a/src/renderer/constants.ts b/src/renderer/constants.ts index 87e88f476..d6a922b1f 100644 --- a/src/renderer/constants.ts +++ b/src/renderer/constants.ts @@ -81,12 +81,16 @@ export const Constants = { REFRESH_ACCOUNTS_INTERVAL_MS: 60 * 60 * 1000, // 1 hour - // Query stale time in milliseconds, used by TanStack Query client - QUERY_STALE_TIME_MS: 30 * 1000, // 30 seconds - // Cooldown before retrying a failed query, used by TanStack Query client QUERY_RETRY_DELAY_MS: 30 * 1000, // 30 seconds + // Garbage-collection time for inactive query cache entries, used by + // TanStack Query client. Comfortably longer than either query's own + // refetch interval (notifications: 60s-60min, accounts: 1hr), so stale + // cache entries (e.g. from account churn) are collected well after they + // stop being read. + QUERY_GC_TIME_MS: 10 * 60 * 1000, // 10 minutes + // GraphQL Argument Defaults GRAPHQL_ARGS: { FIRST_LABELS: 100, diff --git a/src/renderer/hooks/useAccounts.test.tsx b/src/renderer/hooks/useAccounts.test.tsx new file mode 100644 index 000000000..ca78a6339 --- /dev/null +++ b/src/renderer/hooks/useAccounts.test.tsx @@ -0,0 +1,75 @@ +import { act, renderHook, waitFor } from '@testing-library/react'; +import type { ReactNode } from 'react'; + +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; + +import { mockGitHubCloudAccount } from '../__mocks__/account-mocks'; + +import { Constants } from '../constants'; + +import { useAccountsStore } from '../stores'; + +import * as authUtils from '../utils/auth/utils'; +import { useAccounts } from './useAccounts'; + +describe('renderer/hooks/useAccounts.ts', () => { + const refreshAccountSpy = vi + .spyOn(authUtils, 'refreshAccount') + .mockImplementation(async (account) => account); + + beforeEach(() => { + refreshAccountSpy.mockClear(); + useAccountsStore.setState({ accounts: [mockGitHubCloudAccount] }); + }); + + const createWrapper = () => { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + refetchOnWindowFocus: false, + refetchInterval: false, + }, + }, + }); + + return { + queryClient, + wrapper: ({ children }: { children: ReactNode }) => ( + {children} + ), + }; + }; + + it('refreshes accounts on mount', async () => { + const { wrapper } = createWrapper(); + + renderHook(() => useAccounts(), { wrapper }); + + await waitFor(() => expect(refreshAccountSpy).toHaveBeenCalledWith(mockGitHubCloudAccount)); + }); + + it('exposes a manual refetch that re-runs the refresh', async () => { + const { wrapper } = createWrapper(); + + const { result } = renderHook(() => useAccounts(), { wrapper }); + await waitFor(() => expect(refreshAccountSpy).toHaveBeenCalledTimes(1)); + + await act(async () => { + await result.current.refetchAccounts(); + }); + + await waitFor(() => expect(refreshAccountSpy).toHaveBeenCalledTimes(2)); + }); + + it('sets an explicit staleTime aligned to the refresh interval', async () => { + const { queryClient, wrapper } = createWrapper(); + + renderHook(() => useAccounts(), { wrapper }); + await waitFor(() => expect(refreshAccountSpy).toHaveBeenCalledTimes(1)); + + const [query] = queryClient.getQueryCache().getAll(); + const options = query.options as { staleTime?: number }; + expect(options.staleTime).toBe(Constants.REFRESH_ACCOUNTS_INTERVAL_MS); + }); +}); diff --git a/src/renderer/hooks/useAccounts.ts b/src/renderer/hooks/useAccounts.ts index 2d5d1fa0d..3dc327ff3 100644 --- a/src/renderer/hooks/useAccounts.ts +++ b/src/renderer/hooks/useAccounts.ts @@ -42,6 +42,10 @@ export const useAccounts = (): AccountsState => { enabled: accounts.length > 0, + // Fresh for the whole refresh interval, so mounting a new observer (or + // this hook's own remount on account-list changes) doesn't trigger an + // immediate extra refetch on top of the hourly poll. + staleTime: Constants.REFRESH_ACCOUNTS_INTERVAL_MS, refetchInterval: Constants.REFRESH_ACCOUNTS_INTERVAL_MS, refetchOnWindowFocus: false, }); diff --git a/src/renderer/hooks/useNotifications.test.tsx b/src/renderer/hooks/useNotifications.test.tsx index 6e37269e4..95b97e738 100644 --- a/src/renderer/hooks/useNotifications.test.tsx +++ b/src/renderer/hooks/useNotifications.test.tsx @@ -13,6 +13,8 @@ import { mockSingleAccountNotifications, } from '../__mocks__/notifications-mocks'; +import { Constants } from '../constants'; + import { useAccountsStore, useFiltersStore, useSettingsStore } from '../stores'; import type { AccountNotifications, Percentage } from '../types'; @@ -286,6 +288,32 @@ describe('renderer/hooks/useNotifications.ts', () => { }); }); + describe('query cache configuration', () => { + it('sets an explicit staleTime aligned to the minimum poll interval', async () => { + getAllNotificationsMock.mockResolvedValue(mockSingleAccountNotifications); + + const queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + refetchOnWindowFocus: false, + refetchInterval: false, + }, + }, + }); + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + + renderHook(() => useNotifications({ withSideEffects: true }), { wrapper }); + await waitFor(() => expect(getAllNotificationsMock).toHaveBeenCalledTimes(1)); + + const [query] = queryClient.getQueryCache().getAll(); + const options = query.options as { staleTime?: number }; + expect(options.staleTime).toBe(Constants.MIN_FETCH_NOTIFICATIONS_INTERVAL_MS); + }); + }); + describe('window focus', () => { it('refreshes the enrichment cache when the window regains focus', async () => { getAllNotificationsMock.mockResolvedValue(mockSingleAccountNotifications); diff --git a/src/renderer/hooks/useNotifications.ts b/src/renderer/hooks/useNotifications.ts index 57affdc60..f6f62eff0 100644 --- a/src/renderer/hooks/useNotifications.ts +++ b/src/renderer/hooks/useNotifications.ts @@ -8,6 +8,8 @@ import { useQueryClient, } from '@tanstack/react-query'; +import { Constants } from '../constants'; + import { useAccountsStore, useFiltersStore, useSettingsStore } from '../stores'; import { @@ -165,6 +167,13 @@ export const useNotifications = ({ placeholderData: keepPreviousData, + // Fresh for at least one poll cycle at the fastest possible interval, so + // data isn't treated as stale before a refetch could realistically have + // occurred (see query-cache-timing-alignment design Decision 1 for why + // this uses the static floor rather than the live, possibly-stretched + // interval). + staleTime: Constants.MIN_FETCH_NOTIFICATIONS_INTERVAL_MS, + // Only the singleton side-effects host polls. Other consumers share the // cached data and would otherwise each schedule their own refetch timer. // The interval is re-evaluated after each fetch so it stretches to the diff --git a/src/renderer/utils/api/queryClient.test.ts b/src/renderer/utils/api/queryClient.test.ts new file mode 100644 index 000000000..c0f1be836 --- /dev/null +++ b/src/renderer/utils/api/queryClient.test.ts @@ -0,0 +1,27 @@ +import { Constants } from '../../constants'; + +import { queryClient } from './queryClient'; + +describe('renderer/utils/api/queryClient.ts', () => { + const { queries } = queryClient.getDefaultOptions(); + + it('sets an explicit garbage-collection time', () => { + expect(queries?.gcTime).toBe(Constants.QUERY_GC_TIME_MS); + }); + + it('sets refetchIntervalInBackground at the client level', () => { + expect(queries?.refetchIntervalInBackground).toBe(true); + }); + + it('does not set staleTime at the client level', () => { + // Each query configures its own staleTime at the point of use, since a + // single global value would be wrong for at least one query (see + // useNotifications/useAccounts, which poll on different cadences). + expect(queries?.staleTime).toBeUndefined(); + }); + + it('retries failed queries once after a cooldown', () => { + expect(queries?.retry).toBe(1); + expect(queries?.retryDelay).toBe(Constants.QUERY_RETRY_DELAY_MS); + }); +}); diff --git a/src/renderer/utils/api/queryClient.ts b/src/renderer/utils/api/queryClient.ts index 0dd974e9b..87483510b 100644 --- a/src/renderer/utils/api/queryClient.ts +++ b/src/renderer/utils/api/queryClient.ts @@ -9,6 +9,12 @@ import { Constants } from '../../constants'; * queries retry once after a cooldown instead of the default near-instant * backoff, so a struggling GitHub/GHES instance is not hammered with * back-to-back polls while it recovers. + * + * `staleTime` is intentionally not set here: + * each query has its own refetch cadence (notifications poll on a + * user-configurable interval, accounts poll hourly), so both are configured + * explicitly at each `useQuery` call site instead of a single value that + * would be wrong for at least one of them. */ export const queryClient = new QueryClient({ defaultOptions: { @@ -16,7 +22,7 @@ export const queryClient = new QueryClient({ retry: 1, retryDelay: Constants.QUERY_RETRY_DELAY_MS, refetchIntervalInBackground: true, - staleTime: Constants.QUERY_STALE_TIME_MS, + gcTime: Constants.QUERY_GC_TIME_MS, networkMode: 'online', }, mutations: {