Skip to content

refactor: extract a generic useStorageValue hook for localStorage state#163

Merged
MkDev11 merged 2 commits into
MkDev11:mainfrom
cleanjunc:refactor/use-storage-value-hook
May 26, 2026
Merged

refactor: extract a generic useStorageValue hook for localStorage state#163
MkDev11 merged 2 commits into
MkDev11:mainfrom
cleanjunc:refactor/use-storage-value-hook

Conversation

@cleanjunc
Copy link
Copy Markdown
Contributor

@cleanjunc cleanjunc commented May 26, 2026

Summary

Adds src/lib/use-storage-value.ts, a generic hook that encapsulates the localStorage read, write, parse, serialize, and cross tab event subscription pattern that was previously duplicated across three files. useTrackedRepos, useTrackedMiners, and useSettings are now thin wrappers around it.

The public hook APIs and the custom event names (tracked-repos-changed, tracked-miners-changed, settings-changed) are unchanged, so no call sites needed to be touched. The wrappers preserve the original behavior of always reflecting the parsed disk value after a write (toggle and update still re read from localStorage so rapid successive writes do not race).

Related Issues

Closes #162

Type of Change

  • Bug fix
  • New feature
  • Enhancement
  • Refactor
  • Documentation
  • Other (describe):

Testing

  • pnpm build passes
  • Manual browser smoke test: toggled repo tracking on the Repositories page, toggled miner tracking on the Miners page, and changed several Settings values (layout, page size, render markdown) with two tabs open to confirm same tab and cross tab updates still propagate
  • N/A, docs / config only

Checklist

  • Self-reviewed the diff
  • Follows existing code patterns and naming
  • No unrelated changes included
  • Documentation updated if behavior changed

Summary by CodeRabbit

  • Refactor
    • Consolidated internal storage handling for application settings and tracking features to improve data synchronization reliability across browser tabs and sessions.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

📝 Walkthrough

Walkthrough

Three React hooks that independently managed localStorage state with identical boilerplate patterns are consolidated into thin wrappers around a new shared useStorageValue hook. The shared hook handles storage reads, parsing, serialization, event listening (both native and custom), and SSR-safe defaults; existing hooks are refactored to delegate state management while maintaining their public APIs.

Changes

localStorage State Consolidation via useStorageValue

Layer / File(s) Summary
useStorageValue shared hook foundation
src/lib/use-storage-value.ts
New generic useStorageValue<T> hook unifies localStorage persistence, safe parsing/serialization, SSR-safe defaults, and event coordination. Listens for both native storage events and an optional custom eventName. The write callback persists to localStorage, updates React state immediately, and dispatches the custom event when provided.
Settings hook refactor to useStorageValue
src/lib/settings.ts
useSettings migrates to useStorageValue. Adds parse/serialize helpers with layout field validation and a readFresh function for atomic disk reads. The update and reset callbacks now use the hook's write function instead of manual state/event management.
Tracked repos and miners hooks refactor to useStorageValue
src/lib/tracked-miners.ts, src/lib/tracked-repos.ts
Both hooks refactor to useStorageValue for Set-based storage. Each introduces parse/serialize/readFresh helpers. toggle, clear, and setMany callbacks rewrite to use the hook's write function instead of direct localStorage mutation and custom event dispatch.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Three hooks once danced alone with localStorage dreams,
Now bundled up together in useStorageValue beams,
No more boilerplate ballet, no drift or duplicate strain,
One shared, reusable friend to reign and maintain!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: extracting a generic useStorageValue hook to eliminate localStorage duplication across three modules.
Linked Issues check ✅ Passed The code implementation fully satisfies all requirements from issue #162: new useStorageValue hook with correct signature, subscriptions to storage and custom events, SSR safety, and all three hooks rewritten as thin wrappers preserving public APIs and event names.
Out of Scope Changes check ✅ Passed All changes are within scope: the PR adds useStorageValue and refactors three existing hooks to use it, aligning perfectly with issue #162's objectives for eliminating localStorage duplication.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/lib/settings.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

src/lib/tracked-miners.ts

ESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.

src/lib/tracked-repos.ts

ESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.

  • 1 others

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@src/lib/settings.ts`:
- Around line 43-52: The parse() function must guard against non-object JSON
(e.g., JSON null or primitives) before treating parsed as an AppSettings object:
after JSON.parse(raw) (referenced as parsed), check that parsed is a non-null
object (typeof parsed === 'object' && parsed !== null) and only then spread its
properties and read parsed.layout; otherwise fall back to returning
DEFAULT_SETTINGS (or merging only safe defaults). Update parse to validate
parsed before accessing parsed.layout and only cast to Partial<AppSettings>
after the object check so corrupt/legacy storage values use default settings
instead of causing runtime errors.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2720c91f-4a56-4d35-bc4a-19c7f4ed126d

📥 Commits

Reviewing files that changed from the base of the PR and between e896ce0 and a175d45.

📒 Files selected for processing (4)
  • src/lib/settings.ts
  • src/lib/tracked-miners.ts
  • src/lib/tracked-repos.ts
  • src/lib/use-storage-value.ts

Comment thread src/lib/settings.ts
@MkDev11 MkDev11 merged commit e07dba8 into MkDev11:main May 26, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Three React hooks reimplement the same localStorage state pattern

2 participants