Improve/add support for sync providers, ie rn-mmkv

filipef101 commented 1 year ago

Is your feature request related to a problem?

Would be nice if we could use MMKV instead of async storage

Describe the solution you'd like

Describe alternatives you've considered

Related sub-libraries

[ ] All of them
[ ] posthog-web
[ ] posthog-node
[x] posthog-react-native

Additional context

Thank you for your feature request – we love each and every one!

filipef101 commented 1 year ago

Some people may have mmkv instead of async storage, posthog could detect if async storage not present fallback to mmkv + option to prefer mmkv

benjackwhite commented 1 year ago

We didn't want to end up with 100s of if statements trying to cover every possible dependency 😅 But you can just configure your own interface like so:

const options: PostHogOptions = {
  customAsyncStorage: {
    getItem: (key: string) => Promise<string | null>,
    setItem: (key: string, value: string) => Promise<void>
  }
}

And then you can use whatever storage you like :)

filipef101 commented 1 year ago

mmkv is sync, any chance to get a sync storage provider? to be able to have feature flags on first render/paint @benjackwhite

benjackwhite commented 1 year ago

I'm not 100% certain but I would expect if you simply resolve immediately in those custom functions then it basically will be sync, especially from a first-render point of view?

marandaneto commented 11 months ago

The bridge is async by design, so even if you resolve/await a function, it does not guarantee the order of execution, it might be that something is executed before your awaited function (it's an event loop/single thread), that has also been awaited for.

A solution for all the async problems is to migrate to the RN new architecture and define when a method is async/sync or even have both. Since all the storage plugins are Native libs (android/ios), this would be the best/most reliable option.

For most cases, just awaiting the methods is enough, showing loading progress. At the same time, the calls get resolved, and after you decide what to do depending on the resolved feature flags, it's not necessary to be before the first render/paint, you can work around that by just using this suggestion for now.

filipef101 commented 11 months ago

@marandaneto I don't see any native code, so what needs to be migrated or am i missing something?

filipef101 commented 11 months ago

I'm not 100% certain but I would expect if you simply resolve immediately in those custom functions then it basically will be sync, especially from a first-render point of view?

@benjackwhite doesnt matter how fast it is, if you need to await it wont be available on first paint/render

marandaneto commented 11 months ago

@marandaneto I don't see any native code, so what needs to be migrated or am i missing something?

Right now we try to use expo-file-system or AsyncStorage, both are async by design.

We'd need to either change SemiAsyncStorage or create an SyncStorage type where all methods are sync. including the preloadAsync. Also create a customSyncStorage instead of customAsyncStorage, etc.

Also, use another file system plugin that supports the new architecture/JSI with only sync. methods, for example, react-native-mmkv.

So it's a bit of refactoring on the TS code actually, not native code, plus supporting react-native-mmkv.