Skip to content

Conversation

@yusinto
Copy link
Contributor

@yusinto yusinto commented Dec 18, 2023

Please run the example app as well if you can by running yarn ios from the react-native folder. Note that expo 49 only supports rn 0.72. Expo 50 will work with 0.73 and it's in beta with full release coming in about month. Thank you for reviewing.

  • Added bootstrap from storage.
  • Added local copy of fast-deep-equal (see my comments here).
  • Cleaned up and improved React Native SDK api and example.

…n check for patch and delete. added test for inExperiment.
…nges. add patch and delete unit tests. mock streamer now supports patch and delete commands.
@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #225809: Use localstorage for bootstrapping rn sdk.

},
"dependencies": {
"@launchdarkly/js-client-sdk-common": "0.0.1",
"@react-native-async-storage/async-storage": "^1.21.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need this now since AsyncStorage is removed in 0.73.

options.logger ??
new BasicLogger({
level: 'info',
level: 'debug',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I intentionally left this as debug for the alpha.

@@ -1,4 +1,4 @@
export type EventName = 'connecting' | 'ready' | 'error' | 'change';
Copy link
Member

Choose a reason for hiding this comment

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

Are you keeping a list of breaking changes for when we do a js-client release? Things like this seem like they could be easy to slip through the cracks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I plan to submit a follow up PR to update all comments, readme and docs including breaking changes. I want to get the code reviewed asap first. The connecting event is updated to initializing though to follow @cwaldren-ld 's spec.

This implements feedback from #326.

* Run synchronization logic for all `change` events.
* Removed `ready` event and use `change` instead.
* Callback argument for `change` is now just a string array of flag
keys.
* Removed config options `stream` and `useReport` for now for this
alpha.
@yusinto yusinto enabled auto-merge (squash) December 22, 2023 17:58
@yusinto yusinto requested a review from kinyoklion December 22, 2023 17:58
@yusinto yusinto merged commit 37ff40e into main Dec 22, 2023
@yusinto yusinto deleted the yus/sc-225809/use-localstorage-for-bootstrapping-rn-sdk branch December 22, 2023 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants