diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index b501cbbd..f02665a5 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -942,6 +942,40 @@ describe("ChatView timeline estimator parity (full app)", () => { document.body.innerHTML = ""; }); + it("mounts the regular chat surface without a transport render loop", async () => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: createSnapshotForTargetUser({ + targetMessageId: "msg-user-transport-mount" as MessageId, + targetText: "transport mount", + }), + }); + + try { + await vi.waitFor(() => { + expect( + wsRequests.some((request) => request._tag === ORCHESTRATION_WS_METHODS.getSnapshot), + ).toBe(true); + }); + + await expect.element(page.getByTestId("new-thread-button")).toBeInTheDocument(); + expect( + consoleErrorSpy.mock.calls.some((call) => + call.some( + (value) => + typeof value === "string" && + (value.includes("Too many re-renders") || + value.includes("Minified React error #301")), + ), + ), + ).toBe(false); + } finally { + consoleErrorSpy.mockRestore(); + await mounted.cleanup(); + } + }); + it.each(TEXT_VIEWPORT_MATRIX)( "keeps long user message estimate close at the $name viewport", async (viewport) => { diff --git a/apps/web/src/hooks/useConnectionHealth.test.tsx b/apps/web/src/hooks/useConnectionHealth.test.tsx index d282b3d1..46936d2d 100644 --- a/apps/web/src/hooks/useConnectionHealth.test.tsx +++ b/apps/web/src/hooks/useConnectionHealth.test.tsx @@ -4,17 +4,22 @@ import { act, create, type ReactTestRenderer } from "react-test-renderer"; import type { ConnectionMetrics, TransportState } from "../wsTransport"; import { type ConnectionHealth, useConnectionHealth } from "./useConnectionHealth"; -const { createWsNativeApiMock, getTransportMetricsMock, onTransportStateChangeMock } = vi.hoisted( - () => ({ - createWsNativeApiMock: vi.fn(), - getTransportMetricsMock: vi.fn(), - onTransportStateChangeMock: vi.fn(), - }), -); +const { + createWsNativeApiMock, + getTransportMetricsMock, + getTransportStateSnapshotMock, + onTransportStateChangeMock, +} = vi.hoisted(() => ({ + createWsNativeApiMock: vi.fn(), + getTransportMetricsMock: vi.fn(), + getTransportStateSnapshotMock: vi.fn(), + onTransportStateChangeMock: vi.fn(), +})); vi.mock("../wsNativeApi", () => ({ createWsNativeApi: createWsNativeApiMock, getTransportMetrics: getTransportMetricsMock, + getTransportStateSnapshot: getTransportStateSnapshotMock, onTransportStateChange: onTransportStateChangeMock, })); @@ -88,6 +93,7 @@ beforeEach(() => { createWsNativeApiMock.mockReset(); getTransportMetricsMock.mockReset().mockImplementation(() => currentMetrics); + getTransportStateSnapshotMock.mockReset().mockImplementation(() => currentState); onTransportStateChangeMock.mockReset().mockImplementation((listener) => { transportStateListeners.add(listener); listener(currentState); @@ -120,6 +126,8 @@ describe("useConnectionHealth", () => { await mountHook(); expect(createWsNativeApiMock.mock.calls.length).toBeGreaterThanOrEqual(1); + expect(getTransportStateSnapshotMock.mock.calls.length).toBeGreaterThan(0); + expect(onTransportStateChangeMock).toHaveBeenCalledTimes(1); expect(latestHealth).toEqual({ state: "connecting", isConnected: false, diff --git a/apps/web/src/hooks/useConnectionHealth.ts b/apps/web/src/hooks/useConnectionHealth.ts index c70b728f..4afe3103 100644 --- a/apps/web/src/hooks/useConnectionHealth.ts +++ b/apps/web/src/hooks/useConnectionHealth.ts @@ -1,6 +1,11 @@ import { useEffect, useRef, useState, useSyncExternalStore } from "react"; -import { createWsNativeApi, getTransportMetrics, onTransportStateChange } from "../wsNativeApi"; +import { + createWsNativeApi, + getTransportMetrics, + getTransportStateSnapshot, + onTransportStateChange, +} from "../wsNativeApi"; import type { ConnectionMetrics, TransportState } from "../wsTransport"; export interface ConnectionHealth { @@ -24,6 +29,11 @@ const DEFAULT_METRICS: ConnectionMetrics = { uptimeMs: 0, }; +function subscribe(callback: () => void): () => void { + createWsNativeApi(); + return onTransportStateChange(() => callback()); +} + /** * Returns a reactive snapshot of the WebSocket connection health. * The transport state updates synchronously; metrics are polled at @@ -31,18 +41,8 @@ const DEFAULT_METRICS: ConnectionMetrics = { */ export function useConnectionHealth(): ConnectionHealth { const state = useSyncExternalStore( - (callback) => { - createWsNativeApi(); - return onTransportStateChange(() => callback()); - }, - () => { - let current: TransportState = "connecting"; - const unsub = onTransportStateChange((next) => { - current = next; - }); - unsub(); - return current; - }, + subscribe, + getTransportStateSnapshot, () => "connecting" as TransportState, ); diff --git a/apps/web/src/hooks/useTransportState.test.tsx b/apps/web/src/hooks/useTransportState.test.tsx new file mode 100644 index 00000000..76041ebf --- /dev/null +++ b/apps/web/src/hooks/useTransportState.test.tsx @@ -0,0 +1,109 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { act, create, type ReactTestRenderer } from "react-test-renderer"; + +import type { TransportState } from "../wsTransport"; +import { useTransportState } from "./useTransportState"; + +const { createWsNativeApiMock, getTransportStateSnapshotMock, onTransportStateChangeMock } = + vi.hoisted(() => ({ + createWsNativeApiMock: vi.fn(), + getTransportStateSnapshotMock: vi.fn(), + onTransportStateChangeMock: vi.fn(), + })); + +vi.mock("../wsNativeApi", () => ({ + createWsNativeApi: createWsNativeApiMock, + getTransportStateSnapshot: getTransportStateSnapshotMock, + onTransportStateChange: onTransportStateChangeMock, +})); + +const transportStateListeners = new Set<(state: TransportState) => void>(); + +let currentState: TransportState = "connecting"; +let latestState: TransportState | null = null; +let renderer: ReactTestRenderer | null = null; +let consoleErrorSpy: ReturnType | null = null; +const originalConsoleError = console.error; + +function HookHarness() { + latestState = useTransportState(); + return null; +} + +function emitTransportState(nextState: TransportState) { + currentState = nextState; + for (const listener of new Set(transportStateListeners)) { + listener(nextState); + } +} + +async function mountHook() { + await act(async () => { + renderer = create(); + }); +} + +async function unmountHook() { + if (!renderer) { + return; + } + + await act(async () => { + renderer?.unmount(); + }); + renderer = null; +} + +beforeEach(() => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + consoleErrorSpy = vi.spyOn(console, "error").mockImplementation((message, ...args) => { + if (typeof message === "string" && message.includes("react-test-renderer is deprecated")) { + return; + } + originalConsoleError.call(console, message, ...args); + }); + + currentState = "connecting"; + latestState = null; + renderer = null; + transportStateListeners.clear(); + + createWsNativeApiMock.mockReset(); + getTransportStateSnapshotMock.mockReset().mockImplementation(() => currentState); + onTransportStateChangeMock.mockReset().mockImplementation((listener) => { + transportStateListeners.add(listener); + listener(currentState); + return () => { + transportStateListeners.delete(listener); + }; + }); +}); + +afterEach(async () => { + await unmountHook(); + consoleErrorSpy?.mockRestore(); + consoleErrorSpy = null; + transportStateListeners.clear(); + vi.restoreAllMocks(); +}); + +describe("useTransportState", () => { + it("reads the current state from a pure snapshot and subscribes once", async () => { + await mountHook(); + + expect(createWsNativeApiMock.mock.calls.length).toBeGreaterThanOrEqual(1); + expect(getTransportStateSnapshotMock.mock.calls.length).toBeGreaterThan(0); + expect(onTransportStateChangeMock).toHaveBeenCalledTimes(1); + expect(latestState).toBe("connecting"); + }); + + it("updates when the transport emits a new state", async () => { + await mountHook(); + + await act(async () => { + emitTransportState("open"); + }); + + expect(latestState).toBe("open"); + }); +}); diff --git a/apps/web/src/hooks/useTransportState.ts b/apps/web/src/hooks/useTransportState.ts index 970078ed..63b7a7b1 100644 --- a/apps/web/src/hooks/useTransportState.ts +++ b/apps/web/src/hooks/useTransportState.ts @@ -1,26 +1,21 @@ import { useSyncExternalStore } from "react"; -import { createWsNativeApi, onTransportStateChange } from "../wsNativeApi"; +import { + createWsNativeApi, + getTransportStateSnapshot, + onTransportStateChange, +} from "../wsNativeApi"; import type { TransportState } from "../wsTransport"; +function subscribe(callback: () => void): () => void { + createWsNativeApi(); + return onTransportStateChange(() => callback()); +} + function getServerSnapshot(): TransportState { return "connecting"; } export function useTransportState(): TransportState { - return useSyncExternalStore( - (callback) => { - createWsNativeApi(); - return onTransportStateChange(() => callback()); - }, - () => { - let state: TransportState = "connecting"; - const unsubscribe = onTransportStateChange((nextState) => { - state = nextState; - }); - unsubscribe(); - return state; - }, - getServerSnapshot, - ); + return useSyncExternalStore(subscribe, getTransportStateSnapshot, getServerSnapshot); } diff --git a/apps/web/src/wsNativeApi.ts b/apps/web/src/wsNativeApi.ts index f9d9219a..bbc24f43 100644 --- a/apps/web/src/wsNativeApi.ts +++ b/apps/web/src/wsNativeApi.ts @@ -105,6 +105,14 @@ export function onTransportStateChange(listener: (state: TransportState) => void }; } +/** + * Read-only snapshot of the current transport state. + * Returns "connecting" until the transport is initialised. + */ +export function getTransportStateSnapshot(): TransportState { + return instance?.transport.getState() ?? "connecting"; +} + /** * Read-only snapshot of transport connection metrics. * Returns null if the transport is not initialised yet.