Skip to content

Remove react-native-mmkv dependency#208

Open
vishnumad wants to merge 2 commits into2.0from
vishnu/remove-mmkv-dep
Open

Remove react-native-mmkv dependency#208
vishnumad wants to merge 2 commits into2.0from
vishnu/remove-mmkv-dep

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

@nevotgnab
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

@nevotgnab
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

@nevotgnab nevotgnab 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.

@Stanmar01
Copy link

gracias

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

10 participants