fixes some optimistic action bugs#1171
Conversation
WalkthroughThe updates modify the mail application's folder page loader to redirect to the inbox if the folder parameter is missing, refactor the optimistic actions hook to use toast lifecycle callbacks instead of timeouts for deferred execution, and adjust the memoization dependencies in the threads hook for more precise recalculation based on query updates. Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/mail/hooks/use-threads.ts (1)
38-45: Remove unused dependency from useMemo.The
backgroundQueuevariable is included in the dependency array but is not used within theuseMemocallback. This causes unnecessary recalculations of the threads array whenever the background queue changes.const threads = useMemo(() => { return threadsQuery.data ? threadsQuery.data.pages .flatMap((e) => e.threads) .filter(Boolean) .filter((e) => !isInQueue(`thread:${e.id}`)) : []; - }, [threadsQuery.data, threadsQuery.dataUpdatedAt, isInQueue, backgroundQueue]); + }, [threadsQuery.data, threadsQuery.dataUpdatedAt, isInQueue]);apps/mail/app/(routes)/mail/[folder]/page.tsx (1)
1-1: Remove unused import.The
Navigatecomponent is imported but never used in this file. OnlyuseNavigateis used on line 25.-import { Navigate, useLoaderData, useNavigate } from 'react-router'; +import { useLoaderData, useNavigate } from 'react-router';apps/mail/hooks/use-optimistic-actions.ts (3)
142-149: Remove console.log statements from production code.Debug console.log statements should not be included in production code.
toast(bulkActionMessage, { onAutoClose: () => { - console.log('auto closed'); doAction(); }, onDismiss: () => { - console.log('dismissed'); doAction(); },
141-158: Consider setting explicit toast duration.The toast no longer has an explicit duration set, which might lead to inconsistent behavior across different environments or toast configurations.
toast(bulkActionMessage, { + duration: UNDO_DELAY, onAutoClose: () => {
298-306: Extract duplicated background queue cleanup logic.The background queue deletion logic is duplicated across multiple execute and undo callbacks. Consider extracting this into a helper function.
+ const cleanupBackgroundQueue = useCallback((threadIds: string[]) => { + threadIds.forEach((id) => { + setBackgroundQueue({ type: 'delete', threadId: `thread:${id}` }); + }); + }, [setBackgroundQueue]); // Then in the execute and undo callbacks: - threadIds.forEach((id) => { - setBackgroundQueue({ type: 'delete', threadId: `thread:${id}` }); - }); + cleanupBackgroundQueue(threadIds);Also applies to: 354-363
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/mail/app/(routes)/mail/[folder]/page.tsx(2 hunks)apps/mail/hooks/use-optimistic-actions.ts(5 hunks)apps/mail/hooks/use-threads.ts(1 hunks)
🔇 Additional comments (2)
apps/mail/app/(routes)/mail/[folder]/page.tsx (1)
13-14: Good defensive programming!The early redirect ensures users always land on a valid folder page, preventing potential errors from missing folder parameters.
apps/mail/hooks/use-optimistic-actions.ts (1)
55-57: Correct usage of infiniteQueryKey for infinite queries.Good fix! Using
infiniteQueryKeyinstead ofqueryKeyfor thelistThreadsquery ensures proper invalidation of infinite query data.
Merge activity
|
2 participants
TL;DR
Added redirect for missing folder parameter and fixed query invalidation for mail threads.
What changed?
Navigatecomponent from 'react-router'queryKeytoinfiniteQueryKeyfor thelistThreadsqueryHow to test?
/mail/) and verify it redirects to/mail/inboxWhy make this change?
The redirect ensures users always land on a valid mail folder page even if they access an incomplete URL. The query invalidation fix ensures that the UI properly refreshes after optimistic actions are performed, maintaining data consistency between the client and server.
Summary by CodeRabbit
Bug Fixes
Refactor