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

Added sessionStorage support #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Anon0x19
Copy link

I was anticipating the merge of pull/40. However, since it hasn't been merged yet, I thought the PR might have been discontinued. Given that others are also interested in this feature, I decided to implement it myself.

Previously, I created a PR but I noticed a few issues when integrating it into my own project. As a result, I decided to close that PR and have now raised this one with the necessary changes to ensure it works properly.

I hope we can get this feature merged soon, as it would be quite useful. If there are any further changes needed, please do let me know, as I am more than happy to assist with further contributions.

@sushinpv
Copy link
Owner

Hi @Anon0x19 the only issue we faced is on the NextJs when the component is rendering on the server side! We are working on it, and we will be closing this Request with this week without any delay

@Anon0x19
Copy link
Author

Hi @Anon0x19 the only issue we faced is on the NextJs when the component is rendering on the server side! We are working on it, and we will be closing this Request with this week without any delay

Thank you for the quick response @sushinpv! This fix should work on NextJs. I just tested it using nextjs-react-secure-storage-example.
The only change I made was to modify the import statement from import secureLocalStorage from "react-secure-storage"; to import {secureLocalStorage} from "react-secure-storage";. Additionally, I used yarn link and yarn link react-secure-storage with the branch in this PR

@Anon0x19 Anon0x19 force-pushed the dev/sessionstorage branch from 625b3e0 to 04b16d2 Compare July 24, 2024 16:44
@Anon0x19 Anon0x19 force-pushed the dev/sessionstorage branch from 04b16d2 to 1671b62 Compare July 24, 2024 17:58
@sushinpv
Copy link
Owner

Thanks a lot @Anon0x19 , Let me go through all the test cases and if everything looks good we will be able to release this new version within this week

@Anon0x19
Copy link
Author

Anon0x19 commented Jul 24, 2024

Thanks a lot @Anon0x19 , Let me go through all the test cases and if everything looks good we will be able to release this new version within this week

Awesome! Thank you again for getting back to me so quickly. You were right about the issue with certain Next.js versions. I tested it using 14.2.5 and 13.4.13 and it seems to work on both without issues now.

By the way, for react, I had to change the usage from i.e secureLocalStorage.setItem("number", 12); to secureLocalStorage?.setItem("number", 12); to avoid errors.

Furthermore, it seems the issue arose from new secureStorage(localStorage), but I've now changed it to typeof window !== 'undefined' && typeof window.localStorage !== 'undefined' ? new secureStorage(window.localStorage) : null;

Thank you for your time and for looking into this and testing it further.

@sushinpv
Copy link
Owner

This will be breaking change for the existing users, and integrations, I'll make the necessary changes and planning to close this in next two days

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