diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 023f2f2ad..9430bb3a1 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -36,7 +36,7 @@ The renderer is layered. **Runtime imports flow in one direction only** — a mo | routes screens | - App.tsx / index.tsx composition root - may import any layer + App.tsx / router.tsx / index.tsx composition root - may import any layer ``` - `utils/` is split by concern: `api`, `auth`, `core`, `forges`, `notifications`, `system`, `ui`. @@ -45,7 +45,8 @@ The renderer is layered. **Runtime imports flow in one direction only** — a mo - `stores/index.ts` is the public store facade for `hooks`, `components`, and `routes`. Lower layers (`utils`) import the specific store module instead (for example `stores/useFiltersStore`): the barrel re-exports every store and its dependencies, which would pull the account/auth/forge graph into leaf utilities and create cycles. - `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`, `avatars`, `layout`, `metrics`, `primitives`). -- The composition root (`App.tsx`, `index.tsx`) is the only place allowed to wire every layer together. +- The composition root (`App.tsx`, `router.tsx`, `index.tsx`) is the only place allowed to wire every layer together. +- `router.tsx` owns the single route table and creates the hash data router. A root layout route renders the window chrome, global shortcuts, and sidebar around an ``; a nested protected layout route owns the login redirect for every authenticated screen. Destinations shared with the sidebar and shortcuts live in `nav.ts`. ## State ownership diff --git a/package.json b/package.json index 51128275b..4edc3f31e 100644 --- a/package.json +++ b/package.json @@ -139,7 +139,7 @@ "react-devtools": "8.0.0", "react-dom": "19.3.1", "react-i18next": "17.1.9", - "react-router-dom": "7.18.5", + "react-router": "8.5.0", "rimraf": "6.1.3", "semver": "7.8.5", "tailwindcss": "4.3.3", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 83a578664..ac1fee7d4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -358,9 +358,9 @@ importers: react-i18next: specifier: 17.1.9 version: 17.1.9(i18next@26.5.11(typescript@7.0.2))(react-dom@19.3.1(react@19.3.1))(react@19.3.1)(typescript@7.0.2) - react-router-dom: - specifier: 7.18.5 - version: 7.18.5(react-dom@19.3.1(react@19.3.1))(react@19.3.1) + react-router: + specifier: 8.5.0 + version: 8.5.0(@types/react-dom@19.3.0(@types/react@19.3.0))(@types/react@19.3.0)(react-dom@19.3.1(react@19.3.1))(react@19.3.1) rimraf: specifier: 6.1.3 version: 6.1.3 @@ -1541,6 +1541,9 @@ packages: '@types/react-is': optional: true + '@remix-run/route-pattern@0.22.1': + resolution: {integrity: sha512-czdGJWh09Lapvcqt7Vb09KlrU2PhRJ8xQaI+AItTuD9aDJ5MAgxnS38lnys6Ll+6RJEqPiaUqTwIQhVLwFvv+w==} + '@repeaterjs/repeater@3.1.0': resolution: {integrity: sha512-TaoVksZRSx2KWYYpyLQtMQXXeS98VsgZImzW65xmiVgbYhXLk+aEsmzPLirqVuE4/XuUapH2iMtxUzaBNDzdSQ==} @@ -2612,9 +2615,8 @@ packages: convert-source-map@2.0.0: resolution: {integrity: sha512-Kvp459HrV2FEJ1CAsi1Ku+MY3kasH19TFykTz2xWmMeq6bk2NU3XXvfJ+Q61m0xktWwt+1HSYf3JZsTms3aRJg==} - cookie@1.1.1: - resolution: {integrity: sha512-ei8Aos7ja0weRpFzJnEA9UHJ/7XQmqglbRwnf2ATjcB9Wq874VKH9kfjjirM6UhU2/E5fFYadylyhFldcqSidQ==} - engines: {node: '>=18'} + cookie-es@3.1.1: + resolution: {integrity: sha512-UaXxwISYJPTr9hwQxMFYZ7kNhSXboMXP+Z3TRX6f1/NyaGPfuNUZOWP1pUEb75B2HjfklIYLVRfWiFZJyC6Npg==} core-util-is@1.0.3: resolution: {integrity: sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==} @@ -4133,20 +4135,19 @@ packages: react-is@19.3.1: resolution: {integrity: sha512-RRieF6IyxQBkcK6pP8xOHprLTPW6QjRkbGdImtqNme+/hYVwCZkLQLdlZGOhUqXwkS9IEynaGvPJtv6XB/cz0Q==} - react-router-dom@7.18.5: - resolution: {integrity: sha512-9HcfDMU6JJ/fp6gJZYGwNE6RknVGS4kBjH1HkNw959mCJi70TVwdipQWv2FEol9uGpi+SZTFtVpJPtEQfayeOw==} - engines: {node: '>=20.0.0'} - peerDependencies: - react: '>=18' - react-dom: '>=18' - - react-router@7.18.5: - resolution: {integrity: sha512-dTgvcXDOrLPrlLHrcZAqWveiK1hMSqN+rGAJzegP8HtmNkfRxOoshGWBNX3ZJ0cZjOHkRdlUJtaIhZMLLyGyPw==} - engines: {node: '>=20.0.0'} + react-router@8.5.0: + resolution: {integrity: sha512-9nEd/deifcTJIIcUaNIyQGkyXD0D6QVB9XD2O5GgHpP3xu3U5RXm87YQ8++Wn8JQWgMBM/5+PjWPM/7n7O7WHA==} + engines: {node: '>=22.22.0'} peerDependencies: - react: '>=18' - react-dom: '>=18' + '@types/react': '>=19.2.0' + '@types/react-dom': '>=19.2.0' + react: '>=19.2.7' + react-dom: '>=19.2.7' peerDependenciesMeta: + '@types/react': + optional: true + '@types/react-dom': + optional: true react-dom: optional: true @@ -4296,9 +4297,6 @@ packages: resolution: {integrity: sha512-8I8TjW5KMOKsZQTvoxjuSIa7foAwPWGOts+6o7sgjz41/qMD9VQHEDxi6PBvK2l0MXUmqZyNpUK+T2tQaaElvw==} engines: {node: '>=10'} - set-cookie-parser@2.7.3: - resolution: {integrity: sha512-irZHtTqurLna57KQG/s+fzOwv/J6XSH4cs8M/vsrY3uCaQ3S6+QJguAH0WdN47SXlk82voMZ4naWMQf8fdEoIg==} - shebang-command@2.0.0: resolution: {integrity: sha512-kHxr2zZpYtdmrN1qDjrrX/Z1rR1kG8Dx+gkpK1G4eXmvXswmcE1hTWBWYUzlraYw1/yZp6YuDY77YtvbN0dmDA==} engines: {node: '>=8'} @@ -6116,6 +6114,8 @@ snapshots: '@types/react': 19.3.0 '@types/react-dom': 19.3.0(@types/react@19.3.0) + '@remix-run/route-pattern@0.22.1': {} + '@repeaterjs/repeater@3.1.0': {} '@rolldown/pluginutils@1.0.1': {} @@ -7011,7 +7011,7 @@ snapshots: convert-source-map@2.0.0: {} - cookie@1.1.1: {} + cookie-es@3.1.1: {} core-util-is@1.0.3: {} @@ -8535,18 +8535,14 @@ snapshots: react-is@19.3.1: {} - react-router-dom@7.18.5(react-dom@19.3.1(react@19.3.1))(react@19.3.1): + react-router@8.5.0(@types/react-dom@19.3.0(@types/react@19.3.0))(@types/react@19.3.0)(react-dom@19.3.1(react@19.3.1))(react@19.3.1): dependencies: + '@remix-run/route-pattern': 0.22.1 + cookie-es: 3.1.1 react: 19.3.1 - react-dom: 19.3.1(react@19.3.1) - react-router: 7.18.5(react-dom@19.3.1(react@19.3.1))(react@19.3.1) - - react-router@7.18.5(react-dom@19.3.1(react@19.3.1))(react@19.3.1): - dependencies: - cookie: 1.1.1 - react: 19.3.1 - set-cookie-parser: 2.7.3 optionalDependencies: + '@types/react': 19.3.0 + '@types/react-dom': 19.3.0(@types/react@19.3.0) react-dom: 19.3.1(react@19.3.1) react@19.3.1: {} @@ -8678,8 +8674,6 @@ snapshots: type-fest: 0.13.1 optional: true - set-cookie-parser@2.7.3: {} - shebang-command@2.0.0: dependencies: shebang-regex: 3.0.0 diff --git a/scripts/verifyPackagedDependencies.js b/scripts/verifyPackagedDependencies.js index 0ee86633f..bfbb411d7 100755 --- a/scripts/verifyPackagedDependencies.js +++ b/scripts/verifyPackagedDependencies.js @@ -27,7 +27,7 @@ const FORBIDDEN_PACKAGES = [ 'react', 'react-dom', 'react-i18next', - 'react-router-dom', + 'react-router', ]; const BUILD_ONLY_PACKAGES = [ 'electron', diff --git a/src/renderer/App.test.tsx b/src/renderer/App.test.tsx new file mode 100644 index 000000000..6e40ee172 --- /dev/null +++ b/src/renderer/App.test.tsx @@ -0,0 +1,121 @@ +import { act, cleanup, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { createMemoryRouter, RouterProvider } from 'react-router'; + +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; + +import { mockGitHubCloudAccount } from './__mocks__/account-mocks'; +import { mockSettings } from './__mocks__/state-mocks'; + +import { useAccountsStore, useSettingsStore } from './stores'; + +import type { Account } from './types'; + +import { routes } from './router'; + +// Route resolution is the subject of these tests, so use the real router +// instead of the global `useNavigate` mock from vitest.setup.ts. +vi.unmock('react-router'); + +function renderAt(path: string, { accounts = [] }: { accounts?: Account[] } = {}) { + useAccountsStore.setState({ accounts }); + useSettingsStore.setState(mockSettings); + + const router = createMemoryRouter(routes, { initialEntries: [path] }); + const queryClient = new QueryClient({ + defaultOptions: { queries: { refetchOnWindowFocus: false, retry: false } }, + }); + + const result = render( + + + , + ); + + return { ...result, router }; +} + +afterEach(() => { + cleanup(); +}); + +describe('renderer/App routing', () => { + it('redirects an unauthenticated protected route to login and preserves the destination', async () => { + const { router } = renderAt('/settings'); + + expect(await screen.findByTestId('login-scroll-container')).toBeInTheDocument(); + await waitFor(() => expect(router.state.location.pathname).toBe('/login')); + expect(router.state.location.state).toEqual({ + from: expect.objectContaining({ pathname: '/settings' }), + }); + }); + + it('returns to the requested destination after login', async () => { + const { router } = renderAt('/settings'); + await screen.findByTestId('login-scroll-container'); + + act(() => { + useAccountsStore.setState({ accounts: [mockGitHubCloudAccount] }); + }); + + await waitFor(() => expect(router.state.location.pathname).toBe('/settings')); + expect(await screen.findByTestId('settings')).toBeInTheDocument(); + }); + + it('lands on the default route when login is opened directly', async () => { + const { router } = renderAt('/login'); + await screen.findByTestId('login-scroll-container'); + + act(() => { + useAccountsStore.setState({ accounts: [mockGitHubCloudAccount] }); + }); + + await waitFor(() => expect(router.state.location.pathname).toBe('/')); + expect(await screen.findByText('No new notifications')).toBeInTheDocument(); + }); + + it('resolves an unknown route to the default screen', async () => { + const { router } = renderAt('/unknown', { accounts: [mockGitHubCloudAccount] }); + + await waitFor(() => expect(router.state.location.pathname).toBe('/')); + expect(await screen.findByText('No new notifications')).toBeInTheDocument(); + }); + + it('marks the active sidebar destination with aria-current', async () => { + renderAt('/filters', { accounts: [mockGitHubCloudAccount] }); + + expect(await screen.findByTestId('filters')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Filters' })).toHaveAttribute('aria-current', 'page'); + expect(screen.getByRole('button', { name: 'Settings' })).not.toHaveAttribute('aria-current'); + }); + + it('keeps back navigation working', async () => { + const { router } = renderAt('/', { accounts: [mockGitHubCloudAccount] }); + + await userEvent.click(screen.getByRole('button', { name: 'Settings' })); + await waitFor(() => expect(router.state.location.pathname).toBe('/settings')); + + await act(async () => { + await router.navigate(-1); + }); + + await waitFor(() => expect(router.state.location.pathname).toBe('/')); + expect(await screen.findByText('No new notifications')).toBeInTheDocument(); + }); + + it('settles on the last route for rapid navigation', async () => { + const { router } = renderAt('/', { accounts: [mockGitHubCloudAccount] }); + + const settings = screen.getByRole('button', { name: 'Settings' }); + const home = screen.getByRole('button', { name: 'Gitify' }); + + act(() => { + settings.click(); + home.click(); + }); + + await waitFor(() => expect(router.state.location.pathname).toBe('/')); + expect(screen.queryByTestId('settings')).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Settings' })).not.toHaveFocus(); + }); +}); diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index e72a9992c..cd0325887 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -1,43 +1,19 @@ import { useEffect } from 'react'; -import { Navigate, Route, HashRouter as Router, Routes, useLocation } from 'react-router-dom'; import { BaseStyles } from '@primer/react'; import { ThemeProvider } from '@primer/react/next'; import { QueryClientProvider } from '@tanstack/react-query'; +import { RouterProvider } from 'react-router/dom'; -import { AccountsRoute } from './routes/Accounts'; -import { AccountScopesRoute } from './routes/AccountScopes'; -import { BitbucketLoginWithPersonalAccessTokenRoute } from './routes/bitbucket/LoginWithPersonalAccessToken'; -import { FiltersRoute } from './routes/Filters'; -import { GiteaLoginWithPersonalAccessTokenRoute } from './routes/gitea/LoginWithPersonalAccessToken'; -import { GitHubLoginWithCustomOAuthAppRoute } from './routes/github/LoginWithCustomOAuthApp'; -import { GitHubLoginWithDeviceFlowRoute } from './routes/github/LoginWithDeviceFlow'; -import { GitHubLoginWithCLIRoute } from './routes/github/LoginWithGitHubCLI'; -import { GitHubLoginWithPersonalAccessTokenRoute } from './routes/github/LoginWithPersonalAccessToken'; -import { GitLabLoginWithPersonalAccessTokenRoute } from './routes/gitlab/LoginWithPersonalAccessToken'; -import { LoginRoute } from './routes/Login'; -import { NotificationsRoute } from './routes/Notifications'; -import { SettingsRoute } from './routes/Settings'; -import { useAccountsStore } from './stores'; import { initializeStoreSubscriptions } from './stores/subscriptions'; import { GlobalEffects } from './components/GlobalEffects'; -import { GlobalShortcuts } from './components/GlobalShortcuts'; +import { router } from './router'; import './App.css'; -import { AppLayout } from './components/layout/AppLayout'; - import { queryClient } from './utils/api/queryClient'; -function RequireAuth({ children }: { children: React.ReactNode }) { - const location = useLocation(); - - const isLoggedIn = useAccountsStore((s) => s.isLoggedIn()); - - return isLoggedIn ? children : ; -} - export function App() { // Initialize store subscriptions with proper cleanup useEffect(() => { @@ -50,79 +26,7 @@ export function App() { - - - - - - - - } - path="/" - /> - - - - } - path="/filters" - /> - - - - } - path="/settings" - /> - - - - } - path="/accounts" - /> - - - - } - path="/account-scopes" - /> - } path="/login" /> - } - path="/login/github/device-flow" - /> - } path="/login/github/cli" /> - } - path="/login/github/personal-access-token" - /> - } - path="/login/github/custom-oauth-app" - /> - } - path="/login/gitea/personal-access-token" - /> - } - path="/login/bitbucket/personal-access-token" - /> - } - path="/login/gitlab/personal-access-token" - /> - - - + diff --git a/src/renderer/__helpers__/test-utils.tsx b/src/renderer/__helpers__/test-utils.tsx index 333e35ab0..49b2ded96 100644 --- a/src/renderer/__helpers__/test-utils.tsx +++ b/src/renderer/__helpers__/test-utils.tsx @@ -1,6 +1,6 @@ import { render } from '@testing-library/react'; import type { ReactElement } from 'react'; -import { type InitialEntry, MemoryRouter } from 'react-router-dom'; +import { type InitialEntry, MemoryRouter } from 'react-router'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; diff --git a/src/renderer/__helpers__/visual-utils.tsx b/src/renderer/__helpers__/visual-utils.tsx index 1da4438a6..4bc2ca3b7 100644 --- a/src/renderer/__helpers__/visual-utils.tsx +++ b/src/renderer/__helpers__/visual-utils.tsx @@ -1,6 +1,6 @@ import { render, waitFor } from '@testing-library/react'; import type { ReactElement } from 'react'; -import { type InitialEntry, MemoryRouter } from 'react-router-dom'; +import { type InitialEntry, MemoryRouter } from 'react-router'; import { BaseStyles } from '@primer/react'; import { ThemeProvider } from '@primer/react/next'; diff --git a/src/renderer/__helpers__/vitest.setup.ts b/src/renderer/__helpers__/vitest.setup.ts index 7448f0f13..ce3e2fd37 100644 --- a/src/renderer/__helpers__/vitest.setup.ts +++ b/src/renderer/__helpers__/vitest.setup.ts @@ -16,8 +16,8 @@ import useSettingsStore from '../stores/useSettingsStore'; * Shared navigate mock — import from test-utils in any test that needs to assert on navigation */ export const navigateMock = vi.fn(); -vi.mock('react-router-dom', async () => ({ - ...(await vi.importActual('react-router-dom')), +vi.mock('react-router', async () => ({ + ...(await vi.importActual('react-router')), useNavigate: () => navigateMock, })); diff --git a/src/renderer/components/Oops.tsx b/src/renderer/components/Oops.tsx index e302676ca..498355bba 100644 --- a/src/renderer/components/Oops.tsx +++ b/src/renderer/components/Oops.tsx @@ -1,5 +1,5 @@ import { useMemo } from 'react'; -import { useNavigate } from 'react-router-dom'; +import { useNavigate } from 'react-router'; import { Button } from '@primer/react'; diff --git a/src/renderer/components/Sidebar.test.tsx b/src/renderer/components/Sidebar.test.tsx index 4be7f15ba..4cb2fd714 100644 --- a/src/renderer/components/Sidebar.test.tsx +++ b/src/renderer/components/Sidebar.test.tsx @@ -29,18 +29,19 @@ describe('renderer/components/Sidebar.tsx', () => { }); for (const name of [ - new RegExp(`^${APPLICATION.NAME}`), /^Notifications/, /^Toggle focused mode/, - /^Filters/, /^My issues/, /^My pull requests/, /^Refresh/, - /^Settings/, ]) { expect(screen.getByRole('button', { name })).toBeInTheDocument(); } + for (const name of [new RegExp(`^${APPLICATION.NAME}`), /^Filters/, /^Settings/]) { + expect(screen.getByRole('button', { name })).toBeInTheDocument(); + } + expect( screen.queryByRole('button', { name: new RegExp(`^Quit ${APPLICATION.NAME}`) }), ).not.toBeInTheDocument(); diff --git a/src/renderer/components/Sidebar.tsx b/src/renderer/components/Sidebar.tsx index 22be2463d..685f8a844 100644 --- a/src/renderer/components/Sidebar.tsx +++ b/src/renderer/components/Sidebar.tsx @@ -1,4 +1,5 @@ import { useTranslation } from 'react-i18next'; +import { useLocation } from 'react-router'; import { BellIcon, @@ -20,9 +21,11 @@ import { useShortcutActions } from '../hooks/useShortcutActions'; import { useAccountsStore, useFiltersStore, useSettingsStore } from '../stores'; import { LogoIcon } from '../icons/LogoIcon'; +import { getNavItem } from '../nav'; export function Sidebar() { const { t } = useTranslation(); + const location = useLocation(); const { status, notificationCount, hasUnreadNotifications, isFetching } = useNotifications(); const { shortcuts } = useShortcutActions(); @@ -36,6 +39,30 @@ export function Sidebar() { const isLoading = status === 'loading'; + const home = getNavItem('home'); + const filters = getNavItem('filters'); + const settings = getNavItem('settings'); + + // Literal t() keys keep the labels discoverable by the i18n extractor. + const copy = { + filters: { + label: t('sidebar.filters', 'Filters'), + description: t('sidebar.filter_notifications', 'Filter notifications'), + }, + home: { + description: t('sidebar.home', 'Home'), + }, + settings: { + label: t('sidebar.settings', 'Settings'), + description: t('settings.title', 'Settings'), + }, + }; + + const isActive = (path: string, exact = false) => + exact + ? location.pathname === path + : location.pathname === path || location.pathname.startsWith(`${path}/`); + return ( } keybindingHint={shortcuts.home.key} onClick={() => shortcuts.home.action()} @@ -91,8 +119,9 @@ export function Sidebar() { /> shortcuts.filters.action()} @@ -141,8 +170,9 @@ export function Sidebar() { /> shortcuts.settings.action()} diff --git a/src/renderer/components/layout/AppLayout.tsx b/src/renderer/components/layout/AppLayout.tsx index 1faeab219..b81542072 100644 --- a/src/renderer/components/layout/AppLayout.tsx +++ b/src/renderer/components/layout/AppLayout.tsx @@ -31,7 +31,9 @@ export function AppLayout({ children }: Readonly) { -
{children}
+
+ {children} +
); } diff --git a/src/renderer/components/layout/__snapshots__/AppLayout.test.tsx.snap b/src/renderer/components/layout/__snapshots__/AppLayout.test.tsx.snap index 794b5b692..7eb5ed6f2 100644 --- a/src/renderer/components/layout/__snapshots__/AppLayout.test.tsx.snap +++ b/src/renderer/components/layout/__snapshots__/AppLayout.test.tsx.snap @@ -31,6 +31,7 @@ exports[`renderer/components/layout/AppLayout.tsx > renders itself & its childre data-wrap="nowrap" >