Merge pull request #41 from atomantic/better/dry

atomantic · web-flow · commit 34e9fb42c19b · 2026-03-05T18:27:58.000-08:00
refactor: deduplicate SSE managers and localStorage sync patterns
diff --git a/client/src/context/SidebarContext.tsx b/client/src/context/SidebarContext.tsx
@@ -1,6 +1,7 @@
 import { createContext, useContext, useState, useCallback, useEffect, ReactNode } from 'react';
 import type { DatabaseInfo } from '@fsf/shared';
 import { api } from '../services/api';
+import { useLocalStorage } from '../hooks/useLocalStorage';
 
 interface SidebarContextType {
   isCollapsed: boolean;
@@ -18,23 +19,23 @@ interface SidebarContextType {
 const SidebarContext = createContext<SidebarContextType | null>(null);
 
 export function SidebarProvider({ children }: { children: ReactNode }) {
-  const [isCollapsed, setIsCollapsed] = useState(() => {
-    const stored = localStorage.getItem('sidebar-collapsed');
-    return stored === 'true';
-  });
+  const [isCollapsed, setIsCollapsed] = useLocalStorage(
+    'sidebar-collapsed',
+    false,
+    String,
+    (raw) => raw === 'true'
+  );
   const [isMobileOpen, setIsMobileOpen] = useState(false);
-  const [expandedDatabases, setExpandedDatabases] = useState<Set<string>>(() => {
-    const stored = localStorage.getItem('sidebar-expanded-databases');
-    return stored ? new Set(JSON.parse(stored)) : new Set();
-  });
+  const [expandedDatabases, setExpandedDatabases] = useLocalStorage<Set<string>>(
+    'sidebar-expanded-databases',
+    () => new Set(),
+    (s) => JSON.stringify([...s]),
+    (raw) => new Set(JSON.parse(raw))
+  );
 
   const toggleCollapsed = useCallback(() => {
-    setIsCollapsed(prev => {
-      const newValue = !prev;
-      localStorage.setItem('sidebar-collapsed', String(newValue));
-      return newValue;
-    });
-  }, []);
+    setIsCollapsed(prev => !prev);
+  }, [setIsCollapsed]);
 
   const toggleMobile = useCallback(() => {
     setIsMobileOpen(prev => !prev);
@@ -52,20 +53,18 @@ export function SidebarProvider({ children }: { children: ReactNode }) {
       } else {
         next.add(dbId);
       }
-      localStorage.setItem('sidebar-expanded-databases', JSON.stringify([...next]));
       return next;
     });
-  }, []);
+  }, [setExpandedDatabases]);
 
   const expandDatabase = useCallback((dbId: string) => {
     setExpandedDatabases(prev => {
       if (prev.has(dbId)) return prev;
       const next = new Set(prev);
       next.add(dbId);
-      localStorage.setItem('sidebar-expanded-databases', JSON.stringify([...next]));
       return next;
     });
-  }, []);
+  }, [setExpandedDatabases]);
 
   // Database state management
   const [databases, setDatabases] = useState<DatabaseInfo[]>([]);
diff --git a/client/src/context/ThemeContext.tsx b/client/src/context/ThemeContext.tsx
@@ -1,4 +1,5 @@
-import { createContext, useContext, useState, useCallback, useEffect, ReactNode } from 'react';
+import { createContext, useContext, useCallback, useEffect, ReactNode } from 'react';
+import { useLocalStorage } from '../hooks/useLocalStorage';
 
 type Theme = 'light' | 'dark';
 
@@ -11,15 +12,12 @@ interface ThemeContextType {
 const ThemeContext = createContext<ThemeContextType | null>(null);
 
 export function ThemeProvider({ children }: { children: ReactNode }) {
-  const [theme, setThemeState] = useState<Theme>(() => {
-    const stored = localStorage.getItem('theme') as Theme | null;
-    if (stored) return stored;
-    // Default to dark, or use system preference
-    if (window.matchMedia('(prefers-color-scheme: light)').matches) {
-      return 'light';
-    }
-    return 'dark';
-  });
+  const [theme, setTheme] = useLocalStorage<Theme>(
+    'theme',
+    () => window.matchMedia('(prefers-color-scheme: light)').matches ? 'light' : 'dark',
+    String,
+    (raw) => raw as Theme
+  );
 
   // Apply theme class to document
   useEffect(() => {
@@ -31,14 +29,9 @@ export function ThemeProvider({ children }: { children: ReactNode }) {
     }
   }, [theme]);
 
-  const setTheme = useCallback((newTheme: Theme) => {
-    setThemeState(newTheme);
-    localStorage.setItem('theme', newTheme);
-  }, []);
-
   const toggleTheme = useCallback(() => {
-    setTheme(theme === 'dark' ? 'light' : 'dark');
-  }, [theme, setTheme]);
+    setTheme(prev => prev === 'dark' ? 'light' : 'dark');
+  }, [setTheme]);
 
   return (
     <ThemeContext.Provider value={{ theme, toggleTheme, setTheme }}>
diff --git a/client/src/hooks/useLocalStorage.ts b/client/src/hooks/useLocalStorage.ts
@@ -0,0 +1,36 @@
+import { useState, useCallback } from 'react';
+
+/**
+ * Like useState, but persists to localStorage.
+ * @param key localStorage key
+ * @param fallback value when nothing is stored (or a factory function)
+ * @param serialize convert value to string for storage (default: String)
+ * @param deserialize convert stored string to value (default: identity cast)
+ */
+export function useLocalStorage<T>(
+  key: string,
+  fallback: T | (() => T),
+  serialize: (v: T) => string = String as (v: T) => string,
+  deserialize?: (raw: string) => T
+): [T, (value: T | ((prev: T) => T)) => void] {
+  const [state, setState] = useState<T>(() => {
+    const stored = localStorage.getItem(key);
+    if (stored !== null && deserialize) {
+      return deserialize(stored);
+    }
+    if (stored !== null) {
+      return stored as unknown as T;
+    }
+    return typeof fallback === 'function' ? (fallback as () => T)() : fallback;
+  });
+
+  const setValue = useCallback((value: T | ((prev: T) => T)) => {
+    setState(prev => {
+      const next = typeof value === 'function' ? (value as (prev: T) => T)(prev) : value;
+      localStorage.setItem(key, serialize(next));
+      return next;
+    });
+  }, [key, serialize]);
+
+  return [state, setValue];
+}
diff --git a/server/src/services/test-runner.service.ts b/server/src/services/test-runner.service.ts
@@ -1,9 +1,9 @@
 import { spawn, ChildProcess } from 'child_process';
 import { Response } from 'express';
-import crypto from 'crypto';
 import fs from 'fs';
 import path from 'path';
 import { fileURLToPath } from 'url';
+import { createSseManager } from '../utils/createSseManager.js';
 
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = path.dirname(__filename);
@@ -20,23 +20,14 @@ interface TestRun {
   exitCode?: number;
 }
 
-interface SSEClient {
-  id: string;
-  response: Response;
-}
-
-// SSE clients for test events
-const clients: SSEClient[] = [];
+const sse = createSseManager('test-runner');
 
 // Current test run state
 let currentRun: TestRun | null = null;
 let currentProcess: ChildProcess | null = null;
 
 function broadcast(event: string, data: object) {
-  const message = `event: ${event}\ndata: ${JSON.stringify({ data })}\n\n`;
-  for (const { response } of clients) {
-    response.write(message);
-  }
+  sse.broadcast(event, { data });
 }
 
 function getTestCommand(type: TestType): { command: string; args: string[] } {
@@ -56,14 +47,7 @@ function getTestCommand(type: TestType): { command: string; args: string[] } {
 
 export const testRunnerService = {
   addClient(response: Response): string {
-    const id = crypto.randomUUID();
-    response.writeHead(200, {
-      'Content-Type': 'text/event-stream',
-      'Cache-Control': 'no-cache',
-      'Connection': 'keep-alive',
-    });
-    response.write(`data: ${JSON.stringify({ type: 'connected', clientId: id })}\n\n`);
-    clients.push({ id, response });
+    const id = sse.addClient(response);
 
     // Send current status immediately
     if (currentRun) {
@@ -73,10 +57,7 @@ export const testRunnerService = {
     return id;
   },
 
-  removeClient(id: string) {
-    const index = clients.findIndex(c => c.id === id);
-    if (index !== -1) clients.splice(index, 1);
-  },
+  removeClient: sse.removeClient,
 
   getStatus(): TestRun | null {
     return currentRun;
diff --git a/server/src/utils/browserSseManager.ts b/server/src/utils/browserSseManager.ts
@@ -1,26 +1,14 @@
 import { Response } from 'express';
-import crypto from 'crypto';
 import type { BrowserStatus } from '../services/browser.service';
 import { emitBrowserEvent } from '../services/socket.service.js';
+import { createSseManager } from './createSseManager.js';
 
-interface SSEClient {
-  id: string;
-  response: Response;
-}
-
-const clients: SSEClient[] = [];
+const base = createSseManager('browser');
 let lastStatus: BrowserStatus | null = null;
 
 export const browserSseManager = {
   addClient(response: Response): string {
-    const id = crypto.randomUUID();
-    response.writeHead(200, {
-      'Content-Type': 'text/event-stream',
-      'Cache-Control': 'no-cache',
-      'Connection': 'keep-alive'
-    });
-    response.write(`data: ${JSON.stringify({ type: 'connected', clientId: id })}\n\n`);
-    clients.push({ id, response });
+    const id = base.addClient(response);
 
     // Send current status immediately if available
     if (lastStatus) {
@@ -31,34 +19,21 @@ export const browserSseManager = {
     return id;
   },
 
-  removeClient(id: string) {
-    const index = clients.findIndex(c => c.id === id);
-    if (index !== -1) clients.splice(index, 1);
-  },
+  removeClient: base.removeClient,
 
   broadcast(event: string, data: object) {
     // Emit via Socket.IO (always, even if no SSE clients)
     emitBrowserEvent(event, data);
 
-    // Also send via SSE for backwards compatibility
-    if (clients.length === 0) return;
-
-    const message = `event: ${event}\ndata: ${JSON.stringify({ data })}\n\n`;
-    clients.forEach(({ response }) => {
-      response.write(message);
-    });
+    // Also send via SSE for backwards compatibility — wrap in { data } envelope
+    base.broadcast(event, { data });
   },
 
   broadcastStatus(status: BrowserStatus) {
     lastStatus = status;
     this.broadcast('status', status);
   },
 
-  getClientCount(): number {
-    return clients.length;
-  },
-
-  hasClients(): boolean {
-    return clients.length > 0;
-  }
+  getClientCount: base.getClientCount,
+  hasClients: base.hasClients,
 };
diff --git a/server/src/utils/createSseManager.ts b/server/src/utils/createSseManager.ts
@@ -0,0 +1,54 @@
+import { Response } from 'express';
+import crypto from 'crypto';
+
+interface SSEClient {
+  id: string;
+  response: Response;
+}
+
+export interface SseManager {
+  addClient(response: Response): string;
+  removeClient(id: string): void;
+  broadcast(event: string, data: object): void;
+  getClientCount(): number;
+  hasClients(): boolean;
+}
+
+export function createSseManager(name?: string): SseManager {
+  const clients: SSEClient[] = [];
+
+  return {
+    addClient(response: Response): string {
+      const id = crypto.randomUUID();
+      response.writeHead(200, {
+        'Content-Type': 'text/event-stream',
+        'Cache-Control': 'no-cache',
+        'Connection': 'keep-alive',
+      });
+      response.write(`data: ${JSON.stringify({ type: 'connected', clientId: id })}\n\n`);
+      clients.push({ id, response });
+      return id;
+    },
+
+    removeClient(id: string) {
+      const index = clients.findIndex(c => c.id === id);
+      if (index !== -1) clients.splice(index, 1);
+    },
+
+    broadcast(event: string, data: object) {
+      if (clients.length === 0) return;
+      const message = `event: ${event}\ndata: ${JSON.stringify(data)}\n\n`;
+      for (const { response } of clients) {
+        response.write(message);
+      }
+    },
+
+    getClientCount(): number {
+      return clients.length;
+    },
+
+    hasClients(): boolean {
+      return clients.length > 0;
+    },
+  };
+}
diff --git a/server/src/utils/sseManager.ts b/server/src/utils/sseManager.ts
@@ -1,39 +1,3 @@
-import { Response } from 'express';
-import crypto from 'crypto';
+import { createSseManager } from './createSseManager.js';
 
-interface SSEClient {
-  id: string;
-  response: Response;
-}
-
-const clients: SSEClient[] = [];
-
-export const sseManager = {
-  addClient(response: Response): string {
-    const id = crypto.randomUUID();
-    response.writeHead(200, {
-      'Content-Type': 'text/event-stream',
-      'Cache-Control': 'no-cache',
-      'Connection': 'keep-alive'
-    });
-    response.write(`data: ${JSON.stringify({ type: 'connected', clientId: id })}\n\n`);
-    clients.push({ id, response });
-    return id;
-  },
-
-  removeClient(id: string) {
-    const index = clients.findIndex(c => c.id === id);
-    if (index !== -1) clients.splice(index, 1);
-  },
-
-  broadcast(event: string, data: object) {
-    const message = `event: ${event}\ndata: ${JSON.stringify(data)}\n\n`;
-    clients.forEach(({ response }) => {
-      response.write(message);
-    });
-  },
-
-  getClientCount(): number {
-    return clients.length;
-  }
-};
+export const sseManager = createSseManager('sse');
