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

feat: add in-memory storage as a fallback for disabled LocalStorage #533

Merged
merged 8 commits into from Apr 11, 2023

Conversation

jpudysz
Copy link
Contributor

@jpudysz jpudysz commented Apr 7, 2023

Hello @mrousavy

I am submitting this pull request to propose the addition of a new feature webFallbackStorage. The purpose of this enhancement is to provide a more resilient storage option for web environments, particularly when users disable localStorage in their browsers.

The webFallbackStorage is designed as an optional property that conforms to the Storage type. It allows the application to handle disabled localStorage situations gracefully by offering eg. in-memory storage as an alternative. This ensures smooth functioning of the application despite any user-imposed limitations.

Right now, react-native-mmkv throws an error and there is no way to run the monorepo with disabled localStorage.

Here's a brief overview of the changes introduced with this feature:

  • The webFallbackStorage property has been added as an optional attribute, maintaining backward compatibility with existing implementations.
  • When a user disables localStorage, the application will seamlessly transition to using webFallbackStorage, ensuring no disruption in the user experience.
  • If the webFallbackStorage property is not provided, the default behavior remains unchanged, and an error will be thrown in case localStorage is disabled.

I believe this enhancement will significantly improve the robustness and flexibility of the react-native-mmkv library.
Later we can add default in-memory storage to the lib.

Please feel free to reach out to me with any questions or suggestions.

@jpudysz
Copy link
Contributor Author

jpudysz commented Apr 7, 2023

This is default behaviour:
White screen + such error message in the console:
err

Copy link
Owner

@mrousavy mrousavy left a comment

Choose a reason for hiding this comment

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

hey! thanks for your PR man! Left some comments, lmk :)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/createMMKV.web.ts Outdated Show resolved Hide resolved
@jpudysz jpudysz changed the title feat: allow user to pass fallback storage in case localStorage is unavailable feat: add in-memory storage as a fallback for disabled LocalStorage Apr 8, 2023
@jpudysz
Copy link
Contributor Author

jpudysz commented Apr 8, 2023

Ready to be checked @mrousavy

warn

We have a single warning and my app is up and running 🪄

@jpudysz
Copy link
Contributor Author

jpudysz commented Apr 11, 2023

@mrousavy is there anything I can do with failed job?

@mrousavy
Copy link
Owner

Hey - for some reason the linter failed. I'll fix that, your PR looks good!

I'm thinking if we should also use the in-memory Map fallback for when JSI is unavailable, e.g. when running in a Chrome debugger?

@mrousavy mrousavy merged commit a60d0b3 into mrousavy:master Apr 11, 2023
2 of 3 checks passed
@jpudysz
Copy link
Contributor Author

jpudysz commented Apr 11, 2023

Hey - for some reason the linter failed. I'll fix that, your PR looks good!

I'm thinking if we should also use the in-memory Map fallback for when JSI is unavailable, e.g. when running in a Chrome debugger?

If it causes a crash then yes, it's still better to have limited experience, yet let the user debug the app.

@jpudysz jpudysz deleted the feature/web-fallback branch April 11, 2023 11:57
@hirbod
Copy link
Contributor

hirbod commented Jun 23, 2023

Hi @jpudysz @mrousavy,

while this PR is great and works as expected inside the browser, it does flood and spam the Next.js server console now.
We're using server side props / rendering, which does not have access to window.localStorage, but the client has access.

This is throwing an incredible amount of console.log's now while navigating for us.

@hirbod
Copy link
Contributor

hirbod commented Jun 23, 2023

I think the console.log should only be printed if typeof window !== "undefined"

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.

None yet

3 participants