refactor(web): migrate reaction optimistic updates from cache to UI pattern
Replace cache-level optimistic updates (onMutate with temp IDs) with TQ v5's UI-level pattern (useMutationState + render-time derivation) for both issue-level and comment-level reaction toggles. The cache-level approach caused a race condition: temp IDs in the cache couldn't be deduplicated against real IDs from WS events, causing reaction counts to briefly flash incorrect values (e.g. 0→1→2→1). The UI pattern keeps the cache clean (always server-confirmed data) and derives optimistic state at render time from pending mutation variables. WS events safely update the cache without conflicting with temp data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Naiyuan Qing
parent
1903b886f6
commit
060afc848c
3 changed files with 119 additions and 118 deletions
|
|
@ -334,6 +334,7 @@ export function useDeleteComment(issueId: string) {
|
|||
export function useToggleCommentReaction(issueId: string) {
|
||||
const qc = useQueryClient();
|
||||
return useMutation({
|
||||
mutationKey: ["toggleCommentReaction", issueId] as const,
|
||||
mutationFn: async ({
|
||||
commentId,
|
||||
emoji,
|
||||
|
|
@ -349,73 +350,6 @@ export function useToggleCommentReaction(issueId: string) {
|
|||
}
|
||||
return api.addReaction(commentId, emoji);
|
||||
},
|
||||
onMutate: async ({ commentId, emoji, existing }) => {
|
||||
await qc.cancelQueries({ queryKey: issueKeys.timeline(issueId) });
|
||||
const prev = qc.getQueryData<TimelineEntry[]>(issueKeys.timeline(issueId));
|
||||
|
||||
if (existing) {
|
||||
// Remove
|
||||
qc.setQueryData<TimelineEntry[]>(
|
||||
issueKeys.timeline(issueId),
|
||||
(old) =>
|
||||
old?.map((e) =>
|
||||
e.id === commentId
|
||||
? {
|
||||
...e,
|
||||
reactions: (e.reactions ?? []).filter(
|
||||
(r) => r.id !== existing.id,
|
||||
),
|
||||
}
|
||||
: e,
|
||||
),
|
||||
);
|
||||
} else {
|
||||
// Add temp
|
||||
const tempReaction: Reaction = {
|
||||
id: `temp-${Date.now()}`,
|
||||
comment_id: commentId,
|
||||
actor_type: "",
|
||||
actor_id: "",
|
||||
emoji,
|
||||
created_at: new Date().toISOString(),
|
||||
};
|
||||
qc.setQueryData<TimelineEntry[]>(
|
||||
issueKeys.timeline(issueId),
|
||||
(old) =>
|
||||
old?.map((e) =>
|
||||
e.id === commentId
|
||||
? { ...e, reactions: [...(e.reactions ?? []), tempReaction] }
|
||||
: e,
|
||||
),
|
||||
);
|
||||
}
|
||||
return { prev };
|
||||
},
|
||||
onSuccess: (reaction, { commentId }) => {
|
||||
if (reaction) {
|
||||
// Replace temp with real
|
||||
qc.setQueryData<TimelineEntry[]>(
|
||||
issueKeys.timeline(issueId),
|
||||
(old) =>
|
||||
old?.map((e) =>
|
||||
e.id === commentId
|
||||
? {
|
||||
...e,
|
||||
reactions: (e.reactions ?? []).map((r) =>
|
||||
r.id.startsWith("temp-") && r.emoji === reaction.emoji
|
||||
? reaction
|
||||
: r,
|
||||
),
|
||||
}
|
||||
: e,
|
||||
),
|
||||
);
|
||||
}
|
||||
},
|
||||
onError: (_err, _vars, ctx) => {
|
||||
if (ctx?.prev)
|
||||
qc.setQueryData(issueKeys.timeline(issueId), ctx.prev);
|
||||
},
|
||||
onSettled: () => {
|
||||
qc.invalidateQueries({ queryKey: issueKeys.timeline(issueId) });
|
||||
},
|
||||
|
|
@ -429,6 +363,7 @@ export function useToggleCommentReaction(issueId: string) {
|
|||
export function useToggleIssueReaction(issueId: string) {
|
||||
const qc = useQueryClient();
|
||||
return useMutation({
|
||||
mutationKey: ["toggleIssueReaction", issueId] as const,
|
||||
mutationFn: async ({
|
||||
emoji,
|
||||
existing,
|
||||
|
|
@ -442,48 +377,6 @@ export function useToggleIssueReaction(issueId: string) {
|
|||
}
|
||||
return api.addIssueReaction(issueId, emoji);
|
||||
},
|
||||
onMutate: async ({ emoji, existing }) => {
|
||||
await qc.cancelQueries({ queryKey: issueKeys.reactions(issueId) });
|
||||
const prev = qc.getQueryData<IssueReaction[]>(issueKeys.reactions(issueId));
|
||||
|
||||
if (existing) {
|
||||
qc.setQueryData<IssueReaction[]>(
|
||||
issueKeys.reactions(issueId),
|
||||
(old) => old?.filter((r) => r.id !== existing.id),
|
||||
);
|
||||
} else {
|
||||
const temp: IssueReaction = {
|
||||
id: `temp-${Date.now()}`,
|
||||
issue_id: issueId,
|
||||
actor_type: "",
|
||||
actor_id: "",
|
||||
emoji,
|
||||
created_at: new Date().toISOString(),
|
||||
};
|
||||
qc.setQueryData<IssueReaction[]>(
|
||||
issueKeys.reactions(issueId),
|
||||
(old) => [...(old ?? []), temp],
|
||||
);
|
||||
}
|
||||
return { prev };
|
||||
},
|
||||
onSuccess: (reaction) => {
|
||||
if (reaction) {
|
||||
qc.setQueryData<IssueReaction[]>(
|
||||
issueKeys.reactions(issueId),
|
||||
(old) =>
|
||||
old?.map((r) =>
|
||||
r.id.startsWith("temp-") && r.emoji === reaction.emoji
|
||||
? reaction
|
||||
: r,
|
||||
),
|
||||
);
|
||||
}
|
||||
},
|
||||
onError: (_err, _vars, ctx) => {
|
||||
if (ctx?.prev)
|
||||
qc.setQueryData(issueKeys.reactions(issueId), ctx.prev);
|
||||
},
|
||||
onSettled: () => {
|
||||
qc.invalidateQueries({ queryKey: issueKeys.reactions(issueId) });
|
||||
},
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
"use client";
|
||||
|
||||
import { useCallback } from "react";
|
||||
import { useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import { useCallback, useMemo } from "react";
|
||||
import { useQuery, useQueryClient, useMutationState } from "@tanstack/react-query";
|
||||
import type { IssueReaction } from "@/shared/types";
|
||||
import type {
|
||||
IssueReactionAddedPayload,
|
||||
|
|
@ -13,7 +13,7 @@ import { useWSEvent, useWSReconnect } from "@/features/realtime";
|
|||
|
||||
export function useIssueReactions(issueId: string, userId?: string) {
|
||||
const qc = useQueryClient();
|
||||
const { data: reactions = [], isLoading: loading } = useQuery(
|
||||
const { data: serverReactions = [], isLoading: loading } = useQuery(
|
||||
issueReactionsOptions(issueId),
|
||||
);
|
||||
|
||||
|
|
@ -26,7 +26,7 @@ export function useIssueReactions(issueId: string, userId?: string) {
|
|||
}, [qc, issueId]),
|
||||
);
|
||||
|
||||
// --- WS event handlers ---
|
||||
// --- WS event handlers (update server cache for other users' actions) ---
|
||||
|
||||
useWSEvent(
|
||||
"issue_reaction:added",
|
||||
|
|
@ -70,12 +70,62 @@ export function useIssueReactions(issueId: string, userId?: string) {
|
|||
),
|
||||
);
|
||||
|
||||
// --- Optimistic UI derivation ---
|
||||
// Instead of writing temp data into the cache (which races with WS events),
|
||||
// derive optimistic state at render time from pending mutation variables.
|
||||
|
||||
const pendingVars = useMutationState({
|
||||
filters: {
|
||||
mutationKey: ["toggleIssueReaction", issueId],
|
||||
status: "pending",
|
||||
},
|
||||
select: (m) =>
|
||||
m.state.variables as
|
||||
| { emoji: string; existing: IssueReaction | undefined }
|
||||
| undefined,
|
||||
});
|
||||
|
||||
const reactions = useMemo(() => {
|
||||
if (pendingVars.length === 0) return serverReactions;
|
||||
|
||||
let result = [...serverReactions];
|
||||
for (const vars of pendingVars) {
|
||||
if (!vars) continue;
|
||||
if (vars.existing) {
|
||||
// Pending removal
|
||||
result = result.filter((r) => r.id !== vars.existing!.id);
|
||||
} else {
|
||||
// Pending add — skip if server already has it (WS arrived first)
|
||||
const alreadyExists = result.some(
|
||||
(r) =>
|
||||
r.emoji === vars.emoji &&
|
||||
r.actor_type === "member" &&
|
||||
r.actor_id === userId,
|
||||
);
|
||||
if (!alreadyExists) {
|
||||
result = [
|
||||
...result,
|
||||
{
|
||||
id: `optimistic-${vars.emoji}`,
|
||||
issue_id: issueId,
|
||||
actor_type: "member",
|
||||
actor_id: userId ?? "",
|
||||
emoji: vars.emoji,
|
||||
created_at: new Date().toISOString(),
|
||||
},
|
||||
];
|
||||
}
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}, [serverReactions, pendingVars, issueId, userId]);
|
||||
|
||||
// --- Mutation ---
|
||||
|
||||
const toggleReaction = useCallback(
|
||||
async (emoji: string) => {
|
||||
if (!userId) return;
|
||||
const existing = reactions.find(
|
||||
const existing = serverReactions.find(
|
||||
(r) =>
|
||||
r.emoji === emoji &&
|
||||
r.actor_type === "member" &&
|
||||
|
|
@ -83,7 +133,7 @@ export function useIssueReactions(issueId: string, userId?: string) {
|
|||
);
|
||||
toggleMutation.mutate({ emoji, existing });
|
||||
},
|
||||
[userId, reactions, toggleMutation],
|
||||
[userId, serverReactions, toggleMutation],
|
||||
);
|
||||
|
||||
return { reactions, loading, toggleReaction };
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
"use client";
|
||||
|
||||
import { useState, useCallback } from "react";
|
||||
import { useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import { useState, useCallback, useMemo } from "react";
|
||||
import { useQuery, useQueryClient, useMutationState } from "@tanstack/react-query";
|
||||
import type { Comment, TimelineEntry, Reaction } from "@/shared/types";
|
||||
import type {
|
||||
CommentCreatedPayload,
|
||||
|
|
@ -259,9 +259,67 @@ export function useIssueTimeline(issueId: string, userId?: string) {
|
|||
[deleteCommentMutation],
|
||||
);
|
||||
|
||||
// --- Optimistic UI derivation for comment reactions ---
|
||||
// Instead of writing temp data into the cache (which races with WS events),
|
||||
// derive optimistic state at render time from pending mutation variables.
|
||||
|
||||
const pendingReactionVars = useMutationState({
|
||||
filters: {
|
||||
mutationKey: ["toggleCommentReaction", issueId],
|
||||
status: "pending",
|
||||
},
|
||||
select: (m) =>
|
||||
m.state.variables as
|
||||
| { commentId: string; emoji: string; existing: Reaction | undefined }
|
||||
| undefined,
|
||||
});
|
||||
|
||||
const optimisticTimeline = useMemo(() => {
|
||||
if (pendingReactionVars.length === 0) return timeline;
|
||||
|
||||
return timeline.map((entry) => {
|
||||
const pendingForEntry = pendingReactionVars.filter(
|
||||
(v) => v && v.commentId === entry.id,
|
||||
);
|
||||
if (pendingForEntry.length === 0) return entry;
|
||||
|
||||
let reactions = entry.reactions ?? [];
|
||||
for (const vars of pendingForEntry) {
|
||||
if (!vars) continue;
|
||||
if (vars.existing) {
|
||||
// Pending removal
|
||||
reactions = reactions.filter((r) => r.id !== vars.existing!.id);
|
||||
} else {
|
||||
// Pending add — skip if server already has it (WS arrived first)
|
||||
const alreadyExists = reactions.some(
|
||||
(r) =>
|
||||
r.emoji === vars.emoji &&
|
||||
r.actor_type === "member" &&
|
||||
r.actor_id === userId,
|
||||
);
|
||||
if (!alreadyExists) {
|
||||
reactions = [
|
||||
...reactions,
|
||||
{
|
||||
id: `optimistic-${vars.emoji}`,
|
||||
comment_id: vars.commentId,
|
||||
actor_type: "member",
|
||||
actor_id: userId ?? "",
|
||||
emoji: vars.emoji,
|
||||
created_at: new Date().toISOString(),
|
||||
},
|
||||
];
|
||||
}
|
||||
}
|
||||
}
|
||||
return { ...entry, reactions };
|
||||
});
|
||||
}, [timeline, pendingReactionVars, userId]);
|
||||
|
||||
const toggleReaction = useCallback(
|
||||
async (commentId: string, emoji: string) => {
|
||||
if (!userId) return;
|
||||
// Read from server timeline (not optimistic) to find the real reaction
|
||||
const entry = timeline.find((e) => e.id === commentId);
|
||||
const existing: Reaction | undefined = (entry?.reactions ?? []).find(
|
||||
(r) =>
|
||||
|
|
@ -275,7 +333,7 @@ export function useIssueTimeline(issueId: string, userId?: string) {
|
|||
);
|
||||
|
||||
return {
|
||||
timeline,
|
||||
timeline: optimisticTimeline,
|
||||
loading,
|
||||
submitting,
|
||||
submitComment,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue