Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/renderer/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
75 changes: 75 additions & 0 deletions src/renderer/hooks/useAccounts.test.tsx
Original file line number Diff line number Diff line change
@@ -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 }) => (
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
),
};
};

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);
});
});
4 changes: 4 additions & 0 deletions src/renderer/hooks/useAccounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down
28 changes: 28 additions & 0 deletions src/renderer/hooks/useNotifications.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 }) => (
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
);

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);
Expand Down
9 changes: 9 additions & 0 deletions src/renderer/hooks/useNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
useQueryClient,
} from '@tanstack/react-query';

import { Constants } from '../constants';

import { useAccountsStore, useFiltersStore, useSettingsStore } from '../stores';

import {
Expand Down Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions src/renderer/utils/api/queryClient.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
8 changes: 7 additions & 1 deletion src/renderer/utils/api/queryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,20 @@ 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: {
queries: {
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: {
Expand Down