From 635c4c7e52707fcb5950147393f2134928aa1014 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Mon, 31 Aug 2026 15:03:40 -0700 Subject: [PATCH] Keep a settings change made before the first read, and wait for it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects on the path that decides what language the app starts in. **A change made before the settings file came back was thrown away.** `queueSettingsPush` returned early while unarmed, dropping the value instead of holding it, so a language picked in the second or so after a page load was never written up: it survived until the next reload and no further. That is a better account of "sometimes it takes several clicks" than the remount race fixed in #160 — the click that stuck was one made after the read had finished. Keeping it is safe because hydrate already refuses to overwrite a key that is still queued. Proof it was real: before this, no `ihasmail` folder was ever created in the account's files, because the seed write never fired. After it, the folder appears. **Without a cached settings object the tree painted too early.** The cache is not read on an untrusted device, and it is cleared by the sign-out that every deploy causes, so in both cases the first frame is the defaults — and the defaults are English. Anything computed in that window is computed in the wrong language. The interface recovers, since it is rebuilt when the catalogue lands, but a string emitted once does not: this is why the stale-folder toast came out in English on an otherwise German screen. So without a cache the authenticated tree now waits for the account's settings and their catalogue, which costs nothing — there was nothing worth painting yet. With a cache it does not wait, and the first frame is as quick as it was. Neither fix makes the toast German yet: the account settings file is neither written nor read successfully in the mock, and both failures are swallowed. That is a third problem, and this commit does not touch it. --- web/src/App.tsx | 54 ++++++++++++++++++++++++++++++------- web/src/lib/settingsSync.ts | 18 ++++++++++++- web/src/lib/storage.ts | 19 +++++++++++++ web/src/store/settings.ts | 13 ++++++++- 4 files changed, 92 insertions(+), 12 deletions(-) diff --git a/web/src/App.tsx b/web/src/App.tsx index 8fb91d3..eae8f58 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -17,7 +17,7 @@ import { AppShell } from "@/views/AppShell"; import { MailView } from "@/views/mail/MailView"; import { ComposerDock } from "@/views/compose/ComposerDock"; import { setUnreadBadge } from "@/lib/notify"; -import { useSettings, syncedPart } from "@/store/settings"; +import { PAINTED_FROM_CACHE, useSettings, syncedPart } from "@/store/settings"; import { armSettingsSync, loadRemoteSettings, queueSettingsPush, settingsAlreadyLoadedFor, settingsSyncAvailable } from "@/lib/settingsSync"; import { listenForVerification, renewWebPush } from "@/lib/webpushEnable"; import { useLanguageVersion, whenLanguageReady } from "@/lib/i18n"; @@ -82,21 +82,45 @@ function AuthedApp() { const accountId = useSession((s) => s.accountId); const [location] = useLocation(); - // Settings that live with the account rather than the browser. The cached - // ones have already painted, so this only has to correct them (issue #54). - // - // Once per account, not once per mount: this subtree is keyed on the - // language version, so picking a language throws it away and builds it - // again. Re-reading the settings file there would apply a copy written - // before the change and undo it. + /* + * Settings that live with the account rather than the browser. + * + * When this browser has them cached they have already painted, and this only + * has to correct them (issue #54). When it does not -- an untrusted device, + * or the sign-out that every deploy causes -- the first frame is the + * defaults, and the defaults are English. Rendering then means anything + * computed before the settings land is computed in the wrong language: not + * the interface, which is rebuilt when the catalogue arrives, but a string + * emitted once, like a toast. That is why the stale-folder toast came out + * in English on an otherwise German screen. + * + * So without a cache the tree waits, which costs nothing: there was nothing + * worth painting yet. With one it does not wait, and the screen is as quick + * as it was. + * + * Once per account, not once per mount: this subtree is keyed on the + * language version, so picking a language throws it away and builds it + * again. Re-reading the settings file there would apply a copy written + * before the change and undo it. + */ + const [ready, setReady] = useState(PAINTED_FROM_CACHE); useEffect(() => { - if (settingsAlreadyLoadedFor(accountId)) return; + if (settingsAlreadyLoadedFor(accountId)) { + setReady(true); + return; + } let cancelled = false; void (async () => { const remote = await loadRemoteSettings(); if (cancelled) return; if (remote) useSettings.getState().hydrate(remote); - // Pushes were held back until now so they could not race the load. + // The catalogue for whatever language that turned out to be. Hydrating + // asks for it; this is waiting for the answer. + await whenLanguageReady(); + if (cancelled) return; + setReady(true); + // Pushes were held back until now so they could not race the load. A + // change made while it was in flight was kept, and goes out here. armSettingsSync(); // No file yet — seed one from what this browser has, so the next device // to sign in starts from these rather than from the defaults. @@ -188,6 +212,16 @@ function AuthedApp() { if (notif) void import("@/lib/notify").then((m) => m.requestNotificationPermission()); }, [notif]); + // Nothing worth painting until the account's settings are in force; see the + // comment on `ready` above. With a cache this was true from the first frame. + if (!ready) { + return ( +
+ +
+ ); + } + return ( }> diff --git a/web/src/lib/settingsSync.ts b/web/src/lib/settingsSync.ts index 19b46aa..7ac9548 100644 --- a/web/src/lib/settingsSync.ts +++ b/web/src/lib/settingsSync.ts @@ -85,6 +85,8 @@ export function settingsAlreadyLoadedFor(accountId: string | null | undefined): export function armSettingsSync(): void { armed = true; bindFlushListeners(); + // A change made while the read was in flight has been waiting for this. + if (pending) void flushSettingsPush(); } /** Stop syncing and drop anything queued (logout). */ @@ -104,8 +106,22 @@ export function stopSettingsSync(): void { * one request goes out once the changes stop. */ export function queueSettingsPush(synced: Record): void { - if (!armed || !settingsSyncAvailable()) return; + if (!settingsSyncAvailable()) return; + /* + * Held, not dropped, before the first load has settled. + * + * This used to return here, which silently threw the change away: a + * language picked in the second or so before the settings file came back + * was never written, so it survived until the next reload and no further. + * That is the other half of "sometimes it takes several clicks" -- the + * click that stuck was one made after the read had finished. + * + * Keeping it is safe because `hydrate` refuses to overwrite a key that is + * still queued, so the newer local change wins over the older file rather + * than racing it. `armSettingsSync` writes whatever is waiting. + */ pending = synced; + if (!armed) return; if (timer !== null) window.clearTimeout(timer); timer = window.setTimeout(() => { timer = null; diff --git a/web/src/lib/storage.ts b/web/src/lib/storage.ts index e9b634e..c861ddd 100644 --- a/web/src/lib/storage.ts +++ b/web/src/lib/storage.ts @@ -107,6 +107,25 @@ export function loadJson(key: string, fallback: T): T { } } +/** + * Did `loadJson` have something to return, or did it hand back the fallback? + * + * The difference decides whether a first paint is worth anything. With a + * cached value the screen can be right immediately and the account's copy only + * has to correct it; without one -- an untrusted device, or the sign-out that + * every deploy causes -- the first paint is the defaults, and painting it + * before the account's settings arrive shows English to somebody who chose + * otherwise. + */ +export function hasCachedJson(key: string): boolean { + if (!trusted) return false; + try { + return localStorage.getItem(PREFIX + key) != null; + } catch { + return false; + } +} + export function loadRaw(key: string, fallback: T): T { if (!trusted) return fallback; try { diff --git a/web/src/store/settings.ts b/web/src/store/settings.ts index dc3ecf5..b4944c2 100644 --- a/web/src/store/settings.ts +++ b/web/src/store/settings.ts @@ -1,6 +1,6 @@ import { useEffect, useState } from "react"; import { create } from "zustand"; -import { loadJson, saveJson } from "@/lib/storage"; +import { hasCachedJson, loadJson, saveJson } from "@/lib/storage"; import { pendingSettingsKeys, queueSettingsPush } from "@/lib/settingsSync"; import { setDateTimePrefs, setUiLanguageForFormatting, type DateFormat, type TimeFormat } from "@/lib/datetime"; import type { SwipeAction } from "@/lib/swipe"; @@ -311,6 +311,17 @@ interface SettingsState { } const initialSettings = loadJson("settings", DEFAULT_SETTINGS); + +/** + * Whether the first frame is this account's settings or merely the defaults. + * + * False after every deploy, because deploys sign everyone out and sign-out + * clears the cache -- and false on any untrusted device, where the cache is + * never read. In that state `uiLanguage` starts as English and only becomes + * the account's choice once the settings file lands, which is why the + * authenticated tree waits for it. + */ +export const PAINTED_FROM_CACHE = hasCachedJson("settings"); applyDateTimePrefs(initialSettings); export const useSettings = create((set, get) => ({