Skip to content

Centralize localStorage access to fix SSR hydration warnings & improve type-safety#412

Open
abhijit9040 wants to merge 9 commits into
shopstr-eng:mainfrom
abhijit9040:fix/storage-manager
Open

Centralize localStorage access to fix SSR hydration warnings & improve type-safety#412
abhijit9040 wants to merge 9 commits into
shopstr-eng:mainfrom
abhijit9040:fix/storage-manager

Conversation

@abhijit9040

@abhijit9040 abhijit9040 commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Description
This PR resolves the localStorage architectural debt raised in #[ReplaceWithYourIssueNumber]. It introduces a central abstraction layer for handling client-side persistence safely across the Next.js lifecycle.

Key Changes:

  • Added utils/storage.ts: Created a centralized StorageManager class and instantiated a global storage singleton.
  • Implemented STORAGE_KEYS: Added a canonical STORAGE_KEYS catalog, effectively removing Magic Strings for storage

issue reference - #410

@vercel

vercel Bot commented Apr 15, 2026

Copy link
Copy Markdown

@abhijit9040 is attempting to deploy a commit to the shopstr-eng Team on Vercel.

A member of the Team first needs to authorize it.

@calvadev

Copy link
Copy Markdown
Collaborator

This is a good start, but needs to be applied across all instances of localStorage across the codebase.

@abhijit9040

Copy link
Copy Markdown
Contributor Author

@calvadev ,

This is a good start, but needs to be applied across all instances of localStorage across the codebase.

Thanks for the feedback! I’ll go through the codebase and apply this consistently across all localStorage usages.

…GE_KEYS. Resolve technical debt by replacing direct localStorage/sessionStorage calls and legacy constants.
…age. Integrated upstream discount validation with centralized storage manager.
@abhijit9040

Copy link
Copy Markdown
Contributor Author

@calvadev , have a look on the changes.

@theAnuragMishra

Copy link
Copy Markdown
Contributor

@calvadev @abhijit9040 i am trying to review these changes. one of my observations is that most of the places where access is guarded by typeof window !== "undefined" is inside useEffect. useEffect itself is a browser only api so i wonder why we would need the guards in the first place.

I like the idea of having typed keys though.

@calvadev

Copy link
Copy Markdown
Collaborator

@theAnuragMishra the guards were there because otherwise the local storage data would often be attemped to be fetched before the windo element loaded, causing breaks. The useEffect alone wasn't a safe enough guard.

@abhijit9040

Copy link
Copy Markdown
Contributor Author

"Thanks for the review! You're right — I'll extend the refactor to cover all remaining localStorage/sessionStorage calls across the full codebase. I'll keep the isBrowser guards inside the StorageManager so the API is safe by default regardless of call context. Update incoming soon!"

- mint-button.tsx: migrate tokens/history writes to storage.setJson()
- claim-button.tsx: migrate tokens/mints/history writes to storage.setJson()
- shopstr-slider.tsx: migrate wot write to storage.setItem()
- storefront-theme-wrapper.tsx: migrate sf_seller_pubkey, sf_shop_slug, cart reads to storage; sessionStorage calls to storage.setSessionItem()
- preferences.tsx: migrate theme get/set to storage.getItem/setItem()
- onboarding/wallet.tsx: migrate nwcInfo write to storage.setJson()
- nostr-helper-functions.ts: migrate migrationComplete to storage.setItem(); fix stray brace from prior merge conflict

All production localStorage/sessionStorage calls now go through StorageManager, ensuring SSR-safety and type-safe key management via STORAGE_KEYS constants.
@abhijit9040

Copy link
Copy Markdown
Contributor Author

@calvadev , @theAnuragMishra I've extended the migration to cover all remaining localStorage and sessionStorage calls across the codebase. Every production storage access now goes through StorageManager using typed STORAGE_KEYS constants.

Files updated in this pass:

mint-button.tsx — tokens, history
claim-button.tsx — tokens, mints, history
shopstr-slider.tsx — wot
storefront-theme-wrapper.tsx — sf_seller_pubkey, sf_shop_slug, cart + sessionStorage
preferences.tsx — theme
onboarding/wallet.tsx — nwcInfo
nostr-helper-functions.ts — migrationComplete
Also fixed a stray } leftover from a previous merge conflict in nostr-helper-functions.ts.

The isBrowser guard lives inside StorageManager, so the API is SSR-safe by default regardless of call context. Test files are left as-is since they mock localStorage directly by design.

Ready for re-review!

@GautamBytes

Copy link
Copy Markdown
Contributor

@abhijit9040 can you solve the conflicts please?

… legacy getLocalStorageData

- Remove getLocalStorageData() and getLocalStorageJson() helpers entirely
- Migrate all internal calls in nostr-helper-functions.ts, fetch-service.ts,
  encryption-migration.ts, wallet-recovery.ts, pending-mint-operations.ts,
  nav-top.tsx, product-invoice-card.tsx, cart-invoice-card.tsx, _app.tsx,
  wallet/index.tsx, and transactions.tsx to storage.getJson/getItem
- Add PENDING_MINT_QUOTES to STORAGE_KEYS enum
- Update all affected tests to seed localStorage directly instead of
  mocking the legacy helper (transactions, receive-button, wallet-recovery,
  local-storage-data)
- Ensure SSR safety across all storage operations via StorageManager singleton
@abhijit9040

Copy link
Copy Markdown
Contributor Author

@GautamBytes , resloved the conflicts . have a review .

@GautamBytes GautamBytes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants