mirror of
https://github.com/Luchexx/wacrm.git
synced 2026-11-02 17:29:45 +00:00
fix(inbox): replace broken updater-closure flag, add reconnect/visibility resync
Addresses #106 (issue stays open until the reporter confirms in prod). #105's `let foundInList = false; setState(p => { foundInList = ...; ... })` pattern is broken: setState updaters run during reconciliation, *after* the synchronous handler returns, so `foundInList` reads `false` in the same tick and `hydrateConversation()` fires on every code path the flag was meant to gate. Every incoming message and every conversation UPDATE (read receipts, status flips, agent assignment) triggered a redundant DB fetch, queuing HTTP behind realtime WS frames on the same supabase client and starving the channel — which surfaced as exactly what #106 reports: messages that never arrive, threads stuck on "No messages yet", events landing out of order. The fix: 1. Synchronous known-IDs ref. `knownConvIdsRef` mirrors the conv ids currently in state, kept in sync via a useEffect on `conversations`. Event handlers check it synchronously to decide between a cheap in-place patch and a hydrate refetch — no more closure-flag pattern. 2. Reconnect + visibility resync. Realtime is best-effort: events sent while the WS was disconnected or while the tab was background-throttled are gone. A `resyncToken` state bumped on (a) `isConnected` false→true transitions after the initial connect and (b) tab visibility → visible drives `ConversationList` and `MessageThread` to refire their fetch effects. The reactions effect in MessageThread is split into fetch + channel-subscribe so a resync only refetches the rows without tearing down and rebuilding the realtime channel. 3. Keep the conv-INSERT hydrate. Still the right fix for #104 (realtime payloads never carry the `contact` join); now it only fires when the conversation is genuinely unknown to state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
aecad2e769
commit
138ae8d6f4
@ -30,6 +30,14 @@ export default function InboxPage() {
|
|||||||
const [whatsappConnected, setWhatsappConnected] = useState<boolean | null>(
|
const [whatsappConnected, setWhatsappConnected] = useState<boolean | null>(
|
||||||
null
|
null
|
||||||
);
|
);
|
||||||
|
/**
|
||||||
|
* Bumped whenever we want children (ConversationList, MessageThread)
|
||||||
|
* to refetch from the DB — used as a safety net against missed
|
||||||
|
* realtime events. Bumped on WS reconnect and on tab visibility →
|
||||||
|
* visible. The initial mount fetches don't depend on this; they fire
|
||||||
|
* once on conversationId-change as usual.
|
||||||
|
*/
|
||||||
|
const [resyncToken, setResyncToken] = useState(0);
|
||||||
|
|
||||||
// Fire the deep-link auto-select exactly once per URL — subsequent
|
// Fire the deep-link auto-select exactly once per URL — subsequent
|
||||||
// list refreshes (realtime, manual refetch) must not snap the user
|
// list refreshes (realtime, manual refetch) must not snap the user
|
||||||
@ -43,6 +51,24 @@ export default function InboxPage() {
|
|||||||
// new conversation even when both events arrive within milliseconds.
|
// new conversation even when both events arrive within milliseconds.
|
||||||
const hydratingConvIdsRef = useRef<Set<string>>(new Set());
|
const hydratingConvIdsRef = useRef<Set<string>>(new Set());
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Synchronous mirror of the conversation ids currently in `conversations`
|
||||||
|
* state. Event handlers need to know "do we already have this conv?"
|
||||||
|
* without waiting for a setState updater to run — updaters fire during
|
||||||
|
* reconciliation, *after* the synchronous handler code returns, so a
|
||||||
|
* `let foundInList = false; setState(p => { foundInList = ...; return ... })`
|
||||||
|
* flag reads as `false` in the same tick (this exact bug shipped in #105
|
||||||
|
* and caused #106: every incoming message and every status flip fired a
|
||||||
|
* redundant DB hydrate, swamping the supabase client and starving the
|
||||||
|
* realtime channel). The ref is kept in sync via the effect below.
|
||||||
|
*/
|
||||||
|
const knownConvIdsRef = useRef<Set<string>>(new Set());
|
||||||
|
useEffect(() => {
|
||||||
|
const next = new Set<string>();
|
||||||
|
for (const c of conversations) next.add(c.id);
|
||||||
|
knownConvIdsRef.current = next;
|
||||||
|
}, [conversations]);
|
||||||
|
|
||||||
// Pull the conversation row with its `contact` joined and merge it
|
// Pull the conversation row with its `contact` joined and merge it
|
||||||
// into state. Needed because Supabase Realtime payloads only carry the
|
// into state. Needed because Supabase Realtime payloads only carry the
|
||||||
// row's own columns — a brand-new conversation arrives without a
|
// row's own columns — a brand-new conversation arrives without a
|
||||||
@ -142,32 +168,33 @@ export default function InboxPage() {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Update conversation list preview. If the conv isn't in state
|
// Update conversation list preview. We need to know *synchronously*
|
||||||
// yet (its INSERT event was missed, delayed, or this is the
|
// whether the conv is already in state to decide between patching
|
||||||
// first message for a conversation created out-of-band) the
|
// the preview and triggering a hydrate — see the comment on
|
||||||
// map() below is a no-op and the preview would never appear —
|
// knownConvIdsRef for why a closure flag inside the updater would
|
||||||
// hydrate the row from the DB instead so the inbox self-heals.
|
// always read false here.
|
||||||
let foundInList = false;
|
if (knownConvIdsRef.current.has(newMsg.conversation_id)) {
|
||||||
setConversations((prev) => {
|
setConversations((prev) =>
|
||||||
if (!prev.some((c) => c.id === newMsg.conversation_id)) {
|
prev.map((c) =>
|
||||||
return prev;
|
c.id === newMsg.conversation_id
|
||||||
}
|
? {
|
||||||
foundInList = true;
|
...c,
|
||||||
return prev.map((c) =>
|
last_message_text: newMsg.content_text ?? "",
|
||||||
c.id === newMsg.conversation_id
|
last_message_at: newMsg.created_at,
|
||||||
? {
|
unread_count:
|
||||||
...c,
|
activeConversation?.id === newMsg.conversation_id
|
||||||
last_message_text: newMsg.content_text ?? "",
|
? 0
|
||||||
last_message_at: newMsg.created_at,
|
: c.unread_count + 1,
|
||||||
unread_count:
|
}
|
||||||
activeConversation?.id === newMsg.conversation_id
|
: c,
|
||||||
? 0
|
),
|
||||||
: c.unread_count + 1,
|
|
||||||
}
|
|
||||||
: c,
|
|
||||||
);
|
);
|
||||||
});
|
} else {
|
||||||
if (!foundInList) {
|
// First time we're seeing this conv: the conv-INSERT event
|
||||||
|
// hasn't landed yet, or was missed. Hydrate from the DB so
|
||||||
|
// the row surfaces with its `contact` joined; the conv-UPDATE
|
||||||
|
// event the webhook emits right after the message INSERT will
|
||||||
|
// converge state when it arrives.
|
||||||
hydrateConversation(newMsg.conversation_id);
|
hydrateConversation(newMsg.conversation_id);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -192,27 +219,30 @@ export default function InboxPage() {
|
|||||||
const conv = event.new;
|
const conv = event.new;
|
||||||
|
|
||||||
if (event.eventType === "INSERT") {
|
if (event.eventType === "INSERT") {
|
||||||
// Prepend immediately for snappy UX, then hydrate to fill in
|
// Prepend immediately for snappy UX so the new conv shows in the
|
||||||
// the `contact` join (realtime payloads omit it). The dedupe
|
// list right away, then hydrate to fill in the `contact` join
|
||||||
// inside hydrateConversation keeps the result idempotent if
|
// (realtime payloads never include joins). Skip both if we
|
||||||
// the first-message-INSERT handler also triggers a hydrate.
|
// already have the row — that shouldn't happen normally, but
|
||||||
setConversations((prev) => {
|
// out-of-order delivery would have us prepending a duplicate.
|
||||||
if (prev.some((c) => c.id === conv.id)) return prev;
|
if (!knownConvIdsRef.current.has(conv.id)) {
|
||||||
return [conv, ...prev];
|
setConversations((prev) => {
|
||||||
});
|
if (prev.some((c) => c.id === conv.id)) return prev;
|
||||||
hydrateConversation(conv.id);
|
return [conv, ...prev];
|
||||||
|
});
|
||||||
|
hydrateConversation(conv.id);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (event.eventType === "UPDATE") {
|
if (event.eventType === "UPDATE") {
|
||||||
let foundInList = false;
|
if (knownConvIdsRef.current.has(conv.id)) {
|
||||||
setConversations((prev) => {
|
setConversations((prev) =>
|
||||||
if (!prev.some((c) => c.id === conv.id)) return prev;
|
prev.map((c) => (c.id === conv.id ? { ...c, ...conv } : c)),
|
||||||
foundInList = true;
|
);
|
||||||
return prev.map((c) => (c.id === conv.id ? { ...c, ...conv } : c));
|
} else {
|
||||||
});
|
// UPDATE arrived before the INSERT (or after a missed INSERT)
|
||||||
if (!foundInList) {
|
// — fetch the row so it surfaces with its contact joined. The
|
||||||
// UPDATE arrived before the INSERT (or after a missed
|
// patch contained in `conv` will already be reflected in what
|
||||||
// INSERT) — fetch the row so it surfaces with its contact.
|
// the hydrate fetch returns.
|
||||||
hydrateConversation(conv.id);
|
hydrateConversation(conv.id);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -227,14 +257,59 @@ export default function InboxPage() {
|
|||||||
[activeConversation, hydrateConversation]
|
[activeConversation, hydrateConversation]
|
||||||
);
|
);
|
||||||
|
|
||||||
// Subscribe to realtime
|
// Subscribe to realtime. The `isConnected` flag below feeds the
|
||||||
useRealtime({
|
// reconnect resync: realtime is best-effort and events sent while the
|
||||||
|
// WS was disconnected (laptop sleep, network blip, background-tab
|
||||||
|
// throttle) are simply lost. We need a way to catch up.
|
||||||
|
const { isConnected } = useRealtime({
|
||||||
channelName: "inbox-realtime",
|
channelName: "inbox-realtime",
|
||||||
onMessageEvent: handleMessageEvent,
|
onMessageEvent: handleMessageEvent,
|
||||||
onConversationEvent: handleConversationEvent,
|
onConversationEvent: handleConversationEvent,
|
||||||
enabled: true,
|
enabled: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Bump `resyncToken` whenever the realtime channel transitions from
|
||||||
|
* disconnected → connected *after* the initial connect. The initial
|
||||||
|
* connect is covered by the children's on-mount fetches; only later
|
||||||
|
* reconnects need a manual refetch to fill the gap.
|
||||||
|
*
|
||||||
|
* Tracked via a `was-connected` ref rather than a count so that React
|
||||||
|
* strict-mode's dev-only effect double-fire doesn't read as a
|
||||||
|
* reconnect.
|
||||||
|
*/
|
||||||
|
const wasConnectedRef = useRef(false);
|
||||||
|
const initialConnectDoneRef = useRef(false);
|
||||||
|
useEffect(() => {
|
||||||
|
if (isConnected && !wasConnectedRef.current) {
|
||||||
|
// false → true transition
|
||||||
|
if (initialConnectDoneRef.current) {
|
||||||
|
setResyncToken((n) => n + 1);
|
||||||
|
} else {
|
||||||
|
initialConnectDoneRef.current = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
wasConnectedRef.current = isConnected;
|
||||||
|
}, [isConnected]);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Refetch when the tab regains focus. Background tabs may have their
|
||||||
|
* WS throttled by the browser even without a full disconnect, so a
|
||||||
|
* visibilitychange → visible is a reliable signal that we may have
|
||||||
|
* missed events. Cheap to fire; the children dedupe on their own.
|
||||||
|
*/
|
||||||
|
useEffect(() => {
|
||||||
|
const onVisibility = () => {
|
||||||
|
if (document.visibilityState === "visible") {
|
||||||
|
setResyncToken((n) => n + 1);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
document.addEventListener("visibilitychange", onVisibility);
|
||||||
|
return () => {
|
||||||
|
document.removeEventListener("visibilitychange", onVisibility);
|
||||||
|
};
|
||||||
|
}, []);
|
||||||
|
|
||||||
const handleConversationsLoaded = useCallback(
|
const handleConversationsLoaded = useCallback(
|
||||||
(loaded: Conversation[]) => {
|
(loaded: Conversation[]) => {
|
||||||
setConversations(loaded);
|
setConversations(loaded);
|
||||||
@ -397,6 +472,7 @@ export default function InboxPage() {
|
|||||||
onSelect={handleSelectConversation}
|
onSelect={handleSelectConversation}
|
||||||
conversations={conversations}
|
conversations={conversations}
|
||||||
onConversationsLoaded={handleConversationsLoaded}
|
onConversationsLoaded={handleConversationsLoaded}
|
||||||
|
resyncToken={resyncToken}
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
@ -420,6 +496,7 @@ export default function InboxPage() {
|
|||||||
onStatusChange={handleStatusChange}
|
onStatusChange={handleStatusChange}
|
||||||
onAssignChange={handleAssignChange}
|
onAssignChange={handleAssignChange}
|
||||||
onBack={handleCloseConversation}
|
onBack={handleCloseConversation}
|
||||||
|
resyncToken={resyncToken}
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
|||||||
@ -21,6 +21,13 @@ interface ConversationListProps {
|
|||||||
onSelect: (conversation: Conversation) => void;
|
onSelect: (conversation: Conversation) => void;
|
||||||
conversations: Conversation[];
|
conversations: Conversation[];
|
||||||
onConversationsLoaded: (conversations: Conversation[]) => void;
|
onConversationsLoaded: (conversations: Conversation[]) => void;
|
||||||
|
/**
|
||||||
|
* Increment to force the fetch effect below to refire. The parent
|
||||||
|
* bumps this on realtime reconnect / tab visibility → visible so the
|
||||||
|
* list catches up on any events sent while the WS was disconnected
|
||||||
|
* or the tab was throttled. Optional so existing callers keep working.
|
||||||
|
*/
|
||||||
|
resyncToken?: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
const STATUS_COLORS: Record<ConversationStatus, string> = {
|
const STATUS_COLORS: Record<ConversationStatus, string> = {
|
||||||
@ -41,6 +48,7 @@ export function ConversationList({
|
|||||||
onSelect,
|
onSelect,
|
||||||
conversations,
|
conversations,
|
||||||
onConversationsLoaded,
|
onConversationsLoaded,
|
||||||
|
resyncToken = 0,
|
||||||
}: ConversationListProps) {
|
}: ConversationListProps) {
|
||||||
const [search, setSearch] = useState("");
|
const [search, setSearch] = useState("");
|
||||||
const [filter, setFilter] = useState<ConversationStatus | "all">("all");
|
const [filter, setFilter] = useState<ConversationStatus | "all">("all");
|
||||||
@ -94,7 +102,10 @@ export function ConversationList({
|
|||||||
return () => {
|
return () => {
|
||||||
cancelled = true;
|
cancelled = true;
|
||||||
};
|
};
|
||||||
}, []);
|
// `resyncToken` is included so the parent can force a refetch when
|
||||||
|
// the realtime channel reconnects or the tab regains focus — catches
|
||||||
|
// up on any events sent while the WS was disconnected or throttled.
|
||||||
|
}, [resyncToken]);
|
||||||
|
|
||||||
const filtered = useMemo(() => {
|
const filtered = useMemo(() => {
|
||||||
let result = conversations;
|
let result = conversations;
|
||||||
|
|||||||
@ -70,6 +70,14 @@ interface MessageThreadProps {
|
|||||||
* mobile only.
|
* mobile only.
|
||||||
*/
|
*/
|
||||||
onBack?: () => void;
|
onBack?: () => void;
|
||||||
|
/**
|
||||||
|
* Increment to force the messages + reactions fetch effects to refire.
|
||||||
|
* Parent bumps this on realtime reconnect / tab visibility → visible
|
||||||
|
* so the open thread catches up on any events sent while the WS was
|
||||||
|
* disconnected or the tab was throttled. Optional so existing callers
|
||||||
|
* keep working.
|
||||||
|
*/
|
||||||
|
resyncToken?: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
function formatDateSeparator(dateStr: string): string {
|
function formatDateSeparator(dateStr: string): string {
|
||||||
@ -112,6 +120,7 @@ export function MessageThread({
|
|||||||
onStatusChange,
|
onStatusChange,
|
||||||
onAssignChange,
|
onAssignChange,
|
||||||
onBack,
|
onBack,
|
||||||
|
resyncToken = 0,
|
||||||
}: MessageThreadProps) {
|
}: MessageThreadProps) {
|
||||||
const { user } = useAuth();
|
const { user } = useAuth();
|
||||||
const [loading, setLoading] = useState(false);
|
const [loading, setLoading] = useState(false);
|
||||||
@ -219,12 +228,16 @@ export function MessageThread({
|
|||||||
return () => {
|
return () => {
|
||||||
cancelled = true;
|
cancelled = true;
|
||||||
};
|
};
|
||||||
}, [conversationId]);
|
// `resyncToken` is included so the parent can force a refetch when
|
||||||
|
// the realtime channel reconnects or the tab regains focus —
|
||||||
|
// realtime is best-effort and any message events sent while the WS
|
||||||
|
// was disconnected or throttled are otherwise lost.
|
||||||
|
}, [conversationId, resyncToken]);
|
||||||
|
|
||||||
// Reactions: fetch + realtime per conversation. Subscribing here (not at
|
// Reactions fetch — pulls the current state from the DB. Kept separate
|
||||||
// the page level) keeps the channel scoped to the visible conversation,
|
// from the channel subscription below so a `resyncToken` bump just
|
||||||
// matching the message fetch effect above and avoiding cross-conversation
|
// refetches the rows without also tearing down and rebuilding the
|
||||||
// chatter on a busy inbox.
|
// realtime channel.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!conversationId) {
|
if (!conversationId) {
|
||||||
setReactions([]);
|
setReactions([]);
|
||||||
@ -246,6 +259,18 @@ export function MessageThread({
|
|||||||
setReactions((data as MessageReaction[]) ?? []);
|
setReactions((data as MessageReaction[]) ?? []);
|
||||||
})();
|
})();
|
||||||
|
|
||||||
|
return () => {
|
||||||
|
cancelled = true;
|
||||||
|
};
|
||||||
|
}, [conversationId, resyncToken]);
|
||||||
|
|
||||||
|
// Reactions realtime subscription per conversation. Subscribing here
|
||||||
|
// (not at the page level) keeps the channel scoped to the visible
|
||||||
|
// conversation and avoids cross-conversation chatter on a busy inbox.
|
||||||
|
useEffect(() => {
|
||||||
|
if (!conversationId) return;
|
||||||
|
const supabase = createClient();
|
||||||
|
|
||||||
const channel = supabase
|
const channel = supabase
|
||||||
.channel(`reactions:${conversationId}`)
|
.channel(`reactions:${conversationId}`)
|
||||||
.on(
|
.on(
|
||||||
@ -308,7 +333,6 @@ export function MessageThread({
|
|||||||
.subscribe();
|
.subscribe();
|
||||||
|
|
||||||
return () => {
|
return () => {
|
||||||
cancelled = true;
|
|
||||||
supabase.removeChannel(channel);
|
supabase.removeChannel(channel);
|
||||||
};
|
};
|
||||||
}, [conversationId]);
|
}, [conversationId]);
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user