diff --git a/src/components/layout/__tests__/locale-server-sync-bridge.browser.test.tsx b/src/components/layout/__tests__/locale-server-sync-bridge.browser.test.tsx new file mode 100644 index 000000000..86ea74cbf --- /dev/null +++ b/src/components/layout/__tests__/locale-server-sync-bridge.browser.test.tsx @@ -0,0 +1,71 @@ +/** + * The language reaches the account only when there is an account. + * + * The provider used to PUT the locale on every mount, signed in or not. A + * signed-out page got a 401, the once-per-tab marker was never set, and every + * signed-out page load sent the refused write again. The write now belongs to + * `LocaleServerSyncBridge`, which waits for the account payload without + * fetching it and then sends the choice once. + */ +import { describe, expect, it } from "vitest"; + +import { queryKeys } from "@/lib/query-keys"; + +import { authUser, renderWithApp } from "../../../../tests/browser/render"; +import { stubFetch } from "../../../../tests/browser/fetch-stub"; +import { LocaleServerSyncBridge } from "../locale-server-sync-bridge"; + +const LOCALE_PUT = "/api/auth/me/locale"; + +async function settle(): Promise { + await new Promise((resolve) => setTimeout(resolve, 150)); +} + +describe("LocaleServerSyncBridge", () => { + it("sends nothing while nobody is signed in, and never asks who is", async () => { + const fetchStub = stubFetch().on("PUT", LOCALE_PUT, { data: null }); + await renderWithApp(, { + user: null, + locale: "de", + }); + await settle(); + expect(fetchStub.calls("PUT", LOCALE_PUT)).toHaveLength(0); + expect(fetchStub.calls("GET", "/api/auth/me")).toHaveLength(0); + }); + + it("sends the choice made while signed out once an account appears", async () => { + const fetchStub = stubFetch().on("PUT", LOCALE_PUT, { data: null }); + const { queryClient } = await renderWithApp(, { + user: null, + locale: "de", + }); + await settle(); + expect(fetchStub.calls("PUT", LOCALE_PUT)).toHaveLength(0); + + queryClient.setQueryData( + queryKeys.authMe(), + authUser({ id: "account-signed-in-later" }), + ); + await expect.poll(() => fetchStub.calls("PUT", LOCALE_PUT).length).toBe(1); + expect(fetchStub.calls("PUT", LOCALE_PUT)[0].body).toEqual({ + locale: "de", + }); + }); + + it("writes once per tab for the same account and language", async () => { + const fetchStub = stubFetch().on("PUT", LOCALE_PUT, { data: null }); + const first = await renderWithApp(, { + user: { id: "account-already-synced" }, + locale: "en", + }); + await expect.poll(() => fetchStub.calls("PUT", LOCALE_PUT).length).toBe(1); + await first.screen.unmount(); + + await renderWithApp(, { + user: { id: "account-already-synced" }, + locale: "en", + }); + await settle(); + expect(fetchStub.calls("PUT", LOCALE_PUT)).toHaveLength(1); + }); +}); diff --git a/src/components/layout/locale-server-sync-bridge.tsx b/src/components/layout/locale-server-sync-bridge.tsx new file mode 100644 index 000000000..23bb093f7 --- /dev/null +++ b/src/components/layout/locale-server-sync-bridge.tsx @@ -0,0 +1,39 @@ +"use client"; + +import { useEffect } from "react"; +import { useQuery } from "@tanstack/react-query"; + +import { fetchMe } from "@/hooks/use-auth"; +import { persistLocaleToServer, useTranslations } from "@/lib/i18n/context"; +import { queryKeys } from "@/lib/query-keys"; + +/** + * Keeps `User.locale` in step with the language on screen, for a signed-in + * account only, so the proactive Coach nudge and the other cookie-blind + * background paths write in the language the person reads. + * + * It watches the account payload without fetching it (`enabled: false`): the + * shell already reads `/api/auth/me` on every signed-in page, and a signed-out + * page has no account to write to. Fetching here would trade the refused + * locale PUT for a refused account read on every signed-out load. Same key + * and same fetcher as `useAuth`, so the cache entry is shared, not shadowed. + * + * Fires when an account appears (right after signing in, carrying the choice + * made on the sign-in page) and on every switch; `persistLocaleToServer` + * skips a value this tab already wrote for this account. + */ +export function LocaleServerSyncBridge() { + const { locale } = useTranslations(); + const { data: account } = useQuery({ + queryKey: queryKeys.authMe(), + queryFn: fetchMe, + enabled: false, + }); + const accountId = account?.id ?? null; + + useEffect(() => { + if (accountId) persistLocaleToServer(locale, accountId); + }, [locale, accountId]); + + return null; +} diff --git a/src/components/providers.tsx b/src/components/providers.tsx index 2549b61b0..de8dbec17 100644 --- a/src/components/providers.tsx +++ b/src/components/providers.tsx @@ -22,6 +22,7 @@ import { ServiceWorkerRegistrar } from "@/components/service-worker-registrar"; import { SharedRecordGrantLossBridge } from "@/components/layout/shared-record-grant-loss-bridge"; import { RecordSessionFenceBridge } from "@/components/layout/record-session-fence-bridge"; import { OidcReproofReturnBridge } from "@/components/auth/oidc-reproof-return"; +import { LocaleServerSyncBridge } from "@/components/layout/locale-server-sync-bridge"; import { useAuth, type AccountAccessStatus } from "@/hooks/use-auth"; import { useRecordSessionTransition } from "@/hooks/use-record-session-transition"; import { isDashboardSnapshotEnabled } from "@/lib/dashboard/snapshot-flag"; @@ -434,6 +435,7 @@ export function Providers({ + {children} diff --git a/src/lib/i18n/__tests__/locale-first-paint-single-write.test.tsx b/src/lib/i18n/__tests__/locale-first-paint-single-write.test.tsx index 7ca105bff..1ee834ca9 100644 --- a/src/lib/i18n/__tests__/locale-first-paint-single-write.test.tsx +++ b/src/lib/i18n/__tests__/locale-first-paint-single-write.test.tsx @@ -63,8 +63,11 @@ describe("first paint carries the resolved locale, not a default", () => { /prev\.locale === locale \? \{ locale, messages: loaded \} : prev/, ); - // And the persistence effect keys on the locale alone, so one mount - // with one locale means one write. - expect(src).toMatch(/persistLocaleToServer\(locale\);\s*\}, \[locale\]\);/); + // And the provider writes nothing to the server itself: the mirror lives + // in `LocaleServerSyncBridge`, keyed on the locale and the signed-in + // account, so a signed-out mount sends nothing and one account with one + // locale means one write (its browser test pins both). + expect(src).toMatch(/export function persistLocaleToServer\(/); + expect(src).not.toMatch(/persistLocaleToServer\(locale/); }); }); diff --git a/src/lib/i18n/context.tsx b/src/lib/i18n/context.tsx index 5fd96c77d..13e9734af 100644 --- a/src/lib/i18n/context.tsx +++ b/src/lib/i18n/context.tsx @@ -125,16 +125,23 @@ function persistLocale(newLocale: Locale) { // the server-set cookie the route emits in reply: Safari's ITP caps the // script-written copy at 7 days, and a Set-Cookie is exempt. // -// Once per tab session and per locale, not on every mount: every page load -// mounts the provider, and every authenticated write is charged to the +// Once per tab session, account and locale, not on every mount: every page +// load mounts the provider, and every authenticated write is charged to the // account's shared write bucket, so a mount-time PUT spent one write per page // view on a value that had not changed. A switch still writes at once (the // locale differs from the one recorded), and a new tab writes once. +// +// Only for a signed-in account. The route answers 401 to nobody, the marker +// is then never set, and every signed-out page load used to repeat the +// refused PUT. Signed out, the choice stays in localStorage and the cookie; +// `LocaleServerSyncBridge` calls this once an account is known, so the +// choice made before signing in reaches the column right after. const LOCALE_SYNCED_KEY = "healthlog-locale-synced"; -function persistLocaleToServer(newLocale: Locale) { +export function persistLocaleToServer(newLocale: Locale, accountId: string) { + const marker = `${accountId}:${newLocale}`; try { - if (sessionStorage.getItem(LOCALE_SYNCED_KEY) === newLocale) return; + if (sessionStorage.getItem(LOCALE_SYNCED_KEY) === marker) return; } catch { // Storage unavailable: write as before. } @@ -148,7 +155,7 @@ function persistLocaleToServer(newLocale: Locale) { .then((res) => { if (!res.ok) return; try { - sessionStorage.setItem(LOCALE_SYNCED_KEY, newLocale); + sessionStorage.setItem(LOCALE_SYNCED_KEY, marker); } catch { // Storage unavailable: the next mount writes again. } @@ -238,15 +245,6 @@ export function I18nProvider({ }; }, [locale]); - // v1.25.0 — keep `User.locale` in step with the committed UI locale. Runs on - // the first mount of a tab session (backfilling the column for users who set - // their language before this landed) and on every switch, so the proactive - // Coach nudge and other cookie-blind background paths render in the language - // the user reads. - useEffect(() => { - persistLocaleToServer(locale); - }, [locale]); - const setLocale = useCallback((newLocale: Locale) => { if (!(locales as readonly string[]).includes(newLocale)) return; diff --git a/tests/browser/fetch-stub.ts b/tests/browser/fetch-stub.ts index 5bcbe4fd2..99482907a 100644 --- a/tests/browser/fetch-stub.ts +++ b/tests/browser/fetch-stub.ts @@ -14,10 +14,10 @@ * state. A request no route matches fails with a 404 envelope and is recorded * too, so an unexpected call is visible rather than hanging. * - * One request is ambient: `I18nProvider` mirrors the active locale to - * `PUT /api/auth/me/locale` on every mount, as it does in the app. The stub - * answers it and leaves it out of `requests`, so "nothing was sent" means - * nothing the component under test sent. + * Nothing is ambient: the locale mirror (`PUT /api/auth/me/locale`) belongs + * to `LocaleServerSyncBridge` in the app's providers, which `renderWithApp` + * does not mount, so "nothing was sent" means nothing the component under + * test sent. * * `setup.ts` restores the real `fetch` after every test. */ @@ -72,9 +72,6 @@ export interface FetchStub { let realFetch: typeof fetch | null = null; -/** Sent by the providers on every mount, not by the component under test. */ -const AMBIENT = [{ method: "PUT", path: "/api/auth/me/locale" }]; - function matches(route: Route, method: string, path: string): boolean { if (route.method !== method) return false; const bare = path.split("?")[0]; @@ -123,10 +120,6 @@ export function stubFetch(): FetchStub { window.fetch = async (input, init) => { const request = await record(input, init); - const ambient = AMBIENT.some( - (a) => a.method === request.method && a.path === request.path, - ); - if (ambient) return envelope({ data: request.body ?? null }); requests.push(request); // The newest route wins, so a test can override a default it set up. const route = [...routes]