fix(web): a settings change in one tab no longer reverts another tab's change

apps/web/src/clientPersistenceStorage.ts

@@ -1,6 +1,10 @@
 import { ClientSettingsSchema, type ClientSettings } from "@t3tools/contracts";
 
-import { getLocalStorageItem, setLocalStorageItem } from "./hooks/useLocalStorage";
+import {
+  getLocalStorageItem,
+  setLocalStorageItem,
+  subscribeLocalStorageKey,
+} from "./hooks/useLocalStorage";
 
 const CLIENT_SETTINGS_STORAGE_KEY = "t3code:client-settings:v1";
 
@@ -23,3 +27,12 @@ export function writeBrowserClientSettings(settings: ClientSettings): void {
 
   setLocalStorageItem(CLIENT_SETTINGS_STORAGE_KEY, settings, ClientSettingsSchema);
 }
+
+/** Fires when another tab saves client settings. */
+export function subscribeBrowserClientSettings(onChange: () => void): () => void {
+  if (!hasWindow()) {
+    return () => undefined;
+  }
+
+  return subscribeLocalStorageKey(CLIENT_SETTINGS_STORAGE_KEY, onChange);
+}

apps/web/src/hooks/useLocalStorage.ts

@@ -134,7 +134,8 @@ export function readLocalStorageValue<T, E>(
 /** Fires on writes from this tab and from other tabs. */
 export function subscribeLocalStorageKey(key: string, onChange: () => void): () => void {
   const handleStorageChange = (event: StorageEvent) => {
-    if (event.key === key) {
+    // Storage events carry a null key after clear().
+    if (event.key === key || event.key === null) {
       onChange();
     }
   };

apps/web/src/hooks/useSettings.test.ts

@@ -31,8 +31,27 @@ beforeEach(() => {
 
 afterEach(() => {
   vi.restoreAllMocks();
+  __resetClientSettingsPersistenceForTests();
+  vi.unstubAllGlobals();
 });
 
+const otherTabSettings = { ...DEFAULT_CLIENT_SETTINGS, timestampFormat: "12-hour" as const };
+const readFailure = new Error("storage unavailable");
+
+async function saveInOtherTab(
+  failedReads = 0,
+  saved: ClientSettings | null = otherTabSettings,
+  key: string | null = "t3code:client-settings:v1",
+) {
+  vi.stubGlobal("window", new EventTarget());
+  await ensureClientSettingsHydrated();
+  for (let read = 0; read < failedReads; read += 1) {
+    persistenceMocks.getClientSettings.mockRejectedValueOnce(readFailure);
+  }
+  persistenceMocks.getClientSettings.mockResolvedValue(saved);
+  window.dispatchEvent(Object.assign(new Event("storage"), { key }));
+}
+
 describe("client settings hydration", () => {
   const savedSettings = {
     ...DEFAULT_CLIENT_SETTINGS,
@@ -195,6 +214,43 @@ describe("persistClientSettingsPatch", () => {
     expect(persistenceMocks.setClientSettings).toHaveBeenNthCalledWith(1, firstSettings);
     expect(persistenceMocks.setClientSettings).toHaveBeenNthCalledWith(2, secondSettings);
   });
+
+  it.each([
+    { change: "saved", loaded: null, saved: otherTabSettings },
+    { change: "cleared", loaded: otherTabSettings, saved: null },
+    {
+      change: "cleared with localStorage.clear()",
+      loaded: otherTabSettings,
+      saved: null,
+      key: null,
+    },
+  ])(
+    "follows settings another tab $change after this tab loaded",
+    async ({ loaded, saved, key }) => {
+      persistenceMocks.getClientSettings.mockResolvedValueOnce(loaded);
+      await saveInOtherTab(0, saved, key);
+
+      await persistClientSettingsPatch({ wordWrap: false });
+
+      const expected = { ...(saved ?? DEFAULT_CLIENT_SETTINGS), wordWrap: false };
+      expect(persistenceMocks.setClientSettings).toHaveBeenLastCalledWith(expected);
+      expect(getClientSettings()).toEqual(expected);
+    },
+  );
+
+  it("writes only after re-reading settings another tab saved when the reload failed", async () => {
+    vi.spyOn(console, "error").mockImplementation(() => undefined);
+    // Fails the reload and the first write's re-read.
+    await saveInOtherTab(2);
+
+    await persistClientSettingsPatch({ wordWrap: false });
+    expect(persistenceMocks.setClientSettings).not.toHaveBeenCalled();
+
+    await persistClientSettingsPatch({ snapShotFlash: false });
+    const expected = { ...otherTabSettings, snapShotFlash: false };
+    expect(persistenceMocks.setClientSettings).toHaveBeenCalledExactlyOnceWith(expected);
+    expect(getClientSettings()).toEqual(expected);
+  });
 });
 
 describe("persistClientSettingsUpdate", () => {
@@ -220,19 +276,38 @@ describe("persistClientSettingsUpdate", () => {
     expect(getClientSettings().timestampFormat).toBe("12-hour");
   });
 
+  it("rejects after a failed reload until settings another tab saved can be re-read", async () => {
+    vi.spyOn(console, "error").mockImplementation(() => undefined);
+    // Fails the reload and the first write's re-read.
+    await saveInOtherTab(2);
+    const disableWrap = (current: ClientSettings) => ({ ...current, wordWrap: false });
+
+    await expect(persistClientSettingsUpdate(disableWrap)).rejects.toBe(readFailure);
+    expect(persistenceMocks.setClientSettings).not.toHaveBeenCalled();
+
+    const expected = { ...otherTabSettings, wordWrap: false };
+    await expect(persistClientSettingsUpdate(disableWrap)).resolves.toEqual(expected);
+    expect(persistenceMocks.setClientSettings).toHaveBeenCalledExactlyOnceWith(expected);
+  });
+
   it("preserves an optimistic write made while an awaited update persists", async () => {
     let finishFirstPersistence!: () => void;
     let durableSettings = DEFAULT_CLIENT_SETTINGS;
     const firstPersistence = new Promise<void>((resolve) => {
       finishFirstPersistence = resolve;
     });
+    let markFirstPersistenceStarted!: () => void;
+    const firstPersistenceStarted = new Promise<void>((resolve) => {
+      markFirstPersistenceStarted = resolve;
+    });
     const persist = vi
       .fn<(settings: typeof DEFAULT_CLIENT_SETTINGS) => Promise<void>>()
-      .mockImplementationOnce((settings) =>
-        firstPersistence.then(() => {
+      .mockImplementationOnce((settings) => {
+        markFirstPersistenceStarted();
+        return firstPersistence.then(() => {
           durableSettings = settings;
-        }),
-      )
+        });
+      })
       .mockImplementation(async (settings) => {
         durableSettings = settings;
       });
@@ -246,7 +321,7 @@ describe("persistClientSettingsUpdate", () => {
       }),
       persist,
     );
-    await Promise.resolve();
+    await firstPersistenceStarted;
     const patch = persistClientSettingsPatch({ wordWrap: false }, persist);
     finishFirstPersistence();
     await Promise.all([pending, patch]);

apps/web/src/hooks/useSettings.ts

@@ -30,6 +30,7 @@ import {
   splitSharedServerPatch,
   supportsSharedSettingsSync,
 } from "@t3tools/client-runtime/state/shared-settings";
+import { subscribeBrowserClientSettings } from "~/clientPersistenceStorage";
 import { ensureLocalApi } from "~/localApi";
 import {
   getThemeDefinition,
@@ -75,6 +76,9 @@ let clientSettingsHydrationPromise: Promise<void> | null = null;
 let clientSettingsHydrationGeneration = 0;
 let clientSettingsPersistenceQueue: Promise<void> = Promise.resolve();
 let deferredClientSettingsPatchCount = 0;
+// Set when another tab saves; cleared only by a successful re-read, so writes never use a stale snapshot.
+let clientSettingsStale = false;
+let stopReloadingClientSettings: (() => void) | null = null;
 
 function emitClientSettingsChange() {
   for (const listener of clientSettingsListeners) {
@@ -129,6 +133,13 @@ export function subscribeClientSettingsHydration(listener: () => void): () => vo
   };
 }
 
+async function readPersistedClientSettings(): Promise<ClientSettings> {
+  const persistedSettings = await ensureLocalApi().persistence.getClientSettings();
+  return persistedSettings
+    ? { ...DEFAULT_CLIENT_SETTINGS, ...persistedSettings }
+    : DEFAULT_CLIENT_SETTINGS;
+}
+
 async function hydrateClientSettings(): Promise<void> {
   if (clientSettingsHydrationStatus === "ready") {
     return;
@@ -145,14 +156,13 @@ async function hydrateClientSettings(): Promise<void> {
   );
   const nextHydration = (async () => {
     try {
-      const persistedSettings = await ensureLocalApi().persistence.getClientSettings();
+      const persistedSettings = await readPersistedClientSettings();
       if (hydrationGeneration !== clientSettingsHydrationGeneration) {
         return;
       }
-      if (persistedSettings) {
-        replaceClientSettingsSnapshot({ ...DEFAULT_CLIENT_SETTINGS, ...persistedSettings });
-      }
+      replaceClientSettingsSnapshot(persistedSettings);
       setClientSettingsHydrationStatus("ready");
+      stopReloadingClientSettings ??= subscribeBrowserClientSettings(reloadClientSettings);
     } catch (error) {
       if (hydrationGeneration === clientSettingsHydrationGeneration) {
         setClientSettingsHydrationStatus("failed");
@@ -187,13 +197,35 @@ function enqueueClientSettingsPersistence<A>(work: () => Promise<A>): Promise<A>
   return result;
 }
 
+async function ensureFreshClientSettings(): Promise<void> {
+  if (clientSettingsHydrationStatus !== "ready") {
+    return hydrateClientSettings();
+  }
+  if (clientSettingsStale) {
+    replaceClientSettingsSnapshot(await readPersistedClientSettings());
+    clientSettingsStale = false;
+  }
+}
+
+function reloadClientSettings(): void {
+  clientSettingsStale = true;
+  void enqueueClientSettingsPersistence(ensureFreshClientSettings).catch((error) => {
+    console.error(`${CLIENT_SETTINGS_PERSISTENCE_ERROR_SCOPE} reload failed`, {
+      operation: "reload",
+      ...safeErrorLogAttributes(error),
+    });
+  });
+}
+
 export function persistClientSettingsPatch(
   patch: ClientSettingsPatch,
   persist: (settings: ClientSettings) => Promise<void> = defaultClientSettingsPersistence,
 ): Promise<void> {
-  // Patches queued before hydration must publish before newer optimistic patches.
+  // Patches queued behind hydration or a reload must publish before newer optimistic patches.
   const deferPatch =
-    clientSettingsHydrationStatus !== "ready" || deferredClientSettingsPatchCount > 0;
+    clientSettingsHydrationStatus !== "ready" ||
+    clientSettingsStale ||
+    deferredClientSettingsPatchCount > 0;
   if (deferPatch) {
     deferredClientSettingsPatchCount += 1;
   } else {
@@ -202,9 +234,7 @@ export function persistClientSettingsPatch(
   return enqueueClientSettingsPersistence(async () => {
     if (deferPatch) {
       try {
-        if (clientSettingsHydrationStatus !== "ready") {
-          await hydrateClientSettings();
-        }
+        await ensureFreshClientSettings();
         replaceClientSettingsSnapshot({ ...getClientSettingsSnapshot(), ...patch });
       } finally {
         deferredClientSettingsPatchCount -= 1;
@@ -225,9 +255,7 @@ export async function persistClientSettingsUpdate(
   persist: (settings: ClientSettings) => Promise<void> = defaultClientSettingsPersistence,
 ): Promise<ClientSettings> {
   return enqueueClientSettingsPersistence(async () => {
-    if (clientSettingsHydrationStatus !== "ready") {
-      await hydrateClientSettings();
-    }
+    await ensureFreshClientSettings();
     for (;;) {
       const current = getClientSettingsSnapshot();
       const next = update(current);
@@ -733,6 +761,9 @@ export function updateClientSettings(patch: ClientSettingsPatch): Promise<void>
 
 export function __resetClientSettingsPersistenceForTests(): void {
   clientSettingsHydrationGeneration += 1;
+  stopReloadingClientSettings?.();
+  stopReloadingClientSettings = null;
+  clientSettingsStale = false;
   clientSettingsSnapshot = DEFAULT_CLIENT_SETTINGS;
   clientSettingsHydrationStatus = "pending";
   clientSettingsHydrationPromise = null;