KjellConnelly / react-native-shared-group-preferences

127 stars 57 forks source link

use SharedPreferences, save by key #2

Closed arianne-detorres closed 6 years ago

AndrewHenderson commented 6 years ago

@KjellConnelly Do you think you'll merge this PR?

This change fixes a major issue on Android where you can only store a single value at a time. Every write overwrites the previous value.

I work with @arianne-detorres. Currently, we're pointing to her fork. We want the community to benefit from this fix, so we submitted the PR here.

If anything is missing from our PR, let us know so we can make the changes. We'd really like to help this library continue to evolve.

Thanks!

KjellConnelly commented 6 years ago

Hi Andrew,

I’m pretty sure when I originally wrote this module, I tried the approach you used in this PR but it doesn’t allow for preferences to be shared between apps. It only lets a single app save data within its own container.

KjellConnelly commented 6 years ago

Another option would be to add an option in the function’s parameters to switch between using this code vs my code. I am stuck using RN 0.41 and have been since I started using RN, so maybe things are different on your end, and your PR does work as intended between android apps. But it doesn’t for me.

AndrewHenderson commented 6 years ago

We're currently using these shared preferences in conjunction with RN Share Extension and with this change, everything works as expected. Our share extension is able to access the auth token that we place in the shared group preferences upon authentication. We're able to share from other apps to our app through the extension.

We're on RN 0.55.3 and Android SDK 23-26.

KjellConnelly commented 6 years ago

I think your situation is a bit different than what this module was intended for. For iOS extensions, to share data, you need an app group. But for an android extension, it looks like you only need SharedPreferences, which only shares data between the app and its contents. Since the extension is still within your app’s contents, your code works. But if you were to create a completely different Android app and try to share, it would not work.

Note that for iOS app groups, you can share this data with extensions or other apps. But for Android SharedPreferences, you can only share with extensions.

Also, and I may be wrong about this, but I think the Share extension shares data differently than you would if you wanted to share data without using the share extension. In other words, if you had two apps that shared the same character stats in a game app, your data would not be in sync unless you used the share extension manually first. Am I wrong?

KjellConnelly commented 6 years ago

Ok I just submitted a new version to Github only. Added a new parameter to the set and get functions. Haven't had time to test it yet, and not an expert in java, so there is a good chance of a bug. If you want to test it out, go for it. Otherwise it might be a few days before I get around to testing it. Just add this parameter to your calls: {useAndroidSharedPreferences:true}

For example:

try {
    await SharedGroupPreferences.setItem("savedData", data, appGroupIdentifier, {useAndroidSharedPreferences:true})
    this.loadUsernameFromSharedStorage()
  } catch(errorCode) {
    // errorCode 0 = There is no suite with that name
    console.log(errorCode)
  }

By default, the code should now use Public Storage on Android. But if you add that option, it should use SharedPreferences instead. Let me know if that works for you :)

AndrewHenderson commented 6 years ago

... I think the Share extension shares data differently than you would if you wanted to share data without using the share extension. In other words, if you had two apps that shared the same character stats in a game app, your data would not be in sync unless you used the share extension manually first. Am I wrong?

@KjellConnelly The share extension is intended for sharing data from other apps to your app – a URI from Safari, a photo from Photos, etc.

We are not using your library to share data between apps, but rather between our app and the app's share extension.

My understanding is iOS treats the extension as a separate app, or a "sub-app." For security reasons, the extension doesn't have access to data in the main app. The JS thread has a separate scope, so referencing files used by your main app results in a new instance being returned – you can't reference your app's Redux store. Therefore, we are using this library to set values from within our main app and later retrieve them within our share extension.

I'll try out your latest change and see if it works for us. Regardless, I think you have a bug here.

Sidenote

This might be relevant to you, The RN Share Extension instructs users to create two separate React Native apps, your main one and the share app.

import { AppRegistry } from 'react-native';

import App from './App';
import Share from './Share';

AppRegistry.registerComponent('App', () => App);
AppRegistry.registerComponent('Share', () => Share);
KjellConnelly commented 6 years ago

I'll look into that two separate React Native apps part possibly in the future. The current apps I am working on are iOS first, and Android second. So I'll probably at some point try to iron out the kinks like having to have public storage on Android.

KjellConnelly commented 6 years ago

I added the functionality to use both methods. The intended method for saving data between apps, and my old code that didn’t work for my purposes, but did work for yours. You can set this flag in the options object.

AndrewHenderson commented 6 years ago

Thanks @KjellConnelly. I'm not sure I fully understand the following:

The intended method for saving data between apps, and my old code that didn’t work for my purposes, but did work for yours.

I'll try out {useAndroidSharedPreferences:true}.