mirror of https://github.com/usememos/memos.git
refactor(react-query): optimize config, add error boundary, and remove JSDoc
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 <noreply@anthropic.com>
This commit is contained in:
parent
eed935ce44
commit
d4e08ae2bd
|
|
@ -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 <Spinner />;
|
||||
// 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.
|
||||
|
|
@ -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<Props, State> {
|
||||
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 (
|
||||
<div className="flex items-center justify-center min-h-screen bg-background">
|
||||
<div className="max-w-md w-full p-6 space-y-4">
|
||||
<div className="flex items-center gap-3 text-destructive">
|
||||
<AlertCircle className="w-8 h-8" />
|
||||
<h1 className="text-2xl font-bold">Something went wrong</h1>
|
||||
</div>
|
||||
|
||||
<p className="text-foreground/70">
|
||||
An unexpected error occurred. This could be due to a network issue or a problem with the application.
|
||||
</p>
|
||||
|
||||
{this.state.error && (
|
||||
<details className="bg-muted p-3 rounded-md text-sm">
|
||||
<summary className="cursor-pointer font-medium mb-2">Error details</summary>
|
||||
<pre className="whitespace-pre-wrap break-words text-xs text-foreground/60">{this.state.error.message}</pre>
|
||||
</details>
|
||||
)}
|
||||
|
||||
<Button onClick={this.handleReset} className="w-full gap-2">
|
||||
<RefreshCw className="w-4 h-4" />
|
||||
Reload Application
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return this.props.children;
|
||||
}
|
||||
}
|
||||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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<ListMemosRequest> = {}) {
|
||||
return useQuery({
|
||||
queryKey: memoKeys.list(request),
|
||||
|
|
@ -29,13 +25,6 @@ export function useMemos(request: Partial<ListMemosRequest> = {}) {
|
|||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<ListMemosRequest> = {}) {
|
||||
return useInfiniteQuery({
|
||||
queryKey: memoKeys.list(request),
|
||||
|
|
@ -55,11 +44,6 @@ export function useInfiniteMemos(request: Partial<ListMemosRequest> = {}) {
|
|||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* 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();
|
||||
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<InstanceProvider>
|
||||
<AuthProvider>
|
||||
<ViewProvider>
|
||||
<AppInitializer>
|
||||
<RouterProvider router={router} />
|
||||
<Toaster position="top-right" />
|
||||
</AppInitializer>
|
||||
</ViewProvider>
|
||||
</AuthProvider>
|
||||
</InstanceProvider>
|
||||
<ReactQueryDevtools initialIsOpen={false} />
|
||||
</QueryClientProvider>
|
||||
<ErrorBoundary>
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<InstanceProvider>
|
||||
<AuthProvider>
|
||||
<ViewProvider>
|
||||
<AppInitializer>
|
||||
<RouterProvider router={router} />
|
||||
<Toaster position="top-right" />
|
||||
</AppInitializer>
|
||||
</ViewProvider>
|
||||
</AuthProvider>
|
||||
</InstanceProvider>
|
||||
<ReactQueryDevtools initialIsOpen={false} />
|
||||
</QueryClientProvider>
|
||||
</ErrorBoundary>
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue