refactor(studio): use useSyncExternalStore in useLocalStorage

[aray12]

Summary

Rewrites useLocalStorage (web/packages/studio/src/util/hooks/useLocalStorage.ts) to use React 18's useSyncExternalStore instead of useState + a lazy initializer. The public API — [value, setValue, deleteValue] — is unchanged, so no callsites needed edits.

Resolves ASTD-272.

What changed

• Cross-tab reactivity — subscribes to the storage event, so a write in another tab now re-renders this tab.
• Same-tab reactivity — the storage event only fires in other tabs, so setValue/deleteValue also dispatch a custom nemo-studio:local-storage event. Two hooks on the same key in one tab now stay in sync (previously each had its own isolated useState).
• SSR — the old typeof window === 'undefined' guard is replaced by the getServerSnapshot parameter.

Two correctness details useSyncExternalStore forces

useSyncExternalStore compares snapshots with Object.is:

1. The parsed value is cached against the raw string, so an unchanged value returns a stable reference instead of a fresh JSON.parse result each render.
2. The default is frozen to the first render's reference (via a ref). Two callsites (ExperimentGroupDataView, useRecentWorkspaces) pass an inline [], a new reference every render — returning that directly would trigger React's "getSnapshot should be cached" infinite loop. Freezing it also matches the old lazy-useState "read the default once" semantics.

Minor behavior change

After deleteValue, the value now falls back to the default rather than undefined. For callsites without a default this is identical (undefined either way); for those with one it's arguably more correct.

Testing

• Added useLocalStorage.test.ts (11 tests) — mirrors the sibling useSessionStorage.test.ts plus new cases for cross-tab storage events, same-tab sync, and stable snapshot references.
• typecheck, lint, the new tests, and the ThemeSwitch callsite test all pass.

Summary by CodeRabbit

• Bug Fixes
  • Improved local storage state synchronization across multiple hook instances and browser tabs.
  • Ensured updates and deletions are reflected immediately in the current tab.
  • Added reliable fallback behavior for missing or invalid stored data.
  • Prevented unnecessary re-renders and update loops when default values change by reference.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

Local storage synchronization

Layer / File(s)	Summary
External-store hook and notifications web/packages/studio/src/util/hooks/useLocalStorage.ts	useLocalStorage now uses useSyncExternalStore, cached parsed snapshots, server defaults, native and custom storage events, and notification-based writes and deletions.
Hook behavior coverage web/packages/studio/src/util/hooks/useLocalStorage.test.ts	Tests cover defaults, serialization, deletion, invalid JSON, cross-tab and same-tab synchronization, referential stability, and inline defaults.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant useLocalStorage
  participant localStorage
  participant useSyncExternalStore
  Caller->>useLocalStorage: call setValue
  useLocalStorage->>localStorage: persist value
  useLocalStorage->>useSyncExternalStore: notify key subscribers
  useSyncExternalStore->>useLocalStorage: read updated snapshot
  useLocalStorage-->>Caller: expose updated value

Loading

Suggested reviewers: philipmattingly

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main refactor: switching useLocalStorage to useSyncExternalStore.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch astd-272-upgrade-uselocalstorage-to-use-usesyncexternalstore

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (2)

web/packages/studio/src/util/hooks/useLocalStorage.ts (1)

37-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add an explicit return type to the exported useLocalStorage.

Per coding guidelines, public APIs should have explicit return types. Currently the tuple type is only inferred via as const.

✏️ Suggested fix

-export const useLocalStorage = <T>(key: string, defaultValue?: T) => {
+export const useLocalStorage = <T>(
+  key: string,
+  defaultValue?: T
+): readonly [T | undefined, (value: T) => void, () => void] => {

As per coding guidelines, "Use explicit return types for public APIs and complex functions in TypeScript."

Also applies to: 104-104

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/packages/studio/src/util/hooks/useLocalStorage.ts` at line 37, Update the
exported useLocalStorage function to declare an explicit return type, including
the tuple’s value and setter types, rather than relying on inference from as
const. Apply the same explicit return-type requirement to the related exported
symbol at the other indicated location, preserving the existing API behavior.

Source: Coding guidelines

web/packages/studio/src/util/hooks/useLocalStorage.test.ts (1)

75-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing test for the e.key === null (storage cleared) branch.

subscribe's onStorage handler explicitly special-cases e.key === null (storage cleared) in useLocalStorage.ts, but no test dispatches a StorageEvent('storage', { key: null }) to exercise it.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/packages/studio/src/util/hooks/useLocalStorage.test.ts` around lines 75 -
87, Extend the useLocalStorage tests around the “reacts to storage events from
other tabs” case to cover a StorageEvent with key set to null, representing
cleared storage. Set the stored value, dispatch the event, and assert the hook
updates according to the e.key === null branch in subscribe’s onStorage handler.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@web/packages/studio/src/util/hooks/useLocalStorage.test.ts`:
- Around line 75-87: Extend the useLocalStorage tests around the “reacts to
storage events from other tabs” case to cover a StorageEvent with key set to
null, representing cleared storage. Set the stored value, dispatch the event,
and assert the hook updates according to the e.key === null branch in
subscribe’s onStorage handler.

In `@web/packages/studio/src/util/hooks/useLocalStorage.ts`:
- Line 37: Update the exported useLocalStorage function to declare an explicit
return type, including the tuple’s value and setter types, rather than relying
on inference from as const. Apply the same explicit return-type requirement to
the related exported symbol at the other indicated location, preserving the
existing API behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2789da4b-8fb2-45e8-b182-8e9eb993a8bb

📥 Commits

Reviewing files that changed from the base of the PR and between 275c8cd and 483170c.

📒 Files selected for processing (2)

• web/packages/studio/src/util/hooks/useLocalStorage.test.ts
• web/packages/studio/src/util/hooks/useLocalStorage.ts

[github-actions]

Suite	Lines Covered	Line Rate	Branch Rate
Unit Tests	23845/31009	76.9%	61.7%
Integration Tests	13817/29658	46.6%	19.6%