Refactored client-side data fetching to server side#556
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes update several mail-related components. The mail page now accepts a new folder parameter and displays both a mail list and a thread display using Suspense for asynchronous loading. Several files have been marked for client-side rendering with the addition of the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant MP as MailPage
participant SML as SRMailList
participant STD as SRThreadDisplay
participant Loader as Suspense
U->>MP: Request mail page with threadId & folder
MP->>MP: Fetch thread messages via getMail
MP->>SML: Call SRMailList with folder
SML-->>MP: Return mail list data
MP->>STD: Render thread display with messages
STD-->>MP: Return thread display content
MP-->>U: Complete UI rendered with both mail list and thread display
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
apps/mail/types/index.ts (1)
97-97: Standardized string literals for consistent code styleThe change from double quotes to single quotes for string literals in the MailSelectMode type aligns with TypeScript/JavaScript coding conventions and improves consistency throughout the codebase.
Consider applying this string literal standardization across all type definitions in the file for complete consistency.
apps/mail/app/(routes)/mail/[folder]/[threadId]/page.tsx (1)
26-56: Improved layout with side-by-side mail list and thread display.The two-column layout provides a better user experience by showing both the mail list and the selected thread simultaneously. The use of Suspense for both components handles asynchronous loading states elegantly.
However, there's an inconsistency in the loading states - the mail list has a detailed skeleton loader while the thread display just shows "Loading".
Consider replacing the simple "Loading" text with a more detailed skeleton for the thread display to maintain UI consistency:
- <Suspense fallback={<p>Loading</p>}> + <Suspense + fallback={ + <div className="p-4"> + <div className="flex items-center justify-between mb-4"> + <Skeleton className="h-6 w-1/3" /> + <Skeleton className="h-4 w-24" /> + </div> + <Skeleton className="h-4 w-full mb-2" /> + <Skeleton className="h-4 w-3/4 mb-2" /> + <div className="mt-6"> + <Skeleton className="h-32 w-full mb-4" /> + <Skeleton className="h-24 w-full" /> + </div> + </div> + } + > <SRThreadDisplay messages={threadMessages ?? []} /> </Suspense>apps/mail/components/mail/thread-display.tsx (2)
531-536: Remove commented-out code for better readability.This commented-out effect is leftover code from the original component and no longer applies to this implementation.
Remove the commented-out code since it's not being used:
- // useEffect(() => { - // if (emailData?.[0]) { - // setIsMuted(emailData[0].unread ?? false); - // } - // }, [emailData]);
569-661: Remove large block of commented-out code.This large block of commented-out code for loading states is no longer needed since loading is handled by Suspense in the parent component.
Remove this entire commented-out section to improve code readability and maintainability.
apps/mail/components/mail/mail-list.tsx (1)
587-605: Clean up commented-out load more functionality.The LoadMore button is commented out but still references variables like
isLoadingandisValidatingthat no longer exist in this context after the refactoring.Either remove this commented-out code completely or properly update it to work with the new data flow:
- {/* {items.length >= 9 && ( - <Button - variant={'ghost'} - className="w-full rounded-none" - onClick={handleScroll} - disabled={isLoading || isValidating} - > - {isLoading || isValidating ? ( - <div className="flex items-center gap-2"> - <div className="h-4 w-4 animate-spin rounded-full border-2 border-neutral-900 border-t-transparent dark:border-white dark:border-t-transparent" /> - {t('common.actions.loading')} - </div> - ) : ( - <> - {t('common.mail.loadMore')} <ChevronDown /> - </> - )} - </Button> - )} */}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/mail/app/(routes)/mail/[folder]/[threadId]/page.tsx(1 hunks)apps/mail/components/mail/mail-display.tsx(1 hunks)apps/mail/components/mail/mail-iframe.tsx(1 hunks)apps/mail/components/mail/mail-list.tsx(6 hunks)apps/mail/components/mail/note-panel.tsx(1 hunks)apps/mail/components/mail/thread-display.tsx(4 hunks)apps/mail/types/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/mail/components/mail/thread-display.tsx (4)
apps/mail/types/index.ts (1)
ParsedMessage(37-60)apps/mail/hooks/use-mobile.tsx (1)
useIsMobile(5-19)apps/mail/lib/thread-actions.ts (2)
ThreadDestination(4-4)moveThreadsTo(43-81)apps/mail/actions/mail.ts (1)
modifyLabels(80-113)
🔇 Additional comments (15)
apps/mail/components/mail/mail-iframe.tsx (1)
1-1: Appropriate use of 'use client' directiveThe addition of
'use client';correctly designates this component for client-side rendering, which is necessary since it uses React hooks, DOM manipulation via refs, and browser APIs like URL.createObjectURL().apps/mail/components/mail/mail-display.tsx (1)
1-1: Appropriate use of 'use client' directiveThe addition of
'use client';is necessary for this component as it uses React hooks (useState, useEffect, useMemo), contains complex UI logic, and interacts with client-side APIs. This change ensures proper functioning in Next.js's hybrid rendering model.apps/mail/components/mail/note-panel.tsx (1)
1-1: Appropriate use of 'use client' directiveThe addition of
'use client';correctly marks this component for client-side rendering, which is essential as it contains extensive React hooks, state management, DOM interactions, and browser API calls like navigator.clipboard.apps/mail/types/index.ts (1)
94-94: Required items property ensures proper data flowAdding the
items: InitialThread[]property to the MailListProps interface indicates that external thread data is now required by the mail list component. This change improves type safety and explicitly shows the data dependency.apps/mail/app/(routes)/mail/[folder]/[threadId]/page.tsx (4)
1-8: Good addition of necessary imports for the updated component structure.The imports properly reflect the components needed for the new layout structure, with both mail list and thread display components being imported. The use of Suspense for asynchronous loading is a good practice.
10-15: Appropriate interface update to support folder navigation.The
MailPagePropsinterface has been correctly updated to include thefolderparameter, which is necessary for the enhanced functionality of displaying both mail list and thread details.
17-20: Well-structured server component for mail list retrieval.The
SRMailListfunction is well-implemented as an async component that fetches mail data and passes it to the MailList component. The error handling for empty results is handled via the nullish coalescing operator.
22-25: Clean parameter extraction and data fetching.The destructuring of both
threadIdandfolderfrom params is clear, and the renaming ofthreadtothreadMessagesis more descriptive of the data being fetched.apps/mail/components/mail/thread-display.tsx (3)
1-1: Correct use of 'use client' directive.The 'use client' directive is properly placed at the top of the file, indicating that this component uses client-side features like hooks and event handlers.
497-509: Good component structure with appropriate props and hooks.The
SRThreadDisplaycomponent is well-structured with properly typed props. The useState hooks for managing component state (isMuted, isReplyOpen, etc.) and other hooks for mobile detection and route parameters are appropriate.
663-788: Implementation follows best practices for thread display.The component implementation correctly renders the thread messages with appropriate styling and functionality. The handling of different folder states (inbox, archive, spam) is well-implemented.
apps/mail/components/mail/mail-list.tsx (4)
43-43: Good import of demo data for development.Correctly importing the demo data from a JSON file for use in the demo component.
303-309: Updated props to use the imported demo data.The component now properly uses
demo_itemsas the default value forfilteredItemsand the type has been updated accordingly.
331-331: Improved component architecture with props-based data passing.The
MailListcomponent now accepts anitemsprop directly instead of fetching data internally. This separation of concerns improves reusability and testability.
532-540:⚠️ Potential issueFix dependency array in useEffect.
The effect only includes
isFilteringin its dependency array but also usessetSearchValue. This can lead to stale closures.Update the dependency array to include all external values used in the effect:
useEffect(() => { if (isFiltering) { // Reset the search value when loading is complete setSearchValue((prev) => ({ ...prev, isLoading: false, })); } - }, [isFiltering, setSearchValue]); + }, [isFiltering, setSearchValue]);Likely an incorrect or invalid review comment.
MrgSub
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
apps/mail/components/draft/drafts-list.tsx (1)
343-345: 💡 Verification agent🧩 Analysis chain
TODO comment about proper typing needs to be addressed.
There's a TODO comment about adding proper typing and a @ts-expect-error annotation that suggests there are type issues that need to be fixed.
Consider adding proper type definitions for the props parameter to remove the need for the ts-expect-error comment:
🌐 Web query:
What is the proper typing for react-virtuoso row renderer props?💡 Result:
To properly type React Virtuoso row renderer props, use the library's built-in TypeScript interfaces from the
Componentstype. Here's the standard approach:import { Components } from 'react-virtuoso' const CustomRow: Components['Item'] = React.forwardRef<HTMLDivElement, ItemProps>((props, ref) => { return <div {...props} ref={ref}>{/* Your row content */}</div> }) // Usage in Virtuoso: <Virtuoso components={{ Item: CustomRow }} {/* other props */} />Key typing considerations:
- Component Types: Access specific component props through
Components['ComponentName']- Core Props Interface:
interface ItemProps { index: number; // Item index in the list data?: unknown; // Optional data from itemContent context?: unknown; // Optional context passed via Virtuoso style?: CSSProperties; // Required for positioning }
- Forward Refs: Components must use
React.forwardRefand pass it to DOM elements- Data-driven Lists: Use generics when working with typed data:
const TypedRow = React.forwardRef<HTMLDivElement, ItemProps<YourDataType>>(({ data }, ref) => { return <div ref={ref}>{data.yourProperty}</div> })For table rows in
TableVirtuoso, useTableComponentsinterface instead[4][7]:import { TableComponents } from 'react-virtuoso' const TableRow: TableComponents['TableRow'] = React.forwardRef((props, ref) => { return <tr {...props} ref={ref} /> })Remember to:
- Include all DOM props passed by Virtuoso
- Preserve style positioning calculations
- Use optional context prop for state management[6]
- Avoid inline component definitions to prevent unnecessary remounts[1][4]
Citations:
- 1: https://virtuoso.dev/customize-structure/
- 2: https://www.npmjs.com/package/react-virtuoso/v/0.17.0
- 3: https://blog.logrocket.com/3-ways-render-large-datasets-react/
- 4: https://yourbrandmate.agency/blog/how-to-customize-react-virtuoso-table
- 5: https://www.npmjs.com/package/react-virtuoso/v/0.6.0
- 6: petyosi/react-virtuoso#855
- 7: https://virtuoso.dev/virtuoso-api/
- 8: https://virtuoso.dev/virtuoso-message-list-api/interfaces/VirtuosoMessageListProps/
- 9: https://virtuoso.dev/virtuoso-api/interfaces/VirtuosoProps/
- 10: https://virtuoso.dev
Action Required: Update the Row Renderer Typing
Please remove the temporary
@ts-expect-errorand the accompanying TODO comment by adding proper type definitions for the row renderer component. Based on the React Virtuoso documentation, you can type the props using the built-in interfaces from the library. For example:import { Components } from 'react-virtuoso' const CustomRow: Components['Item'] = React.forwardRef<HTMLDivElement, ItemProps>((props, ref) => ( <div {...props} ref={ref}> {/* Your row content */} </div> ))Key Actions:
- Replace the inline anonymous function at apps/mail/components/draft/drafts-list.tsx (lines 343-345) with a properly typed component using the approach above.
- Remove both the
//TODO: Add proper typingcomment and the// @ts-expect-errorannotation once typing is properly implemented.apps/mail/components/mail/search-bar.tsx (1)
92-156:⚠️ Potential issueMissing dependency in submitSearch useCallback
The submitSearch function has dependencies on
routerandfolderthat are not included in the dependency array.- }, [setSearchValue, isAISearching]); + }, [setSearchValue, isAISearching, router, folder]);This could lead to stale closures where the function doesn't have access to the latest values of these dependencies when it runs.
🧹 Nitpick comments (13)
apps/mail/components/mail/thread-subject.tsx (1)
3-3: Remove unused imports.The
Tooltip,TooltipContent, andTooltipTriggercomponents are imported but not used in the component. They should be removed to keep the code clean and avoid unnecessary imports.-import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip';apps/mail/components/mail/compose.button.tsx (1)
12-17: Link component used as a button.The ComposeButton component uses a Link component with styling that makes it look like a button. Consider using the Button component with an "asChild" prop and wrapping it with the Link component for better semantics.
- return ( - <Link - prefetch={true} - href={'/mail/create'} - className="bg-secondary bg-subtleWhite text-primary hover:bg-subtleWhite dark:bg-subtleBlack dark:hover:bg-subtleBlack relative isolate mt-1 h-8 w-[calc(100%)] overflow-hidden whitespace-nowrap shadow-inner" - onMouseEnter={() => iconRef.current?.startAnimation?.()} - onMouseLeave={() => iconRef.current?.stopAnimation?.()} - > + return ( + <Button + asChild + className="bg-secondary bg-subtleWhite text-primary hover:bg-subtleWhite dark:bg-subtleBlack dark:hover:bg-subtleBlack relative isolate mt-1 h-8 w-[calc(100%)] overflow-hidden whitespace-nowrap shadow-inner" + > + <Link + prefetch={true} + href={'/mail/create'} + onMouseEnter={() => iconRef.current?.startAnimation?.()} + onMouseLeave={() => iconRef.current?.stopAnimation?.()} + >apps/mail/hooks/use-threads.ts (1)
37-38: Consider handling potential null/undefined data more safelyThe non-null assertion (
data!) assumes thatdatawill never be null or undefined, which could lead to runtime errors.Consider adding proper null checking or providing a default value:
- return data!; + return data ?? { nextPageToken: undefined, threads: [], resultSizeEstimate: 0 };apps/mail/components/mail/mail.tsx (1)
301-313: Remove commented-out codeThere are large blocks of commented-out code related to the
ResizablePanelandDrawerfunctionality. Commented code increases the file size and can be confusing for other developers.Consider removing these commented sections if they're no longer needed, or add an explanatory comment if they're kept for future reference.
Also applies to: 316-334
apps/mail/components/mail/thread-display.tsx (2)
533-534: Fix missing dependency in useCallbackThe
moveThreadTofunction's dependency array is missingrouter, which could lead to stale closures if the router changes.- }, [threadId, folder, mutateStats]); + }, [threadId, folder, mutateStats, router, handleClose]);
572-580: Add handleClose to useEffect dependenciesThe useEffect for ESC key handling has an empty dependency array but uses
handleClose. This could cause stale closures.- }, []); + }, [handleClose]);apps/mail/app/(routes)/mail/[folder]/[threadId]/page.tsx (1)
24-64: Well-implemented responsive layout with proper loading statesThe two-column layout with appropriate widths for different screen sizes is well designed. Using Suspense for both the mail list and thread display provides a smooth user experience with appropriate loading states.
However, the fallback for the thread display is quite basic compared to the detailed Skeleton UI for the mail list:
- <Suspense fallback={<p>Loading</p>}> + <Suspense fallback={<div className="flex h-full items-center justify-center p-4"> + <div className="w-full max-w-md space-y-4"> + <Skeleton className="h-8 w-3/4" /> + <Skeleton className="h-4 w-full" /> + <Skeleton className="h-32 w-full" /> + <Skeleton className="h-4 w-1/2" /> + </div> + </div>}>apps/mail/types/index.ts (1)
7-13: Clear interface definition for thread retrieval parametersThe
IGetThreadsinterface clearly defines the parameters needed for retrieving mail threads, with appropriate optional fields.However, the
pageTokenparameter type could be more specific thanstring | number | undefined:- pageToken: string | number | undefined; + pageToken?: string | number;This would be more consistent with TypeScript conventions for optional parameters.
apps/mail/components/mail/search-bar.tsx (2)
125-127: Simplified router navigation with query parametersThe current implementation works but could be more consistent and maintainable:
- if (threadId) router.push(`/mail/${folder}/${threadId}/?q=${searchQuery}`); - else router.push(`/mail/${folder}/?q=${searchQuery}`); + const baseUrl = `/mail/${folder}${threadId ? `/${threadId}` : ''}`; + router.push(`${baseUrl}?q=${encodeURIComponent(searchQuery)}`);This approach encodes the search query properly and makes the pattern more maintainable.
173-182: Inconsistent state management in resetSearch functionThe resetSearch function sets isAISearching to false, but this state isn't fully tracked in the component's state:
const resetSearch = useCallback(() => { form.reset(); + setIsAISearching(false); setSearchValue({ value: '', highlight: '', folder: '', isLoading: false, isAISearching: false, }); }, [form, setSearchValue]);This would ensure the local component state stays in sync with the shared state.
apps/mail/app/(routes)/mail/[folder]/page.tsx (2)
13-27: Clean session handling and data fetchingThe simplified session check and structured data fetching make the code more readable and maintainable.
However, there's potential duplication with the thread page. Consider extracting the common data fetching logic:
// In a shared utility file export async function fetchMailData(params, searchParams) { const { folder } = await params; const { pageToken, q, max, labelIds } = await searchParams; return getMails({ folder, q, max, pageToken, labelIds, }); }Then both pages could use this shared function to reduce duplication.
33-64: Well-implemented Suspense boundary with appropriate fallback UIThe use of Suspense with a detailed Skeleton UI for the loading state provides a smooth user experience. The structured passing of props to SRMailList is clear and complete.
Consider adding error boundaries to handle potential failures gracefully:
+import { ErrorBoundary } from '@/components/error-boundary'; // In the return statement + <ErrorBoundary fallback={<div>Failed to load mail list. Please try again.</div>}> <Suspense fallback={ // ... existing fallback } > <SRMailList threads={threadsResponse?.threads} nextPageToken={threadsResponse?.nextPageToken} resultSizeEstimate={threadsResponse?.resultSizeEstimate} /> </Suspense> + </ErrorBoundary>apps/mail/components/mail/mail-list.tsx (1)
523-540: Implemented pagination UI with Back and Next buttons.The pagination UI is cleanly implemented with conditional rendering based on the size of the data set. The Next button properly constructs the URL with pageToken and prev parameters.
However, consider using a more semantic HTML element for pagination to improve accessibility.
-<div className="flex justify-center pb-3"> +<nav aria-label="Mail pagination" className="flex justify-center pb-3">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/mail/actions/mail.ts(1 hunks)apps/mail/app/(routes)/mail/[folder]/[threadId]/page.tsx(1 hunks)apps/mail/app/(routes)/mail/[folder]/list.server.tsx(1 hunks)apps/mail/app/(routes)/mail/[folder]/page.tsx(1 hunks)apps/mail/app/api/driver/types.ts(2 hunks)apps/mail/components/draft/drafts-list.tsx(2 hunks)apps/mail/components/draft/drafts.tsx(1 hunks)apps/mail/components/mail/compose.button.tsx(1 hunks)apps/mail/components/mail/mail-list.tsx(7 hunks)apps/mail/components/mail/mail-quick-actions.tsx(0 hunks)apps/mail/components/mail/mail.tsx(10 hunks)apps/mail/components/mail/search-bar.tsx(7 hunks)apps/mail/components/mail/thread-display.tsx(6 hunks)apps/mail/components/mail/thread-subject.tsx(1 hunks)apps/mail/components/ui/app-sidebar.tsx(0 hunks)apps/mail/components/ui/settings-content.tsx(3 hunks)apps/mail/config/navigation.ts(3 hunks)apps/mail/hooks/use-threads.ts(3 hunks)apps/mail/next.config.ts(1 hunks)apps/mail/types/index.ts(2 hunks)
💤 Files with no reviewable changes (2)
- apps/mail/components/ui/app-sidebar.tsx
- apps/mail/components/mail/mail-quick-actions.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/mail/config/navigation.ts
🧰 Additional context used
🧬 Code Definitions (10)
apps/mail/components/draft/drafts.tsx (1)
apps/mail/components/draft/drafts-list.tsx (1)
DraftsList(83-379)
apps/mail/components/mail/thread-subject.tsx (1)
apps/mail/lib/utils.ts (1)
cn(48-48)
apps/mail/app/(routes)/mail/[folder]/list.server.tsx (2)
apps/mail/types/index.ts (1)
InitialThread(85-97)apps/mail/components/mail/mail-list.tsx (1)
MailList(264-545)
apps/mail/components/mail/thread-display.tsx (3)
apps/mail/types/index.ts (1)
ParsedMessage(53-76)apps/mail/hooks/use-mobile.tsx (1)
useIsMobile(5-19)apps/mail/lib/thread-actions.ts (2)
ThreadDestination(4-4)moveThreadsTo(43-81)
apps/mail/hooks/use-threads.ts (2)
apps/mail/types/index.ts (1)
IGetThreadsResponse(110-114)apps/mail/actions/mail.ts (1)
getMails(5-18)
apps/mail/actions/mail.ts (2)
apps/mail/types/index.ts (1)
IGetThreads(7-13)apps/mail/app/api/driver/google.ts (1)
driver(81-524)
apps/mail/components/mail/search-bar.tsx (2)
apps/mail/hooks/use-search-value.ts (1)
useSearchValue(19-21)apps/mail/lib/utils.ts (1)
cn(48-48)
apps/mail/app/api/driver/types.ts (1)
apps/mail/types/index.ts (1)
IGetThreadsResponse(110-114)
apps/mail/app/(routes)/mail/[folder]/[threadId]/page.tsx (4)
apps/mail/types/index.ts (2)
MailPageProps(15-21)ParsedMessage(53-76)apps/mail/actions/mail.ts (3)
getMails(5-18)getMail(20-32)markAsRead(34-44)apps/mail/app/(routes)/mail/[folder]/list.server.tsx (1)
SRMailList(5-19)apps/mail/components/mail/thread-display.tsx (1)
SRThreadDisplay(503-747)
apps/mail/components/mail/mail.tsx (2)
apps/mail/app/(routes)/mail/layout.tsx (1)
MailLayout(4-12)apps/mail/lib/utils.ts (1)
cn(48-48)
🪛 Biome (1.9.4)
apps/mail/app/(routes)/mail/[folder]/[threadId]/page.tsx
[error] 22-22: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (39)
apps/mail/components/ui/settings-content.tsx (4)
5-5: Import added for AppSidebar component.The AppSidebar import is now explicitly included, which aligns with its usage on line 14.
13-13: Width adjusted to fill available space.Adding
w-fullclass ensures the component takes the full width of its container, improving layout consistency.
15-15: Class ordering updated for background styling.The
dark:bg-blackclass has been moved to the end of the className list. While this doesn't affect functionality, it helps maintain consistent class ordering patterns.
26-26: Class order reorganized for improved readability.The classes have been reordered with the commonly used flexbox and layout classes grouped together, improving code readability.
apps/mail/components/draft/drafts.tsx (1)
144-144: Simplified component interface by removing isCompact prop.The
isCompactprop has been removed from theDraftsListcomponent call. This aligns with changes in theDraftsListimplementation whereisCompactis now hardcoded tofalsein the row renderer, as shown in the provided code snippets.apps/mail/next.config.ts (1)
30-30:Details
✅ Verification successful
Added cache support in experimental features.
Enabling the
useCacheexperimental feature can improve performance by caching responses and reducing redundant computations or network requests, which is particularly valuable for a mail application where data might be accessed frequently.
🌐 Web query:
What does the useCache experimental feature do in Next.js?💡 Result:
The
useCacheexperimental feature in Next.js introduces a new caching mechanism through the'use cache'directive, enabling granular control over caching functions, components, and entire routes. Here's a detailed breakdown:Key Features
Cache Boundaries:
The'use cache'directive marks components/functions as cacheable, serializing their inputs and outputs. This allows Next.js to reuse cached results for subsequent requests with matching parameters[1][4].Multi-Level Caching:
Apply caching at three levels:// File-level caching (all exports) 'use cache' export default async function Page() { ... } // Component-level caching export async function ProductComponent() { 'use cache' return <></> } // Function-level caching export async function getData() { 'use cache' return await fetch('/api/data') }[1][4][7]
Automatic Dependency Tracking:
Next.js automatically detects serializable arguments/props used in cached functions and includes them in cache keys. Non-serializable values become opaque references that don't affect cache validity[4][7].Configuration Requirements
// next.config.js import type { NextConfig } from 'next' const nextConfig: NextConfig = { experimental: { dynamicIO: true, // Required (enables useCache) // or explicitly: useCache: true } }This flag enables opt-in caching behavior, shifting from Next.js' previous cache-by-default approach[5][9].
Advanced Capabilities
Route-Level Caching:
Apply'use cache'to layout/page files to prerender entire routes:'use cache' import Products from '@/components/products' export default function Page() { return <Products /> // Children inherit caching }[3][6]
Revalidation & Cache Control:
- Default cache duration: 15 minutes[4]
- Customize with
cacheLife:import { unstable_cacheLife as cacheLife } from 'next/cache' export async function getData() { 'use cache' cacheLife('hours') // Set expiration return db.query() }
- Invalidate with
revalidateTag()
[4][6]Serializable Data Support:
Unlike earlierunstable_cache, this works with any serializable data including React components, JSX, and rendered output[3][7].Experimental Status
While powerful,
useCacheremains experimental:
- Requires Next.js canary version
- Potential breaking changes in future releases
- Not recommended for production yet[4][6][9]
This feature represents a paradigm shift toward composable caching, enabling fine-grained performance optimizations while maintaining React's component model[7]. Developers should monitor Next.js documentation for updates as the API stabilizes.
Citations:
- 1: https://blog.nidhin.dev/nextjs-use-cache-directive
- 2: https://nextjs.org/blog/next-15-2
- 3: https://www.nextjscourse.dev/courses/nextjs-course/use-cache-deep-dive
- 4: https://nextjs.org/docs/app/api-reference/directives/use-cache
- 5: https://dev.to/jps27cse/nextjs-15-streamlined-performance-and-experimental-power-462a
- 6: https://nextjs.org/blog/our-journey-with-caching
- 7: https://nextjs.org/blog/composable-caching
- 8: https://nextjs.org/docs/14/app/api-reference/next-config-js/partial-prerendering
- 9: https://www.youtube.com/watch?v=plo5-s4z0Js
- 10: vercel/next.js#12054
Verified: Experimental Cache Feature Enabled Correctly
The use of the experimental
useCache: trueflag inapps/mail/next.config.tsis correctly enabling Next.js’s new caching mechanism to optimize performance through caching responses. This configuration is expected to reduce redundant computations and network requests, particularly benefiting the mail application where data is accessed frequently.
- Location:
apps/mail/next.config.ts, line 30- Purpose: Activates experimental caching as described by Next.js’s documentation, leveraging the
'use cache'directive for granular cache control.No further changes are required based on the current implementation.
apps/mail/app/(routes)/mail/[folder]/list.server.tsx (3)
1-4: Appropriate imports for the mail list component.The imports are clean and focused, bringing in only what's needed for the component's functionality.
5-13: Well-structured component interface with proper TypeScript typing.The component parameters are well defined with optional types that properly match their usage in the component. This makes the component flexible while maintaining type safety.
14-19: Clean implementation with proper fallback for missing threads.The component ensures that even if
threadsis undefined, theMailListcomponent receives an empty array, preventing potential rendering issues. The ScrollArea height calculation uses viewport units which provides good responsiveness.apps/mail/components/mail/thread-subject.tsx (1)
17-25: Subject text now displayed without tooltip.The tooltip functionality has been removed from the subject text, simplifying the component. The subject is now displayed directly in a span element with proper classes for styling.
apps/mail/components/draft/drafts-list.tsx (3)
83-83: Function signature simplified by removing isCompact parameter.The DraftsList function no longer accepts the isCompact parameter, simplifying its interface. This is a good change if the component always needs to be displayed in a non-compact mode.
350-350: isCompact prop hardcoded to false.The isCompact prop is now hardcoded to false instead of being passed from a parameter. This is consistent with the function signature change.
355-355: Updated dependency array by removing isCompact.The dependency array for the rowRenderer callback has been correctly updated to remove isCompact, reflecting its removal from the component's logic.
apps/mail/components/mail/compose.button.tsx (1)
2-2:Details
✅ Verification successful
Verify SquarePenIconHandle type has animation methods.
The code uses
startAnimationandstopAnimationmethods on theSquarePenIconHandle, but it's unclear if these methods exist on this type.Run the following script to verify the definition of the
SquarePenIconHandletype:
🏁 Script executed:
#!/bin/bash # Check for the definition of SquarePenIconHandle grep -r "type SquarePenIconHandle" --include="*.tsx" --include="*.ts" . # Check for the implementation of animation methods grep -r "startAnimation\|stopAnimation" --include="*.tsx" --include="*.ts" .Length of output: 12141
SquarePenIconHandle type validation complete
The verification confirms that the
SquarePenIconHandletype (imported fromapps/mail/components/icons/animated/square-pen.tsx) defines the animation methods (startAnimationandstopAnimation) as expected.apps/mail/app/api/driver/types.ts (2)
1-1: Updated import to use IGetThreadsResponse type.The import statement has been updated to import
IGetThreadsResponseinstead ofInitialThread, which aligns with the updated return type of thelistmethod.
10-16: Updated list method signature to use IGetThreadsResponse return type.The
listmethod's return type has been updated toPromise<IGetThreadsResponse | undefined>, which is a more specific type that includes an array of threads along with pagination information. This change improves type safety and makes the API more predictable.apps/mail/actions/mail.ts (1)
12-12:Details
❓ Verification inconclusive
Verify if
undefinedshould replacelabelIdsindriver.listYou've replaced
labelIdswithundefinedin thedriver.listcall. Make sure this is intentional and won't affect filtering functionality.
🏁 Script executed:
#!/bin/bash # Check how the list method uses the labelIds parameter rg -A 5 -B 5 "list.*labelIds" --glob "apps/mail/app/api/**"Length of output: 61
I'm not seeing conclusive evidence from the previous run. To further verify whether
undefinedis intentional for thelabelIdsparameter in thedriver.listcall, let's search the entire repository for any occurrences of"labelIds". This should help us confirm if this parameter is being used or referenced in the driver implementation.
🏁 Script executed:
#!/bin/bash # Recursively search for "labelIds" in the repository to see how and where this parameter is used. rg "labelIds" .Length of output: 1856
Action Required: Verify Label Filtering Behavior in driver.list
It appears that the current change is forwarding
undefinedfor thelabelIdsargument in thedriver.listcall (atapps/mail/actions/mail.ts:12), even though thegetMailsfunction destructureslabelIdsfrom its parameters. Our repository search shows thatlabelIdsis used elsewhere (e.g., inapps/mail/app/api/driver/google.tsandapps/mail/hooks/use-threads.ts) to control filtering (such as determining unread status vialabelIds.includes('UNREAD')). This discrepancy may unintentionally disable filtering based on labels.
- Location:
apps/mail/actions/mail.ts(line 12)
- The call currently reads:
return await driver.list(folder, q, max, undefined, pageToken);- Concern: Since
labelIdsis expected to influence filtering behavior in the driver’s implementation, confirm that forwardingundefined—thus bypassing any label filtering—is intentional.- Recommendation: If label filtering should remain active, consider passing the
labelIdsparameter instead ofundefined. Otherwise, if disabling label filtering is intended, ensure that any unused parameters (likelabelIdsfromgetMails) are either documented or cleaned up to avoid confusion.apps/mail/components/mail/mail.tsx (2)
232-234: Layout structure changed from ResizablePanelGroup to div elementsThe resizable panel functionality has been removed in favor of simpler div elements. This removes the ability for users to adjust panel sizes.
The simplified layout structure is cleaner and easier to maintain, though it removes the resizable functionality.
207-208: LGTM: Component signature updated to use children propThe
MailLayoutcomponent now accepts achildrenprop and uses destructuring fromuseParamsfor route parameters, making it more flexible and consistent with React component patterns.This change improves component composition and better leverages Next.js routing capabilities.
apps/mail/components/mail/thread-display.tsx (1)
503-747: LGTM: New SRThreadDisplay component implementationThe new
SRThreadDisplaycomponent provides a clean implementation for displaying email threads with good separation of concerns, responsive design handling, and proper state management.The component extracts common UI into a reusable
ThreadContentcomponent and properly handles both desktop and mobile views.apps/mail/types/index.ts (4)
15-21: Well-structured page props with proper typingThe
MailPagePropsinterface provides clear typing for both route parameters and search parameters, which aligns well with the component implementations.
109-114: Clear response interface with appropriate documentationThe
IGetThreadsResponseinterface is well documented and provides a clear structure for thread retrieval responses, aligning with the Gmail API convention as commented.
116-120: Improved MailListProps interface with required items propertyThe refactored interface now properly requires the
itemsarray of threads while keeping pagination-related properties optional.
122-122: Consistent string literal formattingThe update to use single quotes for string literals in
MailSelectModeimproves consistency throughout the codebase.apps/mail/components/mail/search-bar.tsx (3)
1-11: Simplified imports with proper Next.js navigation hooksThe updated imports focus on the essential functionality, removing unnecessary items while adding the proper Next.js navigation hooks.
33-56: Improved form initialization with URL parameter supportThe SearchBar component now properly initializes form values from URL parameters, which maintains state across page loads and navigation.
158-171: Simplified input change handlerThe handleInputChange function has been simplified to focus on the essential functionality, which improves readability and maintainability.
apps/mail/components/mail/mail-list.tsx (12)
8-9: Added navigation icons to support new pagination feature.The addition of
ChevronLeftandChevronRighticons supports the new pagination UI elements that appear later in the file.
17-17: Improved type safety with explicit type import.Good addition of
type ComponentPropsto the import statement, ensuring type-safety for the Badge component variants used ingetDefaultBadgeStyle.
35-35: Renamed import from 'items' to 'demo_items' for clarity.The rename provides better clarity by distinguishing demo data from actual email threads. This change is properly propagated throughout the component.
56-59: Enhanced URL parameter extraction for thread navigation.Good addition of folder parameter extraction and search parameters, which improves the navigation between mail threads while preserving the search context.
61-63: Improved thread ID handling with fallback mechanism.The updated logic elegantly handles both cases where a message might have either a dedicated threadId or use its own id as the thread identifier.
151-153: Enhanced Link component with performance optimizations.The addition of
shallowandprefetchproperties improves navigation performance. The href construction now properly includes all necessary parameters to maintain context between navigations.
237-241: Updated MailListDemo to use renamed demo data.Properly updated the component to use the renamed
demo_itemsboth in the default prop value and the type definition.
264-264: Refactored MailList component to receive data as props.This refactoring follows good practices by making the component receive data rather than fetching it internally, improving reusability and testability.
272-274: Added pagination state management.Good addition of variables to track current page token and previous page token, supporting the new pagination feature.
467-476: Simplified search loading state management.The effect has been streamlined to only depend on
isFiltering, removing unnecessary dependencies that were likely related to the removed data fetching logic.
477-478: Added navigation helper for returning to previous page.Simple but effective implementation of the back navigation functionality using router.back().
483-483: Maintained loading indicator for search operations.Good retention of the loading indicator UI for search operations, ensuring users receive visual feedback during searches.
| onMouseEnter={() => () => iconRef.current?.startAnimation?.()} | ||
| onMouseLeave={() => () => iconRef.current?.stopAnimation?.()} |
There was a problem hiding this comment.
Fix double arrow functions in event handlers.
The onMouseEnter and onMouseLeave handlers are using double arrow functions, which will create a function that returns another function but never executes the animation. This is likely causing the animations to not work properly.
- onMouseEnter={() => () => iconRef.current?.startAnimation?.()}
- onMouseLeave={() => () => iconRef.current?.stopAnimation?.()}
+ onMouseEnter={() => iconRef.current?.startAnimation?.()}
+ onMouseLeave={() => iconRef.current?.stopAnimation?.()}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onMouseEnter={() => () => iconRef.current?.startAnimation?.()} | |
| onMouseLeave={() => () => iconRef.current?.stopAnimation?.()} | |
| onMouseEnter={() => iconRef.current?.startAnimation?.()} | |
| onMouseLeave={() => iconRef.current?.stopAnimation?.()} |
| <Button | ||
| disabled | ||
| variant="ghost" | ||
| className="md:h-fit md:px-2" | ||
| onMouseEnter={() => moreVerticalIconRef.current?.startAnimation?.()} | ||
| onMouseLeave={() => moreVerticalIconRef.current?.stopAnimation?.()} | ||
| > |
There was a problem hiding this comment.
Menu trigger is disabled
The dropdown menu trigger button is set to disabled, which prevents users from accessing the dropdown menu options.
- disabled
+ // Remove disabled attribute to make the menu accessible📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button | |
| disabled | |
| variant="ghost" | |
| className="md:h-fit md:px-2" | |
| onMouseEnter={() => moreVerticalIconRef.current?.startAnimation?.()} | |
| onMouseLeave={() => moreVerticalIconRef.current?.stopAnimation?.()} | |
| > | |
| <Button | |
| // Remove disabled attribute to make the menu accessible | |
| variant="ghost" | |
| className="md:h-fit md:px-2" | |
| onMouseEnter={() => moreVerticalIconRef.current?.startAnimation?.()} | |
| onMouseLeave={() => moreVerticalIconRef.current?.stopAnimation?.()} | |
| > |
| export default async function MailPage({ params, searchParams }: MailPageProps) { | ||
| const { threadId, folder } = await params; | ||
| const { pageToken, q, max, labelIds } = await searchParams; | ||
| const threadsResponse = await getMails({ | ||
| folder, | ||
| q, | ||
| max, | ||
| pageToken, | ||
| labelIds, | ||
| }); | ||
|
|
||
| let threadMessages: ParsedMessage[] = []; | ||
| if (threadId) threadMessages = (await getMail({ id: threadId })) ?? []; | ||
| if (threadMessages && threadMessages.some((e) => e.unread)) void markAsRead({ ids: [threadId] }); |
There was a problem hiding this comment.
Clean data fetching with well-structured error handling
The updated MailPage component now properly handles both params and searchParams, fetching the necessary data in a clean and organized manner. The conditional fetching of thread messages is well implemented.
However, there is a potential null reference issue in line 22:
- if (threadMessages && threadMessages.some((e) => e.unread)) void markAsRead({ ids: [threadId] });
+ if (threadMessages?.some((e) => e.unread)) void markAsRead({ ids: [threadId] });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default async function MailPage({ params, searchParams }: MailPageProps) { | |
| const { threadId, folder } = await params; | |
| const { pageToken, q, max, labelIds } = await searchParams; | |
| const threadsResponse = await getMails({ | |
| folder, | |
| q, | |
| max, | |
| pageToken, | |
| labelIds, | |
| }); | |
| let threadMessages: ParsedMessage[] = []; | |
| if (threadId) threadMessages = (await getMail({ id: threadId })) ?? []; | |
| if (threadMessages && threadMessages.some((e) => e.unread)) void markAsRead({ ids: [threadId] }); | |
| export default async function MailPage({ params, searchParams }: MailPageProps) { | |
| const { threadId, folder } = await params; | |
| const { pageToken, q, max, labelIds } = await searchParams; | |
| const threadsResponse = await getMails({ | |
| folder, | |
| q, | |
| max, | |
| pageToken, | |
| labelIds, | |
| }); | |
| let threadMessages: ParsedMessage[] = []; | |
| if (threadId) threadMessages = (await getMail({ id: threadId })) ?? []; | |
| if (threadMessages?.some((e) => e.unread)) void markAsRead({ ids: [threadId] }); | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
ahmetskilinc
left a comment
ahmetskilinc
left a comment
There was a problem hiding this comment.
lgtm - tested locally works much better
3 participants
READ CAREFULLY THEN REMOVE
Remove bullet points that are not relevant.
PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.
Description
Please provide a clear description of your changes.
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Describe the tests you've done:
Security Considerations
For changes involving data or authentication:
Checklist
Additional Notes
Add any other context about the pull request here.
Screenshots/Recordings
Add screenshots or recordings here if applicable.
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit
New Features
ComposeButtonfor creating new mail items with responsive design considerations.Bug Fixes
Refactor
MailLayoutcomponent by removing unnecessary complexity and adjusting rendering logic.