apps/mail/hooks/use-optimistic-actions.ts (2)

367-402: ⚠️ Potential issue

Fix the syntax error in optimisticToggleImportant function.

The function is missing proper closure and the dependency array is misplaced. This is not a useCallback hook but a regular function.

   function optimisticToggleImportant(threadIds: string[], isImportant: boolean) {
     if (!threadIds.length) return;

     const optimisticId = addOptimisticAction({
       type: 'IMPORTANT',
       threadIds,
       important: isImportant,
     });

     createPendingAction({
       type: 'IMPORTANT',
       threadIds,
       params: { important: isImportant },
       optimisticId,
       execute: async () => {
         await toggleImportant({ ids: threadIds });

         if (mail.bulkSelected.length > 0) {
           setMail({ ...mail, bulkSelected: [] });
         }
       },
       undo: () => {
         removeOptimisticAction(optimisticId);
       },
       toastMessage: isImportant ? 'Marked as important' : 'Unmarked as important',
     });
-    },
-    [
-      addOptimisticAction,
-      createPendingAction,
-      mail,
-      removeOptimisticAction,
-      setMail,
-      toggleImportant,
-    ],
-  );
+  }

🧰 Tools

🪛 Biome (1.9.4)

[error] 393-402: Expected a statement but instead found ',
[
addOptimisticAction,
createPendingAction,
mail,
removeOptimisticAction,
setMail,
toggleImportant,
],
)'.

Expected a statement here.

(parse)

---

467-487: ⚠️ Potential issue

Fix the undoLastAction implementation.

The function has several issues that need to be addressed:

1. It checks lastActionIdRef.current which is an unused ref
2. Missing closing bracket and dependency array for useCallback

   const undoLastAction = useCallback(() => {
-    if (!lastActionIdRef.current) return;
+    if (!optimisticActionsManager.lastActionId) return;

     const lastAction = optimisticActionsManager.pendingActions.get(
       optimisticActionsManager.lastActionId,
     );
     if (!lastAction) return;

     lastAction.undo();

     optimisticActionsManager.pendingActions.delete(optimisticActionsManager.lastActionId);
     optimisticActionsManager.pendingActionsByType
       .get(lastAction.type)
       ?.delete(optimisticActionsManager.lastActionId);

     if (lastAction.toastId) {
       toast.dismiss(lastAction.toastId);
     }

     optimisticActionsManager.lastActionId = null;
-  }
+  }, []);

apps/mail/components/mail/mail-list.tsx (1)

534-582: Consider simplifying the complex conditional rendering structure.

The Draft component refactor correctly integrates optimistic deletion state, but the nested conditional logic is quite complex and impacts readability.

Consider this cleaner structure:

  return (
-    draft ? (
-      <AnimatePresence mode="sync">
-        {!optimisticState.shouldHide && (<DraftContextMenu draftId={message.id}>
+    draft && !optimisticState.shouldHide ? (
+      <DraftContextMenu draftId={message.id}>
          <div className="select-none py-1" onClick={handleMailClick}>
            {/* draft content */}
          </div>
-        </DraftContextMenu> )}
-      </AnimatePresence>
-    ): null
+      </DraftContextMenu>
+    ) : null

Also, consider whether AnimatePresence is necessary here since you're only conditionally rendering based on shouldHide rather than animating transitions.

apps/server/src/trpc/routes/mail.ts (1)

321-339: Well-implemented bulk deletion with proper error handling.

The mutation correctly uses Promise.allSettled for concurrent operations and provides appropriate error handling. The implementation follows established patterns in the codebase.

Consider enhancing error details to include which specific draft IDs failed:

  const results = await Promise.allSettled(ids.map((id) => driver.deleteDraft(id)));
- const errors = results.filter((result) => result.status === 'rejected');
+ const failures = results
+   .map((result, index) => ({ result, draftId: ids[index] }))
+   .filter(({ result }) => result.status === 'rejected');
  if (errors.length > 0) {
-   return { success: false, error: 'Failed to delete some drafts', details: errors };
+   return { 
+     success: false, 
+     error: 'Failed to delete some drafts', 
+     failedIds: failures.map(f => f.draftId),
+     details: failures.map(f => f.result)
+   };
  }

apps/mail/components/context/draft-context.tsx (1)

1-11: Clean up unused imports.

Several context menu components are imported but not used in this file.

Remove the unused imports:

  import {
    ContextMenu,
    ContextMenuContent,
    ContextMenuItem,
-   ContextMenuSeparator,
-   ContextMenuShortcut,
-   ContextMenuSub,
-   ContextMenuSubContent,
-   ContextMenuSubTrigger,
    ContextMenuTrigger,
  } from '../ui/context-menu';

apps/mail/hooks/use-optimistic-actions.ts (3)

423-423: Remove debug console.log statement.

Debug logging should be removed before merging to production.

-          console.log("DELETING DRAFT FROM OPTIMISTIC", draftIds )

---

95-95: Remove all debug console.log statements.

Multiple debug logging statements should be removed throughout the file.

-    console.log('here Generated pending action ID:', pendingActionId);

-      console.log('here Creating new Set for action type:', type);

-    console.log(
-      'here',
-      'Added pending action to type:',
-      type,
-      'Current size:',
-      optimisticActionsManager.pendingActionsByType.get(type)?.size,
-    );

-        console.log('here', {
-          pendingActionsByTypeRef: optimisticActionsManager.pendingActionsByType.get(type)?.size,
-          pendingActionsRef: optimisticActionsManager.pendingActions.size,
-          typeActions: typeActions?.size,
-        });

Also applies to: 98-98, 102-108, 129-133

---

452-465: Consider optimizing the dependency array.

The optimisticDeleteDrafts callback has many dependencies which could cause frequent re-creations. Consider whether all these dependencies are necessary, particularly mail which might change frequently.

You might want to use mail more selectively or use a ref to avoid unnecessary re-renders:

+  const mailRef = useRef(mail);
+  mailRef.current = mail;

   const optimisticDeleteDrafts = useCallback(
     (draftIds: string[]) => {
       // ... existing code ...
-          if (mail.bulkSelected.length > 0) {
-            setMail({ ...mail, bulkSelected: [] });
+          if (mailRef.current.bulkSelected.length > 0) {
+            setMail({ ...mailRef.current, bulkSelected: [] });
           }

And remove mail from the dependency array.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro