-
Notifications
You must be signed in to change notification settings - Fork 31
fix: react context no longer changes on flag change #1081
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/js-client-sdk-common size report |
5936ae2 to
baed8fb
Compare
packages/sdk/react-native/src/hooks/variation/useTypedVariation.ts
Outdated
Show resolved
Hide resolved
packages/sdk/react-native/src/hooks/variation/useTypedVariation.ts
Outdated
Show resolved
Hide resolved
152a584 to
2086a07
Compare
packages/sdk/react-native/src/hooks/variation/useTypedVariation.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
4d4550f to
5669d5d
Compare
4dae4ab to
eba484e
Compare
Requirements
Related issues
sdk-1792
Describe the solution you've provided
Describe alternatives you've considered
I think this change may require a major version, but we can probably avoid that with some compromises.
Additional context
There is a testing gap here and I will create some tasks to address. I want to do this in a different PR because I would like to refactor RN unit test to use https://oss.callstack.com/react-native-testing-library/ instead of the react testing library.
We also need to revisit the detox test, from some research it seems like detox is the correct technology for e2e testing, but we may want to revisit how to plug in to the sdk test harness (and deprecate the example testing)
Note
Medium Risk
Changes core flag-evaluation hooks to manage subscriptions and state updates; incorrect listener cleanup or dependency handling could cause missed updates or leaks, though the change is localized to the React Native SDK.
Overview
Prevents flag updates from changing the React context value by removing provider-level change listeners and memoizing the
{ client }context value inLDProvider(deletingsetupListeners).Adds per-flag subscriptions in
useTypedVariationanduseTypedVariationDetailthat initialize local state, listen onchange:${key}, and update only when the evaluated value/detail actually changes (usingfastDeepEqual), rather than relying on provider re-renders. Tests are updated accordingly, including removal of the now-obsoletesetupListenersunit test.Written by Cursor Bugbot for commit eba484e. This will update automatically on new commits. Configure here.