diff --git a/app/dashboard/consultant/[consultantId]/(features)/messages/MessagesTab.tsx b/app/dashboard/consultant/[consultantId]/(features)/messages/MessagesTab.tsx index 5facfcf03..bb4878454 100644 --- a/app/dashboard/consultant/[consultantId]/(features)/messages/MessagesTab.tsx +++ b/app/dashboard/consultant/[consultantId]/(features)/messages/MessagesTab.tsx @@ -4,6 +4,7 @@ import { Loader2 } from "lucide-react"; import { ChatLayout } from "@/components/chat/ChatLayout"; import { ChatUnavailable } from "@/components/chat/ChatUnavailable"; import { useStreamConnection } from "@/providers/StreamProvider"; +import { StreamChatScope } from "@/components/stream/StreamChatScope"; interface MessagesTabProps { userId: string; @@ -24,7 +25,9 @@ export function MessagesTab({ userId: _userId }: Readonly) { {error ? ( ) : chatConnected ? ( - + + + ) : (
diff --git a/app/dashboard/consultee/[consulteeId]/(features)/messages/MessagesTab.tsx b/app/dashboard/consultee/[consulteeId]/(features)/messages/MessagesTab.tsx index 7f0c3b8fa..80e0bc668 100644 --- a/app/dashboard/consultee/[consulteeId]/(features)/messages/MessagesTab.tsx +++ b/app/dashboard/consultee/[consulteeId]/(features)/messages/MessagesTab.tsx @@ -4,6 +4,7 @@ import { Loader2 } from "lucide-react"; import { ChatLayout } from "@/components/chat/ChatLayout"; import { ChatUnavailable } from "@/components/chat/ChatUnavailable"; import { useStreamConnection } from "@/providers/StreamProvider"; +import { StreamChatScope } from "@/components/stream/StreamChatScope"; /** * Full-bleed chat surface: cancels PageScaffold padding and fills the @@ -20,7 +21,9 @@ export default function MessagesTab() { {error ? ( ) : chatConnected ? ( - + + + ) : (
diff --git a/app/dashboard/organization/[orgId]/messages/MessagesClient.tsx b/app/dashboard/organization/[orgId]/messages/MessagesClient.tsx index 6d377fb2d..18ef986e8 100644 --- a/app/dashboard/organization/[orgId]/messages/MessagesClient.tsx +++ b/app/dashboard/organization/[orgId]/messages/MessagesClient.tsx @@ -3,6 +3,7 @@ import { Loader2 } from "lucide-react"; import { ChatLayout } from "@/components/chat/ChatLayout"; +import { StreamChatScope } from "@/components/stream/StreamChatScope"; import { ChatUnavailable } from "@/components/chat/ChatUnavailable"; import { useStreamConnection } from "@/providers/StreamProvider"; @@ -38,5 +39,9 @@ export function MessagesClient() { ); } - return ; + return ( + + + + ); } diff --git a/app/meetings/layout.tsx b/app/meetings/layout.tsx index 9e0d574e4..2601b0595 100644 --- a/app/meetings/layout.tsx +++ b/app/meetings/layout.tsx @@ -1,5 +1,6 @@ import "@stream-io/video-react-sdk/dist/css/styles.css"; import StreamProvider from "@/providers/StreamProvider"; +import { StreamVideoScope } from "@/components/stream/StreamVideoScope"; import { requireOnboarded } from "@/lib/auth-guard"; export default async function MeetingsLayout({ @@ -16,7 +17,10 @@ export default async function MeetingsLayout({ enableChat={false} enableVideo={true} > - {children} + {/* /meetings is the video surface, so the SDK context is mounted for the + whole route rather than per-page. StreamProvider no longer supplies it + — see components/stream/StreamVideoScope. */} + {children} ); } diff --git a/components/appointments/consultee/ConsulteeAppointmentsAdapter.tsx b/components/appointments/consultee/ConsulteeAppointmentsAdapter.tsx index bfc471bf9..3de3dd631 100644 --- a/components/appointments/consultee/ConsulteeAppointmentsAdapter.tsx +++ b/components/appointments/consultee/ConsulteeAppointmentsAdapter.tsx @@ -3,7 +3,7 @@ import { useState } from "react"; import * as Sentry from "@sentry/nextjs"; import { useParams, useRouter } from "next/navigation"; -import { useStreamVideoClient } from "@stream-io/video-react-sdk"; +import { getGlobalVideoClient } from "@/lib/stream/disconnect"; import { useQueryClient } from "@tanstack/react-query"; import { useToast } from "@/hooks/use-toast"; @@ -113,7 +113,6 @@ const KIND_TO_REPORT_TYPE: Record< export function useConsulteeAppointmentsAdapter(): AppointmentActionAdapter { const router = useRouter(); const { toast } = useToast(); - const client = useStreamVideoClient(); const { data: session } = useSession(); const queryClient = useQueryClient(); const params = useParams<{ consulteeId: string }>(); @@ -144,6 +143,10 @@ export function useConsulteeAppointmentsAdapter(): AppointmentActionAdapter { // Join can't go through useEventActions — its args follow activeVm state, // which wouldn't be committed yet on a same-click join from a row. const joinNow = async (vm: AppointmentVM, slot: SlotLike) => { + // Read the singleton at click time rather than via useStreamVideoClient: + // the SDK context is now scoped to /meetings, and this is the same instance + // would hand back. Matches the #248 lazy-join idiom. + const client = getGlobalVideoClient(); if (!client) { toast({ title: "Not signed in", diff --git a/components/appointments/consultee/useEventActions.ts b/components/appointments/consultee/useEventActions.ts index 075c85039..76b8dd422 100644 --- a/components/appointments/consultee/useEventActions.ts +++ b/components/appointments/consultee/useEventActions.ts @@ -5,7 +5,7 @@ import * as Sentry from "@sentry/nextjs"; import { useToast } from "@/hooks/use-toast"; import { useParams, useRouter } from "next/navigation"; import { useQueryClient } from "@tanstack/react-query"; -import { useStreamVideoClient } from "@stream-io/video-react-sdk"; +import { getGlobalVideoClient } from "@/lib/stream/disconnect"; import { getOrCreateAppointmentMeeting } from "@/lib/meeting"; import type { TAppointment } from "@/types/appointment"; import type { SlotOfAppointment } from "@prisma/client"; @@ -112,7 +112,6 @@ export function useEventActions({ }: UseEventActionsOptions) { const { toast } = useToast(); const router = useRouter(); - const client = useStreamVideoClient(); const queryClient = useQueryClient(); const params = useParams<{ consulteeId: string }>(); const consulteeId = params?.consulteeId; @@ -310,6 +309,9 @@ export function useEventActions({ const handleJoinSession = async (forceSlot?: SlotOfAppointment) => { const slotToUse = forceSlot || getJoinableSlot(); + // Singleton at click time: the SDK context is scoped to /meetings now, and + // this is the same instance would return (#248 idiom). + const client = getGlobalVideoClient(); if (!client) { toast({ title: "Not signed in", diff --git a/components/stream/StreamChatScope.tsx b/components/stream/StreamChatScope.tsx new file mode 100644 index 000000000..e3eda1c71 --- /dev/null +++ b/components/stream/StreamChatScope.tsx @@ -0,0 +1,43 @@ +"use client"; + +import dynamic from "next/dynamic"; +import { useSyncExternalStore } from "react"; +import { + getStreamConnectionServerSnapshot, + getStreamConnectionSnapshot, + subscribeStreamConnection, +} from "@/lib/stream/connection-store"; + +/** + * Mounts the Stream `` context around a chat surface. + * + * This used to wrap the WHOLE dashboard from StreamProvider, which cost two + * things: `ssr: false` on that wrapper meant no dashboard markup was ever + * server-rendered, and the wrapper appearing once the socket settled changed + * the element type at that position and remounted everything under it (#248). + * + * Scoping it here is safe because every `useChatContext` consumer lives under + * `components/chat/`. The sidebar's unread badge is deliberately NOT one of + * them — `hooks/useChatUnreadCount` reads the `StreamChat` singleton directly + * and documents that it works outside the provider. + * + * Renders children unwrapped until the client connects; chat consumers already + * guard a null client, and this keeps the surface visible while connecting. + */ +const ChatProvider = dynamic( + () => import("stream-chat-react").then((m) => ({ default: m.Chat })), + { ssr: false }, +); + +export function StreamChatScope({ + children, +}: Readonly<{ children: React.ReactNode }>) { + const { clients } = useSyncExternalStore( + subscribeStreamConnection, + getStreamConnectionSnapshot, + getStreamConnectionServerSnapshot, + ); + + if (!clients?.chat) return <>{children}; + return {children}; +} diff --git a/components/stream/StreamVideoScope.tsx b/components/stream/StreamVideoScope.tsx new file mode 100644 index 000000000..d1317b0f6 --- /dev/null +++ b/components/stream/StreamVideoScope.tsx @@ -0,0 +1,40 @@ +"use client"; + +import dynamic from "next/dynamic"; +import { useSyncExternalStore } from "react"; +import { + getStreamConnectionServerSnapshot, + getStreamConnectionSnapshot, + subscribeStreamConnection, +} from "@/lib/stream/connection-store"; + +/** + * Mounts the Stream `` context around a video surface. + * + * Sibling of StreamChatScope — see that file for why these contexts no longer + * wrap the entire dashboard. Video consumers (`useStreamVideoClient`, `useCall`) + * are confined to `/meetings` plus the consultee appointments adapter. + * + * Renders children unwrapped until the client connects; the existing video + * consumers already guard a null client. + */ +const VideoProvider = dynamic( + () => + import("@stream-io/video-react-sdk").then((m) => ({ + default: m.StreamVideo, + })), + { ssr: false }, +); + +export function StreamVideoScope({ + children, +}: Readonly<{ children: React.ReactNode }>) { + const { clients } = useSyncExternalStore( + subscribeStreamConnection, + getStreamConnectionSnapshot, + getStreamConnectionServerSnapshot, + ); + + if (!clients?.video) return <>{children}; + return {children}; +} diff --git a/lib/stream/connection-store.ts b/lib/stream/connection-store.ts new file mode 100644 index 000000000..943e074b3 --- /dev/null +++ b/lib/stream/connection-store.ts @@ -0,0 +1,79 @@ +import type { StreamChat } from "stream-chat"; +import type { StreamVideoClient } from "@stream-io/video-react-sdk"; + +/** + * Module-level store for the Stream connection, read via `useSyncExternalStore`. + * + * Why a store and not React state: the provider used to WRAP `children` and + * swap the wrapper set once the sockets settled (`children` → `` → + * ``). React tears down a subtree when the element type at a position + * changes, so that swap remounted the whole dashboard — the remount storm + * behind "I pressed Join ten times" (#248). Publishing to a store instead lets + * the connector render `null` as a SIBLING of `children`, so nothing above the + * dashboard ever changes shape. + * + * It also un-blocks SSR. The connector is still `ssr: false`, but `ssr: false` + * skips server rendering for the component AND its children — so while it + * wrapped the dashboard, no dashboard markup reached the HTML at all. Measured + * on #1102: ` void>(); + +export function subscribeStreamConnection(listener: () => void): () => void { + listeners.add(listener); + return () => listeners.delete(listener); +} + +export function getStreamConnectionSnapshot(): StreamConnectionSnapshot { + return snapshot; +} + +/** + * Server snapshot must be a STABLE reference, not a fresh object — React calls + * this during SSR and would loop forever on a new identity each time. + */ +export function getStreamConnectionServerSnapshot(): StreamConnectionSnapshot { + return INITIAL; +} + +export function setStreamConnection( + patch: Partial, +): void { + const next = { ...snapshot, ...patch }; + // Bail on no-op writes so consumers do not re-render on every heartbeat. + const unchanged = ( + Object.keys(next) as (keyof StreamConnectionSnapshot)[] + ).every((k) => next[k] === snapshot[k]); + if (unchanged) return; + snapshot = next; + for (const l of listeners) l(); +} + +/** Test/logout helper — drops connection state without touching the clients. */ +export function resetStreamConnection(): void { + snapshot = INITIAL; + for (const l of listeners) l(); +} diff --git a/providers/StreamProvider.tsx b/providers/StreamProvider.tsx index 7180e03cb..595b29cb9 100644 --- a/providers/StreamProvider.tsx +++ b/providers/StreamProvider.tsx @@ -1,7 +1,12 @@ "use client"; -import { createContext, useContext } from "react"; +import { createContext, useCallback, useContext, useSyncExternalStore } from "react"; import dynamic from "next/dynamic"; +import { + getStreamConnectionServerSnapshot, + getStreamConnectionSnapshot, + subscribeStreamConnection, +} from "@/lib/stream/connection-store"; // Re-export the logout helper from the SDK-free module so existing importers of // `disconnectStreamClients` from this path keep working unchanged. Callers that @@ -10,10 +15,8 @@ import dynamic from "next/dynamic"; export { disconnectStreamClients } from "@/lib/stream/disconnect"; // ── Connection-state context ──────────────────────────────────────────────── -// Defined in this SDK-free shell (not the heavy impl) so consumers like -// DebugDialog can import useStreamConnection without pulling the SDK, and so the -// context identity is stable across the lazy boundary. The heavy impl imports -// this same context and pushes the live value into it once connected. +// Defined in this SDK-free shell (not the heavy connector) so consumers like +// DebugDialog can import useStreamConnection without pulling the SDK. export interface StreamConnectionState { chatConnected: boolean; @@ -23,11 +26,6 @@ export interface StreamConnectionState { retryConnection: () => void; } -// Default (impl-not-yet-loaded) connection state. Returned by -// useStreamConnection while the dynamic impl is still loading so consumers never -// crash during the lazy-load window. Previously useStreamConnection threw when -// used outside the provider; the dashboard never relied on that throw in prod, -// and a safe default is required now that the provider is lazy. const DEFAULT_CONNECTION_STATE: StreamConnectionState = { chatConnected: false, videoConnected: false, @@ -51,52 +49,58 @@ export interface StreamProviderProps { enableVideo?: boolean; } -// Loading strategy (deliberate, per-scenario — NOT blanket lazy): -// -// • SDK code-split (lazy chunk): YES. The Stream SDK is heavy and the dashboard -// LANDING route is always /home, which renders no chat/video UI. Splitting the -// SDK + its two stylesheets into a chunk keeps them out of /home's synchronous -// bundle, so /home parses/paints without paying for the SDK up front. -// -// • When does that chunk actually load? On dashboard routes the provider is -// mounted by the LAYOUT, so the chunk begins downloading on /home too — but -// IN PARALLEL, off the critical bundle, not blocking first paint. This is the -// right call (deferred/parallel, not "only when chat opens") because the -// consultant sidebar shows a chat-unread badge on every route incl. /home, -// which needs the chat client connected. Gating the whole provider behind -// "user opened chat" would break that badge. The actual connect (websocket) -// is further deferred to requestIdleCallback inside the impl (see #248 there). -// -// • ssr:false: the SDK is browser-only (websockets/WebRTC); SSR-ing it is wasted -// server work and hydration mismatch risk. -// -// Children are rendered through the impl, which renders them DIRECTLY even before -// connection completes (it no longer blocks on a spinner until connected). During -// the brief chunk-download window the fallback below shows — this is strictly -// shorter than the previous behavior, which blocked children until BOTH clients -// finished their websocket handshake. -function StreamProviderLoading() { - return ( -
-
-
- ); -} - -const LazyStreamProviderImpl = dynamic( - () => import("@/providers/StreamProviderImpl"), - { ssr: false, loading: () => }, -); +// The connector holds the SDK + the websocket lifecycle and renders NOTHING. +// ssr:false keeps the browser-only SDK off the server, and because it no longer +// wraps `children`, that no longer costs the dashboard its server rendering. +const StreamConnector = dynamic(() => import("@/providers/StreamProviderImpl"), { + ssr: false, +}); /** - * SDK-free shell. Renders children through the lazily-loaded implementation, - * which provides the chat/video contexts + connection lifecycle once its chunk - * loads. The impl renders children directly even before connection completes; - * video consumers already guard a null client, and chat consumers only render on - * the chat route (under ). + * SDK-free shell. Renders `children` DIRECTLY — server-side included — and + * mounts the connector as a sibling. + * + * Two bugs this shape exists to prevent, both measured: + * + * 1. `ssr: false` skips server rendering for the component AND its children. + * While the connector wrapped the dashboard, no dashboard markup reached + * the HTML: `` → ``). A changed element type at a + * position remounts that whole subtree — the storm behind "I pressed Join + * ten times" (#248). Children now sit in a fixed position forever. + * + * The SDK's own `` / `` contexts are mounted by the surfaces + * that actually consume them (the Messages tabs and /meetings), not here. */ -const StreamProvider = (props: StreamProviderProps) => { - return ; +const StreamProvider = ({ children, ...connectorProps }: StreamProviderProps) => { + const snapshot = useSyncExternalStore( + subscribeStreamConnection, + getStreamConnectionSnapshot, + getStreamConnectionServerSnapshot, + ); + + const retryConnection = useCallback(() => { + // The connector owns the retry loop; it listens for this event so the + // shell does not have to import anything from the SDK bundle to expose it. + window.dispatchEvent(new CustomEvent("stream:retry-connection")); + }, []); + + const value: StreamConnectionState = { + chatConnected: snapshot.chatConnected, + videoConnected: snapshot.videoConnected, + isConnecting: snapshot.isConnecting, + error: snapshot.error, + retryConnection, + }; + + return ( + + {children} + + + ); }; export default StreamProvider; diff --git a/providers/StreamProviderImpl.tsx b/providers/StreamProviderImpl.tsx index a05d093a8..e844b104c 100644 --- a/providers/StreamProviderImpl.tsx +++ b/providers/StreamProviderImpl.tsx @@ -12,8 +12,7 @@ import { useCallback, useEffect, useState, useRef } from "react"; import { StreamChat } from "stream-chat"; -import { Chat } from "stream-chat-react"; -import { StreamVideo, StreamVideoClient } from "@stream-io/video-react-sdk"; +import { StreamVideoClient } from "@stream-io/video-react-sdk"; import { chatTokenProvider, tokenProvider, @@ -23,12 +22,18 @@ import { syncUserEventChannels } from "@/actions/stream/chat/event-channel.actio import { useUserData } from "@/hooks/useUserData"; import { mapRoleToStream } from "@/lib/user"; import { streamLogger } from "@/lib/stream-logger"; -import StreamErrorBoundary from "@/components/stream/StreamErrorBoundary"; -import { - StreamConnectionContext, - type StreamConnectionState, - type StreamProviderProps, -} from "@/providers/StreamProvider"; +import { setStreamConnection } from "@/lib/stream/connection-store"; + +/** + * The connector takes no `children`. It renders nothing and publishes the + * connection to the store instead — see lib/stream/connection-store.ts for why + * (SSR of the dashboard subtree, and the remount storm). + */ +export interface StreamConnectorProps { + userId: string; + enableChat?: boolean; + enableVideo?: boolean; +} // Shared module-level client refs now live in an SDK-free module so SDK-free // callers can disconnect on logout without linking the Stream SDK. #248 import { @@ -71,11 +76,10 @@ interface SettledStreamClients { } const StreamProviderImpl = ({ - children, userId, enableChat = true, enableVideo = true, -}: StreamProviderProps) => { +}: StreamConnectorProps) => { const [clients, setClients] = useState(null); const [chatConnected, setChatConnected] = useState(false); const [videoConnected, setVideoConnected] = useState(false); @@ -84,7 +88,7 @@ const StreamProviderImpl = ({ // We need BOTH a ref and state for retry count: the ref (connectionAttemptsRef) // is used inside setTimeout/async closures where state would be stale, while // this state variable drives re-renders so the UI shows the correct attempt count. - const [retryCount, setRetryCount] = useState(0); + const [, setRetryCount] = useState(0); // Use ref for connection attempts to avoid stale closures in retry logic const connectionAttemptsRef = useRef(0); @@ -493,72 +497,32 @@ const StreamProviderImpl = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [userDetails?.id, isLoading, connectServices]); - // Connection state for context - const connectionState: StreamConnectionState = { - chatConnected, - videoConnected, - isConnecting, - error, - retryConnection, - }; - - // #248: do NOT block children render on connection. Previously this returned - // a full-screen spinner until both clients connected, which gated the entire - // dashboard route (incl. home) behind the Stream connect-storm. We now always - // render children immediately; the Stream context providers wrap them once the - // clients are ready, and the video/chat consumers already guard a null client. - - // The wrapper set is derived from ONE settled value and nested in a fixed - // order — Chat outside, StreamVideo inside — so the shape here is a pure - // function of `clients` rather than of which socket won the race. In the - // normal case that means exactly one shape change for the whole session: - // unwrapped while connecting, then wrapped once both connects settle. - // - // A connect that genuinely FAILS still costs a second change if a later retry - // succeeds. That is accepted: it is a degraded path, the retry loop is capped - // at 5 attempts, and withholding the client that did connect would break the - // sidebar's chat-unread badge on every route (#248). - let content = children; - - if (clients?.video) { - content = {content}; - } - - if (clients?.chat) { - content = {content}; - } - - // The connection-failed banner renders alongside children (not in place of - // them) so the underlying route stays usable while Stream retries/recovers. - return ( - { - streamLogger.error("Stream Provider Error", error, { - componentStack: errorInfo.componentStack, - }); - setError(error.message); - }} - enableRetry={true} - > - - {error && retryCount >= 5 && ( -
-
-

Connection Failed

-

{error}

- -
-
- )} - {content} - - - ); + // Publish to the store rather than wrapping children. The wrapper set used to + // be derived here — `children` → `` → `` — which changed + // the element type at that position once the sockets settled and remounted + // the whole dashboard (#248). The SDK contexts are now mounted by the + // surfaces that consume them; this component only reports state. + useEffect(() => { + setStreamConnection({ + clients, + chatConnected, + videoConnected, + isConnecting, + error, + }); + }, [clients, chatConnected, videoConnected, isConnecting, error]); + + // The shell exposes `retryConnection` without importing the SDK bundle, so it + // asks for a retry by event rather than by calling into here directly. + useEffect(() => { + const onRetry = () => retryConnection(); + window.addEventListener("stream:retry-connection", onRetry); + return () => window.removeEventListener("stream:retry-connection", onRetry); + }, [retryConnection]); + + // Renders nothing: it is a sibling of `children`, not a wrapper. Consumers + // read connection state from the context in providers/StreamProvider.tsx. + return null; }; export default StreamProviderImpl;