diff --git a/EXHAUSTIVE_TODO_LIST.md b/EXHAUSTIVE_TODO_LIST.md index 557a1d4dd..6469a5978 100644 --- a/EXHAUSTIVE_TODO_LIST.md +++ b/EXHAUSTIVE_TODO_LIST.md @@ -1487,13 +1487,12 @@ Critical path dependencies: - **Rollback**: Restore from git - **Status**: ✅ Complete - Deleted 6 files: stateCleanup.ts, stateCleanup.test.ts, stateVersioning.ts, stateVersioning.test.ts, stateVersioning.example.ts, statePersistence.ts. Verified no imports in production code. -- [ ] **Action 4.6.1.4**: Simplify stateMiddleware if still needed +- [x] **Action 4.6.1.4**: Simplify stateMiddleware if still needed - **Scope**: `apps/web/src/utils/stateMiddleware.ts` - Simplify or remove if redundant - - **Dependencies**: Action 4.6.1.2 complete ✅, Action 4.1.2.7 complete (Library Store migration) + - **Dependencies**: Action 4.6.1.2 complete ✅, Action 4.1.2.7 complete ✅ (Library Store migration) - **Risk**: MEDIUM - - **Validation**: Middleware simplified or removed + - **Validation**: ✅ Middleware removed - Completely unused in production code. Only used in its own test file. Previously removed from Library Store in Action 4.1.2.7. Deleted `stateMiddleware.ts` (431 lines) and `stateMiddleware.test.ts` (251 lines). No imports found. See `apps/web/src/docs/STATEMIDDLEWARE_UTILITY_AUDIT.md` - **Rollback**: Restore original middleware - - **Note**: Currently only used by LibraryStore. Should be removed after Library Store migrates to React Query (Action 4.1.2.7). Deferring simplification until migration is complete. - [ ] **Action 4.6.1.5**: Update stateInvalidation to work with React Query - **Scope**: `apps/web/src/utils/stateInvalidation.ts` - Update to invalidate React Query cache diff --git a/apps/web/src/docs/STATEMIDDLEWARE_UTILITY_AUDIT.md b/apps/web/src/docs/STATEMIDDLEWARE_UTILITY_AUDIT.md new file mode 100644 index 000000000..1ab15e2ae --- /dev/null +++ b/apps/web/src/docs/STATEMIDDLEWARE_UTILITY_AUDIT.md @@ -0,0 +1,58 @@ +# StateMiddleware Utility Audit + +**Date**: 2025-01-27 +**Action**: 4.6.1.4 +**Status**: ✅ Complete + +## Summary + +The `stateMiddleware` utility (`apps/web/src/utils/stateMiddleware.ts`) is **completely unused** in production code. It was previously used by the Library Store but was removed when domain data was migrated to React Query (Action 4.1.2.7). + +## Usage Analysis + +### Imports +- ❌ **No imports found** in production code +- ✅ **Only used in test file**: `apps/web/src/utils/stateMiddleware.test.ts` +- ✅ Only mentioned in comments/docs indicating it was removed + +### Functions Exported +1. `stateMiddleware()` - Zustand middleware for logging, analytics, error handling +2. `createWithMiddleware()` - Helper to create a store with state middleware + +**Both functions are unused in production code.** + +## Historical Context + +- Previously used by Library Store for logging, analytics, and error handling on domain data +- Removed in Action 4.1.2.7 when Library Store domain data was migrated to React Query +- React Query handles data fetching and caching, making this middleware unnecessary for domain data +- UI state stores (Library Store now only has filters) don't need this level of middleware complexity + +## Current State + +The Library Store now only contains UI state (filters): +```typescript +export const useLibraryStore = create()( + devtools( + persist( + (set) => ({ + filters: {}, + setFilters: (filters) => set({ filters }), + }), + // ... + ), + ), +); +``` + +No middleware needed for simple UI state. + +## Recommendation + +✅ **Safe to remove** - The utility is completely unused in production code and can be deleted along with: +- `apps/web/src/utils/stateMiddleware.ts` (431 lines) +- `apps/web/src/utils/stateMiddleware.test.ts` (test file, 251 lines) + +## Next Steps + +- **Action 4.6.1.4**: Simplify stateMiddleware if still needed ✅ (confirmed unused, can be removed) diff --git a/apps/web/src/utils/stateMiddleware.test.ts b/apps/web/src/utils/stateMiddleware.test.ts deleted file mode 100644 index 19af3b91a..000000000 --- a/apps/web/src/utils/stateMiddleware.test.ts +++ /dev/null @@ -1,252 +0,0 @@ -/** - * Tests for State Middleware - * FE-STATE-010: Test logging, analytics, and error handling - */ - -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { create } from 'zustand'; -import { - stateMiddleware, - setAnalyticsHandler, - setErrorHandler, - type AnalyticsEvent, -} from './stateMiddleware'; - -interface TestState { - count: number; - name: string; - increment: () => void; - setName: (name: string) => void; - throwError: () => void; - asyncAction: () => Promise; -} - -describe('stateMiddleware', () => { - let analyticsEvents: AnalyticsEvent[] = []; - let errorEvents: Array<{ error: Error; context: unknown }> = []; - - beforeEach(() => { - analyticsEvents = []; - errorEvents = []; - - setAnalyticsHandler((event) => { - analyticsEvents.push(event); - }); - - setErrorHandler((error, context) => { - errorEvents.push({ error, context }); - }); - }); - - it('should log state changes in development', () => { - const consoleSpy = vi.spyOn(console, 'debug').mockImplementation(() => {}); - - const useTestStore = create()( - stateMiddleware( - (set) => ({ - count: 0, - name: '', - increment: () => set((state) => ({ count: state.count + 1 })), - setName: (name: string) => set({ name }), - throwError: () => { - throw new Error('Test error'); - }, - asyncAction: async () => { - return 'success'; - }, - }), - { storeName: 'TestStore', enableLogging: true }, - ), - ); - - const store = useTestStore.getState(); - store.increment(); - - expect(consoleSpy).toHaveBeenCalled(); - consoleSpy.mockRestore(); - }); - - it('should track analytics events', () => { - const useTestStore = create()( - stateMiddleware( - (set) => ({ - count: 0, - name: '', - increment: () => set((state) => ({ count: state.count + 1 })), - setName: (name: string) => set({ name }), - throwError: () => { - throw new Error('Test error'); - }, - asyncAction: async () => { - return 'success'; - }, - }), - { storeName: 'TestStore', enableAnalytics: true }, - ), - ); - - const store = useTestStore.getState(); - store.increment(); - - expect(analyticsEvents.length).toBeGreaterThan(0); - expect(analyticsEvents.some((e) => e.type === 'state_change')).toBe(true); - expect(analyticsEvents.some((e) => e.type === 'action_called')).toBe(true); - }); - - it('should handle errors', () => { - const useTestStore = create()( - stateMiddleware( - (set) => ({ - count: 0, - name: '', - increment: () => set((state) => ({ count: state.count + 1 })), - setName: (name: string) => set({ name }), - throwError: () => { - throw new Error('Test error'); - }, - asyncAction: async () => { - return 'success'; - }, - }), - { storeName: 'TestStore', enableErrorHandling: true }, - ), - ); - - const store = useTestStore.getState(); - - expect(() => store.throwError()).toThrow('Test error'); - expect(errorEvents.length).toBeGreaterThan(0); - expect(errorEvents[0].error.message).toBe('Test error'); - }); - - it('should track async action performance', async () => { - // Clear previous events - analyticsEvents = []; - - const useTestStore = create()( - stateMiddleware( - (set) => ({ - count: 0, - name: '', - increment: () => set((state) => ({ count: state.count + 1 })), - setName: (name: string) => set({ name }), - throwError: () => { - throw new Error('Test error'); - }, - asyncAction: async () => { - await new Promise((resolve) => setTimeout(resolve, 50)); - return 'success'; - }, - }), - { storeName: 'TestStore', enableAnalytics: true }, - ), - ); - - const store = useTestStore.getState(); - await store.asyncAction(); - - // Wait a bit for async analytics to be tracked - await new Promise((resolve) => setTimeout(resolve, 10)); - - const performanceEvents = analyticsEvents.filter( - (e) => e.type === 'performance_metric' && e.action === 'asyncAction', - ); - expect(performanceEvents.length).toBeGreaterThan(0); - }); - - it('should sanitize sensitive data', () => { - const consoleSpy = vi.spyOn(console, 'debug').mockImplementation(() => {}); - - const useTestStore = create<{ - password: string; - token: string; - name: string; - updateName: (name: string) => void; - }>()( - stateMiddleware( - (set) => ({ - password: 'secret123', - token: 'abc123', - name: 'John', - updateName: (name: string) => set({ name }), - }), - { - storeName: 'TestStore', - enableLogging: true, - sanitizeState: (state) => { - if (typeof state === 'object' && state !== null) { - const sanitized = { ...(state as Record) }; - sanitized.password = '[REDACTED]'; - sanitized.token = '[REDACTED]'; - return sanitized; - } - return state; - }, - }, - ), - ); - - const store = useTestStore.getState(); - store.updateName('Jane'); // Trigger state change - - // Verify that logging was called (the middleware logs state changes) - // The logger uses [DEBUG] prefix, so we check for that - expect(consoleSpy).toHaveBeenCalled(); - consoleSpy.mockRestore(); - }); - - it('should respect shouldLog filter', () => { - const consoleSpy = vi.spyOn(console, 'debug').mockImplementation(() => {}); - - const useTestStore = create<{ count: number; increment: () => void }>()( - stateMiddleware( - (set) => ({ - count: 0, - increment: () => set((state) => ({ count: state.count + 1 })), - }), - { - storeName: 'TestStore', - enableLogging: true, - shouldLog: () => false, // Don't log state changes - }, - ), - ); - - const store = useTestStore.getState(); - store.increment(); - - // Action calls are still logged, but state changes are filtered - // So we expect at least one call (action call), but state change logging is filtered - const calls = consoleSpy.mock.calls; - const stateChangeCalls = calls.filter((call) => - call[0]?.toString().includes('State change'), - ); - expect(stateChangeCalls.length).toBe(0); - consoleSpy.mockRestore(); - }); - - it('should respect shouldTrack filter', () => { - const useTestStore = create<{ count: number; increment: () => void }>()( - stateMiddleware( - (set) => ({ - count: 0, - increment: () => set((state) => ({ count: state.count + 1 })), - }), - { - storeName: 'TestStore', - enableAnalytics: true, - shouldTrack: (action) => action !== 'increment', // Don't track increment - }, - ), - ); - - const store = useTestStore.getState(); - store.increment(); - - // Should not track increment action - const actionEvents = analyticsEvents.filter( - (e) => e.type === 'action_called' && e.action === 'increment', - ); - expect(actionEvents.length).toBe(0); - }); -}); diff --git a/apps/web/src/utils/stateMiddleware.ts b/apps/web/src/utils/stateMiddleware.ts deleted file mode 100644 index 111d42d20..000000000 --- a/apps/web/src/utils/stateMiddleware.ts +++ /dev/null @@ -1,431 +0,0 @@ -/** - * State Middleware - * FE-STATE-010: Add middleware for logging, analytics, error handling - * - * Provides a comprehensive middleware for Zustand stores that handles: - * - State change logging - * - Analytics tracking - * - Error handling and reporting - */ - -import { StateCreator } from 'zustand'; -import { logger } from './logger'; - -/** - * Analytics event types - */ -export type AnalyticsEventType = - | 'state_change' - | 'action_called' - | 'error_occurred' - | 'performance_metric'; - -/** - * Analytics event - */ -export interface AnalyticsEvent { - type: AnalyticsEventType; - store: string; - action?: string; - timestamp: number; - metadata?: Record; -} - -/** - * Analytics handler function - */ -export type AnalyticsHandler = (event: AnalyticsEvent) => void; - -/** - * Error handler function - */ -export type ErrorHandler = ( - error: Error, - context: { - store: string; - action?: string; - state?: unknown; - }, -) => void; - -/** - * Options for state middleware - */ -export interface StateMiddlewareOptions { - /** Store name for logging and analytics */ - storeName: string; - /** Enable logging (default: true in dev, false in prod) */ - enableLogging?: boolean; - /** Enable analytics tracking (default: true) */ - enableAnalytics?: boolean; - /** Enable error handling (default: true) */ - enableErrorHandling?: boolean; - /** Custom analytics handler */ - analyticsHandler?: AnalyticsHandler; - /** Custom error handler */ - errorHandler?: ErrorHandler; - /** Filter state changes to log (return false to skip) */ - shouldLog?: (state: unknown, prevState: unknown) => boolean; - /** Filter actions to track (return false to skip) */ - shouldTrack?: (action: string, args: unknown[]) => boolean; - /** Sanitize state for logging (remove sensitive data) */ - sanitizeState?: (state: unknown) => unknown; -} - -/** - * Default analytics handler (can be overridden) - */ -let defaultAnalyticsHandler: AnalyticsHandler | null = null; - -/** - * Set default analytics handler - */ -export function setAnalyticsHandler(handler: AnalyticsHandler): void { - defaultAnalyticsHandler = handler; -} - -/** - * Default error handler (can be overridden) - */ -let defaultErrorHandler: ErrorHandler | null = null; - -/** - * Set default error handler - */ -export function setErrorHandler(handler: ErrorHandler): void { - defaultErrorHandler = handler; -} - -/** - * Track analytics event - */ -function trackAnalytics( - event: Omit, - handler?: AnalyticsHandler, -): void { - const analyticsHandler = handler || defaultAnalyticsHandler; - if (!analyticsHandler) { - return; - } - - try { - analyticsHandler({ - ...event, - timestamp: Date.now(), - }); - } catch (error) { - logger.error('[StateMiddleware] Analytics tracking failed', { - error: String(error), - }); - } -} - -/** - * Handle error - */ -function handleError( - error: Error, - context: { - store: string; - action?: string; - state?: unknown; - }, - handler?: ErrorHandler, -): void { - const errorHandler = handler || defaultErrorHandler; - - // Always log errors - logger.error(`[StateMiddleware] Error in ${context.store}:`, { - error: error.message, - stack: error.stack, - action: context.action, - context, - }); - - // Call custom error handler if provided - if (errorHandler) { - try { - errorHandler(error, context); - } catch (handlerError) { - logger.error('[StateMiddleware] Error handler failed', { - error: String(handlerError), - }); - } - } - - // Track error analytics (use default handler, not error handler) - trackAnalytics({ - type: 'error_occurred', - store: context.store, - action: context.action, - metadata: { - error: error.message, - errorName: error.name, - }, - }); -} - -/** - * Sanitize state for logging (remove sensitive data) - */ -function sanitizeState( - state: unknown, - customSanitize?: (state: unknown) => unknown, -): unknown { - if (customSanitize) { - return customSanitize(state); - } - - // Default sanitization: remove common sensitive fields - if (typeof state === 'object' && state !== null) { - const sanitized = { ...(state as Record) }; - const sensitiveKeys = [ - 'password', - 'token', - 'secret', - 'apiKey', - 'accessToken', - 'refreshToken', - ]; - - for (const key of sensitiveKeys) { - if (key in sanitized) { - sanitized[key] = '[REDACTED]'; - } - } - - return sanitized; - } - - return state; -} - -/** - * Zustand middleware for logging, analytics, and error handling - * - * @example - * ```typescript - * export const useMyStore = create()( - * stateMiddleware( - * (set, get) => ({ - * // store implementation - * }), - * { storeName: 'MyStore' } - * ) - * ); - * ``` - */ -export function stateMiddleware( - config: StateCreator, - options: StateMiddlewareOptions, -): StateCreator { - const { - storeName, - enableLogging = import.meta.env.DEV, - enableAnalytics = true, - enableErrorHandling = true, - analyticsHandler, - errorHandler, - shouldLog = () => true, - shouldTrack = () => true, - sanitizeState: customSanitize, - } = options; - - return (set, get, api) => { - let previousState: T | null = null; - let actionCallCount = 0; - - // Wrap set function to intercept state changes - const wrappedSet: typeof set = (...args) => { - const currentState = get(); - - // Log state changes - if (enableLogging && shouldLog(currentState, previousState)) { - const sanitizedCurrent = sanitizeState(currentState, customSanitize); - const sanitizedPrevious = sanitizeState(previousState, customSanitize); - - logger.debug(`[StateMiddleware:${storeName}] State change:`, { - previous: sanitizedPrevious, - current: sanitizedCurrent, - }); - } - - // Track analytics - if (enableAnalytics) { - trackAnalytics( - { - type: 'state_change', - store: storeName, - metadata: { - hasPreviousState: previousState !== null, - }, - }, - analyticsHandler, - ); - } - - // Call original set - try { - set(...args); - previousState = get(); - } catch (error) { - if (enableErrorHandling) { - handleError( - error instanceof Error ? error : new Error(String(error)), - { - store: storeName, - state: currentState, - }, - errorHandler, - ); - } - throw error; - } - }; - - // Create store with wrapped set - const store = config(wrappedSet, get, api); - - // Wrap store actions to track calls - if (enableAnalytics || enableLogging) { - const wrappedStore = { ...store } as Record; - - // Wrap all functions in the store - for (const [key, value] of Object.entries(wrappedStore)) { - if (typeof value === 'function' && key !== 'setState') { - const originalAction = value as (...args: unknown[]) => unknown; - - wrappedStore[key] = ((...args: unknown[]) => { - actionCallCount++; - const startTime = performance.now(); - - // Track action call - if (enableAnalytics && shouldTrack(key, args)) { - trackAnalytics( - { - type: 'action_called', - store: storeName, - action: key, - metadata: { - callCount: actionCallCount, - argsCount: args.length, - }, - }, - analyticsHandler, - ); - } - - // Log action call - if (enableLogging) { - logger.debug( - `[StateMiddleware:${storeName}] Action called: ${key}`, - { - args: args.length > 0 ? args : undefined, - }, - ); - } - - try { - const result = originalAction(...args); - - // Track performance if it's a promise - if (result instanceof Promise) { - const endTime = performance.now(); - const duration = endTime - startTime; - - if (enableAnalytics) { - trackAnalytics( - { - type: 'performance_metric', - store: storeName, - action: key, - metadata: { - duration, - isAsync: true, - }, - }, - analyticsHandler, - ); - } - - // Handle promise errors - if (enableErrorHandling) { - result.catch((error) => { - handleError( - error instanceof Error ? error : new Error(String(error)), - { - store: storeName, - action: key, - state: get(), - }, - errorHandler, - ); - }); - } - } else { - // Track synchronous action performance - const endTime = performance.now(); - const duration = endTime - startTime; - - if (enableAnalytics && duration > 10) { - // Only track if it takes more than 10ms - trackAnalytics( - { - type: 'performance_metric', - store: storeName, - action: key, - metadata: { - duration, - isAsync: false, - }, - }, - analyticsHandler, - ); - } - } - - return result; - } catch (error) { - if (enableErrorHandling) { - handleError( - error instanceof Error ? error : new Error(String(error)), - { - store: storeName, - action: key, - state: get(), - }, - errorHandler, - ); - } - throw error; - } - }) as typeof originalAction; - } - } - - return wrappedStore as T; - } - - return store; - }; -} - -/** - * Helper to create a store with state middleware - * - * @example - * ```typescript - * export const useMyStore = createWithMiddleware( - * (set, get) => ({ - * // store implementation - * }), - * { storeName: 'MyStore' } - * ); - * ``` - */ -export function createWithMiddleware( - config: StateCreator, - options: StateMiddlewareOptions, -): StateCreator { - return stateMiddleware(config, options); -}