From 5bc7cb9a7445fe59f2ca89b8ab989a20bcdd12a2 Mon Sep 17 00:00:00 2001 From: Kalle <38327916+Sendouc@users.noreply.github.com> Date: Sun, 20 Sep 2026 11:38:17 +0300 Subject: [PATCH] Chat initial load to loader to avoid layout shift --- app/components/MobileNav.tsx | 4 +- app/components/layout/ChatSidebar.tsx | 5 +- app/components/layout/LazyChatSidebar.tsx | 18 - app/components/layout/index.tsx | 6 +- app/features/chat/ChatProvider.tsx | 125 ++++--- .../chat/RouteChatRooms.server.test.ts | 84 +++++ app/features/chat/RouteChatRooms.server.ts | 50 +++ app/features/chat/chat-client.test.ts | 163 +++++++-- app/features/chat/chat-client.ts | 182 +++++++---- app/features/chat/chat-room-list.server.ts | 20 +- app/features/chat/chat-types.ts | 11 +- .../chat/components/Chat.browser.test.tsx | 97 +++++- app/features/chat/components/Chat.module.css | 8 +- app/features/chat/components/Chat.tsx | 309 +++++++++--------- app/features/chat/routes/api.chat.rooms.ts | 19 +- .../events/events-hooks.browser.test.tsx | 20 +- app/features/events/events-hooks.ts | 11 +- .../scrims/loaders/scrims.$id.server.ts | 13 +- .../loaders/q.match.$id.server.test.ts | 17 +- .../loaders/q.match.$id.server.ts | 75 +++-- .../sendouq/loaders/q.looking.server.ts | 13 +- .../loaders/to.$id.matches.$mid.server.ts | 13 +- app/form/SendouForm.tsx | 14 +- app/root.tsx | 7 +- changelog/2026-09-20-chat-loads-with-page.md | 4 + docs/dev/architecture.md | 8 +- 26 files changed, 899 insertions(+), 397 deletions(-) delete mode 100644 app/components/layout/LazyChatSidebar.tsx create mode 100644 app/features/chat/RouteChatRooms.server.test.ts create mode 100644 app/features/chat/RouteChatRooms.server.ts create mode 100644 changelog/2026-09-20-chat-loads-with-page.md diff --git a/app/components/MobileNav.tsx b/app/components/MobileNav.tsx index 715d80654..9df7d6f0d 100644 --- a/app/components/MobileNav.tsx +++ b/app/components/MobileNav.tsx @@ -39,7 +39,7 @@ import { EventsList } from "./EventsList"; import { LinkButton } from "./elements/Button"; import { isOwnToggle } from "./elements/Popover"; import { Image } from "./Image"; -import { LazyChatSidebar } from "./layout/LazyChatSidebar"; +import { ChatSidebar } from "./layout/ChatSidebar"; import { LogInButtonContainer } from "./layout/LogInButtonContainer"; import { NotificationContent, @@ -649,7 +649,7 @@ function ChatPanel({ >
{isOpen ? ( - panelRef.current?.hidePopover()} /> + panelRef.current?.hidePopover()} /> ) : null}
diff --git a/app/components/layout/ChatSidebar.tsx b/app/components/layout/ChatSidebar.tsx index e63fe026c..f80712b94 100644 --- a/app/components/layout/ChatSidebar.tsx +++ b/app/components/layout/ChatSidebar.tsx @@ -158,8 +158,9 @@ function RoomList({ onClose }: { onClose?: () => void }) { const byRecency = (a: ChatRoomListItem, b: ChatRoomListItem) => (b.latestMessageAt ?? 0) - (a.latestMessageAt ?? 0) || b.id - a.id; + // the context's copy of the room over the route's: it carries the live unread count const routeRooms = useCurrentRouteChatRooms().flatMap((entry) => { - const room = chatContext.roomForId(entry.roomId); + const room = chatContext.roomForId(entry.room.id); return room ? [{ ...entry, room }] : []; }); @@ -363,7 +364,7 @@ function SingleChatView({ const chatContext = useChatContext()!; const roomDisplay = useRoomDisplay(); const routeLabel = useCurrentRouteChatRooms().find( - (entry) => entry.roomId === room?.id, + (entry) => entry.room.id === room?.id, )?.label; const otherRoomsUnreadCount = chatContext.rooms diff --git a/app/components/layout/LazyChatSidebar.tsx b/app/components/layout/LazyChatSidebar.tsx deleted file mode 100644 index 34c5f8590..000000000 --- a/app/components/layout/LazyChatSidebar.tsx +++ /dev/null @@ -1,18 +0,0 @@ -import * as React from "react"; -import { Placeholder } from "../Placeholder"; - -const ChatSidebar = React.lazy(() => - import("./ChatSidebar").then((module) => ({ default: module.ChatSidebar })), -); - -export function preloadChatSidebar() { - void import("./ChatSidebar"); -} - -export function LazyChatSidebar({ onClose }: { onClose?: () => void }) { - return ( - }> - - - ); -} diff --git a/app/components/layout/index.tsx b/app/components/layout/index.tsx index 387ed49b8..b9fd55cea 100644 --- a/app/components/layout/index.tsx +++ b/app/components/layout/index.tsx @@ -49,9 +49,9 @@ import { MobileNav } from "../MobileNav"; import { NotificationDot } from "../NotificationDot"; import { ListLink, SideNav, SideNavFooter, SideNavHeader } from "../SideNav"; import { StreamListItems } from "../StreamListItems"; +import { ChatSidebar } from "./ChatSidebar"; import { Footer } from "./Footer"; import styles from "./index.module.css"; -import { LazyChatSidebar } from "./LazyChatSidebar"; import { LogInButtonContainer } from "./LogInButtonContainer"; import { authErrorSearchParams } from "./layout-search-params"; import { NotificationPopover, useNotifications } from "./NotificationPopover"; @@ -451,7 +451,7 @@ export function Layout({ aria-label={t("common:chat.sidebar.title")} onClose={() => setChatSidebarModalOpenAndSync(false)} > - + ) : null}
- setChatSidebarOpen(false)} /> + setChatSidebarOpen(false)} /> ) : null} {typeof authError === "string" ? ( diff --git a/app/features/chat/ChatProvider.tsx b/app/features/chat/ChatProvider.tsx index c17a65b1e..65b6fcd03 100644 --- a/app/features/chat/ChatProvider.tsx +++ b/app/features/chat/ChatProvider.tsx @@ -1,15 +1,19 @@ import * as React from "react"; import { useLocation, useMatches } from "react-router"; -import { preloadChatSidebar } from "~/components/layout/LazyChatSidebar"; import { eventsClient } from "~/features/events/events-client"; import { useEventStreamCatchUp, useEventsConnection, } from "~/features/events/events-hooks"; import { chatRoomChannel } from "~/features/events/events-types"; +import { useHydrated } from "~/hooks/useHydrated"; import { useLayoutSize } from "~/hooks/useLayoutSize"; import type { LoggedInUser } from "~/root"; -import { type ChatSnapshot, chatClient } from "./chat-client"; +import { + type ChatSnapshot, + chatClient, + snapshotFromLoaderData, +} from "./chat-client"; import { useServerRevalidationEvents } from "./chat-hooks"; import type { ChatRoomListItem, @@ -18,6 +22,8 @@ import type { } from "./chat-types"; const EMPTY_MESSAGES: ClientChatMessage[] = []; +const EMPTY_ROOM_LIST: ChatRoomListItem[] = []; +const EMPTY_ROUTE_ROOMS: RouteChatRoom[] = []; const SERVER_SNAPSHOT: ChatSnapshot = { roomsLoaded: false, @@ -60,33 +66,61 @@ export function useChatContext(): ChatContextValue | null { export function ChatProvider({ user, + roomList, children, }: { user?: LoggedInUser | null; + /** The user's rooms as the root loader served them. */ + roomList?: ChatRoomListItem[]; children: React.ReactNode; }) { if (!user) { return <>{children}; } - return {children}; + return ( + + {children} + + ); } function ChatProviderInner({ user, + roomList, children, }: { user: LoggedInUser; + roomList: ChatRoomListItem[]; children: React.ReactNode; }) { useEventsConnection(true); useServerRevalidationEvents(user.id); - const snapshot = React.useSyncExternalStore( + const hydrated = useHydrated(); + const routeRooms = useCurrentRouteChatRooms(); + + const storeSnapshot = React.useSyncExternalStore( chatClient.subscribe, chatClient.getSnapshot, getServerSnapshot, ); + // the loader data stands in until the live client holds it, so the page + // arrives with its chat the way it will stay; memoized to keep the effects + // depending on it off the render loop + const loaderSnapshot = React.useMemo( + () => snapshotFromLoaderData(roomList, routeRooms), + [roomList, routeRooms], + ); + const snapshot = storeSnapshot.roomsLoaded ? storeSnapshot : loaderSnapshot; + + React.useEffect(() => { + chatClient.applyRoomList(roomList); + }, [roomList]); + + React.useEffect(() => { + chatClient.applyRouteRooms(routeRooms); + }, [routeRooms]); React.useEffect(() => { chatClient.start(user.id); @@ -113,25 +147,28 @@ function ChatProviderInner({ useEventStreamCatchUp({ enabled: true, onCatchUp: () => chatClient.catchUp(), + // the loader data is at most as old as the navigation that fetched it + heldSince: performance.timeOrigin, }); - const [chatOpen, setChatOpenState] = React.useState(false); - const [activeRoomIds, setActiveRoomIds] = React.useState([]); - - // the sidebar chunk is fetched as soon as there is something to open it for, - // so that opening chat never waits on a download - const hasRoomToOpen = - snapshot.rooms.length > 0 || activeRoomIds.length > 0 || chatOpen; - React.useEffect(() => { - if (!hasRoomToOpen) return; - - preloadChatSidebar(); - }, [hasRoomToOpen]); + const autoOpenRoomIdsKey = routeRooms + .filter((room) => room.autoOpen) + .map((room) => room.room.id) + .join(","); + const [chatOpenState, setChatOpenState] = React.useState(false); + const [activeRoomIds, setActiveRoomIds] = React.useState(() => + roomIdsFromKey(autoOpenRoomIdsKey), + ); + // the server renders a route's rooms open as the desktop layout has them + // (smaller layouts hide the rail); the route sync settles it once the + // layout is known + const chatOpen = + chatOpenState || (!hydrated && autoOpenRoomIdsKey.length > 0); // messages arriving to a room on screen are read immediately instead of counting unread React.useEffect(() => { - chatClient.setViewedRoomIds(chatOpen ? activeRoomIds : []); - }, [chatOpen, activeRoomIds]); + chatClient.setViewedRoomIds(chatOpenState ? activeRoomIds : []); + }, [chatOpenState, activeRoomIds]); const rooms = snapshot.rooms; @@ -172,9 +209,12 @@ function ChatProviderInner({ useChatRouteSync({ userId: user.id, + hydrated, roomsLoaded: snapshot.roomsLoaded, rooms, observedRoomIds: snapshot.observedRoomIds, + routeRooms, + autoOpenRoomIdsKey, setActiveRoomIds, setChatOpenState, }); @@ -221,27 +261,30 @@ function ChatProviderInner({ function useChatRouteSync({ userId, + hydrated, roomsLoaded, rooms, observedRoomIds, + routeRooms, + autoOpenRoomIdsKey, setActiveRoomIds, setChatOpenState, }: { userId: number; + hydrated: boolean; roomsLoaded: boolean; rooms: ChatRoomListItem[]; observedRoomIds: ReadonlySet; + routeRooms: RouteChatRoom[]; + autoOpenRoomIdsKey: string; setActiveRoomIds: React.Dispatch>; setChatOpenState: (open: boolean) => void; }) { - const routeRooms = useCurrentRouteChatRooms(); // keys rather than the arrays themselves: a route revalidation hands over // equal-but-new loader data that must not re-run the effects - const routeRoomIdsKey = routeRooms.map((room) => room.roomId).join(","); - const autoOpenRoomIdsKey = routeRooms - .filter((room) => room.autoOpen) - .map((room) => room.roomId) - .join(","); + const routeRoomIdsKey = routeRooms.map((room) => room.room.id).join(","); + const latestRouteRoomsRef = React.useRef(routeRooms); + latestRouteRoomsRef.current = routeRooms; const { pathname } = useLocation(); const layoutSize = useLayoutSize(); const previousRouteRoomIdsKeyRef = React.useRef(null); @@ -261,7 +304,9 @@ function useChatRouteSync({ }, [routeRoomIdsKey]); React.useEffect(() => { - if (!roomsLoaded) return; + // the hydration render's layout size is the server's guess, so a room + // opening on arrival waits for the real one + if (!roomsLoaded || !hydrated) return; // route sync opens its own rooms directly: going through the context's // `setChatOpen` would read the previous render's empty `activeRoomIds` and @@ -291,16 +336,15 @@ function useChatRouteSync({ return kept.length === openRoomIds.length ? openRoomIds : kept; }); - // the loader can know about a just-created room before the room list - // does; an observer's room is never in the list at all, so its info is - // fetched separately as an observed room - for (const roomId of roomIdsFromKey(routeRoomIdsKey)) { - if (rooms.some((room) => room.id === roomId)) continue; - - if (autoOpenRoomIds.includes(roomId)) { - void chatClient.refreshRooms(); - } - chatClient.ensureRoomKnown(roomId); + // the loader can know about a just-created room of the user's own before + // the room list does: refetched so it gets listed rather than merely observed + const unlistedOwnRoom = latestRouteRoomsRef.current.find( + (entry) => + entry.room.participantUserIds.includes(userId) && + !rooms.some((room) => room.id === entry.room.id), + ); + if (unlistedOwnRoom) { + void chatClient.refreshRooms(); } } @@ -308,8 +352,12 @@ function useChatRouteSync({ if (!routeRoomIdsChanged) return; setActiveRoomIds(autoOpenRoomIds); - for (const roomId of autoOpenRoomIds) { - chatClient.ensureMessagesLoaded(roomId); + // a room opening on arrival brings its history along; one that did not + // (an older loader response) is fetched + for (const entry of latestRouteRoomsRef.current) { + if (entry.autoOpen && entry.messages === null) { + chatClient.ensureMessagesLoaded(entry.room.id); + } } if (layoutSize === "desktop") { openChatForRooms(autoOpenRoomIds); @@ -333,6 +381,7 @@ function useChatRouteSync({ openChatForRooms([matchedRoom.id]); } }, [ + hydrated, roomsLoaded, routeRoomIdsKey, autoOpenRoomIdsKey, @@ -363,7 +412,7 @@ export function useCurrentRouteChatRooms(): RouteChatRoom[] { } } - return []; + return EMPTY_ROUTE_ROOMS; } function roomIdsFromKey(key: string) { diff --git a/app/features/chat/RouteChatRooms.server.test.ts b/app/features/chat/RouteChatRooms.server.test.ts new file mode 100644 index 000000000..ae5fad56e --- /dev/null +++ b/app/features/chat/RouteChatRooms.server.test.ts @@ -0,0 +1,84 @@ +import { beforeEach, describe, expect, test } from "vitest"; +import * as ChatMessageFactory from "~/db/seed/factories/ChatMessageFactory"; +import * as UserFactory from "~/db/seed/factories/UserFactory"; +import * as RouteChatRooms from "./RouteChatRooms.server"; +import { setupSqMatch } from "./tests/fixtures"; + +const users = UserFactory.pool(); + +const outsiderId = () => users.id(10); + +beforeEach(async () => { + await users.create(10); +}); + +describe("RouteChatRooms.resolve", () => { + test("hands a participant the room as listed along with its history", async () => { + const { match, alphaUserIds } = await setupSqMatch(users); + await ChatMessageFactory.create({ + roomId: match.chatRoomId!, + authorUserId: alphaUserIds[1], + contents: "gg", + }); + + const rooms = await RouteChatRooms.resolve({ id: alphaUserIds[0] }, [ + { roomId: match.chatRoomId!, autoOpen: true }, + ]); + + expect(rooms).toHaveLength(1); + expect(rooms[0].autoOpen).toBe(true); + expect(rooms[0].room).toMatchObject({ + id: match.chatRoomId, + type: "SQ_MATCH", + canPost: true, + unreadCount: 1, + }); + expect(rooms[0].messages?.map((message) => message.contents)).toEqual([ + "gg", + ]); + }); + + test("leaves the history of a room only listed to be fetched on open", async () => { + const { match, alphaUserIds } = await setupSqMatch(users); + + const rooms = await RouteChatRooms.resolve({ id: alphaUserIds[0] }, [ + { roomId: match.chatRoomId!, autoOpen: false, label: "Match" }, + ]); + + expect(rooms).toEqual([ + expect.objectContaining({ + autoOpen: false, + label: "Match", + messages: null, + }), + ]); + }); + + test("drops a room the user may not view", async () => { + const { match } = await setupSqMatch(users); + + const rooms = await RouteChatRooms.resolve({ id: outsiderId() }, [ + { roomId: match.chatRoomId!, autoOpen: true }, + ]); + + expect(rooms).toEqual([]); + }); + + test("drops a room that resolves to nothing", async () => { + const rooms = await RouteChatRooms.resolve({ id: outsiderId() }, [ + { roomId: 999_999, autoOpen: true }, + ]); + + expect(rooms).toEqual([]); + }); + + test("resolves nothing for a logged out viewer", async () => { + const { match } = await setupSqMatch(users); + + const rooms = await RouteChatRooms.resolve(undefined, [ + { roomId: match.chatRoomId!, autoOpen: true }, + ]); + + expect(rooms).toEqual([]); + }); +}); diff --git a/app/features/chat/RouteChatRooms.server.ts b/app/features/chat/RouteChatRooms.server.ts new file mode 100644 index 000000000..fb2cc6b94 --- /dev/null +++ b/app/features/chat/RouteChatRooms.server.ts @@ -0,0 +1,50 @@ +import { hasPermission } from "~/modules/permissions/utils"; +import { logger } from "~/utils/logger"; +import * as ChatRepository from "./ChatRepository.server"; +import * as ChatRoomResolver from "./ChatRoomResolver.server"; +import { roomListItem } from "./chat-room-list.server"; +import type { RouteChatRoom, RouteChatRoomInput } from "./chat-types"; + +/** + * Resolves the rooms a route surfaces into what its page arrives with: each room as the sidebar + * lists it and, for the rooms opening on arrival, their latest messages. A room the user may not + * view is left out, the loader's own access logic being the one that decides who gets it. + */ +export async function resolve( + user: { id: number } | undefined, + inputs: RouteChatRoomInput[], +): Promise { + if (!user || inputs.length === 0) return []; + + const rooms = await ChatRoomResolver.resolveAll( + inputs.map((input) => input.roomId), + ); + const viewableRooms = rooms.filter((room) => + hasPermission(room, "VIEW", user), + ); + const stats = await ChatRepository.findMessageStatsByRoomIds( + user.id, + viewableRooms.map((room) => room.roomId), + ); + const statsByRoomId = new Map(stats.map((row) => [row.roomId, row])); + const roomsById = new Map(viewableRooms.map((room) => [room.roomId, room])); + + const result: RouteChatRoom[] = []; + for (const { roomId, ...input } of inputs) { + const room = roomsById.get(roomId); + if (!room) { + logger.warn(`Route surfaced chat room ${roomId} the user can not view`); + continue; + } + + result.push({ + ...input, + room: roomListItem(room, statsByRoomId.get(roomId), user), + messages: input.autoOpen + ? await ChatRepository.findAllMessagesByRoomId(roomId) + : null, + }); + } + + return result; +} diff --git a/app/features/chat/chat-client.test.ts b/app/features/chat/chat-client.test.ts index 05a3f104a..c5e878115 100644 --- a/app/features/chat/chat-client.test.ts +++ b/app/features/chat/chat-client.test.ts @@ -1,10 +1,15 @@ import { describe, expect, test, vi } from "vitest"; import type { ServerEvent } from "~/features/events/events-types"; -import { type ChatClient, createChatClient } from "./chat-client"; +import { + type ChatClient, + createChatClient, + snapshotFromLoaderData, +} from "./chat-client"; import type { ChatMessageAuthor, ChatMessageWithAuthor, ChatRoomListItem, + RouteChatRoom, } from "./chat-types"; const READ_DEBOUNCE_MS = 20; @@ -56,6 +61,14 @@ function message( }; } +/** A route's room as its loader serves it, with the history an opening room brings along. */ +function routeRoom( + listItem: ChatRoomListItem, + messages: ChatMessageWithAuthor[] | null = null, +): RouteChatRoom { + return { autoOpen: true, room: listItem, messages }; +} + function createHarness({ rooms = [room()], messages = [] as ChatMessageWithAuthor[], @@ -512,17 +525,15 @@ describe("createChatClient", () => { expect(harness.fetchRooms).toHaveBeenCalledTimes(5); }); - test("ensureRoomKnown fetches an observed room outside the user's list", async () => { + test("a route room outside the user's list is held as observed without a fetch", async () => { const observed = room({ id: 50, url: "/to/2/matches/2" }); - const harness = createHarness({ observedRoom: observed }); + const harness = createHarness(); const client = await startedClient(harness); - client.ensureRoomKnown(50); - await flush(); - client.ensureRoomKnown(50); - await flush(); + client.applyRouteRooms([routeRoom(observed)]); + client.applyRouteRooms([routeRoom(observed)]); - expect(harness.fetchRoom).toHaveBeenCalledTimes(1); + expect(harness.fetchRoom).not.toHaveBeenCalled(); expect(client.getSnapshot().roomsById.get(50)).toMatchObject({ id: 50 }); expect(client.getSnapshot().rooms).toHaveLength(1); expect([...client.getSnapshot().observedRoomIds]).toEqual([50]); @@ -530,32 +541,30 @@ describe("createChatClient", () => { test("an observed room starts with no unread of its own", async () => { const observed = room({ id: 50, unreadCount: 12 }); - const harness = createHarness({ observedRoom: observed }); + const harness = createHarness(); const client = await startedClient(harness); - client.ensureRoomKnown(50); - await flush(); + client.applyRouteRooms([routeRoom(observed)]); expect(client.getSnapshot().roomsById.get(50)?.unreadCount).toBe(0); expect(client.getSnapshot().totalUnreadCount).toBe(0); }); - test("ensureRoomKnown is a no-op for a room already in the user's list", async () => { - const harness = createHarness(); + test("a route room already in the user's list stays listed with the list's unread", async () => { + const harness = createHarness({ rooms: [room({ id: 1, unreadCount: 4 })] }); const client = await startedClient(harness); - client.ensureRoomKnown(1); - await flush(); + client.applyRouteRooms([routeRoom(room({ id: 1, unreadCount: 0 }))]); - expect(harness.fetchRoom).not.toHaveBeenCalled(); + expect(client.getSnapshot().observedRoomIds.size).toBe(0); + expect(client.getSnapshot().totalUnreadCount).toBe(4); }); test("a message to an observed room appends without counting unread or refetching the list", async () => { const observed = room({ id: 50 }); - const harness = createHarness({ observedRoom: observed }); + const harness = createHarness(); const client = await startedClient(harness); - client.ensureRoomKnown(50); - await flush(); + client.applyRouteRooms([routeRoom(observed)]); client.ensureMessagesLoaded(50); await flush(); @@ -575,12 +584,10 @@ describe("createChatClient", () => { test("a rooms refetch keeps the held history of an observed room", async () => { const observed = room({ id: 50 }); const harness = createHarness({ - observedRoom: observed, messages: [message({ id: 1, roomId: 50 })], }); const client = await startedClient(harness); - client.ensureRoomKnown(50); - await flush(); + client.applyRouteRooms([routeRoom(observed)]); client.ensureMessagesLoaded(50); await flush(); @@ -593,12 +600,10 @@ describe("createChatClient", () => { test("reopening an observed room refetches its history", async () => { const observed = room({ id: 50 }); const harness = createHarness({ - observedRoom: observed, messages: [message({ id: 1, roomId: 50 })], }); const client = await startedClient(harness); - client.ensureRoomKnown(50); - await flush(); + client.applyRouteRooms([routeRoom(observed)]); client.ensureMessagesLoaded(50); await flush(); @@ -618,10 +623,9 @@ describe("createChatClient", () => { test("an observed room the user's list later carries is superseded by the list version", async () => { const observed = room({ id: 50 }); - const harness = createHarness({ observedRoom: observed }); + const harness = createHarness(); const client = await startedClient(harness); - client.ensureRoomKnown(50); - await flush(); + client.applyRouteRooms([routeRoom(observed)]); expect(client.getSnapshot().rooms).toHaveLength(1); harness.fetchRooms.mockResolvedValue({ @@ -635,6 +639,88 @@ describe("createChatClient", () => { expect(client.getSnapshot().observedRoomIds.size).toBe(0); }); + test("start does not fetch a room list a loader already supplied", async () => { + const harness = createHarness(); + harness.client.applyRoomList([room({ id: 1, unreadCount: 2 })]); + + const client = await startedClient(harness); + + expect(harness.fetchRooms).not.toHaveBeenCalled(); + expect(client.getSnapshot().roomsLoaded).toBe(true); + expect(client.getSnapshot().totalUnreadCount).toBe(2); + }); + + test("a route room's history arrives without a fetch and keeps what was pushed since", async () => { + const harness = createHarness(); + const client = await startedClient(harness); + + client.applyRouteRooms([ + routeRoom(room(), [message({ id: 1 }), message({ id: 2 })]), + ]); + harness.emit({ + kind: "chatMessage", + roomId: 1, + message: message({ id: 3 }), + }); + // a revalidation's snapshot taken before the push + client.applyRouteRooms([ + routeRoom(room(), [message({ id: 1 }), message({ id: 2 })]), + ]); + client.ensureMessagesLoaded(1); + await flush(); + + expect(harness.fetchMessages).not.toHaveBeenCalled(); + expect( + client + .getSnapshot() + .messagesByRoomId.get(1) + ?.map((each) => each.id), + ).toEqual([1, 2, 3]); + }); + + test("a route room's empty history counts as loaded", async () => { + const harness = createHarness(); + const client = await startedClient(harness); + + client.applyRouteRooms([routeRoom(room(), [])]); + client.ensureMessagesLoaded(1); + await flush(); + + expect(harness.fetchMessages).not.toHaveBeenCalled(); + expect(client.getSnapshot().messagesByRoomId.get(1)).toEqual([]); + }); + + test("a history snapshot saying nothing new leaves the held one untouched", async () => { + const harness = createHarness(); + const client = await startedClient(harness); + client.applyRouteRooms([routeRoom(room(), [message({ id: 1 })])]); + const held = client.getSnapshot().messagesByRoomId.get(1); + + client.applyRouteRooms([routeRoom(room(), [message({ id: 1 })])]); + + expect(client.getSnapshot().messagesByRoomId.get(1)).toBe(held); + }); + + test("a pending send survives a route history snapshot", async () => { + const harness = createHarness(); + const client = await startedClient(harness); + client.applyRouteRooms([routeRoom(room(), [])]); + client.send(1, { + publicId: "pending-1", + contents: "hi", + author: author(1), + }); + + client.applyRouteRooms([routeRoom(room(), [message({ id: 1 })])]); + + expect( + client + .getSnapshot() + .messagesByRoomId.get(1) + ?.map((each) => each.publicId), + ).toEqual(["public-1", "pending-1"]); + }); + test("a roomsChanged event refetches the room list", async () => { const harness = createHarness(); await startedClient(harness); @@ -779,3 +865,24 @@ describe("createChatClient", () => { ).toEqual([3, 5]); }); }); + +describe("snapshotFromLoaderData", () => { + test("holds the loader data the way the live client will", () => { + const snapshot = snapshotFromLoaderData( + [room({ id: 1, unreadCount: 2 })], + [ + routeRoom(room({ id: 1, unreadCount: 0 }), [message({ id: 1 })]), + routeRoom(room({ id: 50, unreadCount: 7 }), [ + message({ id: 2, roomId: 50 }), + ]), + ], + ); + + expect(snapshot.roomsLoaded).toBe(true); + expect(snapshot.rooms.map((each) => each.id)).toEqual([1]); + expect(snapshot.totalUnreadCount).toBe(2); + expect([...snapshot.observedRoomIds]).toEqual([50]); + expect(snapshot.messagesByRoomId.get(1)).toHaveLength(1); + expect(snapshot.messagesByRoomId.get(50)).toHaveLength(1); + }); +}); diff --git a/app/features/chat/chat-client.ts b/app/features/chat/chat-client.ts index 6d594d47c..514e2f0a7 100644 --- a/app/features/chat/chat-client.ts +++ b/app/features/chat/chat-client.ts @@ -15,6 +15,7 @@ import type { ChatMessageWithAuthor, ChatRoomListItem, ClientChatMessage, + RouteChatRoom, } from "./chat-types"; const READ_DEBOUNCE_MS = 1_500; @@ -57,16 +58,18 @@ export interface ChatSnapshot { } export interface ChatClient { - /** Starts listening to server events and fetches the room list. */ + /** Starts listening to server events, fetching the room list unless a loader's was applied already. */ start: (ownUserId: number) => void; + /** Takes a loader-served room list as the room list, the way a `GET /api/chat/rooms` response is. */ + applyRoomList: (rooms: ChatRoomListItem[]) => void; + /** Takes in a route's rooms: one not in the user's list is held as observed, and a history that came along is merged into the room's held one. */ + applyRouteRooms: (routeRooms: RouteChatRoom[]) => void; /** Stops event handling and resets all held data. */ stop: () => void; getSnapshot: () => ChatSnapshot; /** Subscribes to snapshot changes, for `useSyncExternalStore`. Returns an unsubscribe function. */ subscribe: (listener: () => void) => () => void; refreshRooms: () => Promise; - /** Fetches a room's info as an observed room when the user's own room list does not carry it (observer access via a route's `chatRooms`). */ - ensureRoomKnown: (roomId: number) => void; /** Fetches the room's history unless it is already loaded or loading. An observed room's held history is refetched instead of trusted: messages only reach an observer while the route surfacing the room keeps its subscription. */ ensureMessagesLoaded: (roomId: number) => void; /** Reconnect catch-up: refetches the room list and every loaded history. */ @@ -263,46 +266,7 @@ export function createChatClient(deps: ChatClientDeps): ChatClient { const data = await deps.fetchRooms(); if (!data) return; - const next = new Map(); - for (const room of data.rooms) { - // a room that arrived in the list is no longer unknown; a later - // recreation under the same owner may need a refetch again - refetchedUnknownRoomIds.delete(room.id); - - // a message that arrived while the fetch was in flight is missing - // from its snapshot: the held room is the newer one - const known = roomsById.get(room.id); - const outrunByPush = - known !== undefined && - (known.latestMessageId ?? 0) > (room.latestMessageId ?? 0); - const newer = outrunByPush ? known : room; - - // a locally-read room stays read even when the server response - // raced the debounced read POST - const readUpTo = locallyReadByRoomId.get(room.id) ?? 0; - const locallyRead = - newer.latestMessageId !== null && readUpTo >= newer.latestMessageId; - - next.set(room.id, { - ...room, - latestMessageId: newer.latestMessageId, - latestMessageAt: newer.latestMessageAt, - unreadCount: locallyRead ? 0 : newer.unreadCount, - observed: false, - }); - } - - // the list version wins over a held observed copy - for (const [roomId, room] of roomsById) { - if (room.observed && !next.has(roomId)) { - next.set(roomId, room); - } - } - - replaceRooms(next); - pruneLostRooms(); - roomsLoaded = true; - notify(); + mergeRoomList(data.rooms); } catch (error) { logger.error("Fetching chat rooms failed", error); } finally { @@ -317,6 +281,75 @@ export function createChatClient(deps: ChatClientDeps): ChatClient { return roomsRefreshInflight; }; + /** Takes a room list snapshot (fetched or loader-served) as the user's own rooms, keeping what the snapshot predates. */ + const mergeRoomList = (rooms: ChatRoomListItem[]) => { + const next = new Map(); + for (const room of rooms) { + // a room that arrived in the list is no longer unknown; a later + // recreation under the same owner may need a refetch again + refetchedUnknownRoomIds.delete(room.id); + + // a message that arrived while the snapshot was on its way is missing + // from it: the held room is the newer one + const known = roomsById.get(room.id); + const outrunByPush = + known !== undefined && + (known.latestMessageId ?? 0) > (room.latestMessageId ?? 0); + const newer = outrunByPush ? known : room; + + // a locally-read room stays read even when the server response + // raced the debounced read POST + const readUpTo = locallyReadByRoomId.get(room.id) ?? 0; + const locallyRead = + newer.latestMessageId !== null && readUpTo >= newer.latestMessageId; + + next.set(room.id, { + ...room, + latestMessageId: newer.latestMessageId, + latestMessageAt: newer.latestMessageAt, + unreadCount: locallyRead ? 0 : newer.unreadCount, + observed: false, + }); + } + + // the list version wins over a held observed copy + for (const [roomId, room] of roomsById) { + if (room.observed && !next.has(roomId)) { + next.set(roomId, room); + } + } + + replaceRooms(next); + pruneLostRooms(); + roomsLoaded = true; + notify(); + }; + + /** Folds a history snapshot into the held one, keeping everything the snapshot predates: optimistic sends, and messages pushed over SSE. */ + const mergeHistory = (roomId: number, messages: ChatMessageWithAuthor[]) => { + const fetchedPublicIds = new Set( + messages.map((message) => message.publicId), + ); + const held = messagesByRoomId.get(roomId) ?? []; + const missedBySnapshot = held.filter( + (message) => !fetchedPublicIds.has(message.publicId), + ); + const merged = sortedMessages([...messages, ...missedBySnapshot]); + + // a snapshot that says nothing new (a page revalidation) must not churn the view + const unchanged = + messagesByRoomId.has(roomId) && + merged.length === held.length && + merged.every( + (message, index) => + message.id === held[index].id && + message.publicId === held[index].publicId, + ); + if (unchanged) return; + + setMessages(roomId, merged); + }; + /** A held history whose room is no longer known belongs to a room the user lost access to (e.g. left the group); drop the local copy. */ const pruneLostRooms = () => { const lostRoomIds = [...messagesByRoomId.keys()].filter( @@ -379,15 +412,7 @@ export function createChatClient(deps: ChatClientDeps): ChatClient { return; } - // keep everything appended while the fetch was in flight that its - // snapshot predates: optimistic sends, and messages pushed over SSE - const fetchedPublicIds = new Set( - data.messages.map((message) => message.publicId), - ); - const missedByFetch = (messagesByRoomId.get(roomId) ?? []).filter( - (message) => !fetchedPublicIds.has(message.publicId), - ); - setMessages(roomId, sortedMessages([...data.messages, ...missedByFetch])); + mergeHistory(roomId, data.messages); notify(); if (viewedRoomIds.has(roomId)) { @@ -414,7 +439,30 @@ export function createChatClient(deps: ChatClientDeps): ChatClient { ownUserId = userId; removeEventListener = deps.addServerEventListener(handleEvent); - void refreshRooms(); + if (!roomsLoaded) { + void refreshRooms(); + } + }, + applyRoomList: mergeRoomList, + applyRouteRooms: (routeRooms) => { + for (const { room, messages } of routeRooms) { + if (!roomsById.has(room.id)) { + // a room outside the user's own list is only observed: it never + // accrues unread, so it must not start out with the server's count + // of everything said in it before the observer showed up + replaceRooms( + new Map(roomsById).set(room.id, { + ...room, + unreadCount: 0, + observed: true, + }), + ); + } + if (messages) { + mergeHistory(room.id, messages); + } + } + notify(); }, stop: () => { removeEventListener?.(); @@ -450,10 +498,6 @@ export function createChatClient(deps: ChatClientDeps): ChatClient { return () => listeners.delete(listener); }, refreshRooms, - ensureRoomKnown: (roomId) => { - if (roomsById.has(roomId)) return; - void loadObservedRoom(roomId); - }, ensureMessagesLoaded: (roomId) => { const canHaveMissedMessages = roomById(roomId)?.observed ?? false; if (messagesByRoomId.has(roomId) && !canHaveMissedMessages) return; @@ -553,6 +597,30 @@ function sortedMessages(messages: ClientChatMessage[]): ClientChatMessage[] { return [...persisted, ...pending]; } +const NEVER_RESOLVING = () => new Promise(() => {}); + +/** Deps of a client that never reaches the network, for a snapshot made from loader data alone. */ +const OFFLINE_DEPS: ChatClientDeps = { + fetchRooms: NEVER_RESOLVING, + fetchRoom: NEVER_RESOLVING, + fetchMessages: NEVER_RESOLVING, + postMessage: NEVER_RESOLVING, + postRead: NEVER_RESOLVING, + onSendFailed: () => {}, + addServerEventListener: () => () => {}, +}; + +/** What a page renders with before the live client has taken the loader data over: held exactly as `applyRoomList` and `applyRouteRooms` hold it. */ +export function snapshotFromLoaderData( + roomList: ChatRoomListItem[], + routeRooms: RouteChatRoom[], +): ChatSnapshot { + const client = createChatClient(OFFLINE_DEPS); + client.applyRoomList(roomList); + client.applyRouteRooms(routeRooms); + return client.getSnapshot(); +} + const fetchJson = async (url: string): Promise => { const response = await fetch(url); if (!response.ok) { diff --git a/app/features/chat/chat-room-list.server.ts b/app/features/chat/chat-room-list.server.ts index 68e9ae85b..986ed2776 100644 --- a/app/features/chat/chat-room-list.server.ts +++ b/app/features/chat/chat-room-list.server.ts @@ -1,12 +1,28 @@ import { hasPermission } from "~/modules/permissions/utils"; -import type * as ChatRepository from "./ChatRepository.server"; -import type * as ChatRoomResolver from "./ChatRoomResolver.server"; +import * as ChatRepository from "./ChatRepository.server"; +import * as ChatRoomResolver from "./ChatRoomResolver.server"; import type { ChatRoomListItem } from "./chat-types"; type MessageStats = Awaited< ReturnType >[number]; +/** The user's open rooms as the chat sidebar lists them, with unread counts; the root loader's first snapshot and the `GET /api/chat/rooms` refetch alike. */ +export async function resolveRoomList(user: { + id: number; +}): Promise { + const rooms = await ChatRoomResolver.findAllByUserId(user.id); + const messageStats = await ChatRepository.findMessageStatsByRoomIds( + user.id, + rooms.map((room) => room.roomId), + ); + const statsByRoomId = new Map(messageStats.map((row) => [row.roomId, row])); + + return rooms.map((room) => + roomListItem(room, statsByRoomId.get(room.roomId), user), + ); +} + /** Shapes a resolved room into the list item the chat client consumes. */ export function roomListItem( room: ChatRoomResolver.ResolvedRoom, diff --git a/app/features/chat/chat-types.ts b/app/features/chat/chat-types.ts index 5c172a1fe..9f20ffde9 100644 --- a/app/features/chat/chat-types.ts +++ b/app/features/chat/chat-types.ts @@ -92,8 +92,8 @@ export interface ChatRoomListItem { latestMessageAt: number | null; } -/** A room the current route surfaces to the viewer, from its loader's `chatRooms`. */ -export interface RouteChatRoom { +/** A room a route asks to surface to the viewer, resolved into a `RouteChatRoom` by `RouteChatRooms.resolve`. */ +export interface RouteChatRoomInput { roomId: number; /** Whether the room opens for the viewer on arrival, rather than only being listed in the sidebar (staff reading a private group chat). */ autoOpen: boolean; @@ -101,4 +101,11 @@ export interface RouteChatRoom { label?: string; } +/** A room the current route surfaces to the viewer, from its loader's `chatRooms`, arriving with everything the chat opens with. */ +export interface RouteChatRoom extends Omit { + room: ChatRoomListItem; + /** Latest messages oldest first; `null` for a room only listed, whose history is fetched when opened. */ + messages: ChatMessageWithAuthor[] | null; +} + export type RevalidateScope = "MATCH_RESULTS"; diff --git a/app/features/chat/components/Chat.browser.test.tsx b/app/features/chat/components/Chat.browser.test.tsx index ea70a87bb..443e05f35 100644 --- a/app/features/chat/components/Chat.browser.test.tsx +++ b/app/features/chat/components/Chat.browser.test.tsx @@ -1,14 +1,59 @@ import * as React from "react"; import { createMemoryRouter, RouterProvider } from "react-router"; -import { describe, expect, test, vi } from "vitest"; +import { afterEach, describe, expect, test, vi } from "vitest"; +import { userEvent } from "vitest/browser"; import { render } from "vitest-browser-react"; +import type { EventsReadyState } from "~/features/events/events-client"; import type { ChatMessageAuthor, ClientChatMessage } from "../chat-types"; import { Chat } from "./Chat"; +const CONNECTION_STATUS_GRACE_MS = 1_500; + vi.mock("~/features/auth/core/user", () => ({ useUser: () => null, })); +// the composer only sends over a live event stream, which the tests have none of +const readyStateStore = vi.hoisted(() => { + let current = "CONNECTED"; + const listeners = new Set<() => void>(); + + return { + get: () => current, + set: (next: string) => { + current = next; + for (const listener of listeners) listener(); + }, + subscribe: (listener: () => void) => { + listeners.add(listener); + return () => listeners.delete(listener); + }, + }; +}); + +vi.mock("~/features/events/events-hooks", async (importOriginal) => { + const react = await import("react"); + + return { + ...(await importOriginal< + typeof import("~/features/events/events-hooks") + >()), + useEventsReadyState: () => + react.useSyncExternalStore( + readyStateStore.subscribe, + readyStateStore.get, + ), + }; +}); + +const setReadyState = (next: EventsReadyState) => readyStateStore.set(next); + +const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +afterEach(() => { + setReadyState("CONNECTED"); +}); + const ALICE: ChatMessageAuthor = { id: 1, username: "Alice", @@ -156,6 +201,32 @@ describe("Chat", () => { expect(screen.getByRole("textbox").elements()).toHaveLength(0); }); + test("sends the draft on enter and clears the composer", async () => { + const onSend = vi.fn(); + const screen = await renderChat([createMessage()], { onSend }); + + const composer = screen.getByPlaceholder("Press enter to send"); + await composer.fill("hello there"); + await userEvent.keyboard("{Enter}"); + + expect(onSend).toHaveBeenCalledWith({ + publicId: expect.any(String), + contents: "hello there", + }); + await expect.element(composer).toHaveValue(""); + }); + + test("a blank draft is not sent", async () => { + const onSend = vi.fn(); + const screen = await renderChat([createMessage()], { onSend }); + + const composer = screen.getByPlaceholder("Press enter to send"); + await composer.fill(" "); + await userEvent.keyboard("{Enter}"); + + expect(onSend).not.toHaveBeenCalled(); + }); + test("renders a splatnet room link with its QR code", async () => { const url = "https://s.nintendo.com/av5ja/lobby"; const screen = await renderChat([ @@ -284,4 +355,28 @@ describe("Chat", () => { await new Promise((resolve) => setTimeout(resolve, 300)); expect(element.scrollTop).toBe(readingPosition); }); + + test("says the stream is down only once it has been down for the grace period", async () => { + const screen = await renderChat([createMessage()]); + + setReadyState("CLOSED"); + await wait(CONNECTION_STATUS_GRACE_MS / 2); + expect(screen.getByText("Disconnected").elements()).toHaveLength(0); + + await wait(CONNECTION_STATUS_GRACE_MS); + await expect.element(screen.getByText("Disconnected")).toBeInTheDocument(); + }); + + test("never says the stream is down when it reconnects inside the grace period", async () => { + const screen = await renderChat([createMessage()]); + + setReadyState("CONNECTING"); + await wait(CONNECTION_STATUS_GRACE_MS / 2); + expect(screen.getByText("Connecting...").elements()).toHaveLength(0); + + setReadyState("CONNECTED"); + await wait(CONNECTION_STATUS_GRACE_MS * 2); + expect(screen.getByText("Connecting...").elements()).toHaveLength(0); + expect(screen.getByText("Disconnected").elements()).toHaveLength(0); + }); }); diff --git a/app/features/chat/components/Chat.module.css b/app/features/chat/components/Chat.module.css index 3965986f1..5e97da44d 100644 --- a/app/features/chat/components/Chat.module.css +++ b/app/features/chat/components/Chat.module.css @@ -139,13 +139,7 @@ align-items: center; gap: var(--s-1); - /* an empty composer is a no-op, so its "required" error is only noise */ - & [id$="-error"] { - display: none; - } - - /* the contents field wrapper takes the row's free space */ - & > div:first-child { + & > input { flex: 1; min-width: 0; } diff --git a/app/features/chat/components/Chat.tsx b/app/features/chat/components/Chat.tsx index 3a9624580..a8fbd4217 100644 --- a/app/features/chat/components/Chat.tsx +++ b/app/features/chat/components/Chat.tsx @@ -3,21 +3,18 @@ import { sub } from "date-fns"; import { SendHorizontal } from "lucide-react"; import { QRCodeSVG } from "qrcode.react"; import * as React from "react"; +import { browser } from "react-dom"; import { useTranslation } from "react-i18next"; -import { useLocation } from "react-router"; +import * as v from "valibot"; import { useEventsReadyState } from "~/features/events/events-hooks"; -import { - type FormRenderProps, - SendouForm, - useFormValue, -} from "~/form/SendouForm"; +import { useDebounce } from "~/hooks/useDebounce"; import { useVirtualizer } from "~/modules/virtualizer/react"; import { databaseTimestampToDate } from "~/utils/dates"; import { shortNanoid } from "~/utils/id"; import { Avatar } from "../../../components/Avatar"; import { SendouButton } from "../../../components/elements/Button"; -import { SubmitButton } from "../../../components/SubmitButton"; import { useDateTimeFormat } from "../../../hooks/intl/useDateTimeFormat"; +import { MESSAGE_MAX_LENGTH } from "../chat-constants"; import { useChatAutoScroll } from "../chat-hooks"; import { findRoomLinks } from "../chat-message-links"; import { sendChatMessageSchema } from "../chat-schemas"; @@ -26,6 +23,8 @@ import styles from "./Chat.module.css"; const MESSAGE_GAP = 8; const ESTIMATED_MESSAGE_HEIGHT = 44; +/** How long the stream may be down before the composer says so, so a connect right after page load never flashes it. */ +const CONNECTION_STATUS_GRACE_MS = 1_500; export interface ChatProps { messages: ClientChatMessage[]; @@ -50,6 +49,55 @@ export function Chat({ disabled, readOnly, }: ChatProps) { + const { t } = useTranslation(["common"]); + + return ( +
+
+ + } + > + + + {readOnly ? ( + // only observers ever see this, so it stays English +
Read-only
+ ) : disabled ? ( +
+ {t("common:chat.expired")} +
+ ) : ( + + )} +
+
+ ); +} + +function MessageLog({ + messages, + labelByUserId, + className, +}: Pick & { className?: string }) { + // the server can't open the pane scrolled to its end, so it stays empty + // (the fallback holding its place) until the browser renders it + React.use(browser("the chat log opens scrolled to its end")); + const { t } = useTranslation(["common"]); const messagesContainerRef = React.useRef(null); @@ -112,160 +160,127 @@ export function Chat({ }); return ( -
-
+ <> +
-
- {virtualizer.items.map(({ index, start }) => { - const msg = messages[index]; - const systemMessage = systemMessageText(msg); + {virtualizer.items.map(({ index, start }) => { + const msg = messages[index]; + const systemMessage = systemMessageText(msg); - return ( -
- {systemMessage ? ( - - ) : ( - - )} -
- ); - })} -
+ return ( +
+ {systemMessage ? ( + + ) : ( + + )} +
+ ); + })}
- {unseenMessagesInTheRoom ? ( - - {t("common:chat.newMessages")} - - ) : null} - {readOnly ? ( - // only observers ever see this, so it stays English -
Read-only
- ) : disabled ? ( -
- {t("common:chat.expired")} -
- ) : ( - - )}
-
+ {unseenMessagesInTheRoom ? ( + + {t("common:chat.newMessages")} + + ) : null} + ); } +/** A plain form on purpose: the chat client POSTs the message itself, so sending never touches the router or revalidates the page's loaders. */ function Composer({ onSend }: { onSend: ChatProps["onSend"] }) { - const { t } = useTranslation(["common"]); - const { pathname } = useLocation(); + const { t } = useTranslation(["common", "forms"]); const readyState = useEventsReadyState(); - const [publicId, setPublicId] = React.useState(() => shortNanoid()); - const [hasSent, setHasSent] = React.useState(false); - - // a send's autofocus must not carry over to an unrelated page - React.useEffect(() => { - setHasSent(false); - }, [pathname]); + const [contents, setContents] = React.useState(""); + const inputRef = React.useRef(null); + const [connectionStatusShown, setConnectionStatusShown] = + React.useState(false); + useDebounce( + () => setConnectionStatusShown(readyState !== "CONNECTED"), + CONNECTION_STATUS_GRACE_MS, + [readyState], + ); const sendingDisabled = readyState !== "CONNECTED"; + const showConnectionStatus = sendingDisabled && connectionStatusShown; + const isEmpty = contents.trim().length === 0; + + const handleSubmit = (event: React.FormEvent) => { + event.preventDefault(); + if (sendingDisabled || isEmpty) return; + + const parsed = v.safeParse(sendChatMessageSchema, { + publicId: shortNanoid(), + contents, + }); + if (!parsed.success) return; + + onSend(parsed.output); + setContents(""); + inputRef.current?.focus(); + }; return ( - { - onSend(values); - setPublicId(shortNanoid()); - setHasSent(true); - }} - > - {({ FormField }) => ( - <> - {readyState !== "CONNECTED" ? ( -
- {t( - readyState === "CONNECTING" - ? "common:chat.connecting" - : "common:chat.disconnected", - )} -
- ) : null} - - - )} -
- ); -} - -function ComposerRow({ - FormField, - sendingDisabled, - hasSent, -}: { - FormField: FormRenderProps["FormField"]; - sendingDisabled: boolean; - hasSent: boolean; -}) { - const { t } = useTranslation(["common"]); - const contents = useFormValue("contents"); - const isEmpty = typeof contents !== "string" || contents.trim().length === 0; - - return ( -
- - } - testId="chat-submit-button" - /> -
+ + {showConnectionStatus ? ( +
+ {t( + readyState === "CONNECTING" + ? "common:chat.connecting" + : "common:chat.disconnected", + )} +
+ ) : null} +
+ setContents(event.target.value)} + placeholder={t("forms:placeholders.chatMessage")} + maxLength={MESSAGE_MAX_LENGTH} + disabled={sendingDisabled} + /> + } + data-testid="chat-submit-button" + /> +
+ ); } diff --git a/app/features/chat/routes/api.chat.rooms.ts b/app/features/chat/routes/api.chat.rooms.ts index ac205f538..d9b32f664 100644 --- a/app/features/chat/routes/api.chat.rooms.ts +++ b/app/features/chat/routes/api.chat.rooms.ts @@ -1,23 +1,10 @@ import { requireUser } from "~/features/auth/core/user.server"; -import * as ChatRepository from "../ChatRepository.server"; -import * as ChatRoomResolver from "../ChatRoomResolver.server"; -import { roomListItem } from "../chat-room-list.server"; +import { resolveRoomList } from "../chat-room-list.server"; import type { ChatRoomListItem } from "../chat-types"; -/** The user's open chat rooms with unread counts; fetched after mount and refetched on `chatMessage` / `roomsChanged` events, never riding a page loader. */ +/** The user's open chat rooms with unread counts, refetched on `chatMessage` / `roomsChanged` events and reconnects; the root loader serves the first snapshot. */ export const loader = async (): Promise<{ rooms: ChatRoomListItem[] }> => { const user = requireUser(); - const rooms = await ChatRoomResolver.findAllByUserId(user.id); - const messageStats = await ChatRepository.findMessageStatsByRoomIds( - user.id, - rooms.map((room) => room.roomId), - ); - const statsByRoomId = new Map(messageStats.map((row) => [row.roomId, row])); - - return { - rooms: rooms.map((room) => - roomListItem(room, statsByRoomId.get(room.roomId), user), - ), - }; + return { rooms: await resolveRoomList(user) }; }; diff --git a/app/features/events/events-hooks.browser.test.tsx b/app/features/events/events-hooks.browser.test.tsx index a87535775..63c6106a6 100644 --- a/app/features/events/events-hooks.browser.test.tsx +++ b/app/features/events/events-hooks.browser.test.tsx @@ -155,13 +155,20 @@ const LATE_FIRST_CONNECT_MS = 2_000; let catchUps = 0; let triggerCatchUp: () => void = () => {}; -function CatchUpHarness({ enabled }: { enabled: boolean }) { +function CatchUpHarness({ + enabled, + heldSince, +}: { + enabled: boolean; + heldSince?: number; +}) { useEventsConnection(true); triggerCatchUp = useEventStreamCatchUp({ enabled, onCatchUp: () => { catchUps++; }, + heldSince, }); return null; @@ -245,6 +252,17 @@ describe("useEventStreamCatchUp", () => { expect(catchUps).toBe(1); }); + test("catches up on a first connect landing long after the data held was current", async () => { + await render( + , + ); + await advanceTimers(); + await helloArrives(); + await advanceTimers(CATCH_UP_MAX_JITTER_MS); + + expect(catchUps).toBe(1); + }); + test("does not catch up on the first connect after being enabled", async () => { const screen = await mountConnecting(false); diff --git a/app/features/events/events-hooks.ts b/app/features/events/events-hooks.ts index 2ab38d2a2..a103fc94b 100644 --- a/app/features/events/events-hooks.ts +++ b/app/features/events/events-hooks.ts @@ -69,9 +69,12 @@ export function useEventsTopic(topic: string, enabled = true) { export function useEventStreamCatchUp({ enabled, onCatchUp, + heldSince, }: { enabled: boolean; onCatchUp: () => void; + /** When the data the caller holds was current (client clock), for data that predates listening: a first connect landing long after it catches up. Defaults to when listening started. */ + heldSince?: number; }) { const connected = useEventsConnected(); const latestOnCatchUp = React.useRef(onCatchUp); @@ -97,7 +100,7 @@ export function useEventStreamCatchUp({ [], ); - useCatchUpOnConnect(enabled, connected, catchUp); + useCatchUpOnConnect(enabled, connected, catchUp, heldSince); React.useEffect(() => { if (!enabled) return; @@ -123,6 +126,7 @@ function useCatchUpOnConnect( enabled: boolean, connected: boolean, onConnect: () => void, + heldSince?: number, ) { const hasConnectedRef = React.useRef(false); const listeningSinceRef = React.useRef(null); @@ -141,13 +145,14 @@ function useCatchUpOnConnect( hasConnectedRef.current = true; if ( isFirstConnect && - Date.now() - listeningSinceRef.current < LATE_FIRST_CONNECT_MS + Date.now() - (heldSince ?? listeningSinceRef.current) < + LATE_FIRST_CONNECT_MS ) { return; } onConnect(); - }, [enabled, connected, onConnect]); + }, [enabled, connected, onConnect, heldSince]); } const returnListeners = new Set<() => void>(); diff --git a/app/features/scrims/loaders/scrims.$id.server.ts b/app/features/scrims/loaders/scrims.$id.server.ts index 62aeb8fd9..4fb1f3996 100644 --- a/app/features/scrims/loaders/scrims.$id.server.ts +++ b/app/features/scrims/loaders/scrims.$id.server.ts @@ -1,5 +1,5 @@ import type { LoaderFunctionArgs } from "react-router"; -import type { RouteChatRoom } from "~/features/chat/chat-types"; +import * as RouteChatRooms from "~/features/chat/RouteChatRooms.server"; import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import * as UserCardRepository from "~/features/user-card/UserCardRepository.server"; import * as UserRepository from "~/features/user-page/UserRepository.server"; @@ -54,10 +54,13 @@ export const loader = async ({ params }: LoaderFunctionArgs) => { })), post, // staff observers chat alongside the participants - chatRooms: (post.chatRoomId !== null && - (participantIds.includes(user.id) || user.roles.includes("STAFF")) - ? [{ roomId: post.chatRoomId, autoOpen: true }] - : []) satisfies RouteChatRoom[], + chatRooms: await RouteChatRooms.resolve( + user, + post.chatRoomId !== null && + (participantIds.includes(user.id) || user.roles.includes("STAFF")) + ? [{ roomId: post.chatRoomId, autoOpen: true }] + : [], + ), anyUserPrefersNoScreen, mapByMap, }; diff --git a/app/features/sendouq-match/loaders/q.match.$id.server.test.ts b/app/features/sendouq-match/loaders/q.match.$id.server.test.ts index 0dfff2bdd..c96676663 100644 --- a/app/features/sendouq-match/loaders/q.match.$id.server.test.ts +++ b/app/features/sendouq-match/loaders/q.match.$id.server.test.ts @@ -39,12 +39,19 @@ describe("q match loader", () => { const loadAs = (userId: number, matchId: number) => matchLoader({ user: userId, params: { id: String(matchId) } }); + const surfacedRooms = (data: Awaited>) => + data.chatRooms.map(({ room, autoOpen, label }) => ({ + roomId: room.id, + autoOpen, + label, + })); + test("surfaces both group chats read-only to staff outside the match", async () => { const match = await createMatch(); const data = await loadAs(staffId(), match.id); - expect(data.chatRooms).toEqual([ + expect(surfacedRooms(data)).toEqual([ { roomId: match.chatRoomId, autoOpen: true }, { roomId: await groupChatRoomId(match.alphaGroup.id), @@ -57,6 +64,12 @@ describe("q match loader", () => { label: "Group Bravo", }, ]); + // only the room opening on arrival brings its history along + expect(data.chatRooms.map((entry) => entry.messages !== null)).toEqual([ + true, + false, + false, + ]); }); test("gives a participant the match chat and their own group chat only", async () => { @@ -64,7 +77,7 @@ describe("q match loader", () => { const data = await loadAs(alphaUserIds()[0], match.id); - expect(data.chatRooms).toEqual([ + expect(surfacedRooms(data)).toEqual([ { roomId: match.chatRoomId, autoOpen: true }, { roomId: await groupChatRoomId(match.alphaGroup.id), diff --git a/app/features/sendouq-match/loaders/q.match.$id.server.ts b/app/features/sendouq-match/loaders/q.match.$id.server.ts index fa6eb11f4..4f740f094 100644 --- a/app/features/sendouq-match/loaders/q.match.$id.server.ts +++ b/app/features/sendouq-match/loaders/q.match.$id.server.ts @@ -1,6 +1,7 @@ import type { LoaderFunctionArgs } from "react-router"; import { getUser } from "~/features/auth/core/user.server"; -import type { RouteChatRoom } from "~/features/chat/chat-types"; +import type { RouteChatRoomInput } from "~/features/chat/chat-types"; +import * as RouteChatRooms from "~/features/chat/RouteChatRooms.server"; import * as Seasons from "~/features/mmr/core/Seasons"; import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import * as ScannerIngestRepository from "~/features/scanner-ingest/ScannerIngestRepository.server"; @@ -60,42 +61,44 @@ export const loader = async ({ params }: LoaderFunctionArgs) => { reportedWeapons, ingestedScoreboards, isOffSeason: Seasons.current() === null, - chatRooms: ((): RouteChatRoom[] => { - if (!user) return []; - - if (isParticipant) { - const ownGroup = matchUnmapped.groupAlpha.members.some( - (member) => member.id === user.id, - ) - ? match.groupAlpha - : match.groupBravo; - - return [match.chatRoomId, ownGroup.chatRoomId] - .filter((id): id is number => typeof id === "number") - .map((roomId) => ({ roomId, autoOpen: true })); - } - - if (!isStaff) return []; - - return [ - // staff observers chat alongside the participants in the match room - { roomId: matchUnmapped.chatRoomId, autoOpen: true }, - // the group chats stay private team spaces: staff only ever reads them - { - roomId: matchUnmapped.groupAlpha.chatRoomId, - autoOpen: false, - label: "Group Alpha", - }, - { - roomId: matchUnmapped.groupBravo.chatRoomId, - autoOpen: false, - label: "Group Bravo", - }, - ].filter( - (room): room is RouteChatRoom => typeof room.roomId === "number", - ); - })(), + chatRooms: await RouteChatRooms.resolve(user, routeChatRoomInputs()), }; + + function routeChatRoomInputs(): RouteChatRoomInput[] { + if (!user) return []; + + if (isParticipant) { + const ownGroup = matchUnmapped.groupAlpha.members.some( + (member) => member.id === user.id, + ) + ? match.groupAlpha + : match.groupBravo; + + return [match.chatRoomId, ownGroup.chatRoomId] + .filter((id): id is number => typeof id === "number") + .map((roomId) => ({ roomId, autoOpen: true })); + } + + if (!isStaff) return []; + + return [ + // staff observers chat alongside the participants in the match room + { roomId: matchUnmapped.chatRoomId, autoOpen: true }, + // the group chats stay private team spaces: staff only ever reads them + { + roomId: matchUnmapped.groupAlpha.chatRoomId, + autoOpen: false, + label: "Group Alpha", + }, + { + roomId: matchUnmapped.groupBravo.chatRoomId, + autoOpen: false, + label: "Group Bravo", + }, + ].filter( + (room): room is RouteChatRoomInput => typeof room.roomId === "number", + ); + } }; export type SendouQMatchLoaderData = SerializeFrom; diff --git a/app/features/sendouq/loaders/q.looking.server.ts b/app/features/sendouq/loaders/q.looking.server.ts index 021cae2a3..8b58caa59 100644 --- a/app/features/sendouq/loaders/q.looking.server.ts +++ b/app/features/sendouq/loaders/q.looking.server.ts @@ -1,7 +1,7 @@ import type { LoaderFunctionArgs } from "react-router"; import * as R from "remeda"; import { requireUser } from "~/features/auth/core/user.server"; -import type { RouteChatRoom } from "~/features/chat/chat-types"; +import * as RouteChatRooms from "~/features/chat/RouteChatRooms.server"; import { resolveNotifications } from "~/features/notifications/core/resolve.server"; import * as SQGroupRepository from "~/features/sendouq/SQGroupRepository.server"; import { cachedStreams } from "~/features/sendouq-streams/core/streams.server"; @@ -70,10 +70,11 @@ export const loader = async ({ url }: LoaderFunctionArgs) => { : [], lastUpdated: Date.now(), streamsCount: (await cachedStreams()).length, - chatRooms: (ownGroup && - ownGroup.members.length > 1 && - ownGroup.chatRoomId !== null - ? [{ roomId: ownGroup.chatRoomId, autoOpen: true }] - : []) satisfies RouteChatRoom[], + chatRooms: await RouteChatRooms.resolve( + user, + ownGroup && ownGroup.members.length > 1 && ownGroup.chatRoomId !== null + ? [{ roomId: ownGroup.chatRoomId, autoOpen: true }] + : [], + ), }; }; diff --git a/app/features/tournament-match/loaders/to.$id.matches.$mid.server.ts b/app/features/tournament-match/loaders/to.$id.matches.$mid.server.ts index d4dc92e68..02085867f 100644 --- a/app/features/tournament-match/loaders/to.$id.matches.$mid.server.ts +++ b/app/features/tournament-match/loaders/to.$id.matches.$mid.server.ts @@ -1,6 +1,6 @@ import cachified from "@epic-web/cachified"; import type { LoaderFunctionArgs } from "react-router"; -import type { RouteChatRoom } from "~/features/chat/chat-types"; +import * as RouteChatRooms from "~/features/chat/RouteChatRooms.server"; import * as ScannerIngestRepository from "~/features/scanner-ingest/ScannerIngestRepository.server"; import * as ReportedWeaponRepository from "~/features/sendouq-match/ReportedWeaponRepository.server"; import * as TournamentRepository from "~/features/tournament/TournamentRepository.server"; @@ -209,10 +209,13 @@ export const loader = async ({ params }: LoaderFunctionArgs) => { endedEarly, noScreen, // observers (TO/streamer/site staff) chat alongside the participants - chatRooms: (match.chatRoomId && - (isParticipant || isSiteStaff || tournament.isOrganizerOrStreamer(user)) - ? [{ roomId: match.chatRoomId, autoOpen: true }] - : []) satisfies RouteChatRoom[], + chatRooms: await RouteChatRooms.resolve( + user, + match.chatRoomId && + (isParticipant || isSiteStaff || tournament.isOrganizerOrStreamer(user)) + ? [{ roomId: match.chatRoomId, autoOpen: true }] + : [], + ), canJoin, // the views can't derive these themselves, the layout ships no bracket match data bracketContext: { diff --git a/app/form/SendouForm.tsx b/app/form/SendouForm.tsx index ec2fbc3e6..ca276a246 100644 --- a/app/form/SendouForm.tsx +++ b/app/form/SendouForm.tsx @@ -108,10 +108,6 @@ type BaseFormProps = { ) => boolean; /** Called once after the action returns without field errors. */ onSuccess?: () => void; - /** For forms that render their own submit control inside `children`. */ - hideSubmitButton?: boolean; - /** When false, navigating away with unsaved edits is not blocked (e.g. a chat draft). */ - guardUnsavedChanges?: boolean; }; /** @@ -187,8 +183,6 @@ function SendouFormInner({ secondarySubmit, hideSubmitButtonWhen, onSuccess, - hideSubmitButton = false, - guardUnsavedChanges = true, }: SendouFormProps) { const { t } = useTranslation(["forms"]); const fetcher = useFetcher<{ fieldErrors?: Record }>(); @@ -264,11 +258,7 @@ function SendouFormInner({ const hasUnsavedChangesRef = React.useRef<() => boolean>(() => false); hasUnsavedChangesRef.current = () => - guardUnsavedChanges && - mode === "submit" && - !readOnly && - store.dirty && - fetcher.state === "idle"; + mode === "submit" && !readOnly && store.dirty && fetcher.state === "idle"; useUnsavedChangesChecker(hasUnsavedChangesRef); const previousFetcherStateRef = React.useRef(fetcher.state); @@ -325,7 +315,7 @@ function SendouFormInner({ <> {title ?

{title}

: null} {resolvedChildren} - {mode !== "submit" || readOnly || hideSubmitButton ? null : ( + {mode !== "submit" || readOnly ? null : ( boolean) | undefined diff --git a/app/root.tsx b/app/root.tsx index 71fa3633c..87400f69a 100644 --- a/app/root.tsx +++ b/app/root.tsx @@ -38,6 +38,7 @@ import { Layout, NPROGRESS_ANCHOR_ID } from "./components/layout"; import { getUser } from "./features/auth/core/user.server"; import { userMiddleware } from "./features/auth/core/user-middleware.server"; import { ChatProvider } from "./features/chat/ChatProvider"; +import { resolveRoomList } from "./features/chat/chat-room-list.server"; import { isMatchResultsScopedRevalidation } from "./features/chat/revalidation-scope"; import { GlobalStatusProvider } from "./features/global-status/GlobalStatusProvider"; import { getSidenavSession } from "./features/layout/core/sidenav-session.server"; @@ -146,6 +147,7 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { return data( { locale, + chatRoomList: user ? await resolveRoomList(user) : [], i18nPreloadUrls: localePreloadUrls(locale), theme: themeSession.getTheme(), sidenavCollapsed: sidenavSession.getCollapsed(), @@ -266,7 +268,10 @@ function Document({ - + diff --git a/changelog/2026-09-20-chat-loads-with-page.md b/changelog/2026-09-20-chat-loads-with-page.md new file mode 100644 index 000000000..7f7191414 --- /dev/null +++ b/changelog/2026-09-20-chat-loads-with-page.md @@ -0,0 +1,4 @@ +--- +type: feature +--- +Chat opens together with the page instead of loading in after it, so pages with a chat no longer shift around when it appears diff --git a/docs/dev/architecture.md b/docs/dev/architecture.md index cda2acf39..9572b8d79 100644 --- a/docs/dev/architecture.md +++ b/docs/dev/architecture.md @@ -223,15 +223,17 @@ There is a single `ChatProvider` mounted near the root, glue over the framework- Two things drive which rooms the client cares about: -1) **The user's own rooms.** `GET /api/chat/rooms` returns every room the user participates in, resolved from the owning entity (SendouQ group/match, tournament match/team, scrim). These show up in the chat list regardless of which page the user is on. -2) **Route-exposed `chatRooms`.** A loader can expose `chatRooms: RouteChatRoom[]` in its returned data. The provider reads this out of `useMatches()` and surfaces those rooms for the duration of the route being active — used for rooms the user is viewing but is not a participant of (e.g. a tournament match chat viewed by a TO). An `autoOpen` room is opened for the viewer, the rest are only listed in the chat sidebar (e.g. the private group chats of a SendouQ match, which staff may read but not post in). +1) **The user's own rooms.** Every room the user participates in, resolved from the owning entity (SendouQ group/match, tournament match/team, scrim). The root loader serves the list with the page (`chatRoomList`) and `GET /api/chat/rooms` refetches it on events and reconnects. These show up in the chat list regardless of which page the user is on. +2) **Route-exposed `chatRooms`.** A loader can expose `chatRooms: RouteChatRoom[]` in its returned data, built with `RouteChatRooms.resolve()` so the page arrives with each room as the sidebar lists it and, for rooms opening on arrival, its latest messages (no fetch after mount, no content shift). The provider reads this out of `useMatches()` and surfaces those rooms for the duration of the route being active — used for rooms the user is viewing but is not a participant of (e.g. a tournament match chat viewed by a TO). An `autoOpen` room is opened for the viewer, the rest are only listed in the chat sidebar (e.g. the private group chats of a SendouQ match, which staff may read but not post in). The loader decides who gets which rooms; the helper drops any the user may not view. Example loader: ```ts return { // ...other loader data - chatRooms: [{ roomId: match.chatRoomId, autoOpen: true }], + chatRooms: await RouteChatRooms.resolve(user, [ + { roomId: match.chatRoomId, autoOpen: true }, + ]), }; ```