optimize session handling

[lukegskw]

No description provided.

[Copilot]

Pull Request Overview

This PR optimizes session handling with improved authentication flow, error management, and validation utilities. The changes focus on refactoring authentication state management, consolidating validation logic, and enhancing error handling for PowerSync synchronization.

• Refactored authentication initialization to use auth state change listeners instead of initial session fetching
• Added validation utilities and consolidated validation rules across components
• Improved PowerSync error handling with retry logic for JWT-related errors

Reviewed Changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
utils/validation.ts	New validation utilities module with email and required field validation functions
utils/toast.ts	Updated toast function parameter order and made message parameter optional
utils/powersync.ts	Added error filtering utility for ignorable PowerSync errors
services/powersync/Connector.ts	Enhanced session handling with token refresh fallback and auth state listener
contexts/PowerSyncProvider.tsx	Improved error handling with retry logic for JWT errors
contexts/AuthContext.tsx	Simplified authentication initialization using auth state change listener
hooks/useItemInteractions.ts	Updated to use new toast function signature and removed unused refresh functionality
components/AlertInput/AlertInput.tsx	Added validation support with error display
components/ItemList/ItemList.tsx	Added email validation for share functionality and removed refresh control
components/Form/FormInput.tsx	Removed duplicate validation rules (moved to utils)
Multiple auth/settings screens	Updated to use new toast function signature

[claude]

Code Review: Optimize Session Handling

✅ Positive Changes

1. Token Management Improvements

• New tokenManager.ts: Excellent proactive token refresh strategy that prevents connection failures
• Token expiry checking with 5-minute buffer is sensible
• Proper error handling in refresh flow

2. Validation System Enhancement

• New validation.ts: Good separation of validation logic from form components
• Consistent validation patterns across the app
• Maintains backward compatibility with existing validationRules API

3. PowerSync Session Handling

• Smart app state management (background/foreground) in PowerSyncProvider.tsx:78-127
• Proactive token refresh before reconnection prevents auth failures
• Better error handling with reduced user-facing noise for JWT/connection errors

4. UI/UX Improvements

• Enhanced AlertInput component with inline validation and error states
• Improved tablet support with responsive dimensions in Dimensions.ts
• Better carousel layout with proper centering and positioning

---

⚠️ Issues & Concerns

1. Error Handling Inconsistency

• MAJOR: Missing error level parameter in several handleErrorAndShowToast calls:
  • contexts/PowerSyncProvider.tsx:65 and 72 - missing error level
  • Fix: Add error level parameter: handleErrorAndShowToast(toast, downloadError, 'error')

2. Performance & Memory Issues

• ItemCard.tsx:43: Changed from useRef to useSharedValue for haptic trigger - this may cause unnecessary re-renders
• Recommendation: Revert to useRef as haptic state doesn't need to trigger re-renders

3. Type Safety Concerns

• PowerSyncProvider.tsx:98,106: Generic error checks !(error instanceof Error) are too broad
• Better approach: Use proper error typing or unknown type handling

4. Logic Issues

• PowerSyncProvider.tsx:154: Condition !isAuthenticated && !isProUser should use || not &&
• Current logic prevents sync even when user is authenticated but not pro

---

🔒 Security Analysis

✅ Good Security Practices

• No sensitive data logging (follows CLAUDE.md guidelines)
• Proper token handling with automatic refresh
• Input validation with email sanitization

⚠️ Minor Security Considerations

• Consider adding rate limiting for token refresh attempts
• Validate token expiry timestamps to prevent manipulation

---

🚀 Performance Considerations

✅ Improvements

• Reduced PowerSync retry delay from 10s to 5s for faster recovery
• Better error filtering to reduce unnecessary user notifications
• Responsive dimensions for better tablet performance

⚠️ Potential Issues

• Frequent app state changes could trigger excessive reconnections
• Consider debouncing the reconnection logic

---

📋 Recommendations

High Priority

1. Fix error handling: Add missing error level parameters
2. Fix logic error: Correct authentication condition in PowerSync setup
3. Revert haptic change: Use useRef instead of useSharedValue in ItemCard

Medium Priority

1. Add unit tests for new tokenManager and validation utilities
2. Consider debouncing app state change reconnections
3. Add error boundary around PowerSync provider

Code Style

• Follows CLAUDE.md conventions well
• Good TypeScript usage with proper typing
• Appropriate separation of concerns

---

📊 Overall Assessment

This PR significantly improves session handling and user experience. The token management strategy is solid, and the validation improvements are valuable. However, there are critical error handling bugs that must be fixed before merging.

Recommendation: Request changes for the critical error handling fixes, then approve.

🔧 Generated with Claude Code

[claude]

Code Review Feedback

Overall Assessment

This PR shows significant improvements in code organization, error handling, and validation. The introduction of custom hooks and better separation of concerns is commendable. However, there are several issues that need attention.

🚨 Critical Issues

1. Incomplete truncated code in ItemCard.tsx (line ~179)

   • The file appears to be cut off mid-function with hasTriggeredHap which should be hasTriggeredHaptic.value = true;
   • This will cause compilation errors and needs immediate fix

2. Typo in CLAUDE.md (line 113)

   • UI and Stylingiiiiichichcccc contains random characters
   • Should be UI and Styling

🔧 Code Quality Issues

AlertInput Component (components/AlertInput/AlertInput.tsx)

• Breaking change: Added validationType prop without default handling for existing usage
• Import issue: Direct import from ../../utils/validation should use absolute path @/utils/validation
• Consider making validation opt-in rather than defaulting to required

Error Handling Pattern Changes

Multiple files show updated error handling signature with new parameter that appears to be a log level. Ensure this change is consistently applied across the entire codebase and add documentation for the new parameter.

🎯 Best Practices Violations

1. Import Organization:

   • auth.tsx removes validationRules import from components but imports from utils - ensure consistent location
   • Same pattern in feedback.tsx

2. File Structure:

   • New .claude/commands/ directory - consider if this belongs in version control
   • *.ipa files added to .gitignore (good practice)

⚡ Performance & Logic Improvements

ItemCard Component

• Good: Replaced useRef with useSharedValue for hasTriggeredHaptic - better for React Native Reanimated
• Good: Improved gesture handling logic

CodeDetails Component

• Good: Better carousel layout with proper container structure
• Good: Improved responsive design with tablet considerations
• Good: Better QR code container styling

Details Screen

• Excellent: Extracted voucher operations logic into useVoucherValueOperations hook
• Good: Cleaner component structure with better separation of concerns

🔐 Security Considerations

1. Validation: New validation in AlertInput is positive for security
2. Error Logging: Ensure sensitive data isn't logged in the new error handling pattern
3. File Handling: PDF/image handling appears secure

📱 React Native Specific

1. Good: Proper use of useSharedValue instead of useRef for animation values
2. Good: Improved dimension handling for tablets
3. Consider: Test the carousel changes on different screen sizes

🧪 Testing Recommendations

1. Test AlertInput component with all validation types
2. Verify error handling across all affected screens
3. Test carousel behavior on tablets and phones
4. Verify the truncated ItemCard functionality once fixed

🚀 Recommended Actions

1. Fix the truncated code in ItemCard.tsx immediately
2. Fix the typo in CLAUDE.md
3. Use absolute imports consistently: @/utils/validation instead of relative paths
4. Document the new error handling signature
5. Consider backward compatibility for the AlertInput validation changes
6. Test thoroughly on both iOS and Android, especially the carousel and gesture changes

✅ What's Working Well

• Excellent separation of concerns with the new custom hooks
• Improved error handling pattern (once documented)
• Better responsive design considerations
• Good use of React Native Reanimated best practices
• Proper TypeScript usage throughout

Status: ⚠️ Needs fixes before merge - primarily the truncated code issue and typo.

[Copilot]

Pull Request Overview

Copilot reviewed 62 out of 69 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

ios/NativePDFBarcodeExtractor/RCTNativePDFBarcodeExtractor.mm:1

• This blocking approach with runloop polling can cause performance issues and unresponsive UI. Consider using proper async/await patterns or completion handlers instead of blocking the thread.

#import "RCTNativePDFBarcodeExtractor.h"