From 18e7d1b3d884db37c264a95e2e296b68d630d32c Mon Sep 17 00:00:00 2001 From: isra el Date: Wed, 22 Jul 2026 13:49:14 +0300 Subject: [PATCH 1/4] perf(web): make dashboard navigation fast and cut session fetches - prefetch route-tab links so tab clicks reuse a cached payload instead of paying an uncached server round trip each time - add loading.tsx boundaries (dashboard root and each section) so clicks paint within a frame; section-level files keep the header and tabs mounted while only the content area swaps - link the sidebar Account item and the checkout page straight to /dashboard/account/billing. The /dashboard/account redirect stub stays for old links, but no internal link pays the extra hop anymore. The stub also dropped query params, which silently ate the plan-change success toast. - set QueryClient defaults (staleTime 60s, no focus refetch, retry 1); mutations already invalidate their keys, so the user's own changes stay instant. Device messages and webhook deliveries get a 15s staleTime since they change from outside the tab. - replace the axios getCachedSession TTL cache with a token seeded from the server session in Providers and kept in sync by a session bridge. Requests attach the token synchronously; /api/auth/session is only a deduped fallback, instead of a refetch every 2 minutes with a thundering herd on expiry. - swap the billing card's 16px loading spinner for a card-shaped skeleton so the tab no longer looks blank while loading Tests: interceptor seeding/dedupe/signed-out behavior, provider defaults and token seeding, nav active-state matching, tab prefetch, billing loading state, plus e2e coverage for direct-to-billing navigation and an at-most-one-session-call budget guard. Co-Authored-By: Claude Fable 5 --- web/app/(app)/checkout/[planName]/page.tsx | 8 +- .../billing/subscription-info.test.tsx | 15 ++++ .../billing/subscription-info.tsx | 10 ++- .../dashboard/(components)/nav-items.test.ts | 37 +++++++++ .../(app)/dashboard/(components)/nav-items.ts | 23 +++-- web/app/(app)/dashboard/account/loading.tsx | 15 ++++ web/app/(app)/dashboard/layout.tsx | 4 +- web/app/(app)/dashboard/loading.tsx | 20 +++++ web/app/(app)/dashboard/messaging/loading.tsx | 14 ++++ web/app/(app)/dashboard/webhooks/loading.tsx | 14 ++++ web/app/(app)/providers.test.tsx | 55 ++++++++++++ web/app/(app)/providers.tsx | 45 ++++++++-- web/components/shared/route-tabs.test.tsx | 45 ++++++++++ web/components/shared/route-tabs.tsx | 5 +- web/e2e/account.spec.ts | 27 ++++++ web/e2e/session-calls.spec.ts | 46 ++++++++++ web/lib/api/hooks.ts | 6 ++ web/lib/httpBrowserClient.test.ts | 83 +++++++++++++++++++ web/lib/httpBrowserClient.ts | 51 +++++++----- web/test/setup.ts | 8 +- 20 files changed, 487 insertions(+), 44 deletions(-) create mode 100644 web/app/(app)/dashboard/(components)/nav-items.test.ts create mode 100644 web/app/(app)/dashboard/account/loading.tsx create mode 100644 web/app/(app)/dashboard/loading.tsx create mode 100644 web/app/(app)/dashboard/messaging/loading.tsx create mode 100644 web/app/(app)/dashboard/webhooks/loading.tsx create mode 100644 web/app/(app)/providers.test.tsx create mode 100644 web/components/shared/route-tabs.test.tsx create mode 100644 web/e2e/session-calls.spec.ts create mode 100644 web/lib/httpBrowserClient.test.ts diff --git a/web/app/(app)/checkout/[planName]/page.tsx b/web/app/(app)/checkout/[planName]/page.tsx index 8c51fdf..e107ad6 100644 --- a/web/app/(app)/checkout/[planName]/page.tsx +++ b/web/app/(app)/checkout/[planName]/page.tsx @@ -117,7 +117,9 @@ export default function CheckoutPage({ planName, billingInterval: urlInterval ?? selected, }) - window.location.href = '/dashboard/account?plan-change-success=1' + // Straight to the billing tab: the /dashboard/account redirect stub + // drops query params, which silently ate the success toast. + window.location.href = '/dashboard/account/billing?plan-change-success=1' } catch (error) { // no auto-retry here: the request may have charged the card setPlanChange(null) @@ -175,7 +177,7 @@ export default function CheckoutPage({ Try again @@ -233,7 +235,7 @@ export default function CheckoutPage({ disabled={isConfirming} asChild > - Cancel + Cancel diff --git a/web/app/(app)/dashboard/(components)/billing/subscription-info.test.tsx b/web/app/(app)/dashboard/(components)/billing/subscription-info.test.tsx index 034ff81..2592f98 100644 --- a/web/app/(app)/dashboard/(components)/billing/subscription-info.test.tsx +++ b/web/app/(app)/dashboard/(components)/billing/subscription-info.test.tsx @@ -216,6 +216,21 @@ describe('SubscriptionInfo', () => { ).toHaveAttribute('href', 'https://textbee.dev/pricing') }) + // The loading state used to be a 16px spinner alone in the content column, + // which read as a blank tab while the subscription loaded. + it('shows a visible loading skeleton while the subscription loads', () => { + useSubscription.mockReturnValue({ + data: undefined, + isLoading: true, + error: null, + }) + render() + + expect(screen.getByRole('status')).toHaveTextContent( + 'Loading subscription' + ) + }) + it('renders an error state rather than an empty card', () => { useSubscription.mockReturnValue({ data: undefined, diff --git a/web/app/(app)/dashboard/(components)/billing/subscription-info.tsx b/web/app/(app)/dashboard/(components)/billing/subscription-info.tsx index 47d1f0e..b479a37 100644 --- a/web/app/(app)/dashboard/(components)/billing/subscription-info.tsx +++ b/web/app/(app)/dashboard/(components)/billing/subscription-info.tsx @@ -11,7 +11,7 @@ import { } from 'lucide-react' import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' -import { Spinner } from '@/components/ui/spinner' +import { Skeleton } from '@/components/ui/skeleton' import { useCurrentUser, useSubscription } from '@/lib/api' import type { SubscriptionStatus } from '@/lib/api/types' import { @@ -235,8 +235,12 @@ export default function SubscriptionInfo() { if (isLoadingSubscription) return ( -
- + // Mirrors the two cards below. The old 16px spinner in an empty column + // read as a blank page while the subscription loaded. +
+ Loading subscription + +
) diff --git a/web/app/(app)/dashboard/(components)/nav-items.test.ts b/web/app/(app)/dashboard/(components)/nav-items.test.ts new file mode 100644 index 0000000..94461b3 --- /dev/null +++ b/web/app/(app)/dashboard/(components)/nav-items.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from 'vitest' +import { isNavItemActive, navItems } from './nav-items' + +describe('isNavItemActive', () => { + it('matches the dashboard home exactly', () => { + expect(isNavItemActive({ href: '/dashboard' }, '/dashboard')).toBe(true) + expect(isNavItemActive({ href: '/dashboard' }, '/dashboard/messaging')).toBe( + false + ) + }) + + it('matches section subroutes by prefix', () => { + const messaging = { href: '/dashboard/messaging' } + expect(isNavItemActive(messaging, '/dashboard/messaging')).toBe(true) + expect(isNavItemActive(messaging, '/dashboard/messaging/bulk')).toBe(true) + expect(isNavItemActive(messaging, '/dashboard/messaging/api-guide')).toBe( + true + ) + }) + + it('does not match sibling routes that share a name prefix', () => { + expect( + isNavItemActive({ href: '/dashboard/message' }, '/dashboard/messaging') + ).toBe(false) + }) + + it('keeps Account active across the section while linking to billing', () => { + const account = navItems.find((item) => item.label === 'Account') + // Direct link skips the /dashboard/account redirect stub. + expect(account?.href).toBe('/dashboard/account/billing') + expect(isNavItemActive(account!, '/dashboard/account/billing')).toBe(true) + expect(isNavItemActive(account!, '/dashboard/account/profile')).toBe(true) + expect(isNavItemActive(account!, '/dashboard/account/security')).toBe(true) + expect(isNavItemActive(account!, '/dashboard/account')).toBe(true) + expect(isNavItemActive(account!, '/dashboard/community')).toBe(false) + }) +}) diff --git a/web/app/(app)/dashboard/(components)/nav-items.ts b/web/app/(app)/dashboard/(components)/nav-items.ts index 7b7c1b0..328fce3 100644 --- a/web/app/(app)/dashboard/(components)/nav-items.ts +++ b/web/app/(app)/dashboard/(components)/nav-items.ts @@ -11,6 +11,10 @@ export type NavItem = { href: string label: string icon: LucideIcon + // Active-state prefix when it differs from href (Account links straight to + // billing so the click skips the /dashboard/account redirect stub, but must + // stay highlighted on every account tab). + match?: string // The mobile tab bar caps at 4 items (375px width); items marked // mobileHidden appear only in the desktop sidebar and the command palette. mobileHidden?: boolean @@ -23,15 +27,24 @@ export const navItems: NavItem[] = [ { href: '/dashboard/messaging', label: 'Messaging', icon: MessageSquareText }, { href: '/dashboard/webhooks', label: 'Webhooks', icon: Webhook, mobileHidden: true }, { href: '/dashboard/community', label: 'Community', icon: Users }, - { href: '/dashboard/account', label: 'Account', icon: UserCircle }, + { + href: '/dashboard/account/billing', + label: 'Account', + icon: UserCircle, + match: '/dashboard/account', + }, ] export const mobileNavItems = navItems.filter((item) => !item.mobileHidden) // /dashboard must match exactly; deeper routes match by prefix so nested pages // keep their parent highlighted. -export function isNavItemActive(href: string, pathname: string): boolean { - return href === '/dashboard' - ? pathname === href - : pathname === href || pathname.startsWith(`${href}/`) || pathname.startsWith(href) +export function isNavItemActive( + item: Pick, + pathname: string +): boolean { + const prefix = item.match ?? item.href + return prefix === '/dashboard' + ? pathname === prefix + : pathname === prefix || pathname.startsWith(`${prefix}/`) } diff --git a/web/app/(app)/dashboard/account/loading.tsx b/web/app/(app)/dashboard/account/loading.tsx new file mode 100644 index 0000000..d7db015 --- /dev/null +++ b/web/app/(app)/dashboard/account/loading.tsx @@ -0,0 +1,15 @@ +import { Skeleton } from '@/components/ui/skeleton' + +// Shown in the content slot while a tab's page segment streams in; the +// section header and tabs from the layout stay mounted above it. max-w-2xl +// matches the account pages (billing, profile, security, support). +export default function Loading() { + return ( +
+ Loading + + + +
+ ) +} diff --git a/web/app/(app)/dashboard/layout.tsx b/web/app/(app)/dashboard/layout.tsx index 975447f..3f1aa8b 100644 --- a/web/app/(app)/dashboard/layout.tsx +++ b/web/app/(app)/dashboard/layout.tsx @@ -57,7 +57,7 @@ export default function DashboardLayout({ ))} @@ -114,7 +114,7 @@ export default function DashboardLayout({ ))}
diff --git a/web/app/(app)/dashboard/loading.tsx b/web/app/(app)/dashboard/loading.tsx new file mode 100644 index 0000000..27a7183 --- /dev/null +++ b/web/app/(app)/dashboard/loading.tsx @@ -0,0 +1,20 @@ +import { Skeleton } from '@/components/ui/skeleton' + +// Section-level fallback for dashboard routes without a nested loading file +// (home, community). Padding mirrors those pages so nothing shifts when the +// real content lands. Sections with their own layout (messaging, webhooks, +// account) use their nested loading files instead, which keep the tabs +// mounted during tab switches. +export default function Loading() { + return ( +
+ Loading +
+ + +
+ + +
+ ) +} diff --git a/web/app/(app)/dashboard/messaging/loading.tsx b/web/app/(app)/dashboard/messaging/loading.tsx new file mode 100644 index 0000000..bd2c348 --- /dev/null +++ b/web/app/(app)/dashboard/messaging/loading.tsx @@ -0,0 +1,14 @@ +import { Skeleton } from '@/components/ui/skeleton' + +// Shown in the content slot while a tab's page segment streams in; the +// section header and tabs from the layout stay mounted above it. +export default function Loading() { + return ( +
+ Loading + + + +
+ ) +} diff --git a/web/app/(app)/dashboard/webhooks/loading.tsx b/web/app/(app)/dashboard/webhooks/loading.tsx new file mode 100644 index 0000000..bd2c348 --- /dev/null +++ b/web/app/(app)/dashboard/webhooks/loading.tsx @@ -0,0 +1,14 @@ +import { Skeleton } from '@/components/ui/skeleton' + +// Shown in the content slot while a tab's page segment streams in; the +// section header and tabs from the layout stay mounted above it. +export default function Loading() { + return ( +
+ Loading + + + +
+ ) +} diff --git a/web/app/(app)/providers.test.tsx b/web/app/(app)/providers.test.tsx new file mode 100644 index 0000000..304a2d2 --- /dev/null +++ b/web/app/(app)/providers.test.tsx @@ -0,0 +1,55 @@ +import { describe, expect, it, vi } from 'vitest' +import { render, screen } from '@testing-library/react' +import { useQueryClient } from '@tanstack/react-query' +import { http, HttpResponse } from 'msw' +import Providers from './providers' +import httpBrowserClient from '@/lib/httpBrowserClient' +import { server } from '@/test/msw/server' +import { API_BASE_URL, TEST_ACCESS_TOKEN } from '@/test/fixtures' +import { mockSession } from '@/test/render' + +// Own getSession mock (overriding the global one in test/setup.ts) so the +// no-session-fetch guarantee below is a real call-count assertion. +const getSessionSpy = vi.hoisted(() => vi.fn()) +vi.mock('next-auth/react', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, getSession: getSessionSpy } +}) + +function DefaultsProbe() { + const queries = useQueryClient().getDefaultOptions().queries + return ( +
+ {`${queries?.staleTime}|${String(queries?.refetchOnWindowFocus)}|${String( + queries?.retry + )}`} +
+ ) +} + +describe('Providers', () => { + it('sets query defaults: 60s staleTime, no focus refetch, one retry', () => { + render( + + + + ) + expect(screen.getByTestId('defaults')).toHaveTextContent('60000|false|1') + }) + + it('seeds the API token from the server session with zero session fetches', async () => { + let authHeader: string | null = null + server.use( + http.get(`${API_BASE_URL}/ping`, ({ request }) => { + authHeader = request.headers.get('authorization') + return HttpResponse.json({ ok: true }) + }) + ) + + render({null}) + await httpBrowserClient.get('/ping') + + expect(authHeader).toBe(`Bearer ${TEST_ACCESS_TOKEN}`) + expect(getSessionSpy).not.toHaveBeenCalled() + }) +}) diff --git a/web/app/(app)/providers.tsx b/web/app/(app)/providers.tsx index 0c1d1b6..d5813be 100644 --- a/web/app/(app)/providers.tsx +++ b/web/app/(app)/providers.tsx @@ -1,26 +1,59 @@ 'use client' import { GoogleOAuthProvider } from '@react-oauth/google' -import { SessionProvider } from 'next-auth/react' +import { SessionProvider, useSession } from 'next-auth/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' -import { useState, type PropsWithChildren } from 'react' +import { useEffect, useState, type PropsWithChildren } from 'react' import type { Session } from 'next-auth' +import { setSessionToken } from '@/lib/httpBrowserClient' + +// Keeps the API client's token in sync with the session (login, logout, tab +// broadcast). Reads from context only: the provider is seeded with the server +// session, so this never hits /api/auth/session. +function SessionTokenBridge() { + const { data: session } = useSession() + + useEffect(() => { + setSessionToken(session?.user?.accessToken ?? null) + }, [session]) + + return null +} // Client-side provider tree for the app. The QueryClient is created once via // useState so it is stable across re-renders (previously a new client was // constructed on every render, throwing away the cache). Session expiry is -// handled globally by a 401 response interceptor in httpBrowserClient, so there -// is no longer a per-navigation whoAmI check here. +// handled globally by a 401 response interceptor in httpBrowserClient, so +// SessionProvider does not refetch on window focus and there is no +// per-navigation whoAmI check here. export default function Providers({ session, children, }: PropsWithChildren<{ session: Session | null }>) { - const [queryClient] = useState(() => new QueryClient()) + const [queryClient] = useState(() => { + // Seeded during the first render, before any child mounts and queries. + setSessionToken(session?.user?.accessToken ?? null) + + // staleTime 60s: mutations invalidate their keys, so the user's own + // changes are always instant; only out-of-tab changes can lag a minute. + // The old defaults (staleTime 0, refetch on focus) refetched every query + // on every mount and window focus. + return new QueryClient({ + defaultOptions: { + queries: { + staleTime: 60_000, + refetchOnWindowFocus: false, + retry: 1, + }, + }, + }) + }) // ThemeProvider lives in the root layout (app/theme-provider.tsx) so its // pre-paint script is not re-rendered on client navigation. return ( - + + ({ + default: ({ children, href, prefetch, ...rest }: any) => ( + + {children} + + ), +})) + +vi.mock('next/navigation', () => ({ + usePathname: () => '/dashboard/messaging', +})) + +const tabs = [ + { href: '/dashboard/messaging', label: 'Send', exact: true }, + { href: '/dashboard/messaging/bulk', label: 'Bulk Send' }, + { href: '/dashboard/messaging/history', label: 'History' }, +] + +describe('RouteTabs', () => { + it('opts every tab link into full prefetch', () => { + render() + // These routes are dynamic (session cookie in the app layout); without an + // explicit prefetch every tab click pays a full server round trip. + for (const link of screen.getAllByRole('link')) { + expect(link).toHaveAttribute('data-prefetch', 'true') + } + expect(screen.getAllByRole('link')).toHaveLength(tabs.length) + }) + + it('marks only the active tab with aria-current', () => { + render() + expect(screen.getByRole('link', { name: 'Send' })).toHaveAttribute( + 'aria-current', + 'page' + ) + expect( + screen.getByRole('link', { name: 'Bulk Send' }) + ).not.toHaveAttribute('aria-current') + }) +}) diff --git a/web/components/shared/route-tabs.tsx b/web/components/shared/route-tabs.tsx index 2b6361a..113a3e7 100644 --- a/web/components/shared/route-tabs.tsx +++ b/web/components/shared/route-tabs.tsx @@ -20,7 +20,9 @@ function isTabActive(tab: RouteTab, pathname: string): boolean { // Link-based segmented control: tabs are real routes, so the active tab // survives refresh and deep links are shareable. Mobile: horizontally -// scrollable pills; the active pill scrolls into view on load. +// scrollable pills; the active pill scrolls into view on load. Links opt into +// full prefetch because these routes are dynamic (session cookie in the app +// layout), which the router's default prefetch skips. export default function RouteTabs({ tabs, className, @@ -54,6 +56,7 @@ export default function RouteTabs({ { ).toHaveCount(0) }) + // The sidebar used to link to /dashboard/account, whose page is a server + // redirect stub, so every click paid navigation + redirect + navigation. + test('the sidebar Account link goes straight to billing with no redirect hop', async ({ + page, + context, + }) => { + await authenticate(context) + await mockApi(page) + + const stubHits: string[] = [] + page.on('request', (request) => { + if (new URL(request.url()).pathname === '/dashboard/account') { + stubHits.push(request.url()) + } + }) + + await page.goto('/dashboard') + await page + .getByRole('navigation', { name: 'Main' }) + .getByRole('link', { name: 'Account' }) + .click() + + await expect(page).toHaveURL(/\/dashboard\/account\/billing$/) + await expect(page.getByRole('heading', { name: 'Pro' })).toBeVisible() + expect(stubHits).toEqual([]) + }) + test('the pricing page is reachable from billing on any plan', async ({ page, context, diff --git a/web/e2e/session-calls.spec.ts b/web/e2e/session-calls.spec.ts new file mode 100644 index 0000000..87c1ad4 --- /dev/null +++ b/web/e2e/session-calls.spec.ts @@ -0,0 +1,46 @@ +import { expect, test } from '@playwright/test' +import { authenticate } from './session' +import { mockApi } from './mock-api' + +// Serverless-cost regression guard. The API client's token is seeded from the +// server-fetched session, so browsing the dashboard must not keep calling +// /api/auth/session. Before this guard, the axios interceptor refetched the +// session every 2 minutes with no dedupe, so a burst of queries fanned out +// one session call each. +test('dashboard navigation makes at most one /api/auth/session call', async ({ + page, + context, +}) => { + await authenticate(context) + await mockApi(page) + + let sessionCalls = 0 + page.on('request', (request) => { + if (new URL(request.url()).pathname === '/api/auth/session') { + sessionCalls += 1 + } + }) + + await page.goto('/dashboard') + await expect( + page.getByRole('heading', { name: 'Welcome back, Test', level: 2 }) + ).toBeVisible() + + const mainNav = page.getByRole('navigation', { name: 'Main' }) + const tabs = page.getByRole('navigation', { name: 'Section navigation' }) + + await mainNav.getByRole('link', { name: 'Messaging' }).click() + await expect(page).toHaveURL(/\/dashboard\/messaging$/) + await tabs.getByRole('link', { name: 'History' }).click() + await expect(page).toHaveURL(/\/dashboard\/messaging\/history$/) + + await mainNav.getByRole('link', { name: 'Webhooks' }).click() + await expect(page).toHaveURL(/\/dashboard\/webhooks$/) + await tabs.getByRole('link', { name: 'Deliveries' }).click() + await expect(page).toHaveURL(/\/dashboard\/webhooks\/deliveries$/) + + await mainNav.getByRole('link', { name: 'Account' }).click() + await expect(page).toHaveURL(/\/dashboard\/account\/billing$/) + + expect(sessionCalls).toBeLessThanOrEqual(1) +}) diff --git a/web/lib/api/hooks.ts b/web/lib/api/hooks.ts index e015cfa..f868ac9 100644 --- a/web/lib/api/hooks.ts +++ b/web/lib/api/hooks.ts @@ -358,6 +358,9 @@ export function useWebhookNotifications(filters: WebhookNotificationFilters) { `${ApiEndpoints.gateway.getWebhookNotifications()}?eventType=${eventType}&page=${page}&limit=${limit}&status=${status}&start=${start}&end=${end}&deviceId=${deviceId}&webhookSubscriptionId=${webhookSubscriptionId}` ) .then(unwrapBody), + // Deliveries arrive from outside the tab, so stay fresher than the 60s + // client-wide default. + staleTime: 15_000, }) } @@ -420,6 +423,9 @@ export function useDeviceMessages( .get(`${ApiEndpoints.gateway.getMessages(deviceId)}?${query}`) .then(unwrapBody) }, + // Inbound messages arrive from outside the tab, so stay fresher than the + // 60s client-wide default. Before ...options so callers can override. + staleTime: 15_000, ...options, }) } diff --git a/web/lib/httpBrowserClient.test.ts b/web/lib/httpBrowserClient.test.ts new file mode 100644 index 0000000..ecd132e --- /dev/null +++ b/web/lib/httpBrowserClient.test.ts @@ -0,0 +1,83 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { http, HttpResponse } from 'msw' +import { server } from '@/test/msw/server' +import { API_BASE_URL } from '@/test/fixtures' + +// Own getSession mock (overriding the global one in test/setup.ts) so calls +// can be counted and resolved per test. +const getSessionMock = vi.hoisted(() => vi.fn()) +vi.mock('next-auth/react', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, getSession: getSessionMock } +}) + +// The token lives in module state, so each test loads a fresh module copy. +async function loadClient() { + vi.resetModules() + return import('@/lib/httpBrowserClient') +} + +function captureAuthHeaders() { + const headers: (string | null)[] = [] + server.use( + http.get(`${API_BASE_URL}/ping`, ({ request }) => { + headers.push(request.headers.get('authorization')) + return HttpResponse.json({ ok: true }) + }) + ) + return headers +} + +afterEach(() => { + getSessionMock.mockReset() +}) + +describe('httpBrowserClient auth interceptor', () => { + it('attaches a seeded token without calling getSession', async () => { + const { default: client, setSessionToken } = await loadClient() + setSessionToken('abc') + const headers = captureAuthHeaders() + + await client.get('/ping') + + expect(headers).toEqual(['Bearer abc']) + expect(getSessionMock).not.toHaveBeenCalled() + }) + + it('dedupes concurrent session fetches when the token is not seeded', async () => { + getSessionMock.mockResolvedValue({ user: { accessToken: 'tok1' } }) + const { default: client } = await loadClient() + const headers = captureAuthHeaders() + + await Promise.all([client.get('/ping'), client.get('/ping')]) + + expect(getSessionMock).toHaveBeenCalledTimes(1) + expect(headers).toEqual(['Bearer tok1', 'Bearer tok1']) + }) + + it('sends no header and no session fetch when seeded signed out', async () => { + const { default: client, setSessionToken } = await loadClient() + // null means known signed out (the 401 handler and the auth pages), so + // requests must not fall back to a /api/auth/session round trip. + setSessionToken(null) + const headers = captureAuthHeaders() + + await client.get('/ping') + + expect(headers).toEqual([null]) + expect(getSessionMock).not.toHaveBeenCalled() + }) + + it('recovers with a fresh fetch after a failed session lookup', async () => { + getSessionMock.mockRejectedValueOnce(new Error('network down')) + getSessionMock.mockResolvedValueOnce({ user: { accessToken: 'tok2' } }) + const { default: client } = await loadClient() + const headers = captureAuthHeaders() + + await expect(client.get('/ping')).rejects.toThrow('network down') + await client.get('/ping') + + expect(getSessionMock).toHaveBeenCalledTimes(2) + expect(headers).toEqual(['Bearer tok2']) + }) +}) diff --git a/web/lib/httpBrowserClient.ts b/web/lib/httpBrowserClient.ts index b906cd0..250ad6e 100644 --- a/web/lib/httpBrowserClient.ts +++ b/web/lib/httpBrowserClient.ts @@ -5,32 +5,41 @@ const httpBrowserClient = axios.create({ baseURL: process.env.NEXT_PUBLIC_API_BASE_URL || '', }) -// Cache for session data to reduce API calls -let sessionCache: any = null -let cacheTimestamp = 0 -const CACHE_DURATION = 2 * 60 * 1000 // 2 minutes +// API access token, seeded from the server-fetched session by Providers and +// kept current by its SessionTokenBridge. Held in module state so the request +// interceptor attaches it synchronously instead of paying a /api/auth/session +// round trip per request (Vercel cost and added latency). undefined means not +// seeded yet; null means known signed out, so auth pages never fetch either. +let sessionToken: string | null | undefined -const getCachedSession = async () => { - const now = Date.now() - - // Return cached session if it's still valid - if (sessionCache && (now - cacheTimestamp) < CACHE_DURATION) { - return sessionCache +export function setSessionToken(token: string | null) { + sessionToken = token +} + +// Fallback for the rare request that fires before the token is seeded (deep +// hard load). One shared promise so a burst of queries costs one session call. +let sessionFetch: Promise | null = null + +function fetchSessionToken() { + if (!sessionFetch) { + sessionFetch = getSession() + .then((session) => { + sessionToken = session?.user?.accessToken ?? null + return sessionToken + }) + .finally(() => { + sessionFetch = null + }) } - - // Fetch fresh session and update cache - const session = await getSession() - sessionCache = session - cacheTimestamp = now - - return session + return sessionFetch } httpBrowserClient.interceptors.request.use(async (config) => { - const session = await getCachedSession() + const token = + sessionToken === undefined ? await fetchSessionToken() : sessionToken - if (session?.user?.accessToken) { - config.headers.Authorization = `Bearer ${session.user.accessToken}` + if (token) { + config.headers.Authorization = `Bearer ${token}` } return config }) @@ -47,7 +56,7 @@ httpBrowserClient.interceptors.response.use( ) { const { pathname } = window.location if (!pathname.includes('/logout') && !pathname.includes('/login')) { - sessionCache = null + setSessionToken(null) window.location.href = '/logout' } } diff --git a/web/test/setup.ts b/web/test/setup.ts index c280596..c5fbfbe 100644 --- a/web/test/setup.ts +++ b/web/test/setup.ts @@ -3,9 +3,11 @@ import { cleanup } from '@testing-library/react' import { afterAll, afterEach, beforeAll, vi } from 'vitest' import { server } from './msw/server' -// The axios browser client's interceptor calls next-auth's getSession(), which -// would otherwise trigger a real /api/auth/session fetch. Mock it so the -// interceptor can attach the Bearer token without any network access. +// The axios browser client's interceptor falls back to next-auth's +// getSession() when no token has been seeded (tests render components without +// the app's Providers), which would otherwise trigger a real /api/auth/session +// fetch. Mock it so the interceptor can attach the Bearer token without any +// network access. const hoisted = vi.hoisted(() => ({ accessToken: 'test-access-token' })) vi.mock('next-auth/react', async (importOriginal) => { const actual = await importOriginal() From ab96e731a68c63bb34f1da6ff5d9fd59f690504f Mon Sep 17 00:00:00 2001 From: isra el Date: Wed, 22 Jul 2026 14:02:13 +0300 Subject: [PATCH 2/4] test(web): wait for hydration before typing in messaging e2e specs The messaging pages now stream behind a loading.tsx boundary, so their HTML can paint before React hydrates. Tests that filled the To field or dropped a CSV immediately after page.goto raced that window: the input was there, the handlers were not, and the interaction was silently lost. Under parallel workers the race lost often enough to fail runs at random. Both specs now navigate through a helper that waits for the client-side devices fetch, which only fires after hydration effects run, so the form is proven interactive before the test touches it. Co-Authored-By: Claude Fable 5 --- web/e2e/bulk-send.spec.ts | 24 +++++++++++++++++------- web/e2e/send-sms.spec.ts | 26 ++++++++++++++++++-------- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/web/e2e/bulk-send.spec.ts b/web/e2e/bulk-send.spec.ts index 57e8072..7fcccc7 100644 --- a/web/e2e/bulk-send.spec.ts +++ b/web/e2e/bulk-send.spec.ts @@ -23,6 +23,16 @@ async function uploadCsv(page: import('@playwright/test').Page, csv: string) { }) } +// The section streams behind a loading.tsx boundary, so the form's HTML can +// paint before React hydrates it; interacting that early is silently lost. +// The devices query only fires after hydration effects run, so its response +// is proof the dropzone and inputs have their handlers attached. +async function gotoBulkPage(page: import('@playwright/test').Page) { + const devicesLoaded = page.waitForResponse('**/gateway/devices') + await page.goto('/dashboard/messaging/bulk') + await devicesLoaded +} + // Keyboard selection rather than clicking the option: the Radix popper can // render outside the viewport on a short page, which made the click flaky. async function selectDevice(page: import('@playwright/test').Page) { @@ -51,7 +61,7 @@ test.describe('bulk send (mocked API, no real backend)', () => { }) }) - await page.goto('/dashboard/messaging/bulk') + await gotoBulkPage(page) await uploadCsv(page, CSV_WITH_PROBLEM_ROWS) @@ -90,7 +100,7 @@ test.describe('bulk send (mocked API, no real backend)', () => { test('offers a downloadable sample CSV', async ({ page, context }) => { await authenticate(context) await mockApi(page) - await page.goto('/dashboard/messaging/bulk') + await gotoBulkPage(page) const link = page.getByRole('link', { name: /download a sample csv/i }) await expect(link).toBeVisible() @@ -110,7 +120,7 @@ test.describe('bulk send (mocked API, no real backend)', () => { }) => { await authenticate(context) await mockApi(page) - await page.goto('/dashboard/messaging/bulk') + await gotoBulkPage(page) // The old copy printed the raw byte count ("max 1048576 bytes"). await expect(page.getByText('1048576')).toHaveCount(0) @@ -124,7 +134,7 @@ test.describe('bulk send (mocked API, no real backend)', () => { await page.setViewportSize({ width: 375, height: 800 }) await authenticate(context) await mockApi(page) - await page.goto('/dashboard/messaging/bulk') + await gotoBulkPage(page) // A wide table is the obvious way to reintroduce the sideways-scroll bug, // and it only exists once a file has been parsed. @@ -144,7 +154,7 @@ test.describe('bulk send (mocked API, no real backend)', () => { }) => { await authenticate(context) await mockApi(page) - await page.goto('/dashboard/messaging/bulk') + await gotoBulkPage(page) await uploadCsv(page, CSV_WITH_PROBLEM_ROWS) await selectDevice(page) @@ -165,7 +175,7 @@ test.describe('bulk send (mocked API, no real backend)', () => { }) => { await authenticate(context) await mockApi(page) - await page.goto('/dashboard/messaging/bulk') + await gotoBulkPage(page) await uploadCsv( page, @@ -187,7 +197,7 @@ test.describe('bulk send (mocked API, no real backend)', () => { test('explains why a non-CSV file was rejected', async ({ page, context }) => { await authenticate(context) await mockApi(page) - await page.goto('/dashboard/messaging/bulk') + await gotoBulkPage(page) // Rejected drops used to return silently, so nothing happened and nothing // said why. diff --git a/web/e2e/send-sms.spec.ts b/web/e2e/send-sms.spec.ts index 0ccf406..054ef14 100644 --- a/web/e2e/send-sms.spec.ts +++ b/web/e2e/send-sms.spec.ts @@ -15,6 +15,16 @@ async function captureSend(page: import('@playwright/test').Page) { return payloads } +// The section streams behind a loading.tsx boundary, so the form's HTML can +// paint before React hydrates it; typing that early into the controlled To +// input is silently wiped when hydration commits. The device name renders +// only after the client-side devices fetch, so its presence proves the form +// is hydrated and its handlers are attached. +async function gotoSendPage(page: import('@playwright/test').Page) { + await page.goto('/dashboard/messaging') + await expect(page.getByLabel('Send from')).toContainText('Pixel 8') +} + test.describe('send sms (mocked API, no real backend)', () => { test('commits recipients as chips and posts exactly those numbers', async ({ page, @@ -24,7 +34,7 @@ test.describe('send sms (mocked API, no real backend)', () => { await mockApi(page) const payloads = await captureSend(page) - await page.goto('/dashboard/messaging') + await gotoSendPage(page) const to = page.getByLabel('To', { exact: true }) await to.fill('+1 (415) 555-0101') @@ -57,7 +67,7 @@ test.describe('send sms (mocked API, no real backend)', () => { }) => { await authenticate(context) await mockApi(page) - await page.goto('/dashboard/messaging') + await gotoSendPage(page) await page .getByLabel('To', { exact: true }) @@ -77,7 +87,7 @@ test.describe('send sms (mocked API, no real backend)', () => { }) => { await authenticate(context) await mockApi(page) - await page.goto('/dashboard/messaging') + await gotoSendPage(page) // Spaces are formatting inside one number here, not separators. const to = page.getByLabel('To', { exact: true }) @@ -94,7 +104,7 @@ test.describe('send sms (mocked API, no real backend)', () => { test('rejects a number that cannot be dialled', async ({ page, context }) => { await authenticate(context) await mockApi(page) - await page.goto('/dashboard/messaging') + await gotoSendPage(page) const to = page.getByLabel('To', { exact: true }) await to.fill('not a number') @@ -113,7 +123,7 @@ test.describe('send sms (mocked API, no real backend)', () => { await mockApi(page) const payloads = await captureSend(page) - await page.goto('/dashboard/messaging') + await gotoSendPage(page) // Type a number then go straight to the message without pressing Enter. await page.getByLabel('To', { exact: true }).fill('+14155550101') @@ -127,7 +137,7 @@ test.describe('send sms (mocked API, no real backend)', () => { test('preselects the only enabled device', async ({ page, context }) => { await authenticate(context) await mockApi(page) - await page.goto('/dashboard/messaging') + await gotoSendPage(page) // Fixtures have one enabled device (Pixel 8) and one disabled. The old // implementation computed this in defaultValues before devices loaded, so @@ -138,7 +148,7 @@ test.describe('send sms (mocked API, no real backend)', () => { test('counts SMS segments', async ({ page, context }) => { await authenticate(context) await mockApi(page) - await page.goto('/dashboard/messaging') + await gotoSendPage(page) await page.getByLabel('Message').fill('a'.repeat(161)) await expect(page.getByText(/161 characters, 2 segments/)).toBeVisible() @@ -149,7 +159,7 @@ test.describe('send sms (mocked API, no real backend)', () => { await mockApi(page) await captureSend(page) - await page.goto('/dashboard/messaging') + await gotoSendPage(page) const to = page.getByLabel('To', { exact: true }) await to.fill('+14155550101') await to.press('Enter') From 3eba8d3444118dd5ced87f578ce46cac0dc8e602 Mon Sep 17 00:00:00 2001 From: isra el Date: Wed, 22 Jul 2026 14:36:34 +0300 Subject: [PATCH 3/4] perf(web): register only the Prism languages used, scope Google OAuth The API guide imported react-syntax-highlighter's default entry, which is the highlight.js build with every language compiled in. It also applied Prism themes to it, and the two use different class names, so the samples were largely uncoloured on top of being heavy. Switch to PrismLight and register the six languages the guide actually renders. GoogleOAuthProvider wrapped the whole app but only the login and register pages use it, so the Google Identity SDK initialised on every dashboard page. Move it inside LoginWithGoogle, which is where both consumers go through. Total client chunks drop from 3.1M to 2.3M. Also fixes four e2e tests that were relying on page.goto returning a fully loaded page. Adding loading.tsx introduced a short skeleton state that did not exist before, and each test measured or interacted during it. They now wait on real signals instead of timing: - bulk send waits for the dropzone row cap, which comes from useSubscription inside that page's own hook. The previous wait on the devices response was wrong: the dashboard layout requests the same query key, so it can resolve before the page hydrates. - the mobile overflow sweep and the API guide replace networkidle, which needs a 500ms window with zero connections that link prefetching keeps pushing out of reach. - the footer/tab-bar check re-scrolls as the page grows, instead of scrolling once while the skeleton is still short. - the overflow sweep measures the billing tab rather than /dashboard/account, a redirect stub with no layout of its own whose client-side redirect tore down the measurement. A new test asserts the samples carry Prism token markup in all five languages, so a revert to the highlight.js entry fails instead of silently dropping the colours. Co-Authored-By: Claude Fable 5 --- .../(auth)/(components)/login-with-google.tsx | 19 +++++++++- .../(components)/api-guide/code-block.tsx | 24 ++++++++++++- web/app/(app)/providers.tsx | 9 +---- web/e2e/api-guide.spec.ts | 36 ++++++++++++++++++- web/e2e/bulk-send.spec.ts | 14 +++++--- web/e2e/chrome.spec.ts | 34 ++++++++++-------- web/e2e/mobile-overflow.spec.ts | 30 +++++++++++----- 7 files changed, 127 insertions(+), 39 deletions(-) diff --git a/web/app/(app)/(auth)/(components)/login-with-google.tsx b/web/app/(app)/(auth)/(components)/login-with-google.tsx index 41329d3..d3d2603 100644 --- a/web/app/(app)/(auth)/(components)/login-with-google.tsx +++ b/web/app/(app)/(auth)/(components)/login-with-google.tsx @@ -2,11 +2,28 @@ import { Routes } from '@/config/routes' import { toast } from '@/hooks/use-toast' -import { CredentialResponse, GoogleLogin } from '@react-oauth/google' +import { + CredentialResponse, + GoogleLogin, + GoogleOAuthProvider, +} from '@react-oauth/google' import { signIn } from 'next-auth/react' import { useRouter, useSearchParams } from 'next/navigation' +// The provider lives here rather than in the app-wide tree so the Google +// Identity SDK is only loaded on the two pages that render this button +// (login and register) instead of on every dashboard page. export default function LoginWithGoogle() { + return ( + + + + ) +} + +function GoogleLoginButton() { const router = useRouter() const searchParams = useSearchParams() const redirect = searchParams.get('redirect') diff --git a/web/app/(app)/dashboard/messaging/(components)/api-guide/code-block.tsx b/web/app/(app)/dashboard/messaging/(components)/api-guide/code-block.tsx index d008b81..0b2ab08 100644 --- a/web/app/(app)/dashboard/messaging/(components)/api-guide/code-block.tsx +++ b/web/app/(app)/dashboard/messaging/(components)/api-guide/code-block.tsx @@ -3,11 +3,33 @@ import { useState } from 'react' import { useTheme } from 'next-themes' import { Check, Copy } from 'lucide-react' -import SyntaxHighlighter from 'react-syntax-highlighter' +import { PrismLight as SyntaxHighlighter } from 'react-syntax-highlighter' +import bash from 'react-syntax-highlighter/dist/esm/languages/prism/bash' +import go from 'react-syntax-highlighter/dist/esm/languages/prism/go' +import javascript from 'react-syntax-highlighter/dist/esm/languages/prism/javascript' +import json from 'react-syntax-highlighter/dist/esm/languages/prism/json' +import php from 'react-syntax-highlighter/dist/esm/languages/prism/php' +import python from 'react-syntax-highlighter/dist/esm/languages/prism/python' import { oneDark, oneLight } from 'react-syntax-highlighter/dist/esm/styles/prism' import { Button } from '@/components/ui/button' import { cn } from '@/lib/utils' +// Prism, registering only the languages the guide uses. The bare +// react-syntax-highlighter entry is the highlight.js build with every language +// compiled in, and it emits hljs-* class names that the Prism themes below do +// not style, so this both shrinks the chunk and makes the theme apply. +// The languages come from LANGUAGES in snippets.ts plus json for responses. +for (const [name, definition] of [ + ['bash', bash], + ['go', go], + ['javascript', javascript], + ['json', json], + ['php', php], + ['python', python], +] as const) { + SyntaxHighlighter.registerLanguage(name, definition) +} + // The previous guide hardcoded a dark slate background regardless of theme, so // code blocks were the only dark thing on a light page. export default function CodeBlock({ diff --git a/web/app/(app)/providers.tsx b/web/app/(app)/providers.tsx index d5813be..601d86e 100644 --- a/web/app/(app)/providers.tsx +++ b/web/app/(app)/providers.tsx @@ -1,6 +1,5 @@ 'use client' -import { GoogleOAuthProvider } from '@react-oauth/google' import { SessionProvider, useSession } from 'next-auth/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { useEffect, useState, type PropsWithChildren } from 'react' @@ -54,13 +53,7 @@ export default function Providers({ return ( - - - {children} - - + {children} ) } diff --git a/web/e2e/api-guide.spec.ts b/web/e2e/api-guide.spec.ts index e0107f6..44d4c36 100644 --- a/web/e2e/api-guide.spec.ts +++ b/web/e2e/api-guide.spec.ts @@ -55,6 +55,33 @@ test.describe('api guide (mocked API, no real backend)', () => { await expect(page.getByText('package main').first()).toBeVisible() }) + // The guide applies Prism themes, so it must render with the Prism + // highlighter. It previously used the package's default export, which is the + // highlight.js build: that emits hljs-* classes the Prism theme never styles + // (so samples went uncoloured) and compiles in every language it supports. + test('samples are highlighted by Prism, in every language', async ({ + page, + context, + }) => { + await authenticate(context) + await mockApi(page) + await page.goto('/dashboard/messaging/api-guide') + + for (const language of ['cURL', 'Node.js', 'Python', 'PHP', 'Go']) { + await page.getByRole('tab', { name: language, exact: true }).click() + await expect( + page.locator('pre code .token').first(), + `${language} samples must carry Prism token markup` + ).toBeVisible() + } + + // A registered language yields more than one token type; a missing one + // would fall back to a single undifferentiated text run. + expect( + await page.locator('pre code .token').count() + ).toBeGreaterThan(5) + }) + test('code can be copied', async ({ page, context }) => { await authenticate(context) await mockApi(page) @@ -63,6 +90,10 @@ test.describe('api guide (mocked API, no real backend)', () => { await page.getByRole('button', { name: 'Copy code' }).first().click() + // The button relabels itself only after the async clipboard write resolves, + // so this is the signal that there is something to read back. + await expect(page.getByRole('button', { name: 'Copied' }).first()).toBeVisible() + const clipboard = await page.evaluate(() => navigator.clipboard.readText() ) @@ -74,7 +105,10 @@ test.describe('api guide (mocked API, no real backend)', () => { await authenticate(context) await mockApi(page) await page.goto('/dashboard/messaging/api-guide') - await page.waitForLoadState('networkidle') + // Not networkidle: link prefetching keeps the network busy, so that state + // can go unreached under parallel workers. A rendered sample proves the + // code blocks (the thing that would widen the page) are laid out. + await expect(page.getByText('curl -X POST').first()).toBeVisible() // Code blocks are the classic way to widen a mobile page. const { scrollWidth, innerWidth } = await page.evaluate(() => ({ diff --git a/web/e2e/bulk-send.spec.ts b/web/e2e/bulk-send.spec.ts index 7fcccc7..9328a3a 100644 --- a/web/e2e/bulk-send.spec.ts +++ b/web/e2e/bulk-send.spec.ts @@ -24,13 +24,17 @@ async function uploadCsv(page: import('@playwright/test').Page, csv: string) { } // The section streams behind a loading.tsx boundary, so the form's HTML can -// paint before React hydrates it; interacting that early is silently lost. -// The devices query only fires after hydration effects run, so its response -// is proof the dropzone and inputs have their handlers attached. +// paint before React hydrates it, and a file offered during that gap is +// silently discarded (react-dropzone binds its change handler on mount). +// The row cap in the dropzone copy is derived from useSubscription inside this +// page's own hook, so seeing it proves this component hydrated and has data. +// Waiting on the devices response would not: the dashboard layout requests the +// same query key, so it can resolve before this page has hydrated. async function gotoBulkPage(page: import('@playwright/test').Page) { - const devicesLoaded = page.waitForResponse('**/gateway/devices') await page.goto('/dashboard/messaging/bulk') - await devicesLoaded + await expect( + page.getByText(/Up to 1 MB and [\d,]+ rows on your plan/) + ).toBeVisible() } // Keyboard selection rather than clicking the option: the Radix popper can diff --git a/web/e2e/chrome.spec.ts b/web/e2e/chrome.spec.ts index 40bea71..2c9001f 100644 --- a/web/e2e/chrome.spec.ts +++ b/web/e2e/chrome.spec.ts @@ -42,23 +42,29 @@ test.describe('app chrome (mocked API, no real backend)', () => { await mockApi(page) await page.goto('/dashboard') - // Scroll to the very bottom so the footer is in view. - await page.evaluate(() => window.scrollTo(0, document.body.scrollHeight)) - const footer = page.locator('footer') await expect(footer).toBeVisible() + const tabBar = page.locator('nav.fixed').first() - const footerBox = await footer.boundingBox() - const tabBarBox = await page.locator('nav.fixed').first().boundingBox() - - expect(footerBox, 'footer must have a layout box').not.toBeNull() - expect(tabBarBox, 'mobile tab bar must have a layout box').not.toBeNull() - - // The footer's last pixel must sit above the tab bar's first pixel. - expect( - footerBox!.y + footerBox!.height, - 'footer bottom must clear the fixed tab bar' - ).toBeLessThanOrEqual(tabBarBox!.y + 1) + // Scrolled again on every attempt rather than once up front: the page + // starts as a short loading skeleton and grows as each card's data lands, + // so a single scroll to the bottom can be measured from a document that is + // still shorter than it ends up, leaving the footer below the viewport. + await expect + .poll( + async () => { + await page.evaluate(() => + window.scrollTo(0, document.body.scrollHeight) + ) + const footerBox = await footer.boundingBox() + const tabBarBox = await tabBar.boundingBox() + if (!footerBox || !tabBarBox) return Number.POSITIVE_INFINITY + // The footer's last pixel must sit above the tab bar's first pixel. + return footerBox.y + footerBox.height - (tabBarBox.y + 1) + }, + { message: 'footer bottom must clear the fixed tab bar' } + ) + .toBeLessThanOrEqual(0) }) test('mobile: footer links stack vertically', async ({ page, context }) => { diff --git a/web/e2e/mobile-overflow.spec.ts b/web/e2e/mobile-overflow.spec.ts index 88b167a..72b0927 100644 --- a/web/e2e/mobile-overflow.spec.ts +++ b/web/e2e/mobile-overflow.spec.ts @@ -15,7 +15,10 @@ const AUTHED_PAGES = [ '/dashboard/messaging/api-guide', '/dashboard/webhooks', '/dashboard/webhooks/deliveries', - '/dashboard/account', + // The billing tab, not /dashboard/account: that route is a redirect stub + // with no layout of its own, and measuring it raced the redirect. The + // redirect itself is covered in account.spec.ts. + '/dashboard/account/billing', '/dashboard/account/profile', '/dashboard/account/security', '/dashboard/account/support', @@ -24,13 +27,22 @@ const AUTHED_PAGES = [ '/dashboard/community', ] +// Deterministic replacement for waitForLoadState('networkidle'), which needs a +// 500ms window with zero connections. Link prefetching keeps issuing and +// aborting RSC requests, so under parallel workers that window can be missed +// until the test times out. Waiting for the page's own heading and for its +// loading placeholders to clear proves the same thing: content is laid out. +async function waitForContent(page: import('@playwright/test').Page) { + await expect(page.locator('main#main-content h2').first()).toBeVisible() + await expect(page.locator('main#main-content [role="status"]')).toHaveCount(0) +} + async function expectNoHorizontalScroll(page: import('@playwright/test').Page) { - const { scrollWidth, innerWidth } = await page.evaluate(() => ({ - scrollWidth: document.documentElement.scrollWidth, - innerWidth: window.innerWidth, - })) - expect(scrollWidth, 'page must not scroll sideways at 375px').toBeLessThanOrEqual( - innerWidth + const overflow = await page.evaluate( + () => document.documentElement.scrollWidth - window.innerWidth + ) + expect(overflow, 'page must not scroll sideways at 375px').toBeLessThanOrEqual( + 0 ) } @@ -42,7 +54,7 @@ test.describe('no horizontal overflow at 375px (mocked API)', () => { await authenticate(context) await mockApi(page) await page.goto(path) - await page.waitForLoadState('networkidle') + await waitForContent(page) await expectNoHorizontalScroll(page) }) } @@ -70,7 +82,7 @@ test.describe('no horizontal overflow at 375px (mocked API)', () => { test('login page', async ({ page }) => { await mockApi(page) await page.goto('/login') - await page.waitForLoadState('networkidle') + await expect(page.getByText('Welcome back')).toBeVisible() await expectNoHorizontalScroll(page) }) }) From 83d0c3803a86137a45037a49169cc09e2dc0d7d2 Mon Sep 17 00:00:00 2001 From: isra el Date: Wed, 22 Jul 2026 14:45:40 +0300 Subject: [PATCH 4/4] fix(web): stop the SupportHQ widget rebuilding itself on every session refetch The effect depended on the whole session object. SessionProvider returns a new object on every refetch, and React compares deps by reference, so an identical session still ran the cleanup and the body again: the widget was destroyed and its script re-injected even though nothing about the user had changed, closing any chat the user had open. It now depends on the individual strings the effect actually reads, which compare by value, so it only re-initialises when the metadata genuinely changes. Two related leaks fixed at the same time: - cleanup called destroy() but left the script tag in the document, so every re-run added another one for the life of the page. It is now removed. - a script whose src is already cached can finish loading after cleanup ran, and initialising then left a widget behind that nothing would destroy. That late load is now ignored. Tests cover all four behaviours and fail against the previous version. Also gives the API guide page focus before the clipboard test: writeText rejects on an unfocused document, which is where a page sits while other workers run, and granting clipboard permissions does not cover it. Co-Authored-By: Claude Fable 5 --- .../shared/support-hq-widget.test.tsx | 129 ++++++++++++++++++ web/components/shared/support-hq-widget.tsx | 69 ++++++---- web/e2e/api-guide.spec.ts | 4 + 3 files changed, 176 insertions(+), 26 deletions(-) create mode 100644 web/components/shared/support-hq-widget.test.tsx diff --git a/web/components/shared/support-hq-widget.test.tsx b/web/components/shared/support-hq-widget.test.tsx new file mode 100644 index 0000000..f434077 --- /dev/null +++ b/web/components/shared/support-hq-widget.test.tsx @@ -0,0 +1,129 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { render } from '@testing-library/react' +import SupportHQWidget from './support-hq-widget' + +const useSessionMock = vi.hoisted(() => vi.fn()) +vi.mock('next-auth/react', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useSession: useSessionMock } +}) + +const init = vi.fn() +const destroy = vi.fn() + +// A fresh object every call, the way SessionProvider hands one back after a +// refetch even when the underlying user has not changed. +const sessionWith = (overrides: Record = {}) => ({ + data: { + user: { + id: 'user_1', + name: 'Test User', + email: 'test@example.com', + phone: '', + ...overrides, + }, + }, +}) + +const scripts = () => + document.querySelectorAll('script[src*="supporthq-widget.js"]') + +// jsdom does not fetch external scripts, so loading is simulated. +const fireLoad = () => + scripts().forEach((s) => (s as HTMLScriptElement).onload?.(new Event('load'))) + +beforeEach(() => { + init.mockClear() + destroy.mockClear() + // @ts-ignore + window.SupportHQWidget = { init, destroy } +}) + +afterEach(() => { + document + .querySelectorAll('script[src*="supporthq-widget.js"]') + .forEach((s) => s.remove()) +}) + +describe('SupportHQWidget', () => { + it('injects the widget script once and initialises with the user metadata', () => { + useSessionMock.mockReturnValue(sessionWith()) + render() + + expect(scripts()).toHaveLength(1) + + fireLoad() + expect(init).toHaveBeenCalledTimes(1) + expect(init.mock.calls[0][0].metadata).toEqual({ + userId: 'user_1', + name: 'Test User', + email: 'test@example.com', + phone: '', + }) + }) + + // The reported defect: the effect depended on the session object, so a + // refetch that returned identical data still destroyed and re-injected the + // widget, closing any chat the user had open. + it('does not reinitialise when a refetch returns identical data', () => { + useSessionMock.mockReturnValue(sessionWith()) + const { rerender } = render() + fireLoad() + + useSessionMock.mockReturnValue(sessionWith()) + rerender() + + expect(scripts()).toHaveLength(1) + expect(destroy).not.toHaveBeenCalled() + expect(init).toHaveBeenCalledTimes(1) + }) + + it('reinitialises when the user metadata actually changes', () => { + useSessionMock.mockReturnValue(sessionWith()) + const { rerender } = render() + fireLoad() + + useSessionMock.mockReturnValue(sessionWith({ email: 'new@example.com' })) + rerender() + fireLoad() + + expect(destroy).toHaveBeenCalledTimes(1) + expect(init).toHaveBeenCalledTimes(2) + expect(init.mock.calls[1][0].metadata.email).toBe('new@example.com') + }) + + // destroy() tears down the widget but leaves the tag, so without an explicit + // removal every re-run left another one behind. + it('leaves no script tags behind when it unmounts', () => { + useSessionMock.mockReturnValue(sessionWith()) + const { unmount } = render() + fireLoad() + + unmount() + + expect(scripts()).toHaveLength(0) + expect(destroy).toHaveBeenCalledTimes(1) + }) + + it('does not initialise if the script loads after cleanup', () => { + useSessionMock.mockReturnValue(sessionWith()) + const { unmount } = render() + + const pending = scripts()[0] as HTMLScriptElement + unmount() + // A cached src can resolve after the effect was torn down; initialising + // then would leave a widget that nothing destroys. + pending.onload?.(new Event('load')) + + expect(init).not.toHaveBeenCalled() + }) + + it('omits metadata when nobody is signed in', () => { + useSessionMock.mockReturnValue({ data: null }) + render() + fireLoad() + + expect(init).toHaveBeenCalledTimes(1) + expect(init.mock.calls[0][0]).not.toHaveProperty('metadata') + }) +}) diff --git a/web/components/shared/support-hq-widget.tsx b/web/components/shared/support-hq-widget.tsx index 6e83bb3..203e97a 100644 --- a/web/components/shared/support-hq-widget.tsx +++ b/web/components/shared/support-hq-widget.tsx @@ -3,32 +3,49 @@ import { useSession } from 'next-auth/react' import React, { useEffect } from 'react' export default function SupportHQWidget() { + const { data: session } = useSession() - const { data: session } = useSession() - - useEffect(() => { - const script = document.createElement('script') - script.src = 'https://cdn.supporthq.app/widget/latest/supporthq-widget.js' - script.async = true - // @ts-ignore - script.onload = () => window.SupportHQWidget?.init({ - projectId: process.env.NEXT_PUBLIC_SUPPORT_HQ_PROJECT_ID, - themeColor: process.env.NEXT_PUBLIC_SUPPORT_HQ_THEME_COLOR ?? '#2563eb', - ...(session?.user && { - metadata: { - userId: session.user.id || '', - name: session.user.name || '', - email: session.user.email || '', - phone: session.user.phone || '', - } - }) - }) - document.body.appendChild(script) - // @ts-ignore - return () => { window.SupportHQWidget?.destroy() } - }, [session]) + // Depended on as individual strings rather than as `session`: SessionProvider + // hands back a new object on every refetch, and comparing that by reference + // tore the widget down and re-injected its script even when nothing about the + // user had changed, closing any open chat. + const hasUser = Boolean(session?.user) + const userId = session?.user?.id ?? '' + const name = session?.user?.name ?? '' + const email = session?.user?.email ?? '' + const phone = session?.user?.phone ?? '' - return ( - <> - ) + useEffect(() => { + let cancelled = false + + const script = document.createElement('script') + script.src = 'https://cdn.supporthq.app/widget/latest/supporthq-widget.js' + script.async = true + script.onload = () => { + // The script can finish loading after this effect was cleaned up (the + // src is cached, so a re-run resolves immediately), and initialising + // then would leave a widget behind that nothing destroys. + if (cancelled) return + // @ts-ignore + window.SupportHQWidget?.init({ + projectId: process.env.NEXT_PUBLIC_SUPPORT_HQ_PROJECT_ID, + themeColor: process.env.NEXT_PUBLIC_SUPPORT_HQ_THEME_COLOR ?? '#2563eb', + ...(hasUser && { + metadata: { userId, name, email, phone }, + }), + }) + } + document.body.appendChild(script) + + return () => { + cancelled = true + // @ts-ignore + window.SupportHQWidget?.destroy() + // destroy() tears down the widget but leaves this tag behind, so each + // re-run used to add another one for the life of the page. + script.remove() + } + }, [hasUser, userId, name, email, phone]) + + return <> } diff --git a/web/e2e/api-guide.spec.ts b/web/e2e/api-guide.spec.ts index 44d4c36..db54852 100644 --- a/web/e2e/api-guide.spec.ts +++ b/web/e2e/api-guide.spec.ts @@ -87,6 +87,10 @@ test.describe('api guide (mocked API, no real backend)', () => { await mockApi(page) await context.grantPermissions(['clipboard-read', 'clipboard-write']) await page.goto('/dashboard/messaging/api-guide') + // navigator.clipboard.writeText rejects when the document is not focused, + // which is the state a backgrounded page sits in while workers run in + // parallel. Granting permissions alone does not cover it. + await page.bringToFront() await page.getByRole('button', { name: 'Copy code' }).first().click()