Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes refactor how processed email content and its metadata are handled in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MailContent
participant useThread
participant processEmailContent
User->>MailContent: Render email view
MailContent->>useThread: Fetch thread data
useThread->>processEmailContent: Prefetch processed content (based on trust & theme)
processEmailContent-->>useThread: Return { html, imagesBlocked }
useThread-->>MailContent: Provide processed content
MailContent->>MailContent: useEffect sets cspViolation if imagesBlocked
MailContent->>User: Render processed HTML
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Enhanced email content handling with prefetching optimization and improved security measures in the Mail-0/Zero email client.
- Added smart prefetching in
apps/mail/hooks/use-threads.tsto preprocess HTML content of latest messages using react-query's prefetchQuery - Improved security in
apps/mail/components/mail/mail-content.tsxby separating CSP violation checks into a dedicated useEffect hook - Implemented theme-aware email content processing for better display consistency
- Added trusted sender verification system for safer image loading
- Optimized component state management by returning both processed HTML and image blocking status as a combined object
2 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
| const prefetchEmailContent = async (message: ParsedMessage) => { | ||
| return queryClient.prefetchQuery({ | ||
| queryKey: ['email-content', message.id, isTrustedSender, resolvedTheme], | ||
| queryFn: async () => { | ||
| const result = await processEmailContent({ | ||
| html: message.decodedBody ?? '', | ||
| shouldLoadImages: isTrustedSender, | ||
| theme: (resolvedTheme as 'light' | 'dark') || 'light', | ||
| }); | ||
|
|
||
| return { | ||
| html: result.processedHtml, | ||
| hasBlockedImages: result.hasBlockedImages, | ||
| }; | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
style: prefetchEmailContent should use useCallback to avoid recreation on every render. Also consider adding error handling for failed prefetches
| useEffect(() => { | ||
| if (!threadQuery.data?.latest?.id) return; | ||
| prefetchEmailContent(threadQuery.data.latest); | ||
| }, [threadQuery.data?.latest]); |
There was a problem hiding this comment.
logic: useEffect dependency array missing prefetchEmailContent function. Add it to prevent stale closure issues
| useEffect(() => { | |
| if (!threadQuery.data?.latest?.id) return; | |
| prefetchEmailContent(threadQuery.data.latest); | |
| }, [threadQuery.data?.latest]); | |
| useEffect(() => { | |
| if (!threadQuery.data?.latest?.id) return; | |
| prefetchEmailContent(threadQuery.data.latest); | |
| }, [threadQuery.data?.latest, prefetchEmailContent]); |
MrgSub
force-pushed
the
07-03-prefetch_latest_html_content
branch
from
525b9c9 to
25c1198
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Prefetch Query Key Mismatch Causes Incomplete Caching
A query key mismatch exists between the prefetchEmailContent function in use-threads.ts and the actual email content query in mail-content.tsx. The prefetch uses isTrustedSender as the third element of the ['email-content', messageId, ..., resolvedTheme] query key, while the actual query uses isTrustedSender || temporaryImagesEnabled. This prevents prefetched data from being used when temporaryImagesEnabled is true but isTrustedSender is false, leading to unnecessary re-fetching and potentially incomplete cached content regarding image loading.
apps/mail/hooks/use-threads.ts#L111-L117
Zero/apps/mail/hooks/use-threads.ts
Lines 111 to 117 in 25c1198
Bug: Prefetch Hook Missing Dependencies
The useEffect responsible for prefetching email content has incomplete dependencies. It calls prefetchEmailContent, which captures isTrustedSender, resolvedTheme, processEmailContent, and queryClient. Since isTrustedSender and resolvedTheme are not included in the useEffect's dependency array, the prefetch will not re-run when these values change. This can lead to stale or incorrect data being prefetched and cached, as these values are crucial for the prefetch's queryKey and queryFn.
apps/mail/hooks/use-threads.ts#L128-L132
Zero/apps/mail/hooks/use-threads.ts
Lines 128 to 132 in 25c1198
BugBot free trial expires on July 22, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
1 participant
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
Bug Fixes
Performance