From a36c1b61bb6d2d691132537a5137a71d207ccfb9 Mon Sep 17 00:00:00 2001 From: Felipe Cardoso Date: Mon, 3 Nov 2025 11:40:46 +0100 Subject: [PATCH] Document Phase 1 lessons learned for AuthContext migration and update hooks for compliance with React Rules of Hooks - Added detailed documentation in `AUTH_CONTEXT_MIGRATION_PLAN.md` for lessons learned during Phase 1 of the `AuthContext` migration. - Highlighted critical implementation insights, including refactoring `useAuth` to call Zustand hooks internally, strict type safety with the `AuthState` interface, and dependency injection via `AuthProvider`. - Defined updated architecture for provider placement and emphasized the importance of documentation, barrel exports, and hook compliance with React rules. - Included comprehensive examples, verification checklists, common mistakes to avoid, and future-proofing guidelines. --- AUTH_CONTEXT_MIGRATION_PLAN.md | 781 ++++++++++++++++++++++++++++----- 1 file changed, 675 insertions(+), 106 deletions(-) diff --git a/AUTH_CONTEXT_MIGRATION_PLAN.md b/AUTH_CONTEXT_MIGRATION_PLAN.md index fd80693..15ad075 100644 --- a/AUTH_CONTEXT_MIGRATION_PLAN.md +++ b/AUTH_CONTEXT_MIGRATION_PLAN.md @@ -47,6 +47,164 @@ --- +## Phase 1 Lessons Learned (CRITICAL - READ FIRST) + +**Phase 1 Status**: ✅ **COMPLETED** - All issues resolved + +### Key Implementation Insights + +#### 1. useAuth Hook Must Call Zustand Hook Internally + +**❌ WRONG (Original Plan)**: +```typescript +export function useAuth() { + const context = useContext(AuthContext); + return context; // Returns the hook function - VIOLATES React Rules of Hooks! +} +``` + +**✅ CORRECT (Implemented)**: +```typescript +export function useAuth(): AuthState; +export function useAuth(selector: (state: AuthState) => T): T; +export function useAuth(selector?: (state: AuthState) => T): AuthState | T { + const storeHook = useContext(AuthContext); + if (!storeHook) { + throw new Error("useAuth must be used within AuthProvider"); + } + // CRITICAL: Call the hook internally + return selector ? storeHook(selector) : storeHook(); +} +``` + +**Why This Matters**: +- ✅ Enables `const { user } = useAuth()` pattern (simple, idiomatic) +- ✅ Also supports `const user = useAuth(s => s.user)` pattern (optimized) +- ✅ Follows React Rules of Hooks (hook called at component top level) +- ❌ Without this, components would need `const { user } = useAuth()()` (wrong!) + +#### 2. Provider Placement Architecture + +**Correct Structure**: +``` +layout.tsx: + ← Provides DI layer + ← Loads auth from storage (needs AuthProvider) + ← Other providers (Theme, Query) + {children} + + +``` + +**Why This Order**: +- AuthProvider must wrap AuthInitializer (AuthInitializer uses auth state) +- AuthProvider should wrap Providers (auth available everywhere) +- Keep provider tree shallow (performance) + +#### 3. Type Safety with Explicit AuthState Interface + +**✅ DO**: Define explicit AuthState interface matching Zustand store: +```typescript +interface AuthState { + user: User | null; + accessToken: string | null; + // ... all properties explicitly typed +} +``` + +**Benefits**: +- IDE autocomplete works perfectly +- Type errors caught at compile time +- Self-documenting code +- Easier to maintain + +#### 4. Comprehensive JSDoc Documentation + +**Pattern to Follow**: +```typescript +/** + * [Component/Function Name] - [One-line purpose] + * + * [Detailed description including:] + * - What it does + * - When to use it + * - How it works + * + * @param {Type} paramName - Parameter description + * @throws {ErrorType} When error occurs + * @returns {Type} Return value description + * + * @example + * ```tsx + * // Concrete usage example + * const { user } = useAuth(); + * ``` + */ +``` + +**Include Examples For**: +- Different usage patterns (with/without selectors) +- Common mistakes to avoid +- Testing scenarios + +#### 5. Barrel Exports for Clean Imports + +**DO**: Add to barrel files (`src/lib/stores/index.ts`): +```typescript +export { useAuth, AuthProvider } from '../auth/AuthContext'; +``` + +**Benefits**: +- Consistent import paths: `import { useAuth } from '@/lib/stores'` +- Easy to refactor internal structure +- Clear public API + +### Critical Mistakes to Avoid + +1. **❌ Don't return hook function from useAuth** + - Returns function, not state + - Violates React Rules of Hooks + - Breaks in Phase 2+ + +2. **❌ Don't nest AuthProvider inside Providers** + - AuthInitializer won't have access to auth + - Wrong dependency order + +3. **❌ Don't forget barrel exports** + - Inconsistent import paths + - Harder to maintain + +4. **❌ Don't skip documentation** + - Future developers won't understand usage + - Leads to incorrect implementations + +5. **❌ Don't use implicit types** + - Harder to debug type issues + - Worse IDE support + +### Verification Checklist (Always Run) + +After any changes: +- [ ] `npm run type-check` - Must pass with 0 errors +- [ ] `npm run dev` - App must start without errors +- [ ] Browser console - Must be clean (no errors/warnings) +- [ ] Test actual usage - Navigate to protected routes +- [ ] Check React DevTools - Verify provider tree structure + +### Performance Considerations + +**Polymorphic useAuth Hook**: +- ✅ No performance overhead (hook called once per component) +- ✅ Selector pattern available for optimization +- ✅ Same performance as direct Zustand usage + +**Context Provider**: +- ✅ Stable value (doesn't change unless store changes) +- ✅ No extra re-renders +- ✅ Negligible memory overhead + +--- + ## Context & Problem Analysis ### Current Architecture @@ -189,71 +347,206 @@ await page.addInitScript((mockStore) => { #### Task 1.1: Create AuthContext Module **File**: `src/lib/auth/AuthContext.tsx` (NEW) +**CRITICAL**: The `useAuth` hook must call the Zustand hook internally to follow React's Rules of Hooks. Do NOT return the hook function itself. + ```typescript -'use client'; +/** + * Authentication Context - Dependency Injection Wrapper for Auth Store + * + * Provides a thin Context layer over Zustand auth store to enable: + * - Test isolation (inject mock stores) + * - E2E testing without backend + * - Clean architecture (DI pattern) + * + * Design: Context handles dependency injection, Zustand handles state management + */ -import { createContext, useContext, ReactNode } from 'react'; -import { useAuthStore as useAuthStoreImpl } from '@/lib/stores/authStore'; +"use client"; -type AuthContextType = ReturnType; +import { createContext, useContext } from "react"; +import type { ReactNode } from "react"; +import { useAuthStore as useAuthStoreImpl } from "@/lib/stores/authStore"; +import type { User } from "@/lib/stores/authStore"; -const AuthContext = createContext(null); +/** + * Authentication state shape + * Matches the Zustand store interface exactly + */ +interface AuthState { + // State + user: User | null; + accessToken: string | null; + refreshToken: string | null; + isAuthenticated: boolean; + isLoading: boolean; + tokenExpiresAt: number | null; + + // Actions + setAuth: (user: User, accessToken: string, refreshToken: string, expiresIn?: number) => Promise; + setTokens: (accessToken: string, refreshToken: string, expiresIn?: number) => Promise; + setUser: (user: User) => void; + clearAuth: () => Promise; + loadAuthFromStorage: () => Promise; + isTokenExpired: () => boolean; +} + +/** + * Type of the Zustand hook function + * Used for Context storage and test injection + */ +type AuthStoreHook = typeof useAuthStoreImpl; + +/** + * Global window extension for E2E test injection + * E2E tests can set window.__TEST_AUTH_STORE__ before navigation + */ +declare global { + interface Window { + __TEST_AUTH_STORE__?: AuthStoreHook; + } +} + +const AuthContext = createContext(null); interface AuthProviderProps { children: ReactNode; - store?: AuthContextType; + /** + * Optional store override for testing + * Used in unit tests to inject mock store + */ + store?: AuthStoreHook; } +/** + * Authentication Context Provider + * + * Wraps Zustand auth store in React Context for dependency injection. + * Enables test isolation by allowing mock stores to be injected via: + * 1. `store` prop (unit tests) + * 2. `window.__TEST_AUTH_STORE__` (E2E tests) + * 3. Production singleton (default) + * + * @example + * ```tsx + * // In root layout + * + * + * + * + * // In unit tests + * + * + * + * + * // In E2E tests (before navigation) + * window.__TEST_AUTH_STORE__ = mockAuthStoreHook; + * ``` + */ export function AuthProvider({ children, store }: AuthProviderProps) { + // Check for E2E test store injection (SSR-safe) + const testStore = + typeof window !== "undefined" && window.__TEST_AUTH_STORE__ + ? window.__TEST_AUTH_STORE__ + : null; + // Priority: explicit prop > E2E test store > production singleton - const testStore = typeof window !== 'undefined' - ? (window as any).__TEST_AUTH_STORE__ - : null; + const authStore = store ?? testStore ?? useAuthStoreImpl; - const authStore = store ?? testStore ?? useAuthStoreImpl(); - - return ( - - {children} - - ); + return {children}; } -export function useAuth() { - const context = useContext(AuthContext); - if (!context) { - throw new Error('useAuth must be used within AuthProvider'); +/** + * Hook to access authentication state and actions + * + * Supports both full state access and selector patterns for performance optimization. + * Must be used within AuthProvider. + * + * @throws {Error} If used outside of AuthProvider + * + * @example + * ```tsx + * // Full state access (simpler, re-renders on any state change) + * function MyComponent() { + * const { user, isAuthenticated } = useAuth(); + * return
{user?.first_name}
; + * } + * + * // Selector pattern (optimized, re-renders only when selected value changes) + * function UserName() { + * const user = useAuth(state => state.user); + * return {user?.first_name}; + * } + * + * // In mutation callbacks (outside React render) + * const handleLogin = async (data) => { + * const response = await loginAPI(data); + * // Use getState() directly for mutations (see useAuth.ts hooks) + * const setAuth = useAuthStore.getState().setAuth; + * await setAuth(response.user, response.token); + * }; + * ``` + */ +export function useAuth(): AuthState; +export function useAuth(selector: (state: AuthState) => T): T; +export function useAuth(selector?: (state: AuthState) => T): AuthState | T { + const storeHook = useContext(AuthContext); + + if (!storeHook) { + throw new Error("useAuth must be used within AuthProvider"); } - return context; + + // CRITICAL: Call the Zustand hook internally (follows React Rules of Hooks) + // This is the key difference from returning the hook function itself + return selector ? storeHook(selector) : storeHook(); } ``` +**Key Implementation Details**: +1. **Polymorphic Hook**: Supports both `useAuth()` and `useAuth(selector)` patterns +2. **Calls Hook Internally**: `storeHook()` is called inside `useAuth`, not by consumers +3. **Type Safety**: `AuthState` interface matches Zustand store exactly +4. **Window Global**: Type-safe extension for E2E test injection + **Verification**: - Run: `npm run type-check` -- Verify: `AuthContextType` correctly infers Zustand store type +- Verify: `AuthState` interface matches all Zustand store properties - Check: No circular import warnings +- Verify: Polymorphic overloads work correctly **Success Criteria**: -- [ ] File created -- [ ] TypeScript compiles without errors -- [ ] Type inference works correctly +- [x] File created with correct implementation +- [x] TypeScript compiles without errors +- [x] Type inference works correctly +- [x] Hook calls Zustand hook internally (not returns it) --- #### Task 1.2: Wrap Application Root **File**: `src/app/layout.tsx` (MODIFY) -**Change**: -```typescript -import { AuthProvider } from '@/lib/auth/AuthContext'; +**Also**: `src/app/providers.tsx` (MODIFY) - Remove AuthInitializer from here +**Also**: `src/lib/stores/index.ts` (MODIFY) - Add barrel exports +**Step 1: Add imports to layout.tsx**: +```typescript +// At the top of layout.tsx, add these imports: +import { AuthProvider } from "@/lib/auth/AuthContext"; +import { AuthInitializer } from "@/components/auth"; +``` + +**Step 2: Wrap body content with AuthProvider**: +```typescript export default function RootLayout({ children }: { children: ReactNode }) { return ( - - + + + {/* Theme initialization script stays here */} +