optimizely / javascript-sdk

JavaScript SDK for Optimizely Feature Experimentation and Optimizely Full Stack (legacy)
https://www.optimizely.com/products/experiment/feature-experimentation/
Apache License 2.0
80 stars 82 forks source link

[ENHANCEMENT] Remove async storage from the core implementation #952

Open matinzd opened 1 month ago

matinzd commented 1 month ago

Description

It would be ideal not to tightly integrate async storage within the SDK, allowing us to inject other types of storage options. There is an object called persistentCacheProvider, but it appears that even if we pass in persistentCacheProvider, the SDK still defaults to using async-storage. Perhaps lazy-loading async storage would be a better option, or not including it as a peer dependency at all.

Benefits

It is possible to use faster and more modern storage options in react native like mmkv or secure storage.

Detail

No response

Examples

No response

Risks/Downsides

No response

raju-opti commented 2 weeks ago

Hi @matinzd, thank you for raising the issue.

but it appears that even if we pass in persistentCacheProvider, the SDK still defaults to using async-storage

To not force the user to provide a persistenceCache implementation, we use react-native-async-storage by default. I assume you mean react-native-async-storage is imported statically even if a persistentCacheProvider is provided, requiring the user to install react-native-async-storage even though it is not being used. I see that is the case in the sdk source code. I agree that we should load it dynamically instead. We are looking into it.

matinzd commented 2 weeks ago

A possible way/workaround for this is to import async storage in the try/catch block with a local variable in a helper class and export it to other parts of the sdk. In that case you can keep the default implementation without forcing users to install async-storage.

let AsyncStorage = null

try {
  AsyncStorage = require('@react-native-async-storage/async-storage').default  
} catch (e) {
  console.warn('AsyncStorage is not available')
}

export AsyncStorage;
raju-opti commented 2 weeks ago

@matinzd Thanks for the suggestion. We will consider something similar to this.