Refactor debug filter from useEffect to Redux middleware

src/client/App.tsx

@@ -3,7 +3,6 @@ import './App.scss';
 import { Typography } from '@mui/material';
 import { lazy, Suspense } from 'react';
 import { Route, Routes } from 'react-router-dom';
-import DebugManager from './features/Debug/DebugManager';
 import Page from './features/Page/Page';
 import SyncManager from './features/Sync/SyncManager';
 import UpdateManager from './features/Update/UpdateManager';
@@ -24,7 +23,6 @@ function App() {
 
   return (
     <Themed>
-      <DebugManager />
       {process.env.NODE_ENV !== 'development' && <UpdateManager />}
       <UserProvider>
         <SyncManager />

src/client/features/Debug/debugSlice.ts

@@ -1,14 +1,48 @@
+import type { Middleware } from '@reduxjs/toolkit';
 import { createSlice } from '@reduxjs/toolkit';
+import debugModule from 'debug';
+
+const DEBUG_KEY = 'debug-filter';
+
+// Initialize from localStorage at module load time
+function getInitialDebugValue(): string | null {
+  const stored = localStorage.getItem(DEBUG_KEY);
+  if (stored) {
+    debugModule.enable(stored);
+  } else {
+    debugModule.disable();
+  }
+  return stored;
+}
 
 export const debugSlice = createSlice({
   name: 'debug',
-  initialState: { value: undefined as unknown as string },
+  initialState: { value: getInitialDebugValue() },
   reducers: {
     set: (state, action) => {
       state.value = action.payload;
     },
   },
 });
 
+// Middleware to sync debug state changes to localStorage and debugModule
+export const debugMiddleware: Middleware = () => (next) => (action) => {
+  const result = next(action);
+
+  if (debugSlice.actions.set.match(action)) {
+    const newValue = action.payload;
+    if (!newValue) {
+      // null or empty string - clear debug
+      debugModule.disable();
+      localStorage.removeItem(DEBUG_KEY);
+    } else {
+      debugModule.enable(newValue);
+      localStorage.setItem(DEBUG_KEY, newValue);
+    }
+  }
+
+  return result;
+};
+
 export const { set } = debugSlice.actions;
 export default debugSlice.reducer;

src/client/store.ts

@@ -1,7 +1,7 @@
 import { configureStore } from '@reduxjs/toolkit';
 import * as RR from 'react-redux';
 
-import debugSlice from './features/Debug/debugSlice';
+import debugSlice, { debugMiddleware } from './features/Debug/debugSlice';
 import pageSlice from './features/Page/pageSlice';
 import syncSlice from './features/Sync/syncSlice';
 import updateSlice from './features/Update/updateSlice';
@@ -18,6 +18,7 @@ function createStore() {
       debug: debugSlice,
       update: updateSlice,
     },
+    middleware: (getDefaultMiddleware) => getDefaultMiddleware().concat(debugMiddleware),
     // devTools: process.env.NODE_ENV !== 'production',
   });
 }

useEffectGone.md

@@ -0,0 +1,202 @@
+# useEffect Elimination Plan
+
+Based on React's guidance from [You Might Not Need an Effect](https://react.dev/learn/you-might-not-need-an-effect), this document identifies useEffect instances that are incorrectly used and should be refactored.
+
+## Quick Reference: When to Avoid useEffect
+
+1. **Transforming data for rendering** → Calculate during render or use `useMemo`
+2. **Handling user events** → Use event handlers
+3. **Resetting state on prop change** → Use `key` prop
+4. **Subscribing to external stores** → Use `useSyncExternalStore`
+5. **Initializing the app** → Use module-level code
+
+---
+
+## LOW RISK - Safe to Refactor
+
+These useEffects can be removed with minimal impact and clear alternatives.
+
+### 1. Page Context Effects (5 instances)
+
+**Pattern**: Dispatching `setContext()` on mount to set page title/navigation state.
+
+| File | Lines | Current Code |
+|------|-------|--------------|
+| [About.tsx](src/client/pages/About.tsx#L68-L76) | 68-76 | `useEffect(() => dispatch(setContext({title: 'About', ...})), [dispatch])` |
+| [History.tsx](src/client/pages/History.tsx#L20-L27) | 20-27 | `useEffect(() => dispatch(setContext({title: 'History', ...})), [dispatch])` |
+| [Home.tsx](src/client/pages/Home.tsx#L28-L35) | 28-35 | `useEffect(() => dispatch(setContext({title: 'Repeatable Checklists', ...})), [dispatch])` |
+| [Template.tsx](src/client/pages/Template.tsx#L62-L70) | 62-70 | `useEffect(() => dispatch(setContext({title: template?.title, ...})))` - **No deps array!** |
+| [Repeatable.tsx](src/client/pages/Repeatable.tsx#L37-L100) | 37-100 | Nested inside larger effect (context set at line ~80) |
+
+**Why Remove**: These are setting state during render based on props/route. This is a classic anti-pattern - dispatching to set derived state that could be computed.
+
+**Recommended Approach**:
+1. Create a custom hook `usePageContext` that calls `dispatch(setContext(...))` during render
+2. Or use a layout component pattern where context is set declaratively
+3. For Template.tsx specifically: the missing dependency array means it runs on EVERY render - this is a bug
+
+**Example Refactor**:
+```typescript
+// Option 1: Custom hook that sets context during render
+function usePageContext(context: PageContext) {
+  const dispatch = useAppDispatch();
+  const contextRef = useRef(context);
+  if (!shallowEqual(contextRef.current, context)) {
+    contextRef.current = context;
+  }
+  useMemo(() => {
+    dispatch(setContext(contextRef.current));
+  }, [dispatch, contextRef.current]);
+}
+
+// Usage
+function About() {
+  usePageContext({ title: 'About', back: true, under: 'about' });
+  // ...
+}
+```
+
+**Risk Level**: LOW
+- No external systems involved
+- No async operations
+- Easy to test
+- Clear migration path
+
+---
+
+### 2. Debug Filter Management
+
+**File**: [DebugManager.tsx](src/client/features/Debug/DebugManager.tsx#L12-L24) (lines 12-24)
+
+**Current Code**:
+```typescript
+useEffect(() => {
+  if (debugFilter === undefined) {
+    dispatch(setDebug(localStorage.getItem(DEBUG_KEY)));
+  } else if (debugFilter === null) {
+    debugModule.disable();
+    localStorage.removeItem(DEBUG_KEY);
+  } else {
+    debugModule.enable(debugFilter);
+    localStorage.setItem(DEBUG_KEY, debugFilter);
+  }
+}, [debugFilter, dispatch]);
+```
+
+**Why Remove**: This is initializing state from localStorage and then syncing state changes back. The initialization should happen once at app startup, and the sync should be handled by Redux middleware - not a render effect.
+
+**Recommended Approach**:
+1. Initialize debug filter in Redux initial state (read from localStorage at store creation)
+2. Use Redux middleware to sync localStorage on state changes
+3. Call `debugModule.enable/disable` from the middleware
+
+**Risk Level**: LOW
+- Debug-only feature
+- No user-facing impact if it breaks
+- Simple state management pattern
+
+---
+
+## MEDIUM RISK - Refactor with Caution
+
+These require more careful consideration and testing.
+
+### 3. Update Manager - Install Update
+
+**File**: [UpdateManager.tsx](src/client/features/Update/UpdateManager.tsx#L27-L33) (lines 27-33)
+
+**Current Code**:
+```typescript
+useEffect(() => {
+  if (registration && waitingToInstall && userReadyToUpdate) {
+    registration.waiting?.postMessage({ type: 'SKIP_WAITING' });
+  }
+}, [registration, userReadyToUpdate, waitingToInstall]);
+```
+
+**Why Remove**: This is reacting to state changes to trigger an action. The `userReadyToUpdate` flag is set by a user clicking a button - the `postMessage` call should happen directly in that click handler, not as a cascading effect.
+
+**Recommended Approach**:
+- Pass `registration` to the component with the update button
+- Call `registration.waiting?.postMessage({ type: 'SKIP_WAITING' })` directly in the click handler
+- Remove `userReadyToUpdate` state entirely
+
+**Risk Level**: MEDIUM
+- Critical functionality (PWA updates)
+- But relatively simple logic
+- Straightforward event-driven refactor
+
+---
+
+### 4. Stale Queue Processing
+
+**File**: [SyncManager.tsx](src/client/features/Sync/SyncManager.tsx#L211-L228) (lines 211-228)
+
+**Current Code**:
+```typescript
+useEffect(() => {
+  const docs = Object.values(stale);
+  if (docs.length && socket && socket.connected && state === State.connected) {
+    socket.emit('docUpdate', docs);
+  }
+  dispatch(cleanStale(docs));
+}, [dispatch, socket, stale, state]);
+```
+
+**Why Remove**: This processes a queue whenever `stale` changes. The stale queue is populated by `userPut` operations - the socket emit should happen as part of that action flow, not as a reactive effect.
+
+**Recommended Approach**:
+- Move to Redux middleware that listens for actions that add to stale queue
+- Emit socket events directly from the middleware when docs are added
+- Or use a Redux thunk that handles both the local update and socket emit
+
+**Risk Level**: MEDIUM
+- Core sync functionality
+- Logic is straightforward
+- Need to ensure socket availability in middleware context
+
+---
+
+### 5. Socket Ready Signal
+
+**File**: [SyncManager.tsx](src/client/features/Sync/SyncManager.tsx#L230-L237) (lines 230-237)
+
+**Current Code**:
+```typescript
+useEffect(() => {
+  if (socket?.connected && state === State.completed) {
+    socket.emit('ready');
+    dispatch(socketConnected());
+  }
+}, [dispatch, socket, state]);
+```
+
+**Why Remove**: This is reacting to sync completion to emit a ready signal. The sync completion happens in another useEffect - the ready signal should be emitted directly at the end of that sync logic, not as a cascading effect.
+
+**Recommended Approach**:
+- At the end of the full sync handler (after `dispatch(completeSync())`), emit the ready signal and dispatch `socketConnected()` directly
+- This makes the flow explicit rather than reactive
+
+**Risk Level**: MEDIUM
+- Part of sync flow
+- Simple conditional logic
+- Timing might be tricky - need to ensure socket is available
+
+---
+
+## Summary
+
+| Risk | Count | Description |
+|------|-------|-------------|
+| LOW | 6 | 5 page context effects + debug filter management |
+| MEDIUM | 3 | Update install trigger, stale queue processing, socket ready signal |
+
+**Total**: 9 useEffects to refactor
+
+## Recommended Priority
+
+1. **First**: Fix the Template.tsx missing dependency array (bug!)
+2. **Second**: Refactor page context effects (LOW risk, 5 instances, same pattern)
+3. **Third**: Move debug filter to Redux middleware (LOW risk)
+4. **Fourth**: Refactor UpdateManager install trigger to event handler (MEDIUM risk)
+5. **Fifth**: Refactor SyncManager stale queue and ready signal (MEDIUM risk, related changes)