From d4e08ae2bd0726756c05bec1b1c36515b9f51b99 Mon Sep 17 00:00:00 2001 From: Steven Date: Thu, 25 Dec 2025 08:46:52 +0800 Subject: [PATCH] refactor(react-query): optimize config, add error boundary, and remove JSDoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit combines multiple improvements to the React Query migration: Performance Optimization: - Increase default staleTime from 10s to 30s for better performance - Reduces unnecessary network requests while maintaining data freshness Error Handling: - Add ErrorBoundary component with user-friendly error UI - Integrated at app root level for comprehensive coverage - Provides error details and reload option Documentation: - Add docs/auth-architecture.md explaining AuthContext design decisions - Document why AuthContext is preferred over React Query for current user Code Cleanup: - Remove all JSDoc comments from hooks and components - Keep essential inline comments for clarity - Simplifies code readability Files modified: - src/lib/query-client.ts - Optimized staleTime - src/main.tsx - Added ErrorBoundary wrapper - src/components/ErrorBoundary.tsx - New component - src/hooks/useMemoQueries.ts - Removed JSDoc - src/hooks/useUserQueries.ts - Removed JSDoc - src/components/PagedMemoList/PagedMemoList.tsx - Removed JSDoc - docs/auth-architecture.md - New documentation All changes verified with TypeScript compilation and production build. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- web/docs/auth-architecture.md | 55 +++++++++++++++ web/src/components/ErrorBoundary.tsx | 70 +++++++++++++++++++ .../PagedMemoList/PagedMemoList.tsx | 4 -- web/src/hooks/useMemoQueries.ts | 28 -------- web/src/hooks/useUserQueries.ts | 44 +----------- web/src/lib/query-client.ts | 9 +-- web/src/main.tsx | 29 ++++---- 7 files changed, 149 insertions(+), 90 deletions(-) create mode 100644 web/docs/auth-architecture.md create mode 100644 web/src/components/ErrorBoundary.tsx diff --git a/web/docs/auth-architecture.md b/web/docs/auth-architecture.md new file mode 100644 index 000000000..be93292f4 --- /dev/null +++ b/web/docs/auth-architecture.md @@ -0,0 +1,55 @@ +# Authentication State Architecture + +## Current Approach: AuthContext + +The application uses **AuthContext** for authentication state management, not React Query's `useCurrentUserQuery`. This is an intentional architectural decision. + +### Why AuthContext Instead of React Query? + +#### 1. **Synchronous Initialization** +- AuthContext fetches user data during app initialization (`main.tsx`) +- Provides synchronous access to `currentUser` throughout the app +- No need to handle loading states in every component + +#### 2. **Single Source of Truth** +- User data fetched once on mount +- All components get consistent, up-to-date user info +- No race conditions from multiple query instances + +#### 3. **Integration with React Query** +- AuthContext pre-populates React Query cache after fetch (line 81-82 in `AuthContext.tsx`) +- Best of both worlds: synchronous access + cache consistency +- React Query hooks like `useNotifications()` can still use the cached user data + +#### 4. **Simpler Component Code** +```typescript +// With AuthContext (current) +const user = useCurrentUser(); // Always returns User | undefined + +// With React Query (alternative) +const { data: user, isLoading } = useCurrentUserQuery(); +if (isLoading) return ; +// Need loading handling everywhere +``` + +### When to Use React Query for Auth? + +Consider migrating auth to React Query if: +- App needs real-time user profile updates from external sources +- Multiple tabs need instant sync +- User data changes frequently during a session + +For Memos (a notes app where user profile rarely changes), AuthContext is the right choice. + +### Future Considerations + +The unused `useCurrentUserQuery()` hook in `useUserQueries.ts` is kept for potential future use. If requirements change (e.g., real-time collaboration on user profiles), migration path is clear: + +1. Remove AuthContext +2. Use `useCurrentUserQuery()` everywhere +3. Handle loading states in components +4. Add suspense boundaries if needed + +## Recommendation + +**Keep the current AuthContext approach.** It provides better DX and performance for this use case. diff --git a/web/src/components/ErrorBoundary.tsx b/web/src/components/ErrorBoundary.tsx new file mode 100644 index 000000000..6bc4fb1f1 --- /dev/null +++ b/web/src/components/ErrorBoundary.tsx @@ -0,0 +1,70 @@ +import { AlertCircle, RefreshCw } from "lucide-react"; +import { Component, type ErrorInfo, type ReactNode } from "react"; +import { Button } from "./ui/button"; + +interface Props { + children: ReactNode; + fallback?: ReactNode; +} + +interface State { + hasError: boolean; + error: Error | null; +} + +export class ErrorBoundary extends Component { + constructor(props: Props) { + super(props); + this.state = { hasError: false, error: null }; + } + + static getDerivedStateFromError(error: Error): State { + return { hasError: true, error }; + } + + componentDidCatch(error: Error, errorInfo: ErrorInfo) { + console.error("ErrorBoundary caught an error:", error, errorInfo); + } + + handleReset = () => { + this.setState({ hasError: false, error: null }); + window.location.reload(); + }; + + render() { + if (this.state.hasError) { + if (this.props.fallback) { + return this.props.fallback; + } + + return ( +
+
+
+ +

Something went wrong

+
+ +

+ An unexpected error occurred. This could be due to a network issue or a problem with the application. +

+ + {this.state.error && ( +
+ Error details + 
{this.state.error.message}
+
+ )} + + +
+
+ ); + } + + return this.props.children; + } +} diff --git a/web/src/components/PagedMemoList/PagedMemoList.tsx b/web/src/components/PagedMemoList/PagedMemoList.tsx index 8c1540a83..1c81588cc 100644 --- a/web/src/components/PagedMemoList/PagedMemoList.tsx +++ b/web/src/components/PagedMemoList/PagedMemoList.tsx @@ -29,10 +29,6 @@ interface Props { showCreator?: boolean; } -/** - * Custom hook to auto-fetch more content when page isn't scrollable. - * This ensures users see content without needing to scroll on large screens. - */ function useAutoFetchWhenNotScrollable({ hasNextPage, isFetchingNextPage, diff --git a/web/src/hooks/useMemoQueries.ts b/web/src/hooks/useMemoQueries.ts index c653c273c..40a152b96 100644 --- a/web/src/hooks/useMemoQueries.ts +++ b/web/src/hooks/useMemoQueries.ts @@ -15,10 +15,6 @@ export const memoKeys = { detail: (name: string) => [...memoKeys.details(), name] as const, }; -/** - * Hook to fetch a list of memos with filtering and sorting. - * @param request - Request parameters (state, orderBy, filter, pageSize) - */ export function useMemos(request: Partial = {}) { return useQuery({ queryKey: memoKeys.list(request), @@ -29,13 +25,6 @@ export function useMemos(request: Partial = {}) { }); } -/** - * Hook for infinite scrolling/pagination of memos. - * Automatically fetches pages as the user scrolls. - * - * @param request - Partial request configuration (state, orderBy, filter, pageSize) - * @returns React Query infinite query result with pages of memos - */ export function useInfiniteMemos(request: Partial = {}) { return useInfiniteQuery({ queryKey: memoKeys.list(request), @@ -55,11 +44,6 @@ export function useInfiniteMemos(request: Partial = {}) { }); } -/** - * Hook to fetch a single memo by its resource name. - * @param name - Memo resource name (e.g., "memos/123") - * @param options - Query options including enabled flag - */ export function useMemo(name: string, options?: { enabled?: boolean }) { return useQuery({ queryKey: memoKeys.detail(name), @@ -72,10 +56,6 @@ export function useMemo(name: string, options?: { enabled?: boolean }) { }); } -/** - * Hook to create a new memo. - * Automatically invalidates memo lists and user stats on success. - */ export function useCreateMemo() { const queryClient = useQueryClient(); @@ -95,10 +75,6 @@ export function useCreateMemo() { }); } -/** - * Hook to update an existing memo with optimistic updates. - * Implements rollback on error for better UX. - */ export function useUpdateMemo() { const queryClient = useQueryClient(); @@ -145,10 +121,6 @@ export function useUpdateMemo() { }); } -/** - * Hook to delete a memo. - * Automatically removes memo from cache and invalidates lists on success. - */ export function useDeleteMemo() { const queryClient = useQueryClient(); diff --git a/web/src/hooks/useUserQueries.ts b/web/src/hooks/useUserQueries.ts index 0d2122026..fdb1a8efb 100644 --- a/web/src/hooks/useUserQueries.ts +++ b/web/src/hooks/useUserQueries.ts @@ -17,19 +17,9 @@ export const userKeys = { notifications: () => [...userKeys.all, "notifications"] as const, }; -/** - * Hook to get the current authenticated user via React Query. - * - * NOTE: This hook is currently UNUSED in favor of the AuthContext-based - * useCurrentUser hook (src/hooks/useCurrentUser.ts) which provides the same - * data from AuthContext. The AuthContext fetches user data on app initialization - * and stores it in React state, avoiding unnecessary re-fetches. - * - * This hook is kept for potential future use if we decide to fully migrate - * auth state to React Query, but currently all components use the AuthContext version. - * - * Data is cached for 5 minutes as auth state changes infrequently. - */ +// NOTE: This hook is currently UNUSED in favor of the AuthContext-based +// useCurrentUser hook (src/hooks/useCurrentUser.ts). This is kept for potential +// future migration to React Query for auth state. export function useCurrentUserQuery() { return useQuery({ queryKey: userKeys.currentUser(), @@ -41,11 +31,6 @@ export function useCurrentUserQuery() { }); } -/** - * Hook to fetch a specific user by name. - * @param name - User resource name (e.g., "users/123") - * @param options - Query options including enabled flag - */ export function useUser(name: string, options?: { enabled?: boolean }) { return useQuery({ queryKey: userKeys.detail(name), @@ -58,10 +43,6 @@ export function useUser(name: string, options?: { enabled?: boolean }) { }); } -/** - * Hook to fetch statistics for a specific user. - * @param username - User resource name (e.g., "users/123") - */ export function useUserStats(username?: string) { return useQuery({ queryKey: username ? userKeys.userStats(username) : userKeys.stats(), @@ -76,9 +57,6 @@ export function useUserStats(username?: string) { }); } -/** - * Hook to fetch shortcuts for the current user. - */ export function useShortcuts() { return useQuery({ queryKey: userKeys.shortcuts(), @@ -89,10 +67,6 @@ export function useShortcuts() { }); } -/** - * Hook to fetch notifications for the current user. - * Only fetches when a user is authenticated. - */ export function useNotifications() { const { data: currentUser } = useCurrentUserQuery(); @@ -110,10 +84,6 @@ export function useNotifications() { }); } -/** - * Hook to fetch tag counts for autocomplete suggestions. - * @param forCurrentUser - If true, fetches only current user's tags; if false, fetches all public tags - */ export function useTagCounts(forCurrentUser = false) { const { data: currentUser } = useCurrentUserQuery(); @@ -148,10 +118,6 @@ export function useTagCounts(forCurrentUser = false) { }); } -/** - * Hook to update a user's profile. - * Automatically updates the cache on success. - */ export function useUpdateUser() { const queryClient = useQueryClient(); @@ -170,10 +136,6 @@ export function useUpdateUser() { }); } -/** - * Hook to delete a user. - * Automatically removes the user from cache on success. - */ export function useDeleteUser() { const queryClient = useQueryClient(); diff --git a/web/src/lib/query-client.ts b/web/src/lib/query-client.ts index 9bac6f7ca..af7e9713c 100644 --- a/web/src/lib/query-client.ts +++ b/web/src/lib/query-client.ts @@ -3,12 +3,13 @@ import { QueryClient } from "@tanstack/react-query"; export const queryClient = new QueryClient({ defaultOptions: { queries: { - // Memos app is real-time focused, so we want fresh data - staleTime: 1000 * 10, // 10 seconds + // Balanced approach: Fresh enough for collaboration, but reduces unnecessary refetches + // Individual queries can override with shorter staleTime if needed (e.g., notifications) + staleTime: 1000 * 30, // 30 seconds (increased from 10s for better performance) gcTime: 1000 * 60 * 5, // 5 minutes (formerly cacheTime) retry: 1, - refetchOnWindowFocus: true, - refetchOnReconnect: true, + refetchOnWindowFocus: true, // Refetch when user returns to tab + refetchOnReconnect: true, // Refetch when network reconnects }, mutations: { retry: 1, diff --git a/web/src/main.tsx b/web/src/main.tsx index aa2dfa17c..35b06a136 100644 --- a/web/src/main.tsx +++ b/web/src/main.tsx @@ -7,6 +7,7 @@ import { Toaster } from "react-hot-toast"; import { RouterProvider } from "react-router-dom"; import "./i18n"; import "./index.css"; +import { ErrorBoundary } from "@/components/ErrorBoundary"; import { AuthProvider, useAuth } from "@/contexts/AuthContext"; import { InstanceProvider, useInstance } from "@/contexts/InstanceContext"; import { ViewProvider } from "@/contexts/ViewContext"; @@ -48,19 +49,21 @@ function AppInitializer({ children }: { children: React.ReactNode }) { function Main() { return ( - - - - - - - - - - - - - + + + + + + + + + + + + + + + ); }