mixpanel / mixpanel-react-native

Official React Native Tracking Library for Mixpanel Analytics
https://mixpanel.com
Apache License 2.0
105 stars 44 forks source link

Require only the used subset of `AsnycStorage` as custom storage #246

Closed bfricka closed 4 months ago

bfricka commented 4 months ago

According to the implementation in javascript/mixpanel-storage.js, only a subset of the AsyncStorage interface is actually used by Mixpanel.

Not having this interface explicitly defined is problematic, because it requires devs to assume that Mixpanel may use a larger subset of the interface at any time. Devs would need to either implement the full interface, or risk breaking changes if Mixpanel's usage changes. Additionally, the types should reflect the required interface, so it's obvious that they have appropriately implemented it.

This PR does two main things:

  1. Defines that interface in the official types. So devs can be sure they are implementing the full interface required by Mixpanel
  2. References this interface in the docs

Additionally, it links to the correct AsyncStorage reference, rather than the one that has been long removed from React Native.

bfricka commented 4 months ago

Note that, the fact that Mixpanel only uses a subset of the interface is fortuitous, as it allows the common WebStorage interface to simply be async wrapped, e.g.:

const wrapAsync = <R>(fn: () => R) => {
  return new Promise<R>((resolve, reject) => {
    try {
      resolve(fn())
    } catch (e) {
      reject(e)
    }
  })
}

export const asyncWebStorage = {
  getItem: (key) => wrapAsync(() => localStorage.getItem(key)),
  setItem: (key, value) => wrapAsync(() => localStorage.setItem(key, value)),
  removeItem: (key) => wrapAsync(() => localStorage.removeItem(key)),
} satisfies MixpanelAsyncStorage

Or in our case, we're wrapping react-native-mmkv

zihejia commented 4 months ago

Thank you so much @bfricka !