Skip to content
Merged
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
79 changes: 79 additions & 0 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Architecture

Gitify is an Electron menubar app that monitors notifications across supported Git forges. This document describes how the code is organised so the structure stays coherent as the app grows. It is the reference for where new code belongs.

## Processes

Gitify runs as three Electron processes plus a small shared module:

| Location | Role |
| --------------- | -------------------------------------------------------------------------------- |
| `src/main/` | Electron main process: window/menubar lifecycle, tray, handlers, updater |
| `src/preload/` | The only bridge between renderer and main, exposed as a typed `window.gitify` |
| `src/shared/` | Code imported by **both** processes (constants, events, logger, platform, theme) |
| `src/renderer/` | The React application |

The renderer never reaches into the main process directly: it goes through `window.gitify`. A capability that requires Node or Electron APIs belongs in the main process behind an IPC handler.

## Renderer layers

The renderer is layered. **Runtime imports flow in one direction only** — a module may import from its own layer or any layer below it, never above. Type-only imports are exempt: importing types never creates a runtime dependency, so it need not follow the direction.

```
types / constants / icons (leaf: shared types, values, presentational icons)
|
+----+----+
| |
utils stores peer domain layers
(domain (client state,
logic) Zustand)
| |
+----+----+
|
hooks orchestration over stores, query, and utils
|
components presentational React components
|
routes screens
|
App.tsx / index.tsx composition root - may import any layer
```

- `utils/` is split by concern: `api`, `auth`, `core`, `forges`, `notifications`, `system`, `ui`.
- `types.ts`, `constants.ts`, and `icons/` are the leaf modules: shared types, shared values, and presentational icons. `types.ts` imports types from `utils/auth/types` and generated GitHub GraphQL types — those imports are type-only, so the direction rule does not apply. `icons/` sits below `utils`, so both `utils` and `components` may import it (a forge adapter declares its brand mark from here).
- `stores/` holds client state (Zustand): persisted `accounts` and `settings`, session `filters` and notification action failures.
- `utils` and `stores` are **peers** — they may reference each other. Neither may import `hooks`, `components`, or `routes`.
- `components/` is grouped by feature (`notifications`, `filters`, `settings`, `login`, `fields`, `avatars`, `layout`, `metrics`, `primitives`).
- The composition root (`App.tsx`, `index.tsx`) is the only place allowed to wire every layer together.

## State ownership

Every kind of renderer state has exactly one owner. Put new state with the mechanism that owns its kind:

| Kind of state | Owner | Examples |
| ---------------------- | ---------------------------------------------------- | -------------------------------------------------------- |
| Server / async data | TanStack Query | the notification list, periodic account refreshes |
| Persisted client state | Zustand with `persist` | `accounts`, `settings` |
| Session client state | Zustand without `persist` | `filters`, notification action failures |
| Derived values | A store selector/action, or a pure `utils/` function | `isLoggedIn`, `hasMultipleAccounts`, notification status |
| Ephemeral view state | React `useState` / `useMemo` | row expand/collapse, exit animations |

Rules:

- **Application state is never owned by a React context.** `AppContext` was retired in favour of hooks and stores; do not reintroduce a context provider to hold app state.
- **Derived values are not stored.** If a value can be computed from existing state, expose it as a selector or a pure function instead of adding new state. Derived predicates are read reactively (invoke the store accessor inside the selector) so consumers update when their source changes.
- Query state is server state: never mirror it into a store.

## Side-effect ownership

App-level side effects are owned by a single host. `GlobalEffects` mounts exactly once (inside the query and theme providers) and owns theme application, tray updates, global shortcut registration, periodic account refreshes, and notification polling and alerts.

Consumers read the shared query cache and stores, and **do not register their own** polling, alerts, wake, or focus listeners. This keeps background work (for example, firing one poll per interval) from multiplying with the number of mounted components. When you add a new background effect, add it to the host, not to a leaf component.

## Forge adapters

Multi-forge support is adapter-based. Each forge implements the `ForgeAdapter` interface in `src/renderer/utils/forges/`, and shared code routes through `getAdapter(forge)` (forge-wide members) or `getAccountAdapter(account)` (account-scoped operations). Shared code never imports forge-specific modules directly, and there is no forge-specific branching outside an adapter module. See `CONTRIBUTING.md` for the conditions for adding a forge.

## Enforcement

The one-way module dependency direction is checked automatically: `vite.config.mts` defines per-layer `no-restricted-imports` overrides for the renderer, and `pnpm check` fails on a violation. The enforced rules are: `utils` and `stores` must not import `hooks`, `components`, or `routes`; `hooks` must not import `components` or `routes`; `components` must not import `routes`; and the leaf layers stay leaves — `icons` must not import any implementation layer, and the leaf modules `types`/`constants` must not import implementation layers either (their type-only imports are permitted). Any future exception must be argued and documented here rather than silently allow-listed.
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ the macOS native vibrancy material, which Chromium cannot render.

- Linting and formatting are configured in `vite.config.mts` (the `lint` and `fmt` blocks). Please run `pnpm check` before submitting a PR.
- Follow existing file and folder naming conventions.
- See [ARCHITECTURE.md](./ARCHITECTURE.md) for the renderer's module layering, state ownership, and side-effect conventions.
- Keep commit messages clear and descriptive.

### How to Report Bugs or Request Features
Expand Down
11 changes: 11 additions & 0 deletions src/renderer/__helpers__/vitest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@ vi.mock('../hooks/useNotifications', async () => {
};
});

vi.mock('../hooks/notifications/useNotificationsHost', async () => {
const actual = await vi.importActual<
typeof import('../hooks/notifications/useNotificationsHost')
>('../hooks/notifications/useNotificationsHost');
const { getMockedNotificationsState } = await import('./hook-mocks');
return {
...actual,
useNotificationsHost: () => getMockedNotificationsState(),
};
});

vi.mock('../hooks/useLogins', async () => {
const actual = await vi.importActual<typeof import('../hooks/useLogins')>('../hooks/useLogins');
const { getMockedLoginsState } = await import('./hook-mocks');
Expand Down
4 changes: 2 additions & 2 deletions src/renderer/components/GlobalEffects.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { type FC, useEffect } from 'react';

import { useQueryClient } from '@tanstack/react-query';

import { useNotificationsHost } from '../hooks/notifications/useNotificationsHost';
import { useAccounts } from '../hooks/useAccounts';
import { useAppearance } from '../hooks/useAppearance';
import { useNotifications } from '../hooks/useNotifications';
import { useOnlineStatus } from '../hooks/useOnlineStatus';
import {
useShortcutRegistration,
Expand Down Expand Up @@ -40,7 +40,7 @@ export const GlobalEffects: FC = () => {

// Notification fetching side effects (sound / native alerts, interval/focus/reconnect
// polling, and the system-wake refetch)
const { status, notificationCount } = useNotifications({ withSideEffects: true });
const { status, notificationCount } = useNotificationsHost();

// Periodic account refreshes (startup + hourly interval)
useAccounts();
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/components/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { useNotifications } from '../hooks/useNotifications';
import { useShortcutActions } from '../hooks/useShortcutActions';
import { useAccountsStore, useFiltersStore, useSettingsStore } from '../stores';

import { LogoIcon } from './icons/LogoIcon';
import { LogoIcon } from '../icons/LogoIcon';

export const Sidebar: FC = () => {
const { status, notificationCount, hasUnreadNotifications, isFetching } = useNotifications();
Expand Down
25 changes: 23 additions & 2 deletions src/renderer/components/settings/AppearanceSettings.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ import { act, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import { renderWithProviders } from '../../__helpers__/test-utils';
import { mockGitHubAppAccount } from '../../__mocks__/account-mocks';
import {
mockGitHubAppAccount,
mockGitHubEnterpriseServerAccount,
} from '../../__mocks__/account-mocks';

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

import { DesignLanguage } from '../../types';

Expand Down Expand Up @@ -128,4 +131,22 @@ describe('renderer/components/settings/AppearanceSettings.tsx', () => {

expect(screen.queryByTestId('checkbox-showStatusIconColors')).not.toBeInTheDocument();
});

it('reactively hides the account header checkbox when a second account is added', async () => {
await act(async () => {
renderWithProviders(<AppearanceSettings />, {
accounts: [mockGitHubAppAccount],
});
});

expect(screen.getByTestId('checkbox-showAccountHeader')).toBeInTheDocument();

await act(async () => {
useAccountsStore.setState({
accounts: [mockGitHubAppAccount, mockGitHubEnterpriseServerAccount],
});
});

expect(screen.queryByTestId('checkbox-showAccountHeader')).not.toBeInTheDocument();
});
});
4 changes: 2 additions & 2 deletions src/renderer/components/settings/AppearanceSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {

export const AppearanceSettings: FC = () => {
// Account store values
const hasMultipleAccounts = useAccountsStore((s) => s.hasMultipleAccounts);
const hasMultipleAccounts = useAccountsStore((s) => s.hasMultipleAccounts());

// Setting store actions
const toggleSetting = useSettingsStore((s) => s.toggleSetting);
Expand Down Expand Up @@ -174,7 +174,7 @@ export const AppearanceSettings: FC = () => {
notifications list.
</Text>
}
visible={!hasMultipleAccounts()}
visible={!hasMultipleAccounts}
/>

<Checkbox
Expand Down
4 changes: 2 additions & 2 deletions src/renderer/components/settings/SystemSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { Title } from '../primitives/Title';

import { type KeyboardAcceleratorShortcut, OpenPreference, toLink } from '../../types';

import { VolumeDownIcon } from '../../icons/VolumeDownIcon';
import { VolumeUpIcon } from '../../icons/VolumeUpIcon';
import { openExternalLink } from '../../utils/system/comms';
import {
formatAcceleratorForDisplay,
Expand All @@ -26,8 +28,6 @@ import {
decreaseVolume,
increaseVolume,
} from '../../utils/ui/volume';
import { VolumeDownIcon } from '../icons/VolumeDownIcon';
import { VolumeUpIcon } from '../icons/VolumeUpIcon';

const defaultSettings = DEFAULT_SETTINGS_STATE;

Expand Down
170 changes: 170 additions & 0 deletions src/renderer/hooks/notifications/useNotificationEffects.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
import { useEffect, useRef } from 'react';

import { focusManager, type QueryKey, useQueryClient } from '@tanstack/react-query';

import {
getNotificationFailureKey,
useNotificationActionFailuresStore,
useSettingsStore,
} from '../../stores';

import type { AccountNotifications, GitifyNotification } from '../../types';

import { doesAllAccountsHaveErrors } from '../../utils/core/errors';
import {
filterBaseNotifications,
filterDetailedNotifications,
} from '../../utils/notifications/filters/filter';
import { refreshEnrichmentCache } from '../../utils/notifications/notifications';
import { getNewNotifications } from '../../utils/notifications/utils';
import { raiseSoundNotification } from '../../utils/system/audio';
import { raiseNativeNotification } from '../../utils/system/native';

export interface NotificationEffectsInput {
/**
* Run the singleton side effects. Only the app-level effects host should
* enable this; all other consumers share the query cache without them.
*/
enabled: boolean;
notifications: AccountNotifications[];
isLoading: boolean;
isError: boolean;
notificationsQueryKey: QueryKey;
refetch: () => void;
fetchIntervalMs: number;
}

/**
* The side-effect half of notification state: sound and native alerts for new
* notifications, refetch on system wake and window focus, and pruning of
* session-local failure entries.
*
* Kept separate from the query and mutations so the side effects can be owned
* by a single host rather than repeated per consumer.
*/
export const useNotificationEffects = ({
enabled,
notifications,
isLoading,
isError,
notificationsQueryKey,
refetch,
fetchIntervalMs,
}: NotificationEffectsInput): void => {
const queryClient = useQueryClient();
const previousNotificationsRef = useRef<AccountNotifications[]>([]);

const playSoundNewNotifications = useSettingsStore((s) => s.playSound);
const showSystemNotifications = useSettingsStore((s) => s.showNotifications);
const notificationVolume = useSettingsStore((s) => s.notificationVolume);

// Refetch notifications immediately when the system wakes from sleep or the
// user unlocks their screen, without waiting for the next poll interval.
// Owned by the singleton side-effects host: every consumer registering a
// wake listener would both multiply refetches and leak ipcRenderer
// listeners on unmount.
useEffect(() => {
if (!enabled) {
return;
}

return window.gitify.onSystemWake(() => {
refetch();
});
}, [enabled, refetch]);

// Enriched subject details are cached per `updatedAt`, which label,
// milestone, and reaction changes never bump, so they can drift while the
// app polls in the background. Drop the cache when the window regains focus
// and refetch, so the inbox the user actually looks at re-enriches at that
// moment. The refetch is explicit because the stale-gated focus refetch
// (`refetchOnWindowFocus` above) does not fire within `staleTime`; when it
// does fire too, TanStack dedupes the concurrent fetches. Clearing is
// synchronous within the focus dispatch while any refetch reads the cache
// only after its own awaits, so the clear always lands first. Throttled to
// the fetch interval to avoid re-enrichment bursts from rapid open/close
// cycles.
useEffect(() => {
if (!enabled) {
return;
}

return focusManager.subscribe((focused) => {
if (focused && refreshEnrichmentCache(fetchIntervalMs)) {
refetch();
}
});
}, [enabled, fetchIntervalMs, refetch]);

// Handle sound and native notifications when new notifications arrive
useEffect(() => {
if (!enabled || isLoading || isError || notifications.length === 0) {
return;
}

const allAccountsHaveErrors = doesAllAccountsHaveErrors(notifications);
if (allAccountsHaveErrors) {
return;
}

const unfilteredNotifications =
queryClient.getQueryData<AccountNotifications[]>(notificationsQueryKey) || [];

const diffNotifications = getNewNotifications(
previousNotificationsRef.current,
unfilteredNotifications,
);

if (diffNotifications.length > 0) {
// Apply filters to new notifications so only filtered notifications trigger alerts
const filteredDiffNotifications = filterDetailedNotifications(
filterBaseNotifications(diffNotifications),
) as GitifyNotification[];

if (filteredDiffNotifications.length > 0) {
if (playSoundNewNotifications) {
void raiseSoundNotification(notificationVolume);
}

if (showSystemNotifications) {
void raiseNativeNotification(filteredDiffNotifications);
}
}
}

previousNotificationsRef.current = unfilteredNotifications;
}, [
enabled,
notifications,
isLoading,
isError,
playSoundNewNotifications,
showSystemNotifications,
notificationVolume,
queryClient,
notificationsQueryKey,
]);

// Session-local failure entries are independent of the notifications
// cache, so they must be pruned separately once a notification no longer
// appears in the (unfiltered) list - e.g. actioned successfully elsewhere,
// or its account was removed. Owned by the singleton side-effects host so
// it runs once per notifications update rather than once per mounted
// row/consumer.
useEffect(() => {
if (!enabled) {
return;
}

const unfilteredNotifications =
queryClient.getQueryData<AccountNotifications[]>(notificationsQueryKey) || [];

const currentNotificationKeys = unfilteredNotifications.flatMap((accountNotifications) =>
accountNotifications.notifications.map((notification) =>
getNotificationFailureKey(notification.account, notification.id),
),
);

useNotificationActionFailuresStore.getState().pruneFailures(currentNotificationKeys);
}, [enabled, notifications, queryClient, notificationsQueryKey]);
};
Loading
Loading