acme/web · Pull Request #247

Add optimistic updates to task list mutations

MO
Mira Okafor
opened 2 days ago
mo/optimistic-tasks → main
+142 / −38 6 files changed

What this PR does

Risk map

useOptimisticTasks.ts TaskList.tsx api/tasks.ts Toast.tsx types/task.ts TaskList.test.tsx
safe worth a look needs attention

Files

src/hooks/useOptimisticTasks.ts
needs attention +58 −0
@@ -0,0 +1,58 @@
1+import { useMutation, useQueryClient } from '@tanstack/react-query';
2+import { updateTask, TaskPatch } from '../api/tasks';
3+import type { Task } from '../types/task';
4+
5+export function useOptimisticTasks(boardId: string) {
6+ const qc = useQueryClient();
7+ const key = ['tasks', boardId];
8+
9+ return useMutation({
10+ mutationFn: (patch: TaskPatch) => updateTask(patch),
11+ onMutate: async (patch) => {
12+ const prev = qc.getQueryData<Task[]>(key);
13+ qc.setQueryData<Task[]>(key, (old = []) =>
14+ old.map(t => t.id === patch.id ? { ...t, ...patch } : t)
15+ );
16+ return { prev };
17+ },
18+ onError: (_e, _p, ctx) => qc.setQueryData(key, ctx?.prev),
19+ });
20+}
line 11

BlockingonMutate doesn’t call qc.cancelQueries(key) first. If a background refetch lands between the optimistic write and the server response, it will clobber the optimistic state and the UI will flicker back to the old value.

line 18

NitRollback restores the list but never surfaces the error. Consider wiring the existing pushToast here so users know the toggle didn’t stick.

src/components/TaskList.tsx
worth a look +31 −24
@@ -42,14 +42,17 @@ export function TaskList({ boardId }: Props) {
42 const { data: tasks } = useTasks(boardId);
43- const [pending, setPending] = useState<string | null>(null);
44-
45- async function toggle(task: Task) {
46- setPending(task.id);
47- await updateTask({ id: task.id, done: !task.done });
48- await refetch();
49- setPending(null);
50- }
43+ const { mutate, isPending } = useOptimisticTasks(boardId);
44+
45+ const toggle = (task: Task) =>
46+ mutate({ id: task.id, done: !task.done });
47
48 return (
49 <ul className="tasks">
50- {tasks?.map(t => <TaskRow key={t.id} task={t} busy={pending === t.id} />)}
50+ {tasks?.map(t => <TaskRow key={t.id} task={t} onToggle={toggle} />)}
51 </ul>
line 43

NitisPending is destructured but never read. Either drop it or pass it to TaskRow so the checkbox can dim while the request is in flight.

src/api/tasks.ts
worth a look +19 −6
@@ -12,10 +12,15 @@ export type TaskPatch = Partial<Task> & { id: string };
12
13-export async function updateTask(patch: TaskPatch) {
14- return http.patch(`/tasks/${patch.id}`, patch);
13+export async function updateTask(
14+ patch: TaskPatch,
15+ key = crypto.randomUUID(),
16+) {
17+ return http.patch(`/tasks/${patch.id}`, patch, {
18+ headers: { 'Idempotency-Key': key },
19+ });
20 }
line 15

BlockingGenerating the idempotency key as a default parameter means retries from the mutation layer get a new key each time, which defeats the purpose. The key should be minted once in onMutate and threaded through.