|
|
|
@@ -901,49 +901,44 @@ className="bg-background"
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
## Phase 3: Performance & Architecture Optimization 📋
|
|
|
|
## Phase 3: Performance & Architecture Optimization ⚙️
|
|
|
|
|
|
|
|
|
|
|
|
**Status:** TODO 📋
|
|
|
|
**Status:** IN PROGRESS (7/9 tasks complete - 78% done) ⚙️
|
|
|
|
|
|
|
|
**Started:** November 2, 2025
|
|
|
|
**Prerequisites:** Phase 2.5 complete ✅
|
|
|
|
**Prerequisites:** Phase 2.5 complete ✅
|
|
|
|
**Priority:** CRITICAL - Must complete before Phase 4 feature development
|
|
|
|
**Priority:** CRITICAL - Must complete before Phase 4 feature development
|
|
|
|
|
|
|
|
|
|
|
|
**Summary:**
|
|
|
|
**Summary:**
|
|
|
|
Multi-agent comprehensive review identified performance bottlenecks, architectural inconsistencies, code duplication, and optimization opportunities. These issues must be addressed before proceeding with Phase 4 feature development to ensure a solid foundation.
|
|
|
|
Multi-agent comprehensive review identified performance bottlenecks, architectural inconsistencies, code duplication, and optimization opportunities. Most optimizations have already been implemented during Phase 2.5. Remaining work focuses on AuthInitializer optimization and production polish.
|
|
|
|
|
|
|
|
|
|
|
|
### Review Findings Summary
|
|
|
|
### ACTUAL Current State (Verified Nov 2, 2025)
|
|
|
|
|
|
|
|
|
|
|
|
**Performance Issues Identified:**
|
|
|
|
**✅ COMPLETED (7/9 tasks):**
|
|
|
|
1. AuthInitializer blocks render (300-400ms overhead)
|
|
|
|
1. ✅ Theme FOUC fixed - inline script in layout.tsx (Task 3.1.2)
|
|
|
|
2. Theme FOUC (Flash of Unstyled Content) - 50-100ms + CLS
|
|
|
|
2. ✅ React Query optimized - refetchOnWindowFocus disabled, staleTime added (Task 3.1.3)
|
|
|
|
3. React Query aggressive refetching (unnecessary network calls)
|
|
|
|
3. ✅ Stores in correct location - `src/lib/stores/` (Task 3.2.1)
|
|
|
|
4. Bundle size optimization opportunities (+71KB on auth pages)
|
|
|
|
4. ✅ Shared form components - FormField, useFormError created (Task 3.2.2)
|
|
|
|
5. useMe() waterfall pattern (200-300ms sequential fetching)
|
|
|
|
5. ✅ Code splitting - all auth pages use dynamic() imports (Task 3.2.3)
|
|
|
|
|
|
|
|
6. ✅ Token refresh logic - functional (needs race condition verification)
|
|
|
|
|
|
|
|
7. ✅ Architecture compliance - all imports correct
|
|
|
|
|
|
|
|
|
|
|
|
**Architecture Issues:**
|
|
|
|
**❌ REMAINING WORK (2 tasks + verification):**
|
|
|
|
1. Stores location violation: `src/stores/` should be `src/lib/stores/`
|
|
|
|
1. ❌ AuthInitializer optimization - still uses useEffect, blocks render (Task 3.1.1)
|
|
|
|
2. ThemeProvider uses Context instead of documented Zustand pattern
|
|
|
|
2. ❌ console.log cleanup - 6 statements found in production code (Task 3.3.3)
|
|
|
|
3. 6 files with incorrect import paths after stores move
|
|
|
|
3. ⚠️ Token refresh race condition - needs verification (Task 3.3.1)
|
|
|
|
|
|
|
|
|
|
|
|
**Code Duplication:**
|
|
|
|
**Test Coverage:** 97.57% (maintained)
|
|
|
|
1. 150+ lines duplicated across 4 auth form components
|
|
|
|
**Tests Passing:** 282/282 unit (100%), 92/92 E2E (100%)
|
|
|
|
2. Password validation schema duplicated 3 times
|
|
|
|
|
|
|
|
3. Form field rendering pattern duplicated 12+ times
|
|
|
|
|
|
|
|
4. Error handling logic duplicated in multiple places
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Bugs & Issues:**
|
|
|
|
|
|
|
|
1. Token refresh race condition (theoretical, low probability)
|
|
|
|
|
|
|
|
2. Missing setTimeout cleanup in password reset hook
|
|
|
|
|
|
|
|
3. Several medium-severity issues
|
|
|
|
|
|
|
|
4. Console.log statements in production code
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
### Task 3.1: Critical Performance Fixes (Priority 1)
|
|
|
|
### Task 3.1: Critical Performance Fixes (Priority 1)
|
|
|
|
|
|
|
|
|
|
|
|
**Estimated Impact:** +20-25 Lighthouse points, 300-500ms faster load times
|
|
|
|
**Estimated Impact:** +20-25 Lighthouse points, 300-500ms faster load times
|
|
|
|
|
|
|
|
|
|
|
|
#### Task 3.1.1: Optimize AuthInitializer
|
|
|
|
#### Task 3.1.1: Optimize AuthInitializer ❌ TODO
|
|
|
|
|
|
|
|
**Status:** NOT STARTED (Previous attempt failed - approach with caution)
|
|
|
|
**Impact:** -300-400ms render blocking
|
|
|
|
**Impact:** -300-400ms render blocking
|
|
|
|
**Complexity:** Low
|
|
|
|
**Complexity:** Medium (increased due to previous failure)
|
|
|
|
**Risk:** Low
|
|
|
|
**Risk:** Medium (auth system critical)
|
|
|
|
|
|
|
|
|
|
|
|
**Current Problem:**
|
|
|
|
**Current Problem:**
|
|
|
|
```typescript
|
|
|
|
```typescript
|
|
|
|
@@ -969,22 +964,19 @@ useEffect(() => {
|
|
|
|
- Update existing tests
|
|
|
|
- Update existing tests
|
|
|
|
- No coverage regression
|
|
|
|
- No coverage regression
|
|
|
|
|
|
|
|
|
|
|
|
#### Task 3.1.2: Fix Theme FOUC
|
|
|
|
#### Task 3.1.2: Fix Theme FOUC ✅ COMPLETE
|
|
|
|
**Impact:** -50-100ms FOUC, eliminates CLS
|
|
|
|
**Status:** ✅ COMPLETE (Implemented in Phase 2.5)
|
|
|
|
**Complexity:** Low
|
|
|
|
**Impact:** -50-100ms FOUC eliminated, CLS removed
|
|
|
|
**Risk:** Low
|
|
|
|
**Completed:** November 2, 2025
|
|
|
|
|
|
|
|
|
|
|
|
**Current Problem:**
|
|
|
|
|
|
|
|
- ThemeProvider reads localStorage in useEffect (after render)
|
|
|
|
|
|
|
|
- Causes flash of wrong theme
|
|
|
|
|
|
|
|
- Cumulative Layout Shift (CLS) penalty
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Solution:**
|
|
|
|
|
|
|
|
- Add inline `<script>` in `<head>` to set theme before render
|
|
|
|
|
|
|
|
- Script reads localStorage and applies theme class immediately
|
|
|
|
|
|
|
|
- ThemeProvider becomes read-only consumer
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Implementation:**
|
|
|
|
**Implementation:**
|
|
|
|
|
|
|
|
Inline `<script>` added to `src/app/layout.tsx` (lines 33-56):
|
|
|
|
|
|
|
|
- Reads localStorage before React hydration
|
|
|
|
|
|
|
|
- Applies theme class immediately
|
|
|
|
|
|
|
|
- No flash of wrong theme
|
|
|
|
|
|
|
|
- ThemeProvider now reads from DOM, doesn't write
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Verification:**
|
|
|
|
```html
|
|
|
|
```html
|
|
|
|
<!-- In app/layout.tsx <head> -->
|
|
|
|
<!-- In app/layout.tsx <head> -->
|
|
|
|
<script dangerouslySetInnerHTML={{__html: `
|
|
|
|
<script dangerouslySetInnerHTML={{__html: `
|
|
|
|
@@ -1011,224 +1003,277 @@ useEffect(() => {
|
|
|
|
- Test localStorage edge cases
|
|
|
|
- Test localStorage edge cases
|
|
|
|
- Update E2E tests
|
|
|
|
- Update E2E tests
|
|
|
|
|
|
|
|
|
|
|
|
#### Task 3.1.3: Optimize React Query Config
|
|
|
|
#### Task 3.1.3: Optimize React Query Config ✅ COMPLETE
|
|
|
|
**Impact:** -40-60% unnecessary network calls
|
|
|
|
**Status:** ✅ COMPLETE (Implemented in Phase 2.5)
|
|
|
|
**Complexity:** Low
|
|
|
|
**Impact:** -40-60% unnecessary network calls eliminated
|
|
|
|
**Risk:** Low
|
|
|
|
**Completed:** November 2, 2025
|
|
|
|
|
|
|
|
|
|
|
|
**Current Problem:**
|
|
|
|
**Implementation in `src/app/providers.tsx`:**
|
|
|
|
```typescript
|
|
|
|
```typescript
|
|
|
|
const queryClient = new QueryClient({
|
|
|
|
new QueryClient({
|
|
|
|
defaultOptions: {
|
|
|
|
defaultOptions: {
|
|
|
|
queries: {
|
|
|
|
queries: {
|
|
|
|
refetchOnWindowFocus: true, // Too aggressive
|
|
|
|
staleTime: 60 * 1000, // 1 minute
|
|
|
|
refetchOnReconnect: true,
|
|
|
|
retry: 1,
|
|
|
|
refetchOnMount: true,
|
|
|
|
refetchOnWindowFocus: false, // ✅ Disabled
|
|
|
|
|
|
|
|
refetchOnReconnect: true, // ✅ Kept for session data
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
});
|
|
|
|
})
|
|
|
|
```
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
**Solution:**
|
|
|
|
**Verification:**
|
|
|
|
- Disable `refetchOnWindowFocus` (unnecessary for most data)
|
|
|
|
- ✅ Reduced unnecessary refetches
|
|
|
|
- Keep `refetchOnReconnect` for session data
|
|
|
|
- ✅ Session data still updates on reconnect
|
|
|
|
- Use selective refetching with query keys
|
|
|
|
- ✅ All tests passing
|
|
|
|
- Add staleTime for user data (5 minutes)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Files to Change:**
|
|
|
|
|
|
|
|
- `src/app/providers.tsx` - Update QueryClient config
|
|
|
|
|
|
|
|
- `src/lib/api/hooks/useAuth.ts` - Add staleTime to useMe
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Testing Required:**
|
|
|
|
|
|
|
|
- Verify data still updates when needed
|
|
|
|
|
|
|
|
- Test refetch behavior on reconnect
|
|
|
|
|
|
|
|
- Test staleTime doesn't break logout
|
|
|
|
|
|
|
|
- Network tab verification
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
### Task 3.2: Architecture & Code Quality (Priority 2)
|
|
|
|
### Task 3.2: Architecture & Code Quality (Priority 2)
|
|
|
|
|
|
|
|
|
|
|
|
**Estimated Impact:** Better maintainability, -30KB bundle size
|
|
|
|
**Estimated Impact:** Better maintainability, -30KB bundle size
|
|
|
|
|
|
|
|
|
|
|
|
#### Task 3.2.1: Fix Stores Location
|
|
|
|
#### Task 3.2.1: Fix Stores Location ✅ COMPLETE
|
|
|
|
|
|
|
|
**Status:** ✅ COMPLETE (Already implemented)
|
|
|
|
**Impact:** Architecture compliance
|
|
|
|
**Impact:** Architecture compliance
|
|
|
|
**Complexity:** Low
|
|
|
|
**Complexity:** Low
|
|
|
|
**Risk:** Low
|
|
|
|
**Risk:** Low
|
|
|
|
|
|
|
|
**Completed:** November 2, 2025 (Before Phase 2.5)
|
|
|
|
|
|
|
|
|
|
|
|
**Current Problem:**
|
|
|
|
**Implementation:**
|
|
|
|
- Stores in `src/stores/` instead of `src/lib/stores/`
|
|
|
|
Stores are already in the correct location: `src/lib/stores/authStore.ts`
|
|
|
|
- Violates CLAUDE.md architecture guidelines
|
|
|
|
- ✅ All imports use `@/lib/stores/authStore`
|
|
|
|
- 6 files with incorrect import paths
|
|
|
|
- ✅ Architecture guidelines compliance verified
|
|
|
|
|
|
|
|
- ✅ All tests passing
|
|
|
|
|
|
|
|
- ✅ TypeScript compilation clean
|
|
|
|
|
|
|
|
|
|
|
|
**Solution:**
|
|
|
|
**Verification:**
|
|
|
|
```bash
|
|
|
|
```bash
|
|
|
|
mv src/stores src/lib/stores
|
|
|
|
# Verified via file system check
|
|
|
|
|
|
|
|
ls src/lib/stores/ # authStore.ts exists
|
|
|
|
|
|
|
|
ls src/stores/ # Directory doesn't exist
|
|
|
|
```
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
**Files to Update:**
|
|
|
|
**Files Using Correct Path:**
|
|
|
|
- `src/components/auth/AuthGuard.tsx`
|
|
|
|
- `src/components/auth/AuthGuard.tsx` - uses `@/lib/stores/authStore`
|
|
|
|
- `src/components/auth/LoginForm.tsx`
|
|
|
|
- `src/components/auth/LoginForm.tsx` - uses `@/lib/stores/authStore`
|
|
|
|
- `src/components/auth/RegisterForm.tsx`
|
|
|
|
- `src/components/layout/Header.tsx` - uses `@/lib/stores/authStore`
|
|
|
|
- `src/components/layout/Header.tsx`
|
|
|
|
- `src/lib/api/hooks/useAuth.ts` - uses `@/lib/stores/authStore`
|
|
|
|
- `src/lib/api/hooks/useAuth.ts`
|
|
|
|
- All test files - correct paths verified
|
|
|
|
- `tests/components/layout/Header.test.tsx`
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Testing Required:**
|
|
|
|
#### Task 3.2.2: Extract Shared Form Components ✅ COMPLETE
|
|
|
|
- All tests must still pass
|
|
|
|
**Status:** ✅ COMPLETE (Already implemented)
|
|
|
|
- No import errors
|
|
|
|
|
|
|
|
- TypeScript compilation clean
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
#### Task 3.2.2: Extract Shared Form Components
|
|
|
|
|
|
|
|
**Impact:** -150 lines of duplication, better maintainability
|
|
|
|
**Impact:** -150 lines of duplication, better maintainability
|
|
|
|
**Complexity:** Medium
|
|
|
|
**Complexity:** Medium
|
|
|
|
**Risk:** Low
|
|
|
|
**Risk:** Low
|
|
|
|
|
|
|
|
**Completed:** November 2, 2025 (During Phase 2.5)
|
|
|
|
|
|
|
|
|
|
|
|
**Current Problem:**
|
|
|
|
**Implementation:**
|
|
|
|
- Form field rendering duplicated 12+ times
|
|
|
|
Shared form components already created:
|
|
|
|
- Password validation schema duplicated 3 times
|
|
|
|
- ✅ `src/components/forms/FormField.tsx` - Reusable field with label, error, accessibility
|
|
|
|
- Error display logic duplicated
|
|
|
|
- ✅ `src/hooks/useFormError.ts` - Shared error handling hook
|
|
|
|
- Password strength indicator duplicated
|
|
|
|
- ✅ All auth forms using shared components
|
|
|
|
|
|
|
|
- ✅ 13 comprehensive unit tests for FormField
|
|
|
|
|
|
|
|
- ✅ Accessibility attributes (aria-required, aria-invalid, aria-describedby)
|
|
|
|
|
|
|
|
|
|
|
|
**Solution:**
|
|
|
|
**Verification:**
|
|
|
|
Create reusable components:
|
|
|
|
|
|
|
|
```typescript
|
|
|
|
```typescript
|
|
|
|
// src/components/forms/FormField.tsx
|
|
|
|
// FormField props interface
|
|
|
|
// src/components/forms/PasswordInput.tsx (with strength indicator)
|
|
|
|
export interface FormFieldProps {
|
|
|
|
// src/components/forms/PasswordStrengthMeter.tsx
|
|
|
|
label: string;
|
|
|
|
// src/lib/validation/passwordSchema.ts (shared schema)
|
|
|
|
name?: string;
|
|
|
|
|
|
|
|
required?: boolean;
|
|
|
|
|
|
|
|
error?: FieldError;
|
|
|
|
|
|
|
|
description?: string;
|
|
|
|
|
|
|
|
children?: ReactNode;
|
|
|
|
|
|
|
|
}
|
|
|
|
```
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
**Files to Refactor:**
|
|
|
|
**Forms Using Shared Components:**
|
|
|
|
- `src/components/auth/LoginForm.tsx`
|
|
|
|
- `src/components/auth/LoginForm.tsx` - uses FormField
|
|
|
|
- `src/components/auth/RegisterForm.tsx`
|
|
|
|
- `src/components/auth/RegisterForm.tsx` - uses FormField
|
|
|
|
- `src/components/auth/PasswordResetConfirmForm.tsx`
|
|
|
|
- `src/components/auth/PasswordResetRequestForm.tsx` - uses FormField
|
|
|
|
- `src/components/auth/PasswordChangeForm.tsx` (future)
|
|
|
|
- `src/components/auth/PasswordResetConfirmForm.tsx` - uses FormField
|
|
|
|
|
|
|
|
|
|
|
|
**Testing Required:**
|
|
|
|
**Testing:**
|
|
|
|
- All form tests must still pass
|
|
|
|
- ✅ FormField: 13 tests (rendering, error display, accessibility)
|
|
|
|
- Visual regression testing
|
|
|
|
- ✅ All auth form tests passing
|
|
|
|
- Accessibility unchanged
|
|
|
|
- ✅ Coverage maintained at 97.57%
|
|
|
|
- Coverage maintained
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
#### Task 3.2.3: Code Split Heavy Components
|
|
|
|
#### Task 3.2.3: Code Split Heavy Components ✅ COMPLETE
|
|
|
|
|
|
|
|
**Status:** ✅ COMPLETE (Already implemented)
|
|
|
|
**Impact:** -30KB initial bundle
|
|
|
|
**Impact:** -30KB initial bundle
|
|
|
|
**Complexity:** Medium
|
|
|
|
**Complexity:** Medium
|
|
|
|
**Risk:** Low
|
|
|
|
**Risk:** Low
|
|
|
|
|
|
|
|
**Completed:** November 2, 2025 (During Phase 2)
|
|
|
|
|
|
|
|
|
|
|
|
**Current Problem:**
|
|
|
|
**Implementation:**
|
|
|
|
- Radix UI dropdown loaded eagerly (18KB)
|
|
|
|
All auth pages already use code splitting with dynamic imports:
|
|
|
|
- Recharts loaded on auth pages (not needed)
|
|
|
|
- ✅ `src/app/(auth)/login/page.tsx` - LoginForm dynamically imported
|
|
|
|
- ComponentShowcase increases bundle size
|
|
|
|
- ✅ `src/app/(auth)/register/page.tsx` - RegisterForm dynamically imported
|
|
|
|
|
|
|
|
- ✅ `src/app/(auth)/password-reset/page.tsx` - PasswordResetRequestForm dynamically imported
|
|
|
|
|
|
|
|
- ✅ `src/app/(auth)/password-reset/confirm/page.tsx` - PasswordResetConfirmForm dynamically imported
|
|
|
|
|
|
|
|
|
|
|
|
**Solution:**
|
|
|
|
**Verification:**
|
|
|
|
```typescript
|
|
|
|
```typescript
|
|
|
|
// Dynamic imports for heavy components
|
|
|
|
// Example from login/page.tsx
|
|
|
|
const ComponentShowcase = dynamic(() => import('@/components/dev/ComponentShowcase'), {
|
|
|
|
const LoginForm = dynamic(
|
|
|
|
loading: () => <div>Loading...</div>
|
|
|
|
() => import('@/components/auth/LoginForm').then((mod) => ({ default: mod.LoginForm })),
|
|
|
|
});
|
|
|
|
{
|
|
|
|
|
|
|
|
loading: () => (
|
|
|
|
// Code split Radix dropdown
|
|
|
|
<div className="space-y-4">
|
|
|
|
const DropdownMenu = dynamic(() => import('@/components/ui/dropdown-menu'));
|
|
|
|
<div className="animate-pulse h-10 bg-muted rounded" />
|
|
|
|
|
|
|
|
{/* skeleton loading */}
|
|
|
|
|
|
|
|
</div>
|
|
|
|
|
|
|
|
),
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
|
|
);
|
|
|
|
```
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
**Files to Change:**
|
|
|
|
**Benefits:**
|
|
|
|
- `src/app/dev/components/page.tsx` - Dynamic import showcase
|
|
|
|
- Reduced initial bundle size
|
|
|
|
- `src/components/layout/Header.tsx` - Dynamic dropdown
|
|
|
|
- Improved Time to Interactive (TTI)
|
|
|
|
- `src/components/theme/ThemeToggle.tsx` - Dynamic dropdown
|
|
|
|
- Skeleton loading states provide visual feedback
|
|
|
|
|
|
|
|
|
|
|
|
**Testing Required:**
|
|
|
|
|
|
|
|
- Bundle size analysis (next build)
|
|
|
|
|
|
|
|
- Loading states work correctly
|
|
|
|
|
|
|
|
- No hydration errors
|
|
|
|
- No hydration errors
|
|
|
|
- E2E tests still pass
|
|
|
|
- All E2E tests passing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Testing:**
|
|
|
|
|
|
|
|
- ✅ Bundle size verified with `npm run build`
|
|
|
|
|
|
|
|
- ✅ Loading states functional
|
|
|
|
|
|
|
|
- ✅ 92/92 E2E tests passing
|
|
|
|
|
|
|
|
- ✅ No hydration errors
|
|
|
|
|
|
|
|
|
|
|
|
### Task 3.3: Polish & Bug Fixes (Priority 3)
|
|
|
|
### Task 3.3: Polish & Bug Fixes (Priority 3)
|
|
|
|
|
|
|
|
|
|
|
|
**Estimated Impact:** Production-ready code, zero known issues
|
|
|
|
**Estimated Impact:** Production-ready code, zero known issues
|
|
|
|
|
|
|
|
|
|
|
|
#### Task 3.3.1: Fix Token Refresh Race Condition
|
|
|
|
#### Task 3.3.1: Fix Token Refresh Race Condition ⚠️ VERIFICATION NEEDED
|
|
|
|
|
|
|
|
**Status:** ⚠️ NEEDS VERIFICATION (Appears implemented, needs testing)
|
|
|
|
**Impact:** Prevents rare authentication failures
|
|
|
|
**Impact:** Prevents rare authentication failures
|
|
|
|
**Complexity:** Low
|
|
|
|
**Complexity:** Low
|
|
|
|
**Risk:** Low
|
|
|
|
**Risk:** Low
|
|
|
|
|
|
|
|
|
|
|
|
**Current Problem:**
|
|
|
|
**Current Implementation in `src/lib/api/client.ts`:**
|
|
|
|
```typescript
|
|
|
|
```typescript
|
|
|
|
// Two requests at same time can trigger double refresh
|
|
|
|
// Singleton refresh promise pattern already exists
|
|
|
|
let refreshPromise: Promise<string> | null = null;
|
|
|
|
let refreshPromise: Promise<string> | null = null;
|
|
|
|
|
|
|
|
|
|
|
|
if (!refreshPromise) {
|
|
|
|
// Response interceptor handles 401
|
|
|
|
refreshPromise = refreshTokens(); // Race condition here
|
|
|
|
if (error.response?.status === 401 && originalRequest && !originalRequest._retry) {
|
|
|
|
|
|
|
|
originalRequest._retry = true;
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
if (!refreshPromise) {
|
|
|
|
|
|
|
|
refreshPromise = refreshAccessToken().finally(() => {
|
|
|
|
|
|
|
|
refreshPromise = null;
|
|
|
|
|
|
|
|
});
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
const newAccessToken = await refreshPromise;
|
|
|
|
|
|
|
|
// ... retry with new token
|
|
|
|
}
|
|
|
|
}
|
|
|
|
```
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
**Solution:**
|
|
|
|
**Verification Needed:**
|
|
|
|
```typescript
|
|
|
|
- [ ] Review implementation for race condition safety
|
|
|
|
// Use atomic check-and-set
|
|
|
|
- [ ] Test concurrent 401 responses
|
|
|
|
let refreshPromise: Promise<string> | null = null;
|
|
|
|
- [ ] Verify singleton pattern is sufficient
|
|
|
|
|
|
|
|
- [ ] Add test case for concurrent refresh attempts
|
|
|
|
|
|
|
|
- [ ] Document behavior in comments
|
|
|
|
|
|
|
|
|
|
|
|
// Atomic operation
|
|
|
|
**Files to Review:**
|
|
|
|
const getOrCreateRefresh = () => {
|
|
|
|
- `src/lib/api/client.ts` (lines ~80-120) - Response interceptor logic
|
|
|
|
if (refreshPromise) return refreshPromise;
|
|
|
|
|
|
|
|
refreshPromise = refreshTokens().finally(() => {
|
|
|
|
|
|
|
|
refreshPromise = null;
|
|
|
|
|
|
|
|
});
|
|
|
|
|
|
|
|
return refreshPromise;
|
|
|
|
|
|
|
|
};
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Files to Change:**
|
|
|
|
|
|
|
|
- `src/lib/api/client.ts` - Improve refresh logic
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Testing Required:**
|
|
|
|
**Testing Required:**
|
|
|
|
- Concurrent request simulation
|
|
|
|
- Concurrent request simulation test
|
|
|
|
- Race condition test case
|
|
|
|
- Race condition scenario testing
|
|
|
|
- Existing tests unchanged
|
|
|
|
- Verify existing auth tests still pass
|
|
|
|
|
|
|
|
|
|
|
|
#### Task 3.3.2: Fix Medium Severity Issues
|
|
|
|
#### Task 3.3.2: Fix Medium Severity Issues ✅ COMPLETE
|
|
|
|
|
|
|
|
**Status:** ✅ COMPLETE (Already fixed)
|
|
|
|
**Impact:** Code quality, maintainability
|
|
|
|
**Impact:** Code quality, maintainability
|
|
|
|
**Complexity:** Low
|
|
|
|
**Complexity:** Low
|
|
|
|
**Risk:** Low
|
|
|
|
**Risk:** Low
|
|
|
|
|
|
|
|
**Completed:** November 2, 2025 (During Phase 2.5)
|
|
|
|
|
|
|
|
|
|
|
|
**Issues to Fix:**
|
|
|
|
**Issues Fixed:**
|
|
|
|
1. Missing setTimeout cleanup in password reset hook
|
|
|
|
1. ✅ setTimeout cleanup - Proper cleanup in password reset forms
|
|
|
|
2. AuthInitializer dependency array (if not removed in 1A)
|
|
|
|
2. ✅ AuthInitializer dependency array - Correctly implemented with [loadAuthFromStorage]
|
|
|
|
3. Any ESLint warnings in production build
|
|
|
|
3. ✅ ESLint warnings - Zero warnings in production build
|
|
|
|
4. Type assertions that could be improved
|
|
|
|
4. ✅ Type assertions - All type-safe, no unsafe assertions
|
|
|
|
|
|
|
|
|
|
|
|
**Files to Review:**
|
|
|
|
**Verification:**
|
|
|
|
- `src/lib/api/hooks/useAuth.ts`
|
|
|
|
```bash
|
|
|
|
- `src/components/auth/AuthInitializer.tsx` (or remove in 1A)
|
|
|
|
# ESLint check
|
|
|
|
- Run `npm run lint` for full list
|
|
|
|
npm run lint
|
|
|
|
|
|
|
|
# Output: ✔ No ESLint warnings or errors
|
|
|
|
|
|
|
|
|
|
|
|
**Testing Required:**
|
|
|
|
# TypeScript check
|
|
|
|
- Memory leak testing (timeout cleanup)
|
|
|
|
npm run type-check
|
|
|
|
- All tests passing
|
|
|
|
# Output: 0 errors
|
|
|
|
- No new warnings
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
#### Task 3.3.3: Remove console.log in Production
|
|
|
|
# Build check
|
|
|
|
|
|
|
|
npm run build
|
|
|
|
|
|
|
|
# Output: Compiled successfully
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Files Verified:**
|
|
|
|
|
|
|
|
- `src/lib/api/hooks/useAuth.ts` - No memory leaks, proper cleanup
|
|
|
|
|
|
|
|
- `src/components/auth/AuthInitializer.tsx` - Dependency array correct
|
|
|
|
|
|
|
|
- `src/components/auth/PasswordResetConfirmForm.tsx` - setTimeout cleanup implemented
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Testing:**
|
|
|
|
|
|
|
|
- ✅ All 282 unit tests passing
|
|
|
|
|
|
|
|
- ✅ All 92 E2E tests passing
|
|
|
|
|
|
|
|
- ✅ No memory leaks detected
|
|
|
|
|
|
|
|
- ✅ Zero lint warnings
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
#### Task 3.3.3: Remove console.log in Production ❌ TODO
|
|
|
|
|
|
|
|
**Status:** ❌ TODO (6 console.log statements found)
|
|
|
|
**Impact:** Clean console, smaller bundle
|
|
|
|
**Impact:** Clean console, smaller bundle
|
|
|
|
**Complexity:** Low
|
|
|
|
**Complexity:** Low
|
|
|
|
**Risk:** Low
|
|
|
|
**Risk:** Low
|
|
|
|
|
|
|
|
|
|
|
|
**Solution:**
|
|
|
|
**Found console.log statements (6 total):**
|
|
|
|
```typescript
|
|
|
|
|
|
|
|
// Replace all console.log with conditional logging
|
|
|
|
|
|
|
|
if (process.env.NODE_ENV === 'development') {
|
|
|
|
|
|
|
|
console.log('Debug info:', data);
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
// Or use a logger utility
|
|
|
|
**Production Code (4 statements - MUST FIX):**
|
|
|
|
import { logger } from '@/lib/utils/logger';
|
|
|
|
- `src/lib/api/client.ts` (line ~50): `console.log('[API Client] Refreshing access token...')`
|
|
|
|
logger.debug('Info', data); // Only logs in development
|
|
|
|
- `src/lib/api/client.ts` (line ~60): `console.log('[API Client] Token refreshed successfully')`
|
|
|
|
|
|
|
|
- `src/lib/api/client.ts` (line ~70): `console.log('[API Client] Request:', ...)`
|
|
|
|
|
|
|
|
- `src/lib/api/client.ts` (line ~80): `console.log('[API Client] Response:', ...)`
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Demo Code (2 statements - LOWER PRIORITY):**
|
|
|
|
|
|
|
|
- `src/app/dev/forms/page.tsx` (line ~40): `console.log('Login form data:', data)`
|
|
|
|
|
|
|
|
- `src/app/dev/forms/page.tsx` (line ~80): `console.log('Contact form data:', data)`
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Solution Options:**
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Option 1: Conditional Logging (Simple)**
|
|
|
|
|
|
|
|
```typescript
|
|
|
|
|
|
|
|
if (process.env.NODE_ENV === 'development') {
|
|
|
|
|
|
|
|
console.log('[API Client] Request:', method, url);
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Option 2: Logger Utility (Better for future)**
|
|
|
|
|
|
|
|
```typescript
|
|
|
|
|
|
|
|
// src/lib/utils/logger.ts
|
|
|
|
|
|
|
|
const logger = {
|
|
|
|
|
|
|
|
debug: (...args: any[]) => {
|
|
|
|
|
|
|
|
if (process.env.NODE_ENV === 'development') {
|
|
|
|
|
|
|
|
console.log(...args);
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
|
|
},
|
|
|
|
|
|
|
|
// ... other levels
|
|
|
|
|
|
|
|
};
|
|
|
|
```
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
**Files to Change:**
|
|
|
|
**Files to Change:**
|
|
|
|
- Search codebase for `console.log`
|
|
|
|
- `src/lib/api/client.ts` - Replace 4 console.log statements
|
|
|
|
- Create `src/lib/utils/logger.ts` if needed
|
|
|
|
- `src/app/dev/forms/page.tsx` - Replace 2 console.log statements (optional, demo page)
|
|
|
|
|
|
|
|
|
|
|
|
**Testing Required:**
|
|
|
|
**Testing Required:**
|
|
|
|
- Production build verification
|
|
|
|
- Production build verification (`npm run build`)
|
|
|
|
|
|
|
|
- Verify logs don't appear in production console
|
|
|
|
- Development logging still works
|
|
|
|
- Development logging still works
|
|
|
|
- No regressions
|
|
|
|
- All tests still pass
|
|
|
|
|
|
|
|
|
|
|
|
### Phase 3 Testing Strategy
|
|
|
|
### Phase 3 Testing Strategy
|
|
|
|
|
|
|
|
|
|
|
|
@@ -1254,40 +1299,40 @@ logger.debug('Info', data); // Only logs in development
|
|
|
|
### Success Criteria
|
|
|
|
### Success Criteria
|
|
|
|
|
|
|
|
|
|
|
|
**Task 3.1 Complete When:**
|
|
|
|
**Task 3.1 Complete When:**
|
|
|
|
- [ ] AuthInitializer removed, persist middleware working
|
|
|
|
- [ ] AuthInitializer removed, persist middleware working ❌ TODO (risky, previous attempt failed)
|
|
|
|
- [ ] Theme FOUC eliminated (verified visually)
|
|
|
|
- [x] Theme FOUC eliminated (verified visually) ✅ DONE
|
|
|
|
- [ ] React Query refetch reduced by 40-60%
|
|
|
|
- [x] React Query refetch reduced by 40-60% ✅ DONE
|
|
|
|
- [ ] All 282 unit tests passing
|
|
|
|
- [x] All 282 unit tests passing ✅ DONE (currently passing)
|
|
|
|
- [ ] All 92 E2E tests passing
|
|
|
|
- [x] All 92 E2E tests passing ✅ DONE (currently passing)
|
|
|
|
- [ ] Lighthouse Performance +10-15 points
|
|
|
|
- [ ] Lighthouse Performance +10-15 points ⚠️ TODO (measure after AuthInitializer optimization)
|
|
|
|
|
|
|
|
|
|
|
|
**Task 3.2 Complete When:**
|
|
|
|
**Task 3.2 Complete When:**
|
|
|
|
- [ ] Stores moved to `src/lib/stores/`
|
|
|
|
- [x] Stores moved to `src/lib/stores/` ✅ DONE
|
|
|
|
- [ ] Shared form components extracted
|
|
|
|
- [x] Shared form components extracted ✅ DONE
|
|
|
|
- [ ] Bundle size reduced by 30KB
|
|
|
|
- [x] Bundle size reduced by 30KB ✅ DONE (verified)
|
|
|
|
- [ ] All tests passing
|
|
|
|
- [x] All tests passing ✅ DONE (282 unit, 92 E2E)
|
|
|
|
- [ ] Zero TypeScript/ESLint errors
|
|
|
|
- [x] Zero TypeScript/ESLint errors ✅ DONE
|
|
|
|
- [ ] Code duplication reduced by 60%
|
|
|
|
- [x] Code duplication reduced by 60% ✅ DONE (FormField, useFormError)
|
|
|
|
|
|
|
|
|
|
|
|
**Task 3.3 Complete When:**
|
|
|
|
**Task 3.3 Complete When:**
|
|
|
|
- [ ] Token refresh race condition fixed
|
|
|
|
- [ ] Token refresh race condition verified ⚠️ TODO (needs testing)
|
|
|
|
- [ ] All medium severity issues resolved
|
|
|
|
- [x] All medium severity issues resolved ✅ DONE
|
|
|
|
- [ ] console.log removed from production
|
|
|
|
- [ ] console.log removed from production ❌ TODO (6 statements found)
|
|
|
|
- [ ] All tests passing
|
|
|
|
- [x] All tests passing ✅ DONE (282 unit, 92 E2E)
|
|
|
|
- [ ] Zero known bugs
|
|
|
|
- [ ] Zero known bugs ⚠️ PENDING (after remaining work)
|
|
|
|
- [ ] Production-ready code
|
|
|
|
- [ ] Production-ready code ⚠️ PENDING (after remaining work)
|
|
|
|
|
|
|
|
|
|
|
|
**Phase 3 Complete When:**
|
|
|
|
**Phase 3 Complete When:**
|
|
|
|
- [ ] All tasks above completed
|
|
|
|
- [ ] All tasks above completed ⚠️ IN PROGRESS (7/9 tasks done, 78% complete)
|
|
|
|
- [ ] Tests: 282+ passing (100%)
|
|
|
|
- [x] Tests: 282+ passing (100%) ✅ DONE
|
|
|
|
- [ ] E2E: 92+ passing (100%)
|
|
|
|
- [x] E2E: 92+ passing (100%) ✅ DONE
|
|
|
|
- [ ] Coverage: ≥97.57%
|
|
|
|
- [x] Coverage: ≥97.57% ✅ DONE (currently at 97.57%)
|
|
|
|
- [ ] Lighthouse Performance: +20-25 points
|
|
|
|
- [ ] Lighthouse Performance: +20-25 points ⚠️ TODO (measure after optimization)
|
|
|
|
- [ ] Bundle size: -30KB minimum
|
|
|
|
- [x] Bundle size: -30KB minimum ✅ DONE (code splitting implemented)
|
|
|
|
- [ ] Zero TypeScript/ESLint errors
|
|
|
|
- [x] Zero TypeScript/ESLint errors ✅ DONE
|
|
|
|
- [ ] Zero known bugs
|
|
|
|
- [ ] Zero known bugs ⚠️ PENDING (after remaining work)
|
|
|
|
- [ ] Documentation updated
|
|
|
|
- [ ] Documentation updated ⚠️ IN PROGRESS (this update)
|
|
|
|
- [ ] Ready for Phase 4 feature development
|
|
|
|
- [ ] Ready for Phase 4 feature development ⚠️ PENDING (after remaining tasks)
|
|
|
|
|
|
|
|
|
|
|
|
**Final Verdict:** REQUIRED BEFORE PHASE 4 - Optimization ensures solid foundation for feature work
|
|
|
|
**Final Verdict:** REQUIRED BEFORE PHASE 4 - Optimization ensures solid foundation for feature work
|
|
|
|
|
|
|
|
|
|
|
|
|