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

Remove react-native-mmkv dependency #208

Open
wants to merge 2 commits into
base: 2.0
Choose a base branch
from
Open

Conversation

vishnumad
Copy link
Contributor

Summary

Replace react-native-mmkv with shared preferences on Android and user defaults on iOS which fixes #199

How did you test your changes?

Handshake

handshake.mp4

Request

make-request.mp4

Address caching

cached-address.mp4

@bangtoven
Copy link
Member

general question: do we need any sort of data migration?

@vishnumad
Copy link
Contributor Author

@bangtoven Yeah, this will lose data for folks that were not passing in a storage object into the provider. I think we have two options:

  • Write a migration that pulls the data from the MMKV default paths (Android, iOS)
  • Or make it clear that this is a breaking change and if they were not passing in a storage object into the provider, with the new release they should pass new MMKV({ id: "mobile_sdk.store" }) in order to keep old data

@bangtoven
Copy link
Member

@vishnumad gotcha. since we don't have that many client apps yet, i think we would just go with your second option instead of implementing+maintaining migration logic

@ikedm
Copy link

ikedm commented May 1, 2023

@vishnumad, thanks for working on this, are there plans on landing this PR?

@cb-heimdall
Copy link
Collaborator

Review Error for Chico1244 @ 2023-08-04 06:20:55 UTC
User must have write permissions to review

@cb-heimdall
Copy link
Collaborator

Review Error for Chico1244 @ 2023-08-04 06:45:00 UTC
User must have write permissions to review

@cb-heimdall
Copy link
Collaborator

Review Error for Chico1244 @ 2023-08-04 06:45:33 UTC
User must have write permissions to review

@cb-heimdall
Copy link
Collaborator

Review Error for Imebeez @ 2023-08-31 12:01:28 UTC
User must have write permissions to review

Pjrich1313

This comment was marked as spam.

@cb-heimdall
Copy link
Collaborator

Review Error for Imebeez @ 2023-09-14 01:09:34 UTC
User must have write permissions to review

@bangtoven bangtoven marked this pull request as ready for review December 19, 2023 00:34
@vishnumad vishnumad requested a review from a team as a code owner December 19, 2023 00:53
Lad005

This comment was marked as spam.

@TaylorMadeUSMC

This comment was marked as spam.

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.

Bug: MMKV Module could not be found
9 participants