Persist in-app feature flags (dev-only feature) #19619
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ilmotta
requested review from
cammellos,
seanstrom,
vkjr,
J-Son89 and
BalogunofAfrica
ilmotta
force-pushed
the
ilmotta/persist-feature-flags
branch
from
b1f280b
to
7d711e3
Compare
Jenkins BuildsClick to see older builds (34)
|
ilmotta
force-pushed
the
ilmotta/persist-feature-flags
branch
4 times, most recently
from
60bd42f
to
6b70b38
Compare
|
@ilmotta This will be a very useful thing to have. Thanks for the initiative. I tested both the iOS & Android PR builds and it doesn't seem to be working. |
Indeed, it only works for me in the emulator, which was the only build I tested. Let's see if I can find the culprit. Thanks! |
ilmotta
force-pushed
the
ilmotta/persist-feature-flags
branch
3 times, most recently
from
27a9133
to
268e49d
Compare
|
@ajayesivan, issue fixed. I had a misconception that |
|
Hi reviewers @cammellos, @seanstrom, @vkjr, @J-Son89, @BalogunofAfrica. This PR is rotting a little bit waiting for more reviews/approvals. It would be great if we could get this PR merged soon, that is, if you agree it's valuable. Thank you! |
ilmotta
force-pushed
the
ilmotta/persist-feature-flags
branch
from
268e49d
to
3474b42
Compare
seanstrom
approved these changes
May 7, 2024
|
@ilmotta what do you think about re-rendering the app when changing the feature flags? I was noticing the other day that I couldn’t immediately see feature-flags affect UI until I forced a re-render. Do you see the same behavior? |
Hey @seanstrom, do you believe this problem is happening in all places? The flags are stored in a Reagent atom, so in theory, if we code views correctly, they should react to changes to this global state. My intuition tells me we should see why it's not re-rendering for each buggy view instead of forcing a full re-render. Wdyt? |
|
@ilmotta yup yup I agree with your approach, the specific screen I’ll debug is related to the settings screen. I’ll try figuring out why it’s not re-rendering properly and ping about what I find n chat. Thanks for your thoughts 🙏 |
I'm curious to see why it's failing to re-render. Let's hope that we find the root cause. Thanks! |
J-Son89
approved these changes
May 7, 2024
|
I like the initiative @ilmotta 🚀 |
ilmotta
force-pushed
the
ilmotta/persist-feature-flags
branch
from
3474b42
to
f65c050
Compare
63% of end-end tests have passedFailed tests (17)Click to expandClass TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletMultipleDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
Expected to fail tests (2)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (33)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR improves in-app feature flags to persist what is currently only stored in a Reagent atom. This should make them more convenient to use, which is a good thing overall for developers.
Additionally, there's now a top-right button in screen
Settings > Feature Flagsthat will reset the flags to the initial values obtained from environment variables. Just a handy little addition.These in-app feature flags are exclusively available in debug builds in
Settings > Feature Flags, and only visible when flagENABLE_QUO_PREVIEWis enabled. There's no impact whatsoever in prod builds. A reminder that they are not meant to be used by users.That's why I think the simple solution presented in this PR is good enough. The developer will now be able to close and reopen the app, and the flags will be recovered using RN Async Storage during app initialization.
It's worth noting that RN has deprecated Async Storage and now recommends other community solutions, but for a dev-only feature, I think it's fine. Regarding this, I don't know if the mobile team has an official recommendation to replace Async Storage.
This PR is kind of a natural progression from the original work done in PR #18602 Add a UI for toggling developer feature flags. We knew from the start it would be useful to persist, but that we could do it later.
Areas that may be impacted
None.
Steps to test
Toggle in-app feature flags, then close and reopen the app. Check if the values were persisted in
Settings > Feature Flags.status: ready