chore: Provider and hooks #306
Conversation
yusinto
requested review from
kinyoklion and
louis-launchdarkly
as code owners
|
This pull request has been linked to Shortcut Story #221870: Implement RN provider api. |
There was a problem hiding this comment.
I ordered the props in this interface alphabetically automatically resulting in an ugly diff. The only significant changes are the additions of typed variation functions.
| type ReactSdkContext = { | ||
| allFlags?: LDFlagSet; | ||
| ldClient?: LDClient; | ||
| }; |
There was a problem hiding this comment.
This is still in flux and will change to have more/less things exposed.
| @@ -0,0 +1,13 @@ | |||
| import useLDClient from './useLDClient'; | |||
|
|
|||
| export const useBoolVariation = (flagKey: string, defaultValue: boolean) => { | |||
There was a problem hiding this comment.
I think these will be very nice for consistent experiment results. (Aside from also allow type safety, which is nice.)
There was a problem hiding this comment.
I agree. Thank you for pushing for these!
|
|
||
| export const useBoolVariationDetail = (flagKey: string, defaultValue: boolean) => { | ||
| const ldClient = useLDClient(); | ||
| return ldClient?.boolVariationDetail(flagKey, defaultValue) ?? defaultValue; |
There was a problem hiding this comment.
Because detailed variations return objects we need to ensure we keep stable object references. Which may happen for flags that exist, because we might be returning a value we have stored.
I am concerned that the variation for a default value will not be stable. Which could lead to the hook triggering more re-renders than desired.
There was a problem hiding this comment.
Ok this is definitely a bug. What do you think of a fix like this:
| return ldClient?.boolVariationDetail(flagKey, defaultValue) ?? defaultValue; | |
| const def = { | |
| value: defaultValue, | |
| variationIndex: null, | |
| reason: null, | |
| }; | |
| return ldClient?.boolVariationDetail(flagKey, defaultValue) ?? def; |
There was a problem hiding this comment.
So, this fixes a case where the ldclient isn't initialized to have a consistent return.
It doesn't affect the stability of the reference returned by the hook. I think you would need to make a memo.
|
I'm closing this PR to make improvements to the provider, hook and error apis. |
This is a second attempt of #306 using the newly added event source. - use StreamingProcessor and event source to fetch flags - added LDProvider - added hooks - added typed variation[Detail] functions Please read the [example README](https://github.com/launchdarkly/js-core/blob/9d458f28625c12169df614c8d2289f9c0952cc45/packages/sdk/react-native/example/README.md#L1) and try running the example app. --------- Co-authored-by: LaunchDarklyReleaseBot <launchdarklyreleasebot@launchdarkly.com> Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Please try and run the app, following the README under sdk/react-native to see if this is better than the existing legacy version in terms of brokenness.