Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nextjs localstorage issue #1193

Merged
merged 4 commits into from
Jan 14, 2025
Merged

Conversation

chakra-guy
Copy link
Collaborator

Explanation

Fixed a critical issue with Next.js 15+ server components where the SDK would attempt to access localStorage in non-browser environments, causing runtime errors. The bug was occurring because the storage manager was unconditionally initialized with browser-specific storage, even in server-side contexts.

The solution:

  1. Made PlatformManager environment detection methods static for better reusability
  2. Added environment-aware storage manager initialization that returns:
    • Regular storage manager for browser environments
    • No-op implementation for non-browser environments (server-side, SSR)

Example error that this fixes:

ReferenceError: localStorage is not defined
    at StorageManagerWeb.getPersistedChannelConfig

This change ensures the SDK works reliably in all Next.js environments while maintaining full functionality in browser contexts. The no-op storage manager gracefully handles all storage operations when running server-side, preventing runtime errors without requiring changes to application code.

References

Related to OpenSea's reported issue with Wagmi + Next.js 15

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@chakra-guy chakra-guy requested a review from a team as a code owner January 13, 2025 14:50
@chakra-guy chakra-guy requested a review from ecp4224 January 14, 2025 10:59
Copy link

codecov bot commented Jan 14, 2025

Codecov Report

Attention: Patch coverage is 76.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 74.02%. Comparing base (801c9ef) to head (1edf4b5).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...kages/sdk/src/storage-manager/getStorageManager.ts 60.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1193      +/-   ##
==========================================
- Coverage   74.35%   74.02%   -0.33%     
==========================================
  Files         182      182              
  Lines        4328     4343      +15     
  Branches     1063     1064       +1     
==========================================
- Hits         3218     3215       -3     
- Misses       1110     1128      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chakra-guy chakra-guy merged commit ea14232 into main Jan 14, 2025
35 of 36 checks passed
@chakra-guy chakra-guy deleted the fix-localstorage-issue-on-serverside branch January 14, 2025 13:53
@christopherferreira9 christopherferreira9 mentioned this pull request Jan 14, 2025
3 tasks
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.

2 participants