Skip to content

useLocalStorage doesn't listen for storage events despite its own doc comment promising cross-tab sync #8384

Description

@JSONbored

Context

apps/loopover-ui/src/lib/use-local-storage.ts is the shared SSR-safe localStorage hook used across
the app (app.runs.tsx's saved views, app.workbench.tsx's last-tab, app.index.tsx's onboarding
state, app.analytics.tsx's window selection, routes/index.tsx's install-tab selection, and
notification-readiness-card.tsx / onboarding-preview-card.tsx's dismiss flags).

The hook's own doc comment (lines 3-11) says:

Tiny SSR-safe localStorage hook. Reads once on mount; writes are persisted synchronously and
broadcast via a storage event for other tabs.

But update() (lines 34-47) only calls window.localStorage.setItem(key, ...) — it never adds a
window.addEventListener("storage", ...) listener anywhere in the hook. The native browser
storage event does fire in other already-open tabs when localStorage changes (that part of
the comment is technically true), but since this hook never listens for it, no already-mounted
component in another tab ever re-renders when the value changes elsewhere. The comment's implied
contract — that other tabs stay in sync — does not hold.

Concretely: open /app/runs in two tabs, save a view in tab A, and tab B's views list is stale
until the user manually reloads tab B. Same for app.workbench.tsx's last-active-tab restore and
app.analytics.tsx's window-days selection.

Requirements

  • useLocalStorage must listen for the native storage event and update its value state when the
    event's key matches the hook's own key (or legacyKey), parsing event.newValue the same way
    the mount-time read does (JSON.parse, wrapped in try/catch, matching the existing catch { /* ignore */ }
    pattern used elsewhere in the file).
  • The storage event only fires in other tabs for the same origin, never the tab that made the
    write — do not attempt to also react to same-tab writes through this path (the local update() call
    already updates that tab's own state synchronously).
  • If event.newValue is null (the key was removed via localStorage.removeItem or .clear()),
    reset to the hook's initial value rather than attempting to parse null.
  • A malformed/unparsable newValue must be ignored (same catch { /* ignore */ } fail-safe posture as
    every other localStorage read in this file), not thrown.
  • Add the window.addEventListener("storage", ...) inside the existing mount useEffect (or a
    second one) and clean it up in the returned teardown function, following the same
    add/remove-listener pattern already used in apps/loopover-ui/src/lib/api/status.ts's
    startHealthPolling() (online/offline/visibilitychange listeners with matching cleanup).
  • Either fix the implementation to match the doc comment, or narrow the doc comment to say writes are
    not currently broadcast to other tabs — pick the implementation fix; the doc comment's promise is
    the one three other engineers reading this hook will rely on, and cross-tab desync on saved views /
    dismiss flags is a real, if minor, correctness bug worth fixing rather than just documenting away.

Deliverables

  • useLocalStorage in apps/loopover-ui/src/lib/use-local-storage.ts listens for the storage
    event and updates its state when another tab changes the same key (or legacy key).
  • A null newValue (key removed elsewhere) resets state to initial.
  • A malformed newValue is ignored, not thrown.
  • The listener is added once per mount and torn down on unmount (verified by a test that mounts,
    unmounts, then fires a storage event and asserts no update / no error).

Test Coverage Requirements

apps/** is outside this repo's Codecov coverage.include (vitest.config.ts / codecov.yml's
ignore: ["apps/**", ...]), so this change owes no Codecov patch-coverage percentage. It still needs
real apps/loopover-ui/src/lib/use-local-storage.test.ts additions (the file already exists with 5
passing tests for the legacy-key migration — add to it, don't replace it) covering:

  • A storage event for the hook's own key with a valid JSON newValue updates value.
  • A storage event for a different key is ignored (value unchanged).
  • A storage event with newValue: null resets value to initial.
  • A storage event with an unparsable newValue is ignored (no throw, value unchanged).
  • A storage event for the legacyKey (when one is configured) is also honored.
  • Unmounting the hook removes the listener (fire a storage event after unmount and assert no error /
    no state change via a fresh renderHook guard, or spy on removeEventListener).

Run npx vitest run apps/loopover-ui/src/lib/use-local-storage.test.ts while iterating, and
npm run ui:test before opening the PR.

Expected Outcome

Two tabs open on the same LoopOver app page (e.g. /app/runs, /app/workbench, /app/analytics)
stay in sync when a useLocalStorage-backed value changes in one of them, matching what the hook's
own doc comment already promises. No visual/layout change — this is a data-correctness fix only.

Links & Resources

  • apps/loopover-ui/src/lib/use-local-storage.ts (the hook)
  • apps/loopover-ui/src/lib/use-local-storage.test.ts (existing tests to extend)
  • apps/loopover-ui/src/lib/api/status.ts's startHealthPolling() for the existing
    add/remove-event-listener-with-cleanup pattern this repo already follows
  • Call sites: apps/loopover-ui/src/routes/app.runs.tsx, app.workbench.tsx, app.analytics.tsx,
    app.index.tsx, routes/index.tsx, components/site/notification-readiness-card.tsx,
    components/site/app-panels/onboarding-preview-card.tsx

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions