Merge pull request #19776 from mozilla/fxa-12738

fix(metrics): Add `useLocalStorageSync` hook for reactive changes to localstorage

Author: vbudhram

packages/fxa-settings/src/lib/cache.ts

@@ -45,7 +45,7 @@ export function getStoredAccountData({
 
 type LocalAccounts = Record<hexstring, StoredAccountData>;
 
-function accounts(accounts?: LocalAccounts) {
+export function accounts(accounts?: LocalAccounts) {
   if (accounts) {
     storage.set('accounts', accounts);
     return accounts;

packages/fxa-settings/src/lib/hooks/useLocalStorageSync.test.ts

@@ -0,0 +1,69 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import { renderHook, act } from '@testing-library/react';
+import { useLocalStorageSync } from './useLocalStorageSync';
+import Storage from '../storage';
+
+jest.mock('../storage');
+
+const mockStorage = {
+  get: jest.fn(),
+  set: jest.fn(),
+  remove: jest.fn(),
+  clear: jest.fn(),
+};
+
+describe('useLocalStorageSync', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+    (Storage.factory as jest.Mock) = jest.fn(() => mockStorage);
+  });
+
+  it('returns value from storage', () => {
+    mockStorage.get.mockReturnValue('test-value');
+    const { result } = renderHook(() => useLocalStorageSync('test-key'));
+
+    expect(result.current).toBe('test-value');
+    expect(mockStorage.get).toHaveBeenCalledWith('test-key');
+  });
+
+  it('returns undefined when key does not exist', () => {
+    mockStorage.get.mockReturnValue(undefined);
+    const { result } = renderHook(() => useLocalStorageSync('non-existent'));
+
+    expect(result.current).toBeUndefined();
+  });
+
+  it('subscribes to localStorageChange events', () => {
+    mockStorage.get.mockReturnValue('value');
+    const { result } = renderHook(() => useLocalStorageSync('test-key'));
+
+    expect(result.current).toBe('value');
+
+    act(() => {
+      window.dispatchEvent(
+        new CustomEvent('localStorageChange', {
+          detail: { key: 'test-key' },
+        })
+      );
+    });
+
+    expect(mockStorage.get).toHaveBeenCalled();
+  });
+
+  it('cleans up event listener on unmount', () => {
+    const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener');
+    const { unmount } = renderHook(() => useLocalStorageSync('test-key'));
+
+    unmount();
+
+    expect(removeEventListenerSpy).toHaveBeenCalledWith(
+      'localStorageChange',
+      expect.any(Function)
+    );
+
+    removeEventListenerSpy.mockRestore();
+  });
+});

packages/fxa-settings/src/lib/hooks/useLocalStorageSync.ts

@@ -0,0 +1,63 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import { useSyncExternalStore } from 'react';
+import Storage from '../storage';
+
+/**
+ * Hook to monitor a specific localStorage key and reactively update when it changes.
+ * Works for both same-tab and cross-tab changes.
+ *
+ * @param key - The localStorage key to monitor (without the '__fxa_storage.' prefix)
+ * @returns The current value from localStorage, or undefined if not set
+ *
+ * @example
+ * ```tsx
+ * const currentAccountUid = useLocalStorageSync('currentAccountUid');
+ *
+ * useEffect(() => {
+ *   if (currentAccountUid) {
+ *     console.log('Account UID changed:', currentAccountUid);
+ *   }
+ * }, [currentAccountUid]);
+ * ```
+ */
+export function useLocalStorageSync(key: string) {
+  const storage = Storage.factory('localStorage');
+
+  const subscribe = (callback: () => void) => {
+    // Listen for custom events (same-tab changes)
+    // We'll dispatch this when we write to localStorage
+    const handleCustomStorage = (e: CustomEvent) => {
+      if (e.detail?.key === key) {
+        callback();
+      }
+    };
+
+    window.addEventListener('localStorageChange' as any, handleCustomStorage as EventListener);
+
+    return () => {
+      window.removeEventListener('localStorageChange' as any, handleCustomStorage as EventListener);
+    };
+  };
+
+  // Cache snapshot to prevent infinite loops - compare by value, not reference
+  let cachedSnapshot: any;
+  let cachedSnapshotString: string | undefined;
+
+  const getSnapshot = () => {
+    const snapshot = storage.get(key);
+    const snapshotString = JSON.stringify(snapshot);
+
+    // Only update cached snapshot if the value actually changed
+    if (snapshotString !== cachedSnapshotString) {
+      cachedSnapshot = snapshot;
+      cachedSnapshotString = snapshotString;
+    }
+
+    return cachedSnapshot;
+  };
+
+  return useSyncExternalStore(subscribe, getSnapshot);
+}

packages/fxa-settings/src/lib/storage.ts

@@ -58,10 +58,27 @@ class Storage {
 
   set(key: string, val: any): void {
     this._backend.setItem(fullKey(key), JSON.stringify(val));
+
+    // This allows useLocalStorageSync to detect changes in the same tab
+    if (this._backend === window.localStorage) {
+      window.dispatchEvent(
+        new CustomEvent('localStorageChange', {
+          detail: { key },
+        })
+      );
+    }
   }
 
   remove(key: string) {
     this._backend.removeItem(fullKey(key));
+
+    if (this._backend === window.localStorage) {
+      window.dispatchEvent(
+        new CustomEvent('localStorageChange', {
+          detail: { key },
+        })
+      );
+    }
   }
 
   clear() {

packages/fxa-settings/src/models/contexts/NimbusContext.test.tsx

@@ -3,26 +3,23 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import React from 'react';
-import { render, screen } from '@testing-library/react';
+import { render, screen, waitFor } from '@testing-library/react';
 import { NimbusProvider, useNimbusContext } from './NimbusContext';
 import { AppContext, AppContextValue } from './AppContext';
 import { useDynamicLocalization } from '../../contexts/DynamicLocalizationContext';
 import { initializeNimbus, NimbusResult } from '../../lib/nimbus';
 import * as Sentry from '@sentry/react';
-import { currentAccount } from '../../lib/cache';
-import { StoredAccountData } from '../../lib/storage-utils';
+import { useLocalStorageSync } from '../../lib/hooks/useLocalStorageSync';
 
 jest.mock('../../contexts/DynamicLocalizationContext');
 jest.mock('../../lib/nimbus');
 jest.mock('@sentry/react');
-jest.mock('../../lib/cache', () => ({
-  currentAccount: jest.fn(),
-}));
+jest.mock('../../lib/hooks/useLocalStorageSync');
 
 const mockUseDynamicLocalization = useDynamicLocalization as jest.MockedFunction<typeof useDynamicLocalization>;
 const mockInitializeNimbus = initializeNimbus as jest.MockedFunction<typeof initializeNimbus>;
 const mockSentryCaptureException = Sentry.captureException as jest.MockedFunction<typeof Sentry.captureException>;
-const mockCurrentAccount = currentAccount as jest.MockedFunction<typeof currentAccount>;
+const mockUseLocalStorageSync = useLocalStorageSync as jest.MockedFunction<typeof useLocalStorageSync>;
 
 const TestComponent = () => {
   const { experiments, loading, error } = useNimbusContext();
@@ -61,9 +58,16 @@ describe('NimbusContext', () => {
       clearLanguagePreference: jest.fn(),
       isLoading: false
     });
-    mockCurrentAccount.mockReturnValue({
-      metricsEnabled: true,
-    } as StoredAccountData);
+    // Mock useLocalStorageSync to return undefined by default (no account)
+    mockUseLocalStorageSync.mockImplementation((key: string) => {
+      if (key === 'currentAccountUid') {
+        return undefined;
+      }
+      if (key === 'accounts') {
+        return undefined;
+      }
+      return undefined;
+    });
     Object.defineProperty(window, 'location', {
       value: { search: '' },
       writable: true
@@ -121,32 +125,39 @@ describe('NimbusContext', () => {
       expect(screen.getByTestId('loading')).toHaveTextContent('false');
     });
 
-    it('does not fetch when metrics are disabled', async () => {
-      mockCurrentAccount.mockReturnValue({
-        metricsEnabled: false,
-      } as StoredAccountData);
+    it('does not fetch when metrics are disabled via localStorage', async () => {
+      const accountUid = 'test-account-uid';
+      mockUseLocalStorageSync.mockImplementation((key: string) => {
+        if (key === 'currentAccountUid') return accountUid;
+        if (key === 'accounts') return { [accountUid]: { metricsEnabled: false } };
+        return undefined;
+      });
 
       renderWithProviders();
 
-      expect(mockInitializeNimbus).not.toHaveBeenCalled();
-      expect(screen.getByTestId('loading')).toHaveTextContent('false');
+      await waitFor(() => {
+        expect(mockInitializeNimbus).not.toHaveBeenCalled();
+      });
       expect(screen.getByTestId('experiments')).toHaveTextContent('no-experiments');
     });
 
-    it('fetches when metrics are enabled', async () => {
-      mockCurrentAccount.mockReturnValue({
-        metricsEnabled: true,
-      } as StoredAccountData);
-      const mockExperiments: NimbusResult = {
+    it('fetches when metrics are enabled via localStorage', async () => {
+      const accountUid = 'test-account-uid';
+      mockUseLocalStorageSync.mockImplementation((key: string) => {
+        if (key === 'currentAccountUid') return accountUid;
+        if (key === 'accounts') return { [accountUid]: { metricsEnabled: true } };
+        return undefined;
+      });
+      mockInitializeNimbus.mockResolvedValue({
         features: { 'test-feature': { enabled: true } },
         nimbusUserId: 'test-user-id'
-      };
-      mockInitializeNimbus.mockResolvedValue(mockExperiments);
+      });
 
       renderWithProviders();
 
-      expect(mockInitializeNimbus).toHaveBeenCalled();
-      await screen.findByTestId('experiments');
+      await waitFor(() => {
+        expect(mockInitializeNimbus).toHaveBeenCalled();
+      });
       expect(screen.getByTestId('experiments')).toHaveTextContent('has-experiments');
     });
 
@@ -191,17 +202,15 @@ describe('NimbusContext', () => {
       expect(screen.getByTestId('experiments')).toHaveTextContent('no-experiments');
     });
 
-    it('handles fetch error', async () => {
+    it('handles fetch error without duplicate error handling', async () => {
       const error = new Error('Network error');
       mockInitializeNimbus.mockRejectedValue(error);
 
       renderWithProviders();
 
       await screen.findByTestId('error');
       expect(screen.getByTestId('error')).toHaveTextContent('Network error');
-      expect(mockSentryCaptureException).toHaveBeenCalledWith(error, expect.objectContaining({
-        tags: { area: 'NimbusProvider', component: 'NimbusContext' }
-      }));
+      expect(mockSentryCaptureException).toHaveBeenCalledTimes(1);
     });
 
     it('handles preview mode from config', async () => {
@@ -234,13 +243,19 @@ describe('NimbusContext', () => {
       );
     });
 
-    it('cleans up on unmount', () => {
+    it('cleans up on unmount', async () => {
+      mockInitializeNimbus.mockResolvedValue({
+        features: { 'test-feature': { enabled: true } },
+        nimbusUserId: 'test-user-id'
+      });
+
       const { unmount } = renderWithProviders();
 
-      unmount();
+      await waitFor(() => {
+        expect(mockInitializeNimbus).toHaveBeenCalled();
+      });
 
-      // Should not throw or cause memory leaks
-      expect(mockInitializeNimbus).toHaveBeenCalled();
+      expect(() => unmount()).not.toThrow();
     });
   });
 });

packages/fxa-settings/src/models/contexts/NimbusContext.ts

@@ -21,7 +21,7 @@ import { useDynamicLocalization } from '../../contexts/DynamicLocalizationContex
 import { parseAcceptLanguage } from '@fxa/shared/l10n';
 import { searchParams } from '../../lib/utilities';
 import * as Sentry from '@sentry/react';
-import { currentAccount } from '../../lib/cache';
+import { useLocalStorageSync } from '../../lib/hooks/useLocalStorageSync';
 
 interface NimbusApiResponse {
   Features?: Record<string, any>;
@@ -45,7 +45,7 @@ const NimbusContext = createContext<NimbusContextValue | undefined>(undefined);
 
 export function useNimbusContext() {
   const context = useContext(NimbusContext);
-  if (context === undefined ) {
+  if (context === undefined) {
     // Return default values when no NimbusProvider is present
     return {
       experiments: null,
@@ -73,21 +73,30 @@ export function NimbusProvider({ children }: NimbusProviderProps) {
   const [loading, setLoading] = useState<boolean>(false);
   const [error, setError] = useState<Error | undefined>(undefined);
 
-  useEffect(() => {
+  // Monitor currentAccountUid changes in localStorage (reacts to same-tab and cross-tab changes)
+  const currentAccountUid = useLocalStorageSync('currentAccountUid');
+  const accounts = useLocalStorageSync('accounts');
+
+  // Get metricsEnabled from accounts object, safely handling undefined values
+  const accountMetricsEnabled = useMemo(() => {
+    if (!currentAccountUid || !accounts || accounts[currentAccountUid]?.metricsEnabled === undefined) {
+      return true;
+    }
+
+    return accounts[currentAccountUid]?.metricsEnabled;
+  }, [currentAccountUid, accounts]);
 
-    // Disable Nimbus if metrics are opted out, note that we specifically check
-    // the local storage account because the account stored in the AppContext is
-    // not available at this point
-    const legacyLocalStorageAccount = currentAccount();
-    if (legacyLocalStorageAccount && legacyLocalStorageAccount.metricsEnabled === false) {
+
+  useEffect(() => {
+    // Disable Nimbus if metrics are opted out
+    const nimbusUserId = currentAccountUid || uniqueUserId;
+    if (accountMetricsEnabled === false) {
       setExperiments(null);
       setLoading(false);
       setError(undefined);
       return;
     }
 
-    const nimbusUserId = legacyLocalStorageAccount?.uid || uniqueUserId;
-
     if (!config?.nimbus.enabled || !nimbusUserId) {
       setExperiments(null);
       setLoading(false);
@@ -160,7 +169,9 @@ export function NimbusProvider({ children }: NimbusProviderProps) {
     config?.nimbus.enabled,
     config?.nimbus.preview,
     uniqueUserId,
-    currentLocale
+    currentLocale,
+    currentAccountUid,
+    accountMetricsEnabled,
   ]);
 
   const value: NimbusContextValue = useMemo(() => ({