fix(inbox): don't drop messages when ?c= URL param updates

Reports: clicking a conversation sometimes left the message thread
stuck on "No messages yet" until the user hit browser refresh.

Root cause — a three-way race kicked off by any click on a conversation
in the list:

1. handleSelectConversation runs:
      setActiveConversation(B)
      setMessages([])
      router.replace("/inbox?c=B")

2. The URL change flips `deepLinkConvId` (from useSearchParams).
   handleConversationsLoaded is a useCallback with
   [deepLinkConvId], so its identity changes. That cascades into
   ConversationList, whose fetchConversations useCallback has
   [onConversationsLoaded], whose useEffect has [fetchConversations] —
   so the list refetches conversations purely because the URL changed.

3. When the refetch lands, handleConversationsLoaded runs its deep-link
   auto-select branch. autoSelectedForDeepLinkRef still points at its
   previous value (null, or the previous deep-link), so the guard
   `ref !== deepLinkConvId` passes and the branch re-applies B:
      setActiveConversation(B)     // no-op, same id
      setMessages([])              // BLOWS AWAY the messages
                                   // MessageThread just fetched

4. MessageThread's fetch effect only re-runs when conversationId
   changes. B is still B. It does not refetch. Messages stuck at [].

A hard reload unsticks things because state starts from scratch and
the initial deep-link flow is the only path that runs the auto-select.

Three defense-in-depth fixes:

- handleSelectConversation now writes the clicked id into
  autoSelectedForDeepLinkRef BEFORE the router.replace, so by the
  time any downstream refetch lands, the auto-select branch sees
  ref === deepLinkConvId and skips.

- handleConversationsLoaded early-returns if the deep-linked
  conversation is already the active one. This is redundant with
  the ref update above but protects against any other code path
  we don't foresee triggering a refetch.

- ConversationList's fetch is now driven by a plain useEffect with
  [] deps, using a ref for the latest onConversationsLoaded
  callback. URL changes no longer cause a conversations refetch.
  Matches the same stabilisation pattern used in
  src/hooks/use-auth.tsx and src/components/inbox/message-thread.tsx.

Verified: typecheck passes; lint count unchanged (37 pre-existing
problems, zero added). Couldn't exercise the rendered /inbox from the
preview sandbox — it's auth-gated and no test credentials are
available. Please confirm in the authenticated inbox:
  - Click back and forth between two conversations with different
    message histories; messages should populate every time.
  - Open /inbox?c=<id> directly; thread opens with messages.
  - Switch conversations after a deep-link landing; still works.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Arnas Donauskas 2026-04-20 20:01:59 +03:00
parent 460b5bf259
commit 6cceb81c46
No known key found for this signature in database
2 changed files with 61 additions and 26 deletions

View File

@ -163,6 +163,16 @@ export default function InboxPage() {
loaded.length > 0 loaded.length > 0
) { ) {
autoSelectedForDeepLinkRef.current = deepLinkConvId; autoSelectedForDeepLinkRef.current = deepLinkConvId;
// If the deep-linked conversation is already the active one
// (e.g. because the user clicked it in the list and we
// router.replace()'d the URL, which made the ConversationList
// refetch and land us back here), do NOT re-apply it. Doing so
// would setMessages([]) on a thread whose messages have
// already been loaded by MessageThread — and because
// conversationId didn't change, MessageThread wouldn't
// refetch. The thread would read "No messages yet" until a
// full page reload rehydrated state from scratch.
if (activeConversation?.id === deepLinkConvId) return;
const match = loaded.find((c) => c.id === deepLinkConvId); const match = loaded.find((c) => c.id === deepLinkConvId);
if (match) { if (match) {
setActiveConversation(match); setActiveConversation(match);
@ -171,7 +181,7 @@ export default function InboxPage() {
} }
} }
}, },
[deepLinkConvId] [deepLinkConvId, activeConversation?.id]
); );
const handleSelectConversation = useCallback( const handleSelectConversation = useCallback(
@ -184,6 +194,14 @@ export default function InboxPage() {
setActiveConversation(conv); setActiveConversation(conv);
setActiveContact(conv.contact ?? null); setActiveContact(conv.contact ?? null);
setMessages([]); setMessages([]);
// Record the selection on the deep-link ref BEFORE we change the
// URL. The router.replace below flips `deepLinkConvId`, which can
// in turn cause ConversationList to refetch and eventually call
// handleConversationsLoaded again. Without this line, the ref
// still points at the previous value, the auto-select block
// sees `ref !== deepLinkConvId`, fires a second time, and
// clobbers the messages MessageThread just fetched.
autoSelectedForDeepLinkRef.current = conv.id;
// Reflect the selection in the URL so a refresh lands the user // Reflect the selection in the URL so a refresh lands the user
// back in the same thread, and so copy-paste links work. Use // back in the same thread, and so copy-paste links work. Use
// replace() to avoid polluting browser history with every click. // replace() to avoid polluting browser history with every click.

View File

@ -1,6 +1,6 @@
"use client"; "use client";
import { useState, useEffect, useCallback, useMemo } from "react"; import { useState, useEffect, useCallback, useMemo, useRef } from "react";
import { createClient } from "@/lib/supabase/client"; import { createClient } from "@/lib/supabase/client";
import { cn } from "@/lib/utils"; import { cn } from "@/lib/utils";
import type { Conversation, ConversationStatus } from "@/types"; import type { Conversation, ConversationStatus } from "@/types";
@ -46,32 +46,49 @@ export function ConversationList({
const [filter, setFilter] = useState<ConversationStatus | "all">("all"); const [filter, setFilter] = useState<ConversationStatus | "all">("all");
const [loading, setLoading] = useState(true); const [loading, setLoading] = useState(true);
const fetchConversations = useCallback(async () => { // Keep the latest callback in a ref so the fetch effect below can
const supabase = createClient(); // have a stable, empty-dep identity. Previously the fetch useCallback
const { data, error } = await supabase // depended on `onConversationsLoaded`, which depends on the parent's
.from("conversations") // `deepLinkConvId` — so every URL change (including one the parent
.select("*, contact:contacts(*)") // triggered via router.replace after a click) caused a fresh
.order("last_message_at", { ascending: false }); // conversations fetch. That extra refetch was the trigger for the
// deep-link auto-select running a second time and wiping the active
if (error) { // thread's messages.
// Supabase errors have non-enumerable properties — log fields explicitly const onConversationsLoadedRef = useRef(onConversationsLoaded);
console.error("Failed to fetch conversations:", { onConversationsLoadedRef.current = onConversationsLoaded;
message: error.message,
details: error.details,
hint: error.hint,
code: error.code,
});
setLoading(false);
return;
}
onConversationsLoaded(data ?? []);
setLoading(false);
}, [onConversationsLoaded]);
useEffect(() => { useEffect(() => {
fetchConversations(); const supabase = createClient();
}, [fetchConversations]); let cancelled = false;
(async () => {
const { data, error } = await supabase
.from("conversations")
.select("*, contact:contacts(*)")
.order("last_message_at", { ascending: false });
if (cancelled) return;
if (error) {
// Supabase errors have non-enumerable properties — log fields explicitly
console.error("Failed to fetch conversations:", {
message: error.message,
details: error.details,
hint: error.hint,
code: error.code,
});
setLoading(false);
return;
}
onConversationsLoadedRef.current(data ?? []);
setLoading(false);
})();
return () => {
cancelled = true;
};
}, []);
const filtered = useMemo(() => { const filtered = useMemo(() => {
let result = conversations; let result = conversations;