Skip to content
This repository was archived by the owner on Mar 13, 2023. It is now read-only.

refactor: make async storage a peer dependency #44

Conversation

mateusz1913
Copy link

Context:

In my project, we use @segment/analytics-react-native & @segment/sovran-react-native and we use react-native-mmkv-storage for key-value storage solution (including custom persistor for segment client). Recently we wanted to remove @react-native-async-storage/async-storage as it's not used in our codebase. However segment libraries declare it as a dependency, even though it should be optional as @segment/analytics-react-native has an option to create custom persistor that matches Persistor type from @segment/sovran-react-native.
If it's possible, then let's mark @react-native-async-storage/async-storage as optional peer dependency to not force users to download it when custom persistor is used.
I'll make follow-up PR for @segment/analytics-react-native

  • @react-native-async-storage/async-storage is marked as optional peer dependency
  • @react-native-async-storage/async-storage is optionally required (instead of being imported) in src/persistor/async-storage-persistor.ts

`@segment/analytics-react-native` has an option to create custom persistor that matches `Persistor` type
from `@segment/sovran-react-native`. If it's possible, then let's mark
`@react-native-async-storage/async-storage` as optional peer dependency to not force users to download it when
custom persistor is used.

- `@react-native-async-storage/async-storage` is marked as optional peer dependency
- `@react-native-async-storage/async-storage` is optionally required (instead of being imported) in
`src/persistor/async-storage-persistor.ts`
@mateusz1913 mateusz1913 force-pushed the refactor/async-storage-optional branch from 539ef04 to 2980ba1 Compare December 19, 2022 19:38
mateusz1913 added a commit to mateusz1913/analytics-react-native that referenced this pull request Dec 19, 2022
mateusz1913 added a commit to mateusz1913/analytics-react-native that referenced this pull request Dec 19, 2022
mateusz1913 added a commit to mateusz1913/analytics-react-native that referenced this pull request Dec 19, 2022
Context: segmentio/sovran-react-native#44

In the README, the installation instructions point to install `@segment/sovran-react-native` &
`@react-native-async-storage/async-storage`. In that case it doesn't make sense to include those as direct
dependencies.
@mateusz1913
Copy link
Author

👋 is there sth I need to improve here?

@oscb
Copy link
Contributor

oscb commented Mar 7, 2023

@mateusz1913 Hey, nothing wrong with your PR, we just weren't tracking this repository lately. We just consolidated this in the monorepo at @segment/analytics-react-native to have an easier time following up on these.
This is something we had in out minds but was never a priority. I will merge your changes over the version of this package in there with proper credit.

Thanks!

@oscb
Copy link
Contributor

oscb commented Mar 13, 2023

Merged at: segmentio/analytics-react-native#779

@oscb oscb closed this Mar 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants