From f248f6ac8c183f7240a60a7d0b6f792dcc117677 Mon Sep 17 00:00:00 2001 From: Stefan du Fresne Date: Wed, 21 Jan 2026 20:04:17 +0000 Subject: [PATCH 1/3] Migrate test mocking from axios to MSW (network-level) Replace axios library mocking with MSW (Mock Service Worker) to intercept HTTP requests at the network level. This prepares for replacing axios with fetch, as MSW works with both. - Add msw as dev dependency - Create msw-handlers.ts with default API response handlers - Create msw-server.ts to set up the test server - Update test-setup.ts to start/stop MSW server - Migrate UserProvider.test.tsx from axios mocks to MSW - Migrate App.test.tsx from axios mocks to MSW - Remove duplicate "unresponsive server" test (same behavior as network error) - Add passthrough handler for socket.io polling requests Co-Authored-By: Claude Opus 4.5 --- package.json | 1 + remove-axios.md | 400 ++++++++++++++++++ src/client/App.test.tsx | 22 +- .../features/User/UserProvider.test.tsx | 68 +-- src/client/test-utils/msw-handlers.ts | 41 ++ src/client/test-utils/msw-server.ts | 4 + src/client/test-utils/test-setup.ts | 10 +- yarn.lock | 309 +++++++++++++- 8 files changed, 805 insertions(+), 50 deletions(-) create mode 100644 remove-axios.md create mode 100644 src/client/test-utils/msw-handlers.ts create mode 100644 src/client/test-utils/msw-server.ts diff --git a/package.json b/package.json index 830b32f..25ed254 100644 --- a/package.json +++ b/package.json @@ -93,6 +93,7 @@ "deep-diff": "^1.0.2", "events": "^3.3.0", "jsdom": "27.4.0", + "msw": "^2.12.7", "parcel": "2.16.3", "pouchdb-adapter-indexeddb": "^9.0.0", "pouchdb-core": "^9.0.0", diff --git a/remove-axios.md b/remove-axios.md new file mode 100644 index 0000000..dbfe201 --- /dev/null +++ b/remove-axios.md @@ -0,0 +1,400 @@ +# Plan: Replace Axios with Fetch + +This document outlines the plan to remove axios from the codebase and replace it with the native `fetch` API. + +## Current State + +### Files Using Axios (4 total, all client-side) + +1. **[UserProvider.tsx](src/client/features/User/UserProvider.tsx)** - Authentication check + - `axios.get('/api/auth', { cancelToken, headers })` + - `axios.CancelToken.source()` for timeout + - `axios.isCancel(error)` for timeout detection + - `axios.isAxiosError(error)` for error type checking + +2. **[SyncManager.tsx](src/client/features/Sync/SyncManager.tsx)** - Server sync operations + - Already has a TODO comment: `// TODO: drop axios and just use fetch` + - `axios.post('/api/sync/begin', { docs })` + - `axios.post('/api/sync/update', { docs })` + - `axios.post('/api/sync/request', { docs })` + - `axios.isAxiosError(e)` for 401 detection + +3. **[UserAuthenticationWidget.tsx](src/client/features/User/UserAuthenticationWidget.tsx)** - Login/signup + - `axios.post('/api/auth', { username, password })` + - `axios.put('/api/auth', { username, password })` + - `axios.isAxiosError(error)` for error type checking + +4. **[About.tsx](src/client/pages/About.tsx)** - Deployment info + - `axios.get('./api/deployment')` + +### Test Files Mocking Axios (2 total) + +1. **[UserProvider.test.tsx](src/client/features/User/UserProvider.test.tsx)** - 7 test cases +2. **[App.test.tsx](src/client/App.test.tsx)** - 8 test cases + +## Axios Features We Use + +| Feature | Fetch Equivalent | Notes | +|---------|-----------------|-------| +| `axios.get(url, config)` | `fetch(url, { method: 'GET', ...config })` | Direct mapping | +| `axios.post(url, data)` | `fetch(url, { method: 'POST', body: JSON.stringify(data), headers: {'Content-Type': 'application/json'} })` | Must set content-type | +| `axios.put(url, data)` | `fetch(url, { method: 'PUT', body: JSON.stringify(data), headers: {'Content-Type': 'application/json'} })` | Must set content-type | +| `response.data` | `await response.json()` | Async in fetch | +| Auto-throw on 4xx/5xx | Check `response.ok` manually | Semantic difference | +| `axios.CancelToken.source()` | `AbortController` | Modern API | +| `axios.isCancel(error)` | Check for `AbortError` | `error.name === 'AbortError'` | +| `axios.isAxiosError(error)` | Custom error handling | Need our own approach | + +## Key Semantic Differences + +### 1. Error Handling +**Axios**: Throws on any non-2xx status code +```typescript +try { + const response = await axios.get('/api/auth'); +} catch (error) { + if (axios.isAxiosError(error) && error.response?.status === 401) { + // Handle 401 + } +} +``` + +**Fetch**: Only throws on network failures, not HTTP errors +```typescript +const response = await fetch('/api/auth'); +if (!response.ok) { + if (response.status === 401) { + // Handle 401 + } +} +``` + +### 2. Request Cancellation +**Axios**: Uses CancelToken (deprecated) or AbortController +```typescript +const source = axios.CancelToken.source(); +setTimeout(() => source.cancel(), 1000); +await axios.get(url, { cancelToken: source.token }); +``` + +**Fetch**: Uses AbortController +```typescript +const controller = new AbortController(); +setTimeout(() => controller.abort(), 1000); +await fetch(url, { signal: controller.signal }); +``` + +### 3. Response Data Access +**Axios**: `response.data` (sync access) +**Fetch**: `await response.json()` (async) + +## Test Mocking Strategy + +### Current Approach (Axios mocking) +```typescript +vi.mock('axios'); +const mockedAxios = axios as Mocked; +mockedAxios.get.mockResolvedValueOnce({ data: user }); +mockedAxios.isAxiosError.mockImplementation((e) => e.isAxiosError); +``` + +### Recommended Approach: MSW (Mock Service Worker) + +**Why MSW?** +- Mocks at the network level, not the library level +- Works with both axios and fetch (test migration can happen before code migration) +- More realistic testing (tests actual request/response cycle) +- Already available in node_modules via @reduxjs/toolkit +- Industry standard for frontend testing + +**Alternative: Direct fetch mocking with vitest** +```typescript +vi.stubGlobal('fetch', vi.fn()); +``` +This is simpler but less realistic. Given that we want to migrate tests first while axios is still in use, MSW is the better choice. + +## Implementation Plan + +### Phase 1: Migrate Tests to MSW + +**Goal**: Tests mock at network level, work with both axios AND fetch + +1. Install MSW explicitly (it's a transitive dependency, but we should own it) + ```bash + yarn add -D msw + ``` + +2. Create MSW handlers in `src/client/test-utils/msw-handlers.ts`: + ```typescript + import { http, HttpResponse } from 'msw'; + + export const handlers = [ + http.get('/api/auth', ({ request }) => { + // Return user or 401 based on test setup + }), + http.post('/api/auth', async ({ request }) => { + // Handle login + }), + http.put('/api/auth', async ({ request }) => { + // Handle signup + }), + http.post('/api/sync/begin', async ({ request }) => { + // Handle sync begin + }), + // ... etc + ]; + ``` + +3. Update test setup in `src/client/test-utils/test-setup.ts`: + ```typescript + import { setupServer } from 'msw/node'; + import { handlers } from './msw-handlers'; + + export const server = setupServer(...handlers); + + beforeAll(() => server.listen()); + afterEach(() => server.resetHandlers()); + afterAll(() => server.close()); + ``` + +4. Migrate [UserProvider.test.tsx](src/client/features/User/UserProvider.test.tsx): + - Remove `vi.mock('axios')` and axios imports + - Use `server.use()` to override handlers per-test + - Test the existing axios code passes with MSW mocks + +5. Migrate [App.test.tsx](src/client/App.test.tsx): + - Same approach as above + - Keep the db mock (that's still needed) + +6. **Verify all tests pass** with axios still in the code + +### Phase 2: Add Missing Test Coverage + +Review each axios usage and ensure adequate test coverage: + +1. **UserProvider.tsx** - Well covered (7 tests) + - ✅ Successful auth + - ✅ 401 response + - ✅ Server down (non-401 error) + - ✅ Network issues + - ✅ Request timeout (cancel) + - ✅ No cookie scenarios + +2. **SyncManager.tsx** - Currently NO direct tests + - Need tests for sync begin/update/request flows + - Need tests for 401 during sync (triggers unauthenticated state) + - Need tests for sync error handling + +3. **UserAuthenticationWidget.tsx** - Currently NO direct tests + - Need tests for successful login + - Need tests for successful signup + - Need tests for 401/403 error display + - Need tests for other error display + +4. **About.tsx** - No critical need + - Only fetches deployment info in production + - Low risk, can be covered minimally + +### Phase 3: Replace Axios with Fetch + +#### 3.1 Create a fetch utility (optional but recommended) + +Create `src/client/utils/api.ts`: +```typescript +export interface ApiResponse { + data: T; + status: number; + ok: boolean; +} + +export class ApiError extends Error { + constructor( + public status: number, + public statusText: string, + public data?: unknown + ) { + super(`HTTP ${status}: ${statusText}`); + this.name = 'ApiError'; + } +} + +export async function api( + url: string, + options?: RequestInit +): Promise> { + const response = await fetch(url, { + ...options, + headers: { + 'Content-Type': 'application/json', + ...options?.headers, + }, + }); + + const data = response.headers.get('content-type')?.includes('application/json') + ? await response.json() + : null; + + if (!response.ok) { + throw new ApiError(response.status, response.statusText, data); + } + + return { data, status: response.status, ok: response.ok }; +} + +export function isApiError(error: unknown): error is ApiError { + return error instanceof ApiError; +} + +export function isAbortError(error: unknown): boolean { + return error instanceof Error && error.name === 'AbortError'; +} +``` + +**Decision point**: We could also just use raw fetch everywhere since our usage is simple. The utility provides: +- Consistent JSON handling +- Error type checking (`isApiError`) similar to `isAxiosError` +- Abort detection (`isAbortError`) similar to `isCancel` + +Given the small number of call sites (8 total), either approach works. Raw fetch is simpler but requires more code at each call site. + +#### 3.2 Migrate each file + +**Order** (simplest to most complex): + +1. **About.tsx** - Simplest, single GET, no error handling + ```typescript + // Before + axios.get('./api/deployment').then((response) => { ... response.data ... }) + + // After + fetch('./api/deployment') + .then((response) => response.json()) + .then((data) => { ... data ... }) + ``` + +2. **UserAuthenticationWidget.tsx** - POST/PUT with error handling + ```typescript + // Before + const response = await (action === Create ? axios.put : axios.post)('/api/auth', { username, password }); + const user: User = response.data; + + // After + const response = await fetch('/api/auth', { + method: action === Create ? 'PUT' : 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ username, password }), + }); + if (!response.ok) { + if ([401, 403].includes(response.status)) { + setError('Incorrect credentials'); + return; + } + throw new Error(`HTTP ${response.status}`); + } + const user: User = await response.json(); + ``` + +3. **UserProvider.tsx** - GET with timeout and cancel handling + ```typescript + // Before + const source = axios.CancelToken.source(); + setTimeout(() => source.cancel(), 1000); + const response = await axios.get('/api/auth', { + cancelToken: source.token, + headers: { 'Cache-Control': 'no-cache' }, + }); + return response.data; + + // After + const controller = new AbortController(); + setTimeout(() => controller.abort(), 1000); + const response = await fetch('/api/auth', { + signal: controller.signal, + headers: { 'Cache-Control': 'no-cache' }, + }); + if (!response.ok) { + if (response.status === 401) { + return 'auth_error'; + } + // Other errors - log and return undefined + debug(`network issues: ${response.status}`); + return undefined; + } + return await response.json(); + ``` + +4. **SyncManager.tsx** - Multiple POSTs with 401 handling + ```typescript + // Before + const serverState: Requests = await axios + .post('/api/sync/begin', { docs: stubs }) + .then(({ data }) => data); + + // After + const response = await fetch('/api/sync/begin', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ docs: stubs }), + }); + if (!response.ok) { + if (response.status === 401) { + dispatch(setUserAsUnauthenticated()); + dispatch(socketDisconnected()); + return; + } + throw new Error(`Sync failed: ${response.status}`); + } + const serverState: Requests = await response.json(); + ``` + +#### 3.3 Remove axios + +1. Remove all axios imports from the 4 files +2. Remove axios from package.json: `yarn remove axios` +3. Run `yarn check && yarn test` to verify + +### Phase 4: Cleanup and Verification + +1. Run full test suite: `yarn test` +2. Run type checking: `yarn check` +3. Manual testing: + - Test login/signup flow + - Test offline -> online sync + - Test with server down + - Test About page in production mode +4. Remove any axios type imports that might remain + +## Risks and Mitigations + +| Risk | Mitigation | +|------|------------| +| Subtle behavior differences | MSW tests will catch these since they test network behavior | +| Missing error cases | Phase 2 adds test coverage before migration | +| Timeout handling differences | AbortController is the modern standard, well-supported | +| Cookie handling | fetch handles cookies by default (same-origin), no change needed | + +## Estimated Scope + +- **4 files** to modify +- **2 test files** to migrate to MSW +- **~2-3 new test files** for SyncManager and UserAuthenticationWidget +- **1 utility file** (optional) +- **0 server changes** (axios was client-only) + +## Checklist + +- [ ] Phase 1: Install MSW and setup test infrastructure +- [ ] Phase 1: Migrate UserProvider.test.tsx to MSW +- [ ] Phase 1: Migrate App.test.tsx to MSW +- [ ] Phase 1: Verify all tests pass with axios still in code +- [ ] Phase 2: Add SyncManager tests +- [ ] Phase 2: Add UserAuthenticationWidget tests +- [ ] Phase 2: Review About.tsx coverage needs +- [ ] Phase 3: Decide on utility vs raw fetch +- [ ] Phase 3: Migrate About.tsx +- [ ] Phase 3: Migrate UserAuthenticationWidget.tsx +- [ ] Phase 3: Migrate UserProvider.tsx +- [ ] Phase 3: Migrate SyncManager.tsx +- [ ] Phase 3: Remove axios dependency +- [ ] Phase 4: Full test suite passes +- [ ] Phase 4: Manual testing complete diff --git a/src/client/App.test.tsx b/src/client/App.test.tsx index 44fb927..349431b 100644 --- a/src/client/App.test.tsx +++ b/src/client/App.test.tsx @@ -1,5 +1,5 @@ import { screen, waitFor } from '@testing-library/react'; -import axios, { type CancelTokenSource } from 'axios'; +import { HttpResponse, http } from 'msw'; import { MemoryRouter } from 'react-router-dom'; import { afterEach, beforeEach, describe, expect, it, type Mocked, vi } from 'vitest'; import { when } from 'vitest-when'; @@ -9,12 +9,10 @@ import db, { type Database } from './db'; import { setUserAsLoggedIn } from './features/User/userSlice'; import { createStore } from './store'; import { render, withStore } from './test-utils'; +import { server } from './test-utils/msw-server'; -// Mock the database and axios +// Mock the database vi.mock('./db'); -vi.mock('axios'); - -const mockedAxios = axios as Mocked; describe('App Routing', () => { let store: ReturnType; @@ -47,13 +45,12 @@ describe('App Routing', () => { return Promise.resolve({ docs: [] }); }); - // Mock axios for UserProvider authentication check - mockedAxios.isAxiosError.mockImplementation((e) => e.isAxiosError); - mockedAxios.isCancel.mockImplementation((e) => e.isCancel); - mockedAxios.CancelToken.source = vi.fn(() => { - return { cancel: () => {} } as CancelTokenSource; - }); - mockedAxios.get.mockResolvedValue({ data: { id: 1, name: 'testuser' } }); + // Mock /api/auth to return the logged-in user + server.use( + http.get('/api/auth', () => { + return HttpResponse.json({ id: 1, name: 'testuser' }); + }), + ); }); afterEach(() => { @@ -209,7 +206,6 @@ describe('App Routing', () => { it('should show guest user message when user is guest', async () => { // biome-ignore lint/suspicious/noDocumentCookie: Required for dual-cookie auth testing document.cookie = 'sanremo-client='; - mockedAxios.get.mockResolvedValue({ data: { id: 1, name: 'testuser' } }); render( withStore( diff --git a/src/client/features/User/UserProvider.test.tsx b/src/client/features/User/UserProvider.test.tsx index 306d64c..837ba5c 100644 --- a/src/client/features/User/UserProvider.test.tsx +++ b/src/client/features/User/UserProvider.test.tsx @@ -1,18 +1,15 @@ import { screen, waitFor } from '@testing-library/react'; -import axios, { type CancelTokenSource } from 'axios'; +import { HttpResponse, http } from 'msw'; import { MemoryRouter } from 'react-router'; import type { AnyAction, Store } from 'redux'; -import { beforeEach, describe, expect, it, type Mocked, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { createStore, type RootState } from '../../store'; import { withStore, render as wrappedRender } from '../../test-utils'; +import { server } from '../../test-utils/msw-server'; import UserProvider from './UserProvider'; import { GuestUser } from './userSlice'; -vi.mock('axios'); - -const mockedAxios = axios as Mocked; - const serverUser = { id: 1, name: 'Tester Test' }; const clientUser = { id: 1, name: 'test' }; const NO_CLIENT_COOKIE = 'sanremo-client='; @@ -23,15 +20,8 @@ const CLIENT_COOKIE = // This should probably be moved somewhere else? Or refactored to just use the UserProvider? describe('user authentication', () => { let store: Store; - beforeEach(() => { - // in theory we can change the jest mock of axios to do a partial mock - // however, this messes with the mocked function that we used to get mockedAxios - mockedAxios.isAxiosError.mockImplementation((e) => e.isAxiosError); - mockedAxios.isCancel.mockImplementation((e) => e.isCancel); - mockedAxios.CancelToken.source = vi.fn(() => { - return { cancel: () => {} } as CancelTokenSource; - }); + beforeEach(() => { store = createStore(); }); @@ -42,7 +32,11 @@ describe('user authentication', () => { it('loads user with valid server credentials', async () => { // biome-ignore lint/suspicious/noDocumentCookie: Required for dual-cookie auth testing document.cookie = CLIENT_COOKIE; - mockedAxios.get.mockResolvedValueOnce({ data: serverUser }); + server.use( + http.get('/api/auth', () => { + return HttpResponse.json(serverUser); + }), + ); render(some text); await waitFor(() => screen.getByText(/some text/)); @@ -50,10 +44,15 @@ describe('user authentication', () => { expect(store.getState().user.value).toStrictEqual(serverUser); expect(store.getState().user.needsServerAuthentication).toBeFalsy(); }); + it('loads user with invalid server credentials but valid client cookie', async () => { // biome-ignore lint/suspicious/noDocumentCookie: Required for dual-cookie auth testing document.cookie = CLIENT_COOKIE; - mockedAxios.get.mockRejectedValue({ response: { status: 401 }, isAxiosError: true }); + server.use( + http.get('/api/auth', () => { + return HttpResponse.json(null, { status: 401 }); + }), + ); render(some text); await waitFor(() => screen.getByText(/some text/)); @@ -61,10 +60,15 @@ describe('user authentication', () => { expect(store.getState().user.value).toStrictEqual(clientUser); expect(store.getState().user.needsServerAuthentication).toBeTruthy(); }); + it('loads user with a down server but valid client cookie', async () => { // biome-ignore lint/suspicious/noDocumentCookie: Required for dual-cookie auth testing document.cookie = CLIENT_COOKIE; - mockedAxios.get.mockRejectedValue({ response: { status: 404 }, isAxiosError: true }); + server.use( + http.get('/api/auth', () => { + return HttpResponse.json(null, { status: 404 }); + }), + ); render(some text); await waitFor(() => screen.getByText(/some text/)); @@ -72,10 +76,15 @@ describe('user authentication', () => { expect(store.getState().user.value).toStrictEqual(clientUser); expect(store.getState().user.needsServerAuthentication).toBeFalsy(); }); + it('loads user with network issues but valid client cookie', async () => { // biome-ignore lint/suspicious/noDocumentCookie: Required for dual-cookie auth testing document.cookie = CLIENT_COOKIE; - mockedAxios.get.mockRejectedValue({ who: 'knows', isAxiosError: true }); + server.use( + http.get('/api/auth', () => { + return HttpResponse.error(); + }), + ); render(some text); await waitFor(() => screen.getByText(/some text/)); @@ -83,21 +92,15 @@ describe('user authentication', () => { expect(store.getState().user.value).toStrictEqual(clientUser); expect(store.getState().user.needsServerAuthentication).toBeFalsy(); }); - it('loads user with an unresponsive server but valid client cookie', async () => { - // biome-ignore lint/suspicious/noDocumentCookie: Required for dual-cookie auth testing - document.cookie = CLIENT_COOKIE; - mockedAxios.get.mockRejectedValue({ isCancel: true }); - - render(some text); - await waitFor(() => screen.getByText(/some text/)); - expect(store.getState().user.value).toStrictEqual(clientUser); - expect(store.getState().user.needsServerAuthentication).toBeFalsy(); - }); it('treats no client cookie as the user being a guest', async () => { // biome-ignore lint/suspicious/noDocumentCookie: Required for dual-cookie auth testing document.cookie = NO_CLIENT_COOKIE; - mockedAxios.get.mockResolvedValueOnce({ data: serverUser }); + server.use( + http.get('/api/auth', () => { + return HttpResponse.json(serverUser); + }), + ); render(some text); await waitFor(() => screen.getByText(/some text/)); @@ -105,11 +108,16 @@ describe('user authentication', () => { expect(store.getState().user.value).toStrictEqual(GuestUser); expect(store.getState().user.needsServerAuthentication).toBeFalsy(); }); + it('for now, treat to corrupted client cookie as the user being a guest', async () => { vi.spyOn(console, 'error').mockImplementation(() => {}); // biome-ignore lint/suspicious/noDocumentCookie: Required for dual-cookie auth testing document.cookie = 'sanremo-client=blah'; - mockedAxios.get.mockResolvedValueOnce({ data: serverUser }); + server.use( + http.get('/api/auth', () => { + return HttpResponse.json(serverUser); + }), + ); render(some text); await waitFor(() => screen.getByText(/some text/)); diff --git a/src/client/test-utils/msw-handlers.ts b/src/client/test-utils/msw-handlers.ts new file mode 100644 index 0000000..e82c7d8 --- /dev/null +++ b/src/client/test-utils/msw-handlers.ts @@ -0,0 +1,41 @@ +import { HttpResponse, http, passthrough } from 'msw'; + +export const handlers = [ + // Socket.io polling requests - let them pass through (they'll fail silently in tests) + http.get('/socket.io/*', () => { + return passthrough(); + }), + + http.get('/api/auth', () => { + return HttpResponse.json(null, { status: 401 }); + }), + + http.post('/api/auth', () => { + return HttpResponse.json(null, { status: 401 }); + }), + + http.put('/api/auth', () => { + return HttpResponse.json(null, { status: 401 }); + }), + + http.post('/api/sync/begin', () => { + return HttpResponse.json({ server: [], client: [] }); + }), + + http.post('/api/sync/update', () => { + return HttpResponse.json({ success: true }); + }), + + http.post('/api/sync/request', () => { + return HttpResponse.json([]); + }), + + http.get('/api/deployment', () => { + return HttpResponse.json({ + deploy_version: 'test', + release_version: 'test', + deploy_commit: 'abc1234', + users: [], + }); + }), +]; diff --git a/src/client/test-utils/msw-server.ts b/src/client/test-utils/msw-server.ts new file mode 100644 index 0000000..6802f90 --- /dev/null +++ b/src/client/test-utils/msw-server.ts @@ -0,0 +1,4 @@ +import { setupServer } from 'msw/node'; +import { handlers } from './msw-handlers'; + +export const server = setupServer(...handlers); diff --git a/src/client/test-utils/test-setup.ts b/src/client/test-utils/test-setup.ts index 3207e0e..53c635e 100644 --- a/src/client/test-utils/test-setup.ts +++ b/src/client/test-utils/test-setup.ts @@ -1,7 +1,13 @@ import '@testing-library/jest-dom/vitest'; -import { vi } from 'vitest'; +import { afterAll, afterEach, beforeAll, vi } from 'vitest'; +import { server } from './msw-server'; + +beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); + +afterEach(() => server.resetHandlers()); + +afterAll(() => server.close()); -// Mock localStorage to ensure it works properly in all test environments const localStorageMock = { getItem: vi.fn((_key: string) => null), setItem: vi.fn((_key: string, _value: string) => {}), diff --git a/yarn.lock b/yarn.lock index b788643..03fd12d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2058,6 +2058,68 @@ __metadata: languageName: node linkType: hard +"@inquirer/ansi@npm:^1.0.2": + version: 1.0.2 + resolution: "@inquirer/ansi@npm:1.0.2" + checksum: 8e408cc628923aa93402e66657482ccaa2ad5174f9db526d9a8b443f9011e9cd8f70f0f534f5fe3857b8a9df3bce1e25f66c96f666d6750490bd46e2b4f3b829 + languageName: node + linkType: hard + +"@inquirer/confirm@npm:^5.0.0": + version: 5.1.21 + resolution: "@inquirer/confirm@npm:5.1.21" + dependencies: + "@inquirer/core": "npm:^10.3.2" + "@inquirer/type": "npm:^3.0.10" + peerDependencies: + "@types/node": ">=18" + peerDependenciesMeta: + "@types/node": + optional: true + checksum: a95bbdbb17626c484735a4193ed6b6a6fbb078cf62116ec8e1667f647e534dd6618e688ecc7962585efcc56881b544b8c53db3914599bbf2ab842e7f224b0fca + languageName: node + linkType: hard + +"@inquirer/core@npm:^10.3.2": + version: 10.3.2 + resolution: "@inquirer/core@npm:10.3.2" + dependencies: + "@inquirer/ansi": "npm:^1.0.2" + "@inquirer/figures": "npm:^1.0.15" + "@inquirer/type": "npm:^3.0.10" + cli-width: "npm:^4.1.0" + mute-stream: "npm:^2.0.0" + signal-exit: "npm:^4.1.0" + wrap-ansi: "npm:^6.2.0" + yoctocolors-cjs: "npm:^2.1.3" + peerDependencies: + "@types/node": ">=18" + peerDependenciesMeta: + "@types/node": + optional: true + checksum: f0f27e07fe288e01e3949b4ad216c19751f025ce77c610366e08d8b0f7a135d064dc074732031d251584b454c576f1e5c849e4abe259186dd5d4974c8f85c13e + languageName: node + linkType: hard + +"@inquirer/figures@npm:^1.0.15": + version: 1.0.15 + resolution: "@inquirer/figures@npm:1.0.15" + checksum: 6e39a040d260ae234ae220180b7994ff852673e20be925f8aa95e78c7934d732b018cbb4d0ec39e600a410461bcb93dca771e7de23caa10630d255692e440f69 + languageName: node + linkType: hard + +"@inquirer/type@npm:^3.0.10": + version: 3.0.10 + resolution: "@inquirer/type@npm:3.0.10" + peerDependencies: + "@types/node": ">=18" + peerDependenciesMeta: + "@types/node": + optional: true + checksum: a846c7a570e3bf2657d489bcc5dcdc3179d24c7323719de1951dcdb722400ac76e5b2bfe9765d0a789bc1921fac810983d7999f021f30a78a6a174c23fc78dc9 + languageName: node + linkType: hard + "@isaacs/cliui@npm:^8.0.2": version: 8.0.2 resolution: "@isaacs/cliui@npm:8.0.2" @@ -2260,6 +2322,20 @@ __metadata: languageName: node linkType: hard +"@mswjs/interceptors@npm:^0.40.0": + version: 0.40.0 + resolution: "@mswjs/interceptors@npm:0.40.0" + dependencies: + "@open-draft/deferred-promise": "npm:^2.2.0" + "@open-draft/logger": "npm:^0.3.0" + "@open-draft/until": "npm:^2.0.0" + is-node-process: "npm:^1.2.0" + outvariant: "npm:^1.4.3" + strict-event-emitter: "npm:^0.5.1" + checksum: 4500f17b65910b2633182fdb15a81ccb6ccd4488a8c45bc2f7acdaaff4621c3cce5362e6b59ddc4fa28d315d0efb0608fd1f0d536bc5345141f8ac03fd7fab22 + languageName: node + linkType: hard + "@mui/core-downloads-tracker@npm:^7.3.7": version: 7.3.7 resolution: "@mui/core-downloads-tracker@npm:7.3.7" @@ -2450,6 +2526,30 @@ __metadata: languageName: node linkType: hard +"@open-draft/deferred-promise@npm:^2.2.0": + version: 2.2.0 + resolution: "@open-draft/deferred-promise@npm:2.2.0" + checksum: eafc1b1d0fc8edb5e1c753c5e0f3293410b40dde2f92688211a54806d4136887051f39b98c1950370be258483deac9dfd17cf8b96557553765198ef2547e4549 + languageName: node + linkType: hard + +"@open-draft/logger@npm:^0.3.0": + version: 0.3.0 + resolution: "@open-draft/logger@npm:0.3.0" + dependencies: + is-node-process: "npm:^1.2.0" + outvariant: "npm:^1.4.0" + checksum: 90010647b22e9693c16258f4f9adb034824d1771d3baa313057b9a37797f571181005bc50415a934eaf7c891d90ff71dcd7a9d5048b0b6bb438f31bef2c7c5c1 + languageName: node + linkType: hard + +"@open-draft/until@npm:^2.0.0": + version: 2.1.0 + resolution: "@open-draft/until@npm:2.1.0" + checksum: 61d3f99718dd86bb393fee2d7a785f961dcaf12f2055f0c693b27f4d0cd5f7a03d498a6d9289773b117590d794a43cd129366fd8e99222e4832f67b1653d54cf + languageName: node + linkType: hard + "@paralleldrive/cuid2@npm:^2.2.2": version: 2.3.1 resolution: "@paralleldrive/cuid2@npm:2.3.1" @@ -4648,6 +4748,13 @@ __metadata: languageName: node linkType: hard +"@types/statuses@npm:^2.0.6": + version: 2.0.6 + resolution: "@types/statuses@npm:2.0.6" + checksum: dd88c220b0e2c6315686289525fd61472d2204d2e4bef4941acfb76bda01d3066f749ac74782aab5b537a45314fcd7d6261eefa40b6ec872691f5803adaa608d + languageName: node + linkType: hard + "@types/superagent@npm:^8.1.0": version: 8.1.9 resolution: "@types/superagent@npm:8.1.9" @@ -5348,6 +5455,24 @@ __metadata: languageName: node linkType: hard +"cli-width@npm:^4.1.0": + version: 4.1.0 + resolution: "cli-width@npm:4.1.0" + checksum: 1fbd56413578f6117abcaf858903ba1f4ad78370a4032f916745fa2c7e390183a9d9029cf837df320b0fdce8137668e522f60a30a5f3d6529ff3872d265a955f + languageName: node + linkType: hard + +"cliui@npm:^8.0.1": + version: 8.0.1 + resolution: "cliui@npm:8.0.1" + dependencies: + string-width: "npm:^4.2.0" + strip-ansi: "npm:^6.0.1" + wrap-ansi: "npm:^7.0.0" + checksum: 4bda0f09c340cbb6dfdc1ed508b3ca080f12992c18d68c6be4d9cf51756033d5266e61ec57529e610dacbf4da1c634423b0c1b11037709cc6b09045cbd815df5 + languageName: node + linkType: hard + "clone@npm:^2.1.2": version: 2.1.2 resolution: "clone@npm:2.1.2" @@ -5523,7 +5648,7 @@ __metadata: languageName: node linkType: hard -"cookie@npm:^1.0.1": +"cookie@npm:^1.0.1, cookie@npm:^1.0.2": version: 1.1.1 resolution: "cookie@npm:1.1.1" checksum: 79c4ddc0fcad9c4f045f826f42edf54bcc921a29586a4558b0898277fa89fb47be95bc384c2253f493af7b29500c830da28341274527328f18eba9f58afa112c @@ -6073,7 +6198,7 @@ __metadata: languageName: node linkType: hard -"escalade@npm:^3.2.0": +"escalade@npm:^3.1.1, escalade@npm:^3.2.0": version: 3.2.0 resolution: "escalade@npm:3.2.0" checksum: ced4dd3a78e15897ed3be74e635110bbf3b08877b0a41be50dcb325ee0e0b5f65fc2d50e9845194d7c4633f327e2e1c6cce00a71b617c5673df0374201d67f65 @@ -6420,6 +6545,13 @@ __metadata: languageName: node linkType: hard +"get-caller-file@npm:^2.0.5": + version: 2.0.5 + resolution: "get-caller-file@npm:2.0.5" + checksum: c6c7b60271931fa752aeb92f2b47e355eac1af3a2673f47c9589e8f8a41adc74d45551c1bc57b5e66a80609f10ffb72b6f575e4370d61cc3f7f3aaff01757cde + languageName: node + linkType: hard + "get-intrinsic@npm:^1.2.5, get-intrinsic@npm:^1.3.0": version: 1.3.1 resolution: "get-intrinsic@npm:1.3.1" @@ -6514,6 +6646,13 @@ __metadata: languageName: node linkType: hard +"graphql@npm:^16.12.0": + version: 16.12.0 + resolution: "graphql@npm:16.12.0" + checksum: b6fffa4e8a4e4a9933ebe85e7470b346dbf49050c1a482fac5e03e4a1a7bed2ecd3a4c97e29f04457af929464bc5e4f2aac991090c2f320111eef26e902a5c75 + languageName: node + linkType: hard + "has-flag@npm:^3.0.0": version: 3.0.0 resolution: "has-flag@npm:3.0.0" @@ -6601,6 +6740,13 @@ __metadata: languageName: node linkType: hard +"headers-polyfill@npm:^4.0.2": + version: 4.0.3 + resolution: "headers-polyfill@npm:4.0.3" + checksum: 53e85b2c6385f8d411945fb890c5369f1469ce8aa32a6e8d28196df38568148de640c81cf88cbc7c67767103dd9acba48f4f891982da63178fc6e34560022afe + languageName: node + linkType: hard + "hoist-non-react-statics@npm:^3.3.1": version: 3.3.2 resolution: "hoist-non-react-statics@npm:3.3.2" @@ -6870,6 +7016,13 @@ __metadata: languageName: node linkType: hard +"is-node-process@npm:^1.2.0": + version: 1.2.0 + resolution: "is-node-process@npm:1.2.0" + checksum: 5b24fda6776d00e42431d7bcd86bce81cb0b6cabeb944142fe7b077a54ada2e155066ad06dbe790abdb397884bdc3151e04a9707b8cd185099efbc79780573ed + languageName: node + linkType: hard + "is-number@npm:^7.0.0": version: 7.0.0 resolution: "is-number@npm:7.0.0" @@ -8141,6 +8294,46 @@ __metadata: languageName: node linkType: hard +"msw@npm:^2.12.7": + version: 2.12.7 + resolution: "msw@npm:2.12.7" + dependencies: + "@inquirer/confirm": "npm:^5.0.0" + "@mswjs/interceptors": "npm:^0.40.0" + "@open-draft/deferred-promise": "npm:^2.2.0" + "@types/statuses": "npm:^2.0.6" + cookie: "npm:^1.0.2" + graphql: "npm:^16.12.0" + headers-polyfill: "npm:^4.0.2" + is-node-process: "npm:^1.2.0" + outvariant: "npm:^1.4.3" + path-to-regexp: "npm:^6.3.0" + picocolors: "npm:^1.1.1" + rettime: "npm:^0.7.0" + statuses: "npm:^2.0.2" + strict-event-emitter: "npm:^0.5.1" + tough-cookie: "npm:^6.0.0" + type-fest: "npm:^5.2.0" + until-async: "npm:^3.0.2" + yargs: "npm:^17.7.2" + peerDependencies: + typescript: ">= 4.8.x" + peerDependenciesMeta: + typescript: + optional: true + bin: + msw: cli/index.js + checksum: e9100fe87dce5411d29392bba88ed1813acc6278991a3068c328ec671e5eb52d1e8ee451ee401c554eab08e04b78b84fcdf2c78f761287c6b43e844e91ce18dd + languageName: node + linkType: hard + +"mute-stream@npm:^2.0.0": + version: 2.0.0 + resolution: "mute-stream@npm:2.0.0" + checksum: 2cf48a2087175c60c8dcdbc619908b49c07f7adcfc37d29236b0c5c612d6204f789104c98cc44d38acab7b3c96f4a3ec2cfdc4934d0738d876dbefa2a12c69f4 + languageName: node + linkType: hard + "nanoid@npm:^3.3.11": version: 3.3.11 resolution: "nanoid@npm:3.3.11" @@ -8332,6 +8525,13 @@ __metadata: languageName: node linkType: hard +"outvariant@npm:^1.4.0, outvariant@npm:^1.4.3": + version: 1.4.3 + resolution: "outvariant@npm:1.4.3" + checksum: 5976ca7740349cb8c71bd3382e2a762b1aeca6f33dc984d9d896acdf3c61f78c3afcf1bfe9cc633a7b3c4b295ec94d292048f83ea2b2594fae4496656eba992c + languageName: node + linkType: hard + "p-map@npm:^4.0.0": version: 4.0.0 resolution: "p-map@npm:4.0.0" @@ -8442,6 +8642,13 @@ __metadata: languageName: node linkType: hard +"path-to-regexp@npm:^6.3.0": + version: 6.3.0 + resolution: "path-to-regexp@npm:6.3.0" + checksum: 73b67f4638b41cde56254e6354e46ae3a2ebc08279583f6af3d96fe4664fc75788f74ed0d18ca44fa4a98491b69434f9eee73b97bb5314bd1b5adb700f5c18d6 + languageName: node + linkType: hard + "path-to-regexp@npm:^8.0.0": version: 8.3.0 resolution: "path-to-regexp@npm:8.3.0" @@ -9331,6 +9538,13 @@ __metadata: languageName: node linkType: hard +"require-directory@npm:^2.1.1": + version: 2.1.1 + resolution: "require-directory@npm:2.1.1" + checksum: 83aa76a7bc1531f68d92c75a2ca2f54f1b01463cb566cf3fbc787d0de8be30c9dbc211d1d46be3497dac5785fe296f2dd11d531945ac29730643357978966e99 + languageName: node + linkType: hard + "require-from-string@npm:^2.0.2": version: 2.0.2 resolution: "require-from-string@npm:2.0.2" @@ -9418,6 +9632,13 @@ __metadata: languageName: node linkType: hard +"rettime@npm:^0.7.0": + version: 0.7.0 + resolution: "rettime@npm:0.7.0" + checksum: 1460539d49415c37e46884bf1db7a5da974b239c1bd6976e1cf076fad169067dc8f55cd2572aec504433162f3627b6d8123eea977d110476258045d620bd051b + languageName: node + linkType: hard + "rollup@npm:^4.43.0": version: 4.55.1 resolution: "rollup@npm:4.55.1" @@ -9589,6 +9810,7 @@ __metadata: express: "npm:^5.2.1" express-session: "npm:1.18.2" jsdom: "npm:27.4.0" + msw: "npm:^2.12.7" parcel: "npm:2.16.3" pg: "npm:8.16.3" pg-protocol: "npm:1.10.3" @@ -9798,7 +10020,7 @@ __metadata: languageName: node linkType: hard -"signal-exit@npm:^4.0.1": +"signal-exit@npm:^4.0.1, signal-exit@npm:^4.1.0": version: 4.1.0 resolution: "signal-exit@npm:4.1.0" checksum: 41602dce540e46d599edba9d9860193398d135f7ff72cab629db5171516cfae628d21e7bfccde1bbfdf11c48726bc2a6d1a8fb8701125852fbfda7cf19c6aa83 @@ -9962,7 +10184,14 @@ __metadata: languageName: node linkType: hard -"string-width-cjs@npm:string-width@^4.2.0, string-width@npm:^4.1.0": +"strict-event-emitter@npm:^0.5.1": + version: 0.5.1 + resolution: "strict-event-emitter@npm:0.5.1" + checksum: f5228a6e6b6393c57f52f62e673cfe3be3294b35d6f7842fc24b172ae0a6e6c209fa83241d0e433fc267c503bc2f4ffdbe41a9990ff8ffd5ac425ec0489417f7 + languageName: node + linkType: hard + +"string-width-cjs@npm:string-width@^4.2.0, string-width@npm:^4.1.0, string-width@npm:^4.2.0, string-width@npm:^4.2.3": version: 4.2.3 resolution: "string-width@npm:4.2.3" dependencies: @@ -10106,6 +10335,13 @@ __metadata: languageName: node linkType: hard +"tagged-tag@npm:^1.0.0": + version: 1.0.0 + resolution: "tagged-tag@npm:1.0.0" + checksum: 91d25c9ffb86a91f20522cefb2cbec9b64caa1febe27ad0df52f08993ff60888022d771e868e6416cf2e72dab68449d2139e8709ba009b74c6c7ecd4000048d1 + languageName: node + linkType: hard + "tar@npm:^6.1.11, tar@npm:^6.1.2": version: 6.2.0 resolution: "tar@npm:6.2.0" @@ -10309,6 +10545,15 @@ __metadata: languageName: node linkType: hard +"type-fest@npm:^5.2.0": + version: 5.4.1 + resolution: "type-fest@npm:5.4.1" + dependencies: + tagged-tag: "npm:^1.0.0" + checksum: 500386d690e634499e6fb765a1e33909a75fea0258acdd56a2d1c9565d6810e222f6d95bd80daa5498377c7ea976af46f518185b5dbd67ff8454562544808aa1 + languageName: node + linkType: hard + "type-is@npm:^2.0.1": version: 2.0.1 resolution: "type-is@npm:2.0.1" @@ -10496,6 +10741,13 @@ __metadata: languageName: node linkType: hard +"until-async@npm:^3.0.2": + version: 3.0.2 + resolution: "until-async@npm:3.0.2" + checksum: 61c8b03895dbe18fe3d90316d0a1894e0c131ea4b1673f6ce78eed993d0bb81bbf4b7adf8477e9ff7725782a76767eed9d077561cfc9f89b4a1ebe61f7c9828e + languageName: node + linkType: hard + "update-browserslist-db@npm:^1.2.0": version: 1.2.3 resolution: "update-browserslist-db@npm:1.2.3" @@ -10844,7 +11096,7 @@ __metadata: languageName: node linkType: hard -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0, wrap-ansi@npm:^7.0.0": version: 7.0.0 resolution: "wrap-ansi@npm:7.0.0" dependencies: @@ -10855,6 +11107,17 @@ __metadata: languageName: node linkType: hard +"wrap-ansi@npm:^6.2.0": + version: 6.2.0 + resolution: "wrap-ansi@npm:6.2.0" + dependencies: + ansi-styles: "npm:^4.0.0" + string-width: "npm:^4.1.0" + strip-ansi: "npm:^6.0.0" + checksum: baad244e6e33335ea24e86e51868fe6823626e3a3c88d9a6674642afff1d34d9a154c917e74af8d845fd25d170c4ea9cf69a47133c3f3656e1252b3d462d9f6c + languageName: node + linkType: hard + "wrap-ansi@npm:^8.1.0": version: 8.1.0 resolution: "wrap-ansi@npm:8.1.0" @@ -10946,6 +11209,13 @@ __metadata: languageName: node linkType: hard +"y18n@npm:^5.0.5": + version: 5.0.8 + resolution: "y18n@npm:5.0.8" + checksum: 4df2842c36e468590c3691c894bc9cdbac41f520566e76e24f59401ba7d8b4811eb1e34524d57e54bc6d864bcb66baab7ffd9ca42bf1eda596618f9162b91249 + languageName: node + linkType: hard + "yallist@npm:^3.0.2": version: 3.1.1 resolution: "yallist@npm:3.1.1" @@ -10967,6 +11237,28 @@ __metadata: languageName: node linkType: hard +"yargs-parser@npm:^21.1.1": + version: 21.1.1 + resolution: "yargs-parser@npm:21.1.1" + checksum: f84b5e48169479d2f402239c59f084cfd1c3acc197a05c59b98bab067452e6b3ea46d4dd8ba2985ba7b3d32a343d77df0debd6b343e5dae3da2aab2cdf5886b2 + languageName: node + linkType: hard + +"yargs@npm:^17.7.2": + version: 17.7.2 + resolution: "yargs@npm:17.7.2" + dependencies: + cliui: "npm:^8.0.1" + escalade: "npm:^3.1.1" + get-caller-file: "npm:^2.0.5" + require-directory: "npm:^2.1.1" + string-width: "npm:^4.2.3" + y18n: "npm:^5.0.5" + yargs-parser: "npm:^21.1.1" + checksum: ccd7e723e61ad5965fffbb791366db689572b80cca80e0f96aad968dfff4156cd7cd1ad18607afe1046d8241e6fb2d6c08bf7fa7bfb5eaec818735d8feac8f05 + languageName: node + linkType: hard + "yn@npm:3.1.1": version: 3.1.1 resolution: "yn@npm:3.1.1" @@ -10974,6 +11266,13 @@ __metadata: languageName: node linkType: hard +"yoctocolors-cjs@npm:^2.1.3": + version: 2.1.3 + resolution: "yoctocolors-cjs@npm:2.1.3" + checksum: 584168ef98eb5d913473a4858dce128803c4a6cd87c0f09e954fa01126a59a33ab9e513b633ad9ab953786ed16efdd8c8700097a51635aafaeed3fef7712fa79 + languageName: node + linkType: hard + "zwitch@npm:^2.0.0": version: 2.0.4 resolution: "zwitch@npm:2.0.4" From f166ba9688757603edbd552fd389307a8cc533c8 Mon Sep 17 00:00:00 2001 From: Stefan du Fresne Date: Thu, 22 Jan 2026 11:13:14 +0000 Subject: [PATCH 2/3] Add test coverage for SyncManager, UserAuthenticationWidget, and About Phase 2 of axios migration: Add comprehensive test coverage for components that use HTTP calls before replacing axios with fetch. - SyncManager.test.tsx: 10 tests covering sync flow, error handling, socket events - UserAuthenticationWidget.test.tsx: 11 tests for login, signup, validation - About.test.tsx: 2 tests for basic rendering - Extended MockDatabase interface with info, changes, allDocs, bulkDocs methods - Updated CLAUDE.md with guidance on magic numbers and type assertions Co-Authored-By: Claude Opus 4.5 --- CLAUDE.md | 35 ++ remove-axios.md | 14 +- src/client/db/__mocks__/index.ts | 8 + src/client/features/Sync/SyncManager.test.tsx | 537 ++++++++++++++++++ .../User/UserAuthenticationWidget.test.tsx | 200 +++++++ src/client/pages/About.test.tsx | 49 ++ 6 files changed, 836 insertions(+), 7 deletions(-) create mode 100644 src/client/features/Sync/SyncManager.test.tsx create mode 100644 src/client/features/User/UserAuthenticationWidget.test.tsx create mode 100644 src/client/pages/About.test.tsx diff --git a/CLAUDE.md b/CLAUDE.md index 8d87996..c11b196 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -194,6 +194,41 @@ import { MyComponent } from './MyComponent'; - **Server**: ES2022 modules, ES2020 target, Node resolution - Strict mode enabled for both +### Magic Numbers +**Avoid magic numbers and array index access.** Numbers should be named constants or accessed via destructuring: +```typescript +// Bad: What does [1] mean? +const handler = mockCalls.find((call) => call[0] === 'connect')?.[1]; + +// Good: Destructure with meaningful names +const handler = mockCalls.find(([eventName]) => eventName === 'connect'); +const [, callback] = handler ?? []; + +// Bad: Magic number +if (retryCount > 3) { ... } + +// Good: Named constant +const MAX_RETRIES = 3; +if (retryCount > MAX_RETRIES) { ... } +``` + +### Type Assertions +**NEVER use the `as` keyword for type assertions.** If you think you need `as`: +1. First, try to fix the types at the source (e.g., update interface definitions, add proper generics) +2. If the types truly cannot be fixed, ask for permission before using `as` +3. Prefer runtime checks (`if (x !== null)`) over type assertions + +Instead of casting, extend mock interfaces or use proper type guards: +```typescript +// Bad: Casting to work around missing mock methods +(handle as unknown as { changes: Mock }).changes = vi.fn(); + +// Good: Add the method to the mock interface +export interface MockDatabase { + changes: Mock<(options?: unknown) => Promise<{ results: unknown[] }>>; +} +``` + ### Testing - **Client tests**: Vitest with jsdom, setup in `src/client/test-utils/test-setup.ts` - **Server tests**: Vitest with default Node environment diff --git a/remove-axios.md b/remove-axios.md index dbfe201..1c33d25 100644 --- a/remove-axios.md +++ b/remove-axios.md @@ -383,13 +383,13 @@ Given the small number of call sites (8 total), either approach works. Raw fetch ## Checklist -- [ ] Phase 1: Install MSW and setup test infrastructure -- [ ] Phase 1: Migrate UserProvider.test.tsx to MSW -- [ ] Phase 1: Migrate App.test.tsx to MSW -- [ ] Phase 1: Verify all tests pass with axios still in code -- [ ] Phase 2: Add SyncManager tests -- [ ] Phase 2: Add UserAuthenticationWidget tests -- [ ] Phase 2: Review About.tsx coverage needs +- [x] Phase 1: Install MSW and setup test infrastructure +- [x] Phase 1: Migrate UserProvider.test.tsx to MSW +- [x] Phase 1: Migrate App.test.tsx to MSW +- [x] Phase 1: Verify all tests pass with axios still in code +- [x] Phase 2: Add SyncManager tests +- [x] Phase 2: Add UserAuthenticationWidget tests +- [x] Phase 2: Review About.tsx coverage needs - [ ] Phase 3: Decide on utility vs raw fetch - [ ] Phase 3: Migrate About.tsx - [ ] Phase 3: Migrate UserAuthenticationWidget.tsx diff --git a/src/client/db/__mocks__/index.ts b/src/client/db/__mocks__/index.ts index 0f5ff60..162d679 100644 --- a/src/client/db/__mocks__/index.ts +++ b/src/client/db/__mocks__/index.ts @@ -5,12 +5,20 @@ export interface MockDatabase { find: Mock<(options?: unknown) => Promise<{ docs: unknown[] }>>; get: Mock<(docId: string) => Promise>; userPut: Mock<(doc: Doc) => Promise>; + info: Mock<() => Promise>>; + changes: Mock<(options?: unknown) => Promise<{ results: unknown[] }>>; + allDocs: Mock<(options?: unknown) => Promise<{ rows: unknown[] }>>; + bulkDocs: Mock<(docs: Doc[], options?: unknown) => Promise>; } const mockDb: MockDatabase = { find: vi.fn(), get: vi.fn(), userPut: vi.fn(), + info: vi.fn(), + changes: vi.fn(), + allDocs: vi.fn(), + bulkDocs: vi.fn(), }; /** diff --git a/src/client/features/Sync/SyncManager.test.tsx b/src/client/features/Sync/SyncManager.test.tsx new file mode 100644 index 0000000..b6cfd53 --- /dev/null +++ b/src/client/features/Sync/SyncManager.test.tsx @@ -0,0 +1,537 @@ +import { act, waitFor } from '@testing-library/react'; +import { HttpResponse, http } from 'msw'; +import type { AnyAction, Store } from 'redux'; +import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from 'vitest'; + +import type { Doc, RepeatableDoc } from '../../../shared/types'; +import { getMockDb, type MockDatabase } from '../../db/__mocks__'; +import { createStore, type RootState } from '../../store'; +import { render, withStore } from '../../test-utils'; +import { server } from '../../test-utils/msw-server'; +import { setUserAsGuest, setUserAsLoggedIn } from '../User/userSlice'; +import SyncManager from './SyncManager'; +import { State } from './syncSlice'; + +vi.mock('../../db'); + +type SocketEventHandler = (...args: unknown[]) => void; + +interface MockSocket { + on: Mock<(event: string, handler: SocketEventHandler) => void>; + emit: Mock<(event: string, ...args: unknown[]) => void>; + close: Mock<() => void>; + connected: boolean; +} + +// FIXME: improve typing here. This should be based on the actual socket.io types +const mockSocket: MockSocket = { + on: vi.fn(), + emit: vi.fn(), + close: vi.fn(), + connected: true, +}; + +vi.mock('socket.io-client', () => ({ + io: vi.fn(() => mockSocket), +})); + +function getSocketHandler(eventName: string): SocketEventHandler | undefined { + const call = mockSocket.on.mock.calls.find(([event]) => event === eventName); + if (call) { + const [, handler] = call; + return handler; + } + return undefined; +} + +function hasSocketHandler(eventName: string): boolean { + return mockSocket.on.mock.calls.some(([event]) => event === eventName); +} + +const testUser = { id: 1, name: 'testuser' }; + +describe('SyncManager', () => { + let store: Store; + let handle: MockDatabase; + + beforeEach(() => { + store = createStore(); + store.dispatch(setUserAsLoggedIn({ user: testUser })); + + handle = getMockDb(); + + mockSocket.on.mockReset(); + mockSocket.emit.mockReset(); + mockSocket.close.mockReset(); + mockSocket.connected = true; + + handle.changes.mockResolvedValue({ + results: [], + }); + handle.allDocs.mockResolvedValue({ + rows: [], + }); + handle.bulkDocs.mockResolvedValue([]); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + function renderSyncManager() { + render(withStore(store, )); + } + + describe('full sync flow', () => { + it('completes sync successfully with no documents to sync', async () => { + server.use( + http.post('/api/sync/begin', () => { + return HttpResponse.json({ server: [], client: [] }); + }), + ); + + renderSyncManager(); + + // Wait for socket connection handler to be registered + await waitFor(() => { + expect(hasSocketHandler('connect')).toBe(true); + }); + + // Simulate socket connect + act(() => { + const connectHandler = getSocketHandler('connect'); + if (!connectHandler) { + expect.unreachable('connect handler should exist'); + } + connectHandler(); + }); + + // Wait for sync to complete + await waitFor(() => { + expect(store.getState().sync.state).toBe(State.connected); + }); + }); + + it('sends local documents to server during sync', async () => { + const localDoc: RepeatableDoc = { + _id: 'repeatable:instance:123', + _rev: '1-abc', + template: 'repeatable:template:test', + values: [], + created: Date.now(), + updated: Date.now(), + slug: 'test', + }; + + handle.changes.mockResolvedValue({ + results: [ + { + id: localDoc._id, + changes: [{ rev: localDoc._rev }], + deleted: false, + }, + ], + }); + + handle.allDocs.mockResolvedValue({ + rows: [{ id: localDoc._id, doc: localDoc }], + }); + + const sentDocs: unknown[] = []; + + server.use( + http.post('/api/sync/begin', () => { + // Server needs our document (it's in server array = server wants these docs) + return HttpResponse.json({ + server: [{ _id: localDoc._id, _rev: localDoc._rev }], + client: [], + }); + }), + http.post('/api/sync/update', async ({ request }) => { + const body = await request.json(); + if (body && typeof body === 'object' && 'docs' in body && Array.isArray(body.docs)) { + sentDocs.push(...body.docs); + } + return HttpResponse.json({ success: true }); + }), + ); + + renderSyncManager(); + + await waitFor(() => { + expect(hasSocketHandler('connect')).toBe(true); + }); + + act(() => { + const connectHandler = getSocketHandler('connect'); + if (!connectHandler) { + expect.unreachable('connect handler should exist'); + } + connectHandler(); + }); + + await waitFor(() => { + expect(store.getState().sync.state).toBe(State.connected); + }); + + // Verify the correct document was sent to the server + expect(sentDocs).toHaveLength(1); + expect(sentDocs[0]).toMatchObject({ + _id: localDoc._id, + _rev: localDoc._rev, + template: localDoc.template, + values: localDoc.values, + slug: localDoc.slug, + }); + }); + + it('receives documents from server during sync', async () => { + const serverDoc: RepeatableDoc = { + _id: 'repeatable:instance:456', + _rev: '2-xyz', + template: 'repeatable:template:test', + values: [true], + created: Date.now(), + updated: Date.now(), + slug: 'server-doc', + }; + + // Mock local database is empty + handle.changes.mockResolvedValue({ + results: [], + }); + + handle.allDocs.mockResolvedValue({ + rows: [], + }); + + server.use( + http.post('/api/sync/begin', () => { + // Client needs a document from server + return HttpResponse.json({ + server: [], + client: [serverDoc], + }); + }), + http.post('/api/sync/request', () => { + return HttpResponse.json([serverDoc]); + }), + ); + + renderSyncManager(); + + // Wait for socket connection handler to be registered + await waitFor(() => { + expect(hasSocketHandler('connect')).toBe(true); + }); + + act(() => { + const connectHandler = getSocketHandler('connect'); + connectHandler?.(); + }); + + // Wait for sync to complete + await waitFor(() => { + expect(store.getState().sync.state).toBe(State.connected); + }); + + // Verify bulkDocs was called with the server document + expect(handle.bulkDocs).toHaveBeenCalled(); + const bulkDocsCall = handle.bulkDocs.mock.calls.find( + (call) => Array.isArray(call[0]) && call[0].some((doc: Doc) => doc._id === serverDoc._id), + ); + expect(bulkDocsCall).toBeDefined(); + if (bulkDocsCall) { + const [docs] = bulkDocsCall; + const writtenDoc = docs.find((doc: Doc) => doc._id === serverDoc._id); + expect(writtenDoc).toMatchObject({ + _id: serverDoc._id, + _rev: serverDoc._rev, + template: serverDoc.template, + values: serverDoc.values, + slug: serverDoc.slug, + }); + } + }); + + it('handles deleted documents from server', async () => { + const deletedDoc: Doc = { + _id: 'repeatable:instance:789', + _rev: '3-def', + _deleted: true, + }; + + // Mock local database is empty + handle.changes.mockResolvedValue({ + results: [], + }); + + handle.allDocs.mockResolvedValue({ + rows: [{ key: deletedDoc._id, error: 'not_found' }], + }); + + server.use( + http.post('/api/sync/begin', () => { + // Client needs a deleted document from server + return HttpResponse.json({ + server: [], + client: [deletedDoc], + }); + }), + ); + + renderSyncManager(); + + // Wait for socket connection handler to be registered + await waitFor(() => { + expect(hasSocketHandler('connect')).toBe(true); + }); + + act(() => { + const connectHandler = getSocketHandler('connect'); + connectHandler?.(); + }); + + // Wait for sync to complete + await waitFor(() => { + expect(store.getState().sync.state).toBe(State.connected); + }); + + // Verify bulkDocs was called with the deleted document + expect(handle.bulkDocs).toHaveBeenCalled(); + const bulkDocsCall = handle.bulkDocs.mock.calls.find( + (call) => Array.isArray(call[0]) && call[0].some((doc: Doc) => doc._id === deletedDoc._id), + ); + expect(bulkDocsCall).toBeDefined(); + if (bulkDocsCall) { + const [docs] = bulkDocsCall; + const writtenDoc = docs.find((doc: Doc) => doc._id === deletedDoc._id); + // When syncing deleted documents, the key properties are _id and _deleted + // The _rev may not be preserved through the sync process + expect(writtenDoc).toMatchObject({ + _id: deletedDoc._id, + _deleted: true, + }); + } + }); + }); + + describe('error handling', () => { + it('handles 401 error during sync by marking user as unauthenticated', async () => { + server.use( + http.post('/api/sync/begin', () => { + return HttpResponse.json(null, { status: 401 }); + }), + ); + + renderSyncManager(); + + // Wait for socket connection handler to be registered + await waitFor(() => { + expect(hasSocketHandler('connect')).toBe(true); + }); + + act(() => { + const connectHandler = getSocketHandler('connect'); + connectHandler?.(); + }); + + // Wait for sync to fail and set unauthenticated + await waitFor(() => { + expect(store.getState().user.needsServerAuthentication).toBe(true); + }); + + // State should be disconnected after 401 + expect(store.getState().sync.state).toBe(State.disconnected); + }); + + it('handles 401 error during sync/update by marking user as unauthenticated', async () => { + const localDoc: RepeatableDoc = { + _id: 'repeatable:instance:123', + _rev: '1-abc', + template: 'repeatable:template:test', + values: [], + created: Date.now(), + updated: Date.now(), + slug: 'test', + }; + + handle.changes.mockResolvedValue({ + results: [ + { + id: localDoc._id, + changes: [{ rev: localDoc._rev }], + deleted: false, + }, + ], + }); + + handle.allDocs.mockResolvedValue({ + rows: [{ id: localDoc._id, doc: localDoc }], + }); + + server.use( + http.post('/api/sync/begin', () => { + return HttpResponse.json({ + server: [{ _id: localDoc._id, _rev: localDoc._rev }], + client: [], + }); + }), + http.post('/api/sync/update', () => { + return HttpResponse.json(null, { status: 401 }); + }), + ); + + renderSyncManager(); + + // Wait for socket connection handler to be registered + await waitFor(() => { + expect(hasSocketHandler('connect')).toBe(true); + }); + + act(() => { + const connectHandler = getSocketHandler('connect'); + connectHandler?.(); + }); + + // Wait for sync to fail and set unauthenticated + await waitFor(() => { + expect(store.getState().user.needsServerAuthentication).toBe(true); + }); + }); + + it('handles other server errors gracefully', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + + server.use( + http.post('/api/sync/begin', () => { + return HttpResponse.json(null, { status: 500 }); + }), + ); + + renderSyncManager(); + + // Wait for socket connection handler to be registered + await waitFor(() => { + expect(hasSocketHandler('connect')).toBe(true); + }); + + act(() => { + const connectHandler = getSocketHandler('connect'); + connectHandler?.(); + }); + + // Wait for sync to fail with error state + await waitFor(() => { + const syncState = store.getState().sync; + expect(syncState.state).toBe(State.error); + }); + + // User should NOT be marked as unauthenticated for non-401 errors + expect(store.getState().user.needsServerAuthentication).toBe(false); + }); + }); + + describe('socket events', () => { + it('handles docUpdate from server', async () => { + const updatedDoc: RepeatableDoc = { + _id: 'repeatable:instance:live-update', + _rev: '1-live', + template: 'repeatable:template:test', + values: [true, false], + created: Date.now(), + updated: Date.now(), + slug: 'live-update', + }; + + handle.allDocs.mockResolvedValue({ + rows: [], + }); + + server.use( + http.post('/api/sync/begin', () => { + return HttpResponse.json({ server: [], client: [] }); + }), + ); + + renderSyncManager(); + + // Wait for docUpdate handler to be registered + await waitFor(() => { + expect(hasSocketHandler('docUpdate')).toBe(true); + }); + + // Simulate socket connect first + act(() => { + const connectHandler = getSocketHandler('connect'); + connectHandler?.(); + }); + + await waitFor(() => { + expect(store.getState().sync.state).toBe(State.connected); + }); + + // Simulate docUpdate event + await act(async () => { + const docUpdateHandler = getSocketHandler('docUpdate'); + await docUpdateHandler?.([updatedDoc]); + }); + + // Verify bulkDocs was called to write the update + expect(handle.bulkDocs).toHaveBeenCalled(); + }); + + it('handles socket disconnect', async () => { + server.use( + http.post('/api/sync/begin', () => { + return HttpResponse.json({ server: [], client: [] }); + }), + ); + + renderSyncManager(); + + // Wait for socket connection handler to be registered + await waitFor(() => { + expect(hasSocketHandler('connect')).toBe(true); + }); + + act(() => { + const connectHandler = getSocketHandler('connect'); + connectHandler?.(); + }); + + await waitFor(() => { + expect(store.getState().sync.state).toBe(State.connected); + }); + + // Simulate disconnect + act(() => { + const disconnectHandler = getSocketHandler('disconnect'); + disconnectHandler?.(); + }); + + // State should be disconnected + expect(store.getState().sync.state).toBe(State.disconnected); + }); + }); + + describe('guest user', () => { + it('does not connect socket for guest users', async () => { + // Reset mocks for this specific test + mockSocket.on.mockReset(); + + // Create a fresh store and set user as guest + const guestStore = createStore(); + guestStore.dispatch(setUserAsGuest()); + + render(withStore(guestStore, )); + + // Wait a bit to ensure no socket connections are attempted + await new Promise((resolve) => setTimeout(resolve, 100)); + + // For guest users, socket shouldn't register any event handlers + // because initSocket returns early when isGuest is true + expect(mockSocket.on).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/client/features/User/UserAuthenticationWidget.test.tsx b/src/client/features/User/UserAuthenticationWidget.test.tsx new file mode 100644 index 0000000..617fb97 --- /dev/null +++ b/src/client/features/User/UserAuthenticationWidget.test.tsx @@ -0,0 +1,200 @@ +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { HttpResponse, http } from 'msw'; +import type { AnyAction, Store } from 'redux'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { User } from '../../../shared/types'; +import { createStore, type RootState } from '../../store'; +import { withStore, render as wrappedRender } from '../../test-utils'; +import { server } from '../../test-utils/msw-server'; +import UserAuthenticationWidget, { Action } from './UserAuthenticationWidget'; +import { GuestUser, setUserAsGuest } from './userSlice'; + +vi.mock('../../db', () => ({ + default: vi.fn(), + migrateFromGuest: vi.fn().mockResolvedValue(undefined), +})); + +const testUser: User = { id: 1, name: 'testuser' }; + +describe('UserAuthenticationWidget', () => { + let store: Store; + + beforeEach(() => { + store = createStore(); + store.dispatch(setUserAsGuest()); + }); + + function render(action: Action, username?: string) { + wrappedRender( + withStore(store, ), + ); + } + + async function fillAndSubmit(username: string, password: string) { + const user = userEvent.setup(); + await user.type(screen.getByLabelText(/username/i), username); + await user.type(screen.getByLabelText(/password/i), password); + await user.click(screen.getByRole('button', { name: /submit/i })); + } + + describe('login (Authenticate)', () => { + it('logs in successfully with valid credentials', async () => { + server.use( + http.post('/api/auth', () => { + return HttpResponse.json(testUser); + }), + ); + + render(Action.Authenticate); + await fillAndSubmit('testuser', 'password123'); + + await waitFor(() => { + expect(store.getState().user.value).toEqual(testUser); + }); + }); + + it('displays error on 401 response', async () => { + server.use( + http.post('/api/auth', () => { + return HttpResponse.json(null, { status: 401 }); + }), + ); + + render(Action.Authenticate); + await fillAndSubmit('testuser', 'wrongpassword'); + + await waitFor(() => { + expect(screen.getByText(/incorrect credentials/i)).toBeInTheDocument(); + }); + expect(store.getState().user.value).toEqual(GuestUser); + }); + + it('displays error on 403 response', async () => { + server.use( + http.post('/api/auth', () => { + return HttpResponse.json(null, { status: 403 }); + }), + ); + + render(Action.Authenticate); + await fillAndSubmit('testuser', 'wrongpassword'); + + await waitFor(() => { + expect(screen.getByText(/incorrect credentials/i)).toBeInTheDocument(); + }); + + expect(store.getState().user.value).toEqual(GuestUser); + }); + + it('displays server error message on other errors', async () => { + server.use( + http.post('/api/auth', () => { + return HttpResponse.json(null, { status: 500 }); + }), + ); + + render(Action.Authenticate);expect(store.getState().user.value).toEqual(GuestUser); + await fillAndSubmit('testuser', 'password123'); + + await waitFor(() => { + expect(screen.getByText(/500/i)).toBeInTheDocument(); + }); + + expect(store.getState().user.value).toEqual(GuestUser); + }); + + it('displays network error on connection failure', async () => { + server.use( + http.post('/api/auth', () => { + return HttpResponse.error(); + }), + ); + + render(Action.Authenticate); + await fillAndSubmit('testuser', 'password123'); + + // Network errors show the error message (varies by environment, but always shown) + await waitFor(() => { + const errorElement = screen.getByRole('paragraph'); + expect(errorElement).toHaveClass('Mui-error'); + }); + + expect(store.getState().user.value).toEqual(GuestUser); + }); + }); + + describe('signup (Create)', () => { + it('creates account successfully', async () => { + server.use( + http.put('/api/auth', () => { + return HttpResponse.json(testUser); + }), + ); + + render(Action.Create); + await fillAndSubmit('newuser', 'newpassword'); + + await waitFor(() => { + expect(store.getState().user.value).toEqual(testUser); + }); + }); + + it('displays error on 401 response during signup', async () => { + server.use( + http.put('/api/auth', () => { + return HttpResponse.json(null, { status: 401 }); + }), + ); + + render(Action.Create); + await fillAndSubmit('existinguser', 'password'); + + await waitFor(() => { + expect(screen.getByText(/incorrect credentials/i)).toBeInTheDocument(); + }); + + expect(store.getState().user.value).toEqual(GuestUser); + }); + }); + + describe('validation', () => { + it('shows error when username is empty', async () => { + render(Action.Authenticate); + const user = userEvent.setup(); + await user.type(screen.getByLabelText(/password/i), 'password123'); + await user.click(screen.getByRole('button', { name: /submit/i })); + + await waitFor(() => { + expect(screen.getByText(/no username/i)).toBeInTheDocument(); + }); + }); + + it('shows error when password is empty', async () => { + render(Action.Authenticate); + const user = userEvent.setup(); + await user.type(screen.getByLabelText(/username/i), 'testuser'); + await user.click(screen.getByRole('button', { name: /submit/i })); + + await waitFor(() => { + expect(screen.getByText(/no password/i)).toBeInTheDocument(); + }); + }); + }); + + describe('autocomplete attributes', () => { + it('uses current-password for login', () => { + render(Action.Authenticate); + expect(screen.getByLabelText(/password/i)).toHaveAttribute( + 'autocomplete', + 'current-password', + ); + }); + + it('uses new-password for signup', () => { + render(Action.Create); + expect(screen.getByLabelText(/password/i)).toHaveAttribute('autocomplete', 'new-password'); + }); + }); +}); diff --git a/src/client/pages/About.test.tsx b/src/client/pages/About.test.tsx new file mode 100644 index 0000000..e350f33 --- /dev/null +++ b/src/client/pages/About.test.tsx @@ -0,0 +1,49 @@ +import { screen, waitFor } from '@testing-library/react'; +import type { AnyAction, Store } from 'redux'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { setUserAsLoggedIn } from '../features/User/userSlice'; +import { createStore, type RootState } from '../store'; +import { render, withStore } from '../test-utils'; +import About from './About'; + +// Mock the database with info method +vi.mock('../db', () => ({ + default: vi.fn(() => ({ + info: vi.fn().mockResolvedValue({ + db_name: 'test-db', + doc_count: 10, + update_seq: 5, + }), + })), +})); + +const testUser = { id: 1, name: 'testuser' }; + +describe('About', () => { + let store: Store; + + beforeEach(() => { + store = createStore(); + store.dispatch(setUserAsLoggedIn({ user: testUser })); + }); + + it('renders deployment type', async () => { + render(withStore(store, )); + + await waitFor(() => { + expect(screen.getByText(/deployment type/i)).toBeInTheDocument(); + }); + }); + + it('renders section headers', async () => { + render(withStore(store, )); + + await waitFor(() => { + expect(screen.getByText('SERVER DETAILS')).toBeInTheDocument(); + expect(screen.getByText('LOCAL DETAILS')).toBeInTheDocument(); + expect(screen.getByText('DATA SYNC')).toBeInTheDocument(); + expect(screen.getByText('DEBUG')).toBeInTheDocument(); + }); + }); +}); From 0bc142d6a226d2c1e5a45d19c84841fcdc40c6dc Mon Sep 17 00:00:00 2001 From: Stefan du Fresne Date: Thu, 22 Jan 2026 11:27:37 +0000 Subject: [PATCH 3/3] Replace axios with native fetch API Remove axios dependency and migrate all HTTP calls to use the native fetch API with AbortController for timeouts. This simplifies the codebase by removing an external dependency for straightforward HTTP operations. Co-Authored-By: Claude Opus 4.5 --- package.json | 1 - remove-axios.md | 14 ++-- src/client/features/Sync/SyncManager.tsx | 75 ++++++++++++------- .../User/UserAuthenticationWidget.test.tsx | 3 +- .../User/UserAuthenticationWidget.tsx | 34 ++++----- src/client/features/User/UserProvider.tsx | 33 ++++---- src/client/pages/About.tsx | 14 ++-- yarn.lock | 31 +------- 8 files changed, 104 insertions(+), 101 deletions(-) diff --git a/package.json b/package.json index 25ed254..42e8363 100644 --- a/package.json +++ b/package.json @@ -87,7 +87,6 @@ "@types/uuid": "^11.0.0", "@vitest/coverage-v8": "4.0.17", "@vitest/ui": "4.0.17", - "axios": "1.13.2", "buffer": "^6.0.3", "date-fns": "4.1.0", "deep-diff": "^1.0.2", diff --git a/remove-axios.md b/remove-axios.md index 1c33d25..962885b 100644 --- a/remove-axios.md +++ b/remove-axios.md @@ -390,11 +390,11 @@ Given the small number of call sites (8 total), either approach works. Raw fetch - [x] Phase 2: Add SyncManager tests - [x] Phase 2: Add UserAuthenticationWidget tests - [x] Phase 2: Review About.tsx coverage needs -- [ ] Phase 3: Decide on utility vs raw fetch -- [ ] Phase 3: Migrate About.tsx -- [ ] Phase 3: Migrate UserAuthenticationWidget.tsx -- [ ] Phase 3: Migrate UserProvider.tsx -- [ ] Phase 3: Migrate SyncManager.tsx -- [ ] Phase 3: Remove axios dependency -- [ ] Phase 4: Full test suite passes +- [x] Phase 3: Decide on utility vs raw fetch (chose raw fetch - simpler for our small codebase) +- [x] Phase 3: Migrate About.tsx +- [x] Phase 3: Migrate UserAuthenticationWidget.tsx +- [x] Phase 3: Migrate UserProvider.tsx +- [x] Phase 3: Migrate SyncManager.tsx +- [x] Phase 3: Remove axios dependency +- [x] Phase 4: Full test suite passes - [ ] Phase 4: Manual testing complete diff --git a/src/client/features/Sync/SyncManager.tsx b/src/client/features/Sync/SyncManager.tsx index 86eb7ac..654ad8f 100644 --- a/src/client/features/Sync/SyncManager.tsx +++ b/src/client/features/Sync/SyncManager.tsx @@ -1,5 +1,3 @@ -// TODO: drop axios and just use fetch -import axios from 'axios'; import { useEffect, useRef, useState } from 'react'; import { io, type Socket } from 'socket.io-client'; @@ -114,11 +112,21 @@ function SyncManager() { // Work out the difference between us and the server debug('checking with the server'); - const serverState: Requests = await axios - .post('/api/sync/begin', { - docs: stubs, - }) - .then(({ data }) => data); + const beginResponse = await fetch('/api/sync/begin', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ docs: stubs }), + }); + if (!beginResponse.ok) { + if (beginResponse.status === 401) { + debug('sync failed as user is no longer authenticated'); + dispatch(setUserAsUnauthenticated()); + dispatch(socketDisconnected()); + return; + } + throw new Error(`Sync begin failed: HTTP ${beginResponse.status}`); + } + const serverState: Requests = await beginResponse.json(); debug( `the server needs ${serverState.server.length}, we need ${serverState.client.length}`, ); @@ -141,12 +149,25 @@ function SyncManager() { }); debug('-> got local'); - await axios.post('/api/sync/update', { - docs: result.rows.map( - // @ts-expect-error FIXME the types changed, work out what to do with erroring rows - (r) => r.doc || { _id: r.id, _rev: r.value.rev, _deleted: r.value.deleted }, - ), + const updateResponse = await fetch('/api/sync/update', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + docs: result.rows.map( + // @ts-expect-error FIXME the types changed, work out what to do with erroring rows + (r) => r.doc || { _id: r.id, _rev: r.value.rev, _deleted: r.value.deleted }, + ), + }), }); + if (!updateResponse.ok) { + if (updateResponse.status === 401) { + debug('sync failed as user is no longer authenticated'); + dispatch(setUserAsUnauthenticated()); + dispatch(socketDisconnected()); + return; + } + throw new Error(`Sync update failed: HTTP ${updateResponse.status}`); + } debug('-> sent'); docCount += batch.length; @@ -168,11 +189,21 @@ function SyncManager() { } if (writes.length) { - const result: Doc[] = await axios - .post('/api/sync/request', { - docs: writes, - }) - .then(({ data }) => data); + const requestResponse = await fetch('/api/sync/request', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ docs: writes }), + }); + if (!requestResponse.ok) { + if (requestResponse.status === 401) { + debug('sync failed as user is no longer authenticated'); + dispatch(setUserAsUnauthenticated()); + dispatch(socketDisconnected()); + return; + } + throw new Error(`Sync request failed: HTTP ${requestResponse.status}`); + } + const result: Doc[] = await requestResponse.json(); debug('<- got server'); await writesFromRemote(handle, result); @@ -200,14 +231,8 @@ function SyncManager() { dispatch(socketConnected()); } } catch (e) { - if (axios.isAxiosError(e) && e.response?.status === 401) { - debug('sync failed as user is no longer authenticated'); - dispatch(setUserAsUnauthenticated()); - dispatch(socketDisconnected()); - } else { - console.error('Failed to sync', e); - dispatch(syncError(e)); - } + console.error('Failed to sync', e); + dispatch(syncError(e)); } finally { debug('finished'); } diff --git a/src/client/features/User/UserAuthenticationWidget.test.tsx b/src/client/features/User/UserAuthenticationWidget.test.tsx index 617fb97..0215faf 100644 --- a/src/client/features/User/UserAuthenticationWidget.test.tsx +++ b/src/client/features/User/UserAuthenticationWidget.test.tsx @@ -95,7 +95,8 @@ describe('UserAuthenticationWidget', () => { }), ); - render(Action.Authenticate);expect(store.getState().user.value).toEqual(GuestUser); + render(Action.Authenticate); + expect(store.getState().user.value).toEqual(GuestUser); await fillAndSubmit('testuser', 'password123'); await waitFor(() => { diff --git a/src/client/features/User/UserAuthenticationWidget.tsx b/src/client/features/User/UserAuthenticationWidget.tsx index 8d99f04..766091f 100644 --- a/src/client/features/User/UserAuthenticationWidget.tsx +++ b/src/client/features/User/UserAuthenticationWidget.tsx @@ -1,5 +1,4 @@ import { Button, Container, FormHelperText, TextField } from '@mui/material'; -import axios from 'axios'; import { type FormEvent, useState } from 'react'; import { useDispatch } from 'react-redux'; import type { User } from '../../../shared/types'; @@ -30,14 +29,21 @@ function UserAuthenticationWidget(props: { username?: string; action: Action }) } try { - const response = await (props.action === Action.Create ? axios.put : axios.post)( - '/api/auth', - { - username, - password, - }, - ); - const user: User = response.data; + const response = await fetch('/api/auth', { + method: props.action === Action.Create ? 'PUT' : 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ username, password }), + }); + + if (!response.ok) { + if ([401, 403].includes(response.status)) { + setError('Incorrect credentials'); + return; + } + throw new Error(`HTTP ${response.status}`); + } + + const user: User = await response.json(); if (props.action === Action.Create) { await migrateFromGuest(user); @@ -47,15 +53,7 @@ function UserAuthenticationWidget(props: { username?: string; action: Action }) dispatch(setUserAsLoggedIn({ user })); } catch (error) { - if ( - axios.isAxiosError(error) && - error.response && - [401, 403].includes(error.response.status) - ) { - setError('Incorrect credentials'); - } else { - setError((error as Error).message || 'Unknown Error'); - } + setError((error as Error).message || 'Unknown Error'); } } diff --git a/src/client/features/User/UserProvider.tsx b/src/client/features/User/UserProvider.tsx index f3ba84b..11c8d3d 100644 --- a/src/client/features/User/UserProvider.tsx +++ b/src/client/features/User/UserProvider.tsx @@ -1,4 +1,3 @@ -import axios from 'axios'; import cookie from 'cookie'; import type React from 'react'; import { type FC, Fragment, useEffect } from 'react'; @@ -19,29 +18,35 @@ const UserProvider: FC<{ children?: React.ReactNode }> = ({ children }) => { // Check the server for authentication against the server-side cookie async function networkCheck(): Promise { debug('server authentication check'); - const source = axios.CancelToken.source(); - setTimeout(() => source.cancel(), 1000); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 1000); try { - const response = await axios.get('/api/auth', { - cancelToken: source.token, + const response = await fetch('/api/auth', { + signal: controller.signal, headers: { 'Cache-Control': 'no-cache', }, }); - debug('got valid server response'); - return response.data; - } catch (error) { - if (axios.isCancel(error)) { - debug('server authentication check timed out'); - } else if (axios.isAxiosError(error)) { - if (error.response?.status === 401) { + clearTimeout(timeoutId); + + if (!response.ok) { + if (response.status === 401) { // authentication failed, either because they have no cookie or because it's wrong / outdated debug('server authentication check failed'); return 'auth_error'; } - debug(`network (axios) issues: ${error.message}`); + debug(`network issues: HTTP ${response.status}`); + return undefined; + } + + debug('got valid server response'); + return await response.json(); + } catch (error) { + clearTimeout(timeoutId); + if (error instanceof Error && error.name === 'AbortError') { + debug('server authentication check timed out'); } else { - console.error('unexpected error in networkCheck', error, source); + console.error('unexpected error in networkCheck', error); } } } diff --git a/src/client/pages/About.tsx b/src/client/pages/About.tsx index a200e7a..d342305 100644 --- a/src/client/pages/About.tsx +++ b/src/client/pages/About.tsx @@ -1,5 +1,4 @@ import { Link } from '@mui/material'; -import axios from 'axios'; import { Fragment, type ReactNode, useEffect, useState } from 'react'; import db from '../db'; @@ -35,14 +34,19 @@ function About() { useEffect(() => { if (process.env.NODE_ENV === 'production') { - axios - .get('./api/deployment') + fetch('./api/deployment') .then((response) => { + if (!response.ok) { + throw new Error(`HTTP ${response.status}`); + } + return response.json(); + }) + .then((responseData) => { const { deploy_version: deployVersion, release_version: releaseVersion, deploy_commit: hash, - } = response.data; + } = responseData; const data: InfoRow[] = [ ['Deploy Version', deployVersion], @@ -55,7 +59,7 @@ function About() { ], ]; - for (const user of response.data.users) { + for (const user of responseData.users) { data.push([`${user.id}.${user.username}`, user.count]); } diff --git a/yarn.lock b/yarn.lock index 03fd12d..e6c3c98 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5119,17 +5119,6 @@ __metadata: languageName: node linkType: hard -"axios@npm:1.13.2": - version: 1.13.2 - resolution: "axios@npm:1.13.2" - dependencies: - follow-redirects: "npm:^1.15.6" - form-data: "npm:^4.0.4" - proxy-from-env: "npm:^1.1.0" - checksum: e8a42e37e5568ae9c7a28c348db0e8cf3e43d06fcbef73f0048669edfe4f71219664da7b6cc991b0c0f01c28a48f037c515263cb79be1f1ae8ff034cd813867b - languageName: node - linkType: hard - "babel-plugin-macros@npm:^3.1.0": version: 3.1.0 resolution: "babel-plugin-macros@npm:3.1.0" @@ -6416,16 +6405,6 @@ __metadata: languageName: node linkType: hard -"follow-redirects@npm:^1.15.6": - version: 1.15.11 - resolution: "follow-redirects@npm:1.15.11" - peerDependenciesMeta: - debug: - optional: true - checksum: d301f430542520a54058d4aeeb453233c564aaccac835d29d15e050beb33f339ad67d9bddbce01739c5dc46a6716dbe3d9d0d5134b1ca203effa11a7ef092343 - languageName: node - linkType: hard - "foreground-child@npm:^3.1.0": version: 3.1.1 resolution: "foreground-child@npm:3.1.1" @@ -6449,7 +6428,7 @@ __metadata: languageName: node linkType: hard -"form-data@npm:^4.0.4, form-data@npm:^4.0.5": +"form-data@npm:^4.0.5": version: 4.0.5 resolution: "form-data@npm:4.0.5" dependencies: @@ -9154,13 +9133,6 @@ __metadata: languageName: node linkType: hard -"proxy-from-env@npm:^1.1.0": - version: 1.1.0 - resolution: "proxy-from-env@npm:1.1.0" - checksum: fe7dd8b1bdbbbea18d1459107729c3e4a2243ca870d26d34c2c1bcd3e4425b7bcc5112362df2d93cc7fb9746f6142b5e272fd1cc5c86ddf8580175186f6ad42b - languageName: node - linkType: hard - "psl@npm:^1.1.33": version: 1.9.0 resolution: "psl@npm:1.9.0" @@ -9797,7 +9769,6 @@ __metadata: "@types/uuid": "npm:^11.0.0" "@vitest/coverage-v8": "npm:4.0.17" "@vitest/ui": "npm:4.0.17" - axios: "npm:1.13.2" bcryptjs: "npm:^3.0.3" buffer: "npm:^6.0.3" compression: "npm:1.8.1"