Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: custom storage option for React Native SDK #539

Merged
merged 6 commits into from
Aug 27, 2024
Merged

feat: custom storage option for React Native SDK #539

merged 6 commits into from
Aug 27, 2024

Conversation

oblador
Copy link
Contributor

@oblador oblador commented Aug 13, 2024

This PR fixes #436 by implementing a custom storage option to the React Native package.

The main motivation is to get rid of the obsolete async storage package, but we also found that having multiple clients as recommended by the official docs when migrating away from secondaryMobileKeys in v9 caused them to overwrite each other's storage and therefore effectively disabling the storage/cache altogether.

@oblador oblador requested a review from a team as a code owner August 13, 2024 16:02
@oblador oblador changed the title Custom storage option for React Native SDK feat: custom storage option for React Native SDK Aug 13, 2024
@kinyoklion
Copy link
Member

Hello @oblador,

Thank you for the contribution. I agree that we should allow custom storage and we will look into your PR.

Part of the issue you were encountering was a bug and we have released a new version which addresses it. Each environment should be stored in its own namespace and not overlap.

Thank you,
Ryan

@kinyoklion
Copy link
Member

Also is there a storage package which you consider idiomatic for the react native ecosystem?

My current understanding is that built-in async storage has been removed, but the @react-native-async-storage/async-storage package is a compatible and maintained replacement.

Thank you,
Ryan

kinyoklion
kinyoklion reviewed Aug 13, 2024
View reviewed changes
packages/sdk/react-native/src/RNOptions.ts Outdated
@@ -25,6 +25,13 @@ export interface RNSpecificOptions {
* Defaults to true.
*/
readonly automaticBackgroundHandling?: boolean;

/**
* Custom persistence layer for offline mode.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storage is used in all modes it is loaded before the networked data source to decrease latency and prevent the usage of default values.

kinyoklion
kinyoklion reviewed Aug 13, 2024
View reviewed changes
packages/sdk/react-native/src/RNOptions.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally the options.ts file validates options.

The validation is only cursory and many examples are available in the common client Configuration.ts.

In this instance we should validate that storage is an object.

The other thing we need to do is remove it from the options passed to the base configuration. Otherwise it will log that it has received an unknown configuration.

@oblador
Copy link
Contributor Author

oblador commented Aug 14, 2024

AsyncStorage has wide adoption mostly due to legacy reasons (previously being part of core). Nowadays I'd argue that most new projects will pick MMKV, but I would personally prefer to decide this myself over being forced by the library.

@kinyoklion
Copy link
Member

Hello @oblador,

I've made some updates to the PR. Please let me know if these work for you.

I am still keeping the async storage base dependency for now.

Thanks,
Ryan

@oblador
Copy link
Contributor Author

oblador commented Aug 27, 2024

Looks good, thanks!

@kinyoklion kinyoklion merged commit 115bd82 into launchdarkly:main Aug 27, 2024
20 checks passed
@github-actions github-actions bot mentioned this pull request Aug 27, 2024
Merged
kinyoklion pushed a commit that referenced this pull request Aug 28, 2024
@github-actions
chore: release main (#553)
fd0c144 
🤖 I have created a release *beep* *boop*
---


<details><summary>akamai-edgeworker-sdk-common: 1.1.13</summary>

##
[1.1.13](akamai-edgeworker-sdk-common-v1.1.12...akamai-edgeworker-sdk-common-v1.1.13)
(2024-08-28)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @launchdarkly/js-server-sdk-common bumped from ^2.4.5 to ^2.5.0
</details>

<details><summary>akamai-server-base-sdk: 2.1.13</summary>

##
[2.1.13](akamai-server-base-sdk-v2.1.12...akamai-server-base-sdk-v2.1.13)
(2024-08-28)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
* @launchdarkly/akamai-edgeworker-sdk-common bumped from ^1.1.12 to
^1.1.13
    * @launchdarkly/js-server-sdk-common bumped from ^2.4.5 to ^2.5.0
</details>

<details><summary>akamai-server-edgekv-sdk: 1.1.13</summary>

##
[1.1.13](akamai-server-edgekv-sdk-v1.1.12...akamai-server-edgekv-sdk-v1.1.13)
(2024-08-28)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
* @launchdarkly/akamai-edgeworker-sdk-common bumped from ^1.1.12 to
^1.1.13
    * @launchdarkly/js-server-sdk-common bumped from ^2.4.5 to ^2.5.0
</details>

<details><summary>cloudflare-server-sdk: 2.5.11</summary>

##
[2.5.11](cloudflare-server-sdk-v2.5.10...cloudflare-server-sdk-v2.5.11)
(2024-08-28)


### Dependencies

* The following workspace dependencies were updated
  * devDependencies
    * @launchdarkly/js-server-sdk-common-edge bumped from 2.3.6 to 2.3.7
</details>

<details><summary>js-client-sdk-common: 1.6.0</summary>

##
[1.6.0](js-client-sdk-common-v1.5.0...js-client-sdk-common-v1.6.0)
(2024-08-28)


### Features

* Correct client evaluation typings.
([#554](#554))
([64ab88d](64ab88d))
* Make timeout optional in LDIdentifyOptions.
([#552](#552))
([fa247b2](fa247b2))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @launchdarkly/js-sdk-common bumped from 2.6.0 to 2.7.0
</details>

<details><summary>js-sdk-common: 2.7.0</summary>

##
[2.7.0](js-sdk-common-v2.6.0...js-sdk-common-v2.7.0)
(2024-08-28)


### Features

* Correct client evaluation typings.
([#554](#554))
([64ab88d](64ab88d))
</details>

<details><summary>js-server-sdk-common: 2.5.0</summary>

##
[2.5.0](js-server-sdk-common-v2.4.5...js-server-sdk-common-v2.5.0)
(2024-08-28)


### Features

* Correct client evaluation typings.
([#554](#554))
([64ab88d](64ab88d))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @launchdarkly/js-sdk-common bumped from 2.6.0 to 2.7.0
</details>

<details><summary>js-server-sdk-common-edge: 2.3.7</summary>

##
[2.3.7](js-server-sdk-common-edge-v2.3.6...js-server-sdk-common-edge-v2.3.7)
(2024-08-28)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @launchdarkly/js-server-sdk-common bumped from 2.4.5 to 2.5.0
</details>

<details><summary>node-server-sdk: 9.5.2</summary>

##
[9.5.2](node-server-sdk-v9.5.1...node-server-sdk-v9.5.2)
(2024-08-28)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @launchdarkly/js-server-sdk-common bumped from 2.4.5 to 2.5.0
</details>

<details><summary>node-server-sdk-dynamodb: 6.1.19</summary>

##
[6.1.19](node-server-sdk-dynamodb-v6.1.18...node-server-sdk-dynamodb-v6.1.19)
(2024-08-28)


### Dependencies

* The following workspace dependencies were updated
  * devDependencies
    * @launchdarkly/node-server-sdk bumped from 9.5.1 to 9.5.2
  * peerDependencies
    * @launchdarkly/node-server-sdk bumped from >=9.4.3 to >=9.5.2
</details>

<details><summary>node-server-sdk-otel: 1.0.11</summary>

##
[1.0.11](node-server-sdk-otel-v1.0.10...node-server-sdk-otel-v1.0.11)
(2024-08-28)


### Dependencies

* The following workspace dependencies were updated
  * devDependencies
    * @launchdarkly/node-server-sdk bumped from 9.5.1 to 9.5.2
  * peerDependencies
    * @launchdarkly/node-server-sdk bumped from >=9.4.3 to >=9.5.2
</details>

<details><summary>node-server-sdk-redis: 4.1.19</summary>

##
[4.1.19](node-server-sdk-redis-v4.1.18...node-server-sdk-redis-v4.1.19)
(2024-08-28)


### Dependencies

* The following workspace dependencies were updated
  * devDependencies
    * @launchdarkly/node-server-sdk bumped from 9.5.1 to 9.5.2
  * peerDependencies
    * @launchdarkly/node-server-sdk bumped from >=9.4.3 to >=9.5.2
</details>

<details><summary>react-native-client-sdk: 10.6.0</summary>

##
[10.6.0](react-native-client-sdk-v10.5.1...react-native-client-sdk-v10.6.0)
(2024-08-28)


### Features

* custom storage option for React Native SDK
([#539](#539))
([115bd82](115bd82))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @launchdarkly/js-client-sdk-common bumped from 1.5.0 to 1.6.0
</details>

<details><summary>vercel-server-sdk: 1.3.14</summary>

##
[1.3.14](vercel-server-sdk-v1.3.13...vercel-server-sdk-v1.3.14)
(2024-08-28)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @launchdarkly/js-server-sdk-common-edge bumped from 2.3.6 to 2.3.7
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kinyoklion kinyoklion kinyoklion approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide an option to set custom storage
2 participants
@oblador @kinyoklion