fix(web): replace actor_id self-event filtering with idempotent cache updates
actor_id identifies the user, not the browser tab. Filtering WS events by actor_id broke multi-tab sync — other tabs of the same user would silently miss updates. Instead, make all WS cache handlers idempotent (dedup checks on add, no-op on duplicate merge/filter) so mutations and WS events coexist safely without filtering. - WSClient: pass actor_id to event handlers for future per-handler use - use-realtime-sync: remove isSelf() gating from onAny and specific handlers - useCreateIssue: add .some() dedup guard + onSettled invalidation - use-issue-reactions: remove payload-level self-filter (dedup already present) - use-issue-timeline: remove payload-level self-filter on comment:created, reaction:added, reaction:removed (dedup already present) - Clean up useCallback deps that no longer reference userId Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
88c2f4ddc4
commit
d58f6cdb33
7 changed files with 20 additions and 32 deletions
|
|
@ -21,11 +21,14 @@ export function useCreateIssue() {
|
|||
mutationFn: (data: CreateIssueRequest) => api.createIssue(data),
|
||||
onSuccess: (newIssue) => {
|
||||
qc.setQueryData<ListIssuesResponse>(issueKeys.list(wsId), (old) =>
|
||||
old
|
||||
old && !old.issues.some((i) => i.id === newIssue.id)
|
||||
? { ...old, issues: [...old.issues, newIssue], total: old.total + 1 }
|
||||
: old,
|
||||
);
|
||||
},
|
||||
onSettled: () => {
|
||||
qc.invalidateQueries({ queryKey: issueKeys.list(wsId) });
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -204,6 +207,9 @@ export function useCreateComment(issueId: string) {
|
|||
},
|
||||
);
|
||||
},
|
||||
onSettled: () => {
|
||||
qc.invalidateQueries({ queryKey: issueKeys.timeline(issueId) });
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -34,8 +34,6 @@ export function useIssueReactions(issueId: string, userId?: string) {
|
|||
(payload: unknown) => {
|
||||
const { reaction, issue_id } = payload as IssueReactionAddedPayload;
|
||||
if (issue_id !== issueId) return;
|
||||
if (reaction.actor_type === "member" && reaction.actor_id === userId)
|
||||
return;
|
||||
qc.setQueryData<IssueReaction[]>(
|
||||
issueKeys.reactions(issueId),
|
||||
(old) => {
|
||||
|
|
@ -45,7 +43,7 @@ export function useIssueReactions(issueId: string, userId?: string) {
|
|||
},
|
||||
);
|
||||
},
|
||||
[qc, issueId, userId],
|
||||
[qc, issueId],
|
||||
),
|
||||
);
|
||||
|
||||
|
|
@ -55,7 +53,6 @@ export function useIssueReactions(issueId: string, userId?: string) {
|
|||
(payload: unknown) => {
|
||||
const p = payload as IssueReactionRemovedPayload;
|
||||
if (p.issue_id !== issueId) return;
|
||||
if (p.actor_type === "member" && p.actor_id === userId) return;
|
||||
qc.setQueryData<IssueReaction[]>(
|
||||
issueKeys.reactions(issueId),
|
||||
(old) =>
|
||||
|
|
@ -69,7 +66,7 @@ export function useIssueReactions(issueId: string, userId?: string) {
|
|||
),
|
||||
);
|
||||
},
|
||||
[qc, issueId, userId],
|
||||
[qc, issueId],
|
||||
),
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -63,11 +63,6 @@ export function useIssueTimeline(issueId: string, userId?: string) {
|
|||
(payload: unknown) => {
|
||||
const { comment } = payload as CommentCreatedPayload;
|
||||
if (comment.issue_id !== issueId) return;
|
||||
if (
|
||||
comment.author_type === "member" &&
|
||||
comment.author_id === userId
|
||||
)
|
||||
return;
|
||||
qc.setQueryData<TimelineEntry[]>(
|
||||
issueKeys.timeline(issueId),
|
||||
(old) => {
|
||||
|
|
@ -77,7 +72,7 @@ export function useIssueTimeline(issueId: string, userId?: string) {
|
|||
},
|
||||
);
|
||||
},
|
||||
[qc, issueId, userId],
|
||||
[qc, issueId],
|
||||
),
|
||||
);
|
||||
|
||||
|
|
@ -161,11 +156,6 @@ export function useIssueTimeline(issueId: string, userId?: string) {
|
|||
(payload: unknown) => {
|
||||
const { reaction, issue_id } = payload as ReactionAddedPayload;
|
||||
if (issue_id !== issueId) return;
|
||||
if (
|
||||
reaction.actor_type === "member" &&
|
||||
reaction.actor_id === userId
|
||||
)
|
||||
return;
|
||||
qc.setQueryData<TimelineEntry[]>(
|
||||
issueKeys.timeline(issueId),
|
||||
(old) =>
|
||||
|
|
@ -177,7 +167,7 @@ export function useIssueTimeline(issueId: string, userId?: string) {
|
|||
}),
|
||||
);
|
||||
},
|
||||
[qc, issueId, userId],
|
||||
[qc, issueId],
|
||||
),
|
||||
);
|
||||
|
||||
|
|
@ -187,7 +177,6 @@ export function useIssueTimeline(issueId: string, userId?: string) {
|
|||
(payload: unknown) => {
|
||||
const p = payload as ReactionRemovedPayload;
|
||||
if (p.issue_id !== issueId) return;
|
||||
if (p.actor_type === "member" && p.actor_id === userId) return;
|
||||
qc.setQueryData<TimelineEntry[]>(
|
||||
issueKeys.timeline(issueId),
|
||||
(old) =>
|
||||
|
|
@ -207,7 +196,7 @@ export function useIssueTimeline(issueId: string, userId?: string) {
|
|||
}),
|
||||
);
|
||||
},
|
||||
[qc, issueId, userId],
|
||||
[qc, issueId],
|
||||
),
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import { useEffect } from "react";
|
|||
import type { WSEventType } from "@/shared/types";
|
||||
import { useWS } from "./provider";
|
||||
|
||||
type EventHandler = (payload: unknown) => void;
|
||||
type EventHandler = (payload: unknown, actorId?: string) => void;
|
||||
|
||||
/**
|
||||
* Hook that subscribes to a WebSocket event and calls the handler.
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ const WS_URL =
|
|||
? `${window.location.protocol === "https:" ? "wss:" : "ws:"}//${window.location.host}/ws`
|
||||
: "ws://localhost:8080/ws");
|
||||
|
||||
type EventHandler = (payload: unknown) => void;
|
||||
type EventHandler = (payload: unknown, actorId?: string) => void;
|
||||
|
||||
interface WSContextValue {
|
||||
subscribe: (event: WSEventType, handler: EventHandler) => () => void;
|
||||
|
|
|
|||
|
|
@ -86,20 +86,16 @@ export function useRealtimeSync(ws: WSClient | null) {
|
|||
]);
|
||||
|
||||
const unsubAny = ws.onAny((msg) => {
|
||||
const myUserId = useAuthStore.getState().user?.id;
|
||||
if (msg.actor_id && msg.actor_id === myUserId) {
|
||||
logger.debug("skipping self-event", msg.type);
|
||||
return;
|
||||
}
|
||||
if (specificEvents.has(msg.type)) return;
|
||||
const prefix = msg.type.split(":")[0] ?? "";
|
||||
const refresh = refreshMap[prefix];
|
||||
if (refresh) debouncedRefresh(prefix, refresh);
|
||||
});
|
||||
|
||||
// --- Specific event handlers (granular updates, no full refetch) ---
|
||||
// NOTE: ws.on() passes msg.payload (no actor_id). Self-event suppression
|
||||
// requires WSClient changes to expose actor_id — tracked as separate task.
|
||||
// --- Specific event handlers (granular cache updates) ---
|
||||
// No self-event filtering: actor_id identifies the USER, not the TAB.
|
||||
// Filtering by actor_id would block other tabs of the same user.
|
||||
// Instead, both mutations and WS handlers use dedup checks to be idempotent.
|
||||
|
||||
const unsubIssueUpdated = ws.on("issue:updated", (p) => {
|
||||
const { issue } = p as IssueUpdatedPayload;
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import type { WSMessage, WSEventType } from "@/shared/types";
|
||||
import { type Logger, noopLogger } from "@/shared/logger";
|
||||
|
||||
type EventHandler = (payload: unknown) => void;
|
||||
type EventHandler = (payload: unknown, actorId?: string) => void;
|
||||
|
||||
export class WSClient {
|
||||
private ws: WebSocket | null = null;
|
||||
|
|
@ -53,7 +53,7 @@ export class WSClient {
|
|||
const eventHandlers = this.handlers.get(msg.type);
|
||||
if (eventHandlers) {
|
||||
for (const handler of eventHandlers) {
|
||||
handler(msg.payload);
|
||||
handler(msg.payload, msg.actor_id);
|
||||
}
|
||||
}
|
||||
for (const handler of this.anyHandlers) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue