Add possibility to override feature flags with environment variable FEATURE_FLAGS #1372

ofreyssinet-ledger · 2022-09-26T14:44:03Z

📝 Description

I did this because I needed to set a feature flag in a playwright test and doing it through an env variable made sense as playwright fixtures are already modifying env.

Now in playwright you can do this to enabled a feature flag:

test.use({ featureFlags: { mockFeature: { enabled: true } } });

It is working both on LLM and LLD.

I also made the appropriate changes in the feature flag debugging UIs in both LLM and LLD.

I'm sure this will be useful in the future for other purposes.

❓ Context

Impacted projects: ledger-live-mobile ledger-live-desktop types-live live-common
Linked resource(s): LIVE-3895

✅ Checklist

Test coverage
Atomic delivery
No breaking changes

📸 Demo

Compiling LLM with `.env` file in `apps/ledger-live-mobile`:

Running LLD like this

FEATURE_FLAGS='{"mockFeature": {"enabled": false, "someRandomKey": "aCustomValue"}}' pnpm dev:lld

🚀 Expectations to reach

Please make sure you follow these Important Steps.

Pull Requests must pass the CI and be internally validated in order to be merged.

vercel · 2022-09-26T14:44:06Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
live-common-tools	✅ Ready (Inspect)	Visit Preview	Oct 13, 2022 at 10:25AM (UTC)

3 Ignored Deployments

Name	Status	Updated
ledger-live-github-bot	⬜️ Ignored (Inspect)	Oct 13, 2022 at 10:25AM (UTC)
native-ui-storybook	⬜️ Ignored (Inspect)	Oct 13, 2022 at 10:25AM (UTC)
react-ui-storybook	⬜️ Ignored (Inspect)	Oct 13, 2022 at 10:25AM (UTC)

changeset-bot · 2022-09-26T14:44:07Z

🦋 Changeset detected

Latest commit: 571ca9a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
live-mobile	Patch
ledger-live-desktop	Patch
@ledgerhq/live-common	Patch
@ledgerhq/types-live	Patch
@ledgerhq/live-cli	Patch
live-common-tools	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov · 2022-09-26T15:03:03Z

Codecov Report

Base: 47.61% // Head: 43.01% // Decreases project coverage by -4.60% ⚠️

Coverage data is based on head (8098f60) compared to base (2c3d6b5).
Patch coverage: 8.33% of modified lines in pull request are covered.

❗ Current head 8098f60 differs from pull request most recent head 571ca9a. Consider uploading reports for the commit 571ca9a to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1372      +/-   ##
===========================================
- Coverage    47.61%   43.01%   -4.61%     
===========================================
  Files          687      609      -78     
  Lines        30258    25477    -4781     
  Branches      7912     6979     -933     
===========================================
- Hits         14407    10958    -3449     
+ Misses       14632    13385    -1247     
+ Partials      1219     1134      -85

Flag	Coverage Δ
test	`43.01% <8.33%> (-4.61%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...er-live-common/src/families/stellar/api/horizon.ts	`22.85% <0.00%> (ø)`
libs/ledger-live-common/src/env.ts	`74.50% <20.00%> (-5.93%)`	⬇️
libs/ledger-live-common/src/range.ts	`94.87% <0.00%> (-2.57%)`	⬇️
libs/ledgerjs/packages/hw-app-tezos/src/Tezos.ts
...s/ledgerjs/packages/cryptoassets/src/currencies.ts
.../ledgerjs/packages/hw-app-btc/src/finalizeInput.ts
libs/ledgerjs/packages/hw-app-eth/src/utils.ts
libs/ledgerjs/packages/errors/src/index.ts
...ibs/ledgerjs/packages/cryptoassets/data/stellar.js
...ledgerjs/packages/hw-transport-mocker/src/index.ts
... and 71 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

github-actions · 2022-09-26T15:19:24Z

@ofreyssinet-ledger

Screenshots: ✅

There are no changes in the screenshots for this PR. If this is expected, you are good to go.

gre

some feedback. good overall.

gre · 2022-09-27T08:46:05Z

apps/ledger-live-desktop/src/renderer/components/FirebaseFeatureFlags.tsx

+          if (feature)
+            return {
+              ...feature,
+              overridesRemote: true,


if it's overriden by env, should remove be false? 🤔

It still overrides the remote value, if the name of the variable was different I would have done what you are suggesting

gre · 2022-09-27T08:47:13Z

.../ledger-live-desktop/src/renderer/screens/settings/sections/Developer/FeatureFlagsButton.tsx

+                    {focusedName === flagName ? (
+                      <Flex flexDirection="column" pl={6} rowGap={3}>
+                        <EditSection
+                          value={inputValues[flagName] || JSON.stringify(pureValue)}


prefer to cache this with a memo. stringify every time it re-renders isn't ideal. you may want to have an intermediary component to ease this as it's included in a "if", generally speaking the JSX tree is quite deep here and splitting into more components isn't bad.

Thanks I'll do that 👍

Done 👌
I split it in 3 components on both LLM & LLD -> FeatureFlagsSettings/index.tsx FeatureFlagDetails.tsx FeatureFlagEdit.tsx.
It's much clearer now I think

github-actions · 2022-10-06T13:55:16Z

@ofreyssinet-ledger

Screenshots: ✅

There are no changes in the screenshots for this PR. If this is expected, you are good to go.

Add feature flags parameter to playwright test fixtures doc:ljs Add env overriding of feature flags in LLM changesets Fix FeatureFlagsButton LLD Fix FeatureFlagsButton remove console log Refactor feature flag settings on LLD Add null to JSONValue type Refactor FeatureFlagsSettings (split in components) Move LLD's FeatureFlagEdit to own component Apply switch toggle directly rather than just modifying the input Remove useless badly typed tryParse

…tically set flag keys

… set flag keys

ofreyssinet-ledger · 2022-10-13T10:05:04Z

Force push: just a squash & rebase, then did some fixes on both apps

github-actions bot assigned ofreyssinet-ledger Sep 26, 2022

github-actions bot added common Has changes in live-common desktop Has changes in LLD ledgerjs Has changes in the ledgerjs open source libs mobile Has changes in LLM labels Sep 26, 2022

vercel bot deployed to Preview – live-common-tools September 26, 2022 14:45 View deployment

ofreyssinet-ledger marked this pull request as ready for review September 26, 2022 14:58

vercel bot deployed to Preview – live-common-tools September 26, 2022 15:30 View deployment

gre reviewed Sep 27, 2022

View reviewed changes

vercel bot deployed to Preview – live-common-tools September 27, 2022 13:22 View deployment

vercel bot deployed to Preview – live-common-tools September 28, 2022 09:30 View deployment

vercel bot deployed to Preview – live-common-tools September 28, 2022 09:42 View deployment

vercel bot deployed to Preview – live-common-tools October 6, 2022 13:00 View deployment

thomasrogerlux approved these changes Oct 13, 2022

View reviewed changes

lambertkevin approved these changes Oct 13, 2022

View reviewed changes

ofreyssinet-ledger added 2 commits October 13, 2022 11:31

Fix styling of LLD feature flags settings, trim input & hide programa…

e1aefef

…tically set flag keys

ofreyssinet-ledger force-pushed the support/env-feature-flags branch from 60ae08a to e1aefef Compare October 13, 2022 09:59

vercel bot deployed to Preview – live-common-tools October 13, 2022 10:01 View deployment

LLM feature flag settings: trim add flag input & hide programatically…

4723ae4

… set flag keys

vercel bot deployed to Preview – live-common-tools October 13, 2022 10:05 View deployment

Add hint on how to use the "add missing flag name" input

64b0e7c

vercel bot deployed to Preview – live-common-tools October 13, 2022 10:20 View deployment

Fix bad "value" prop in inputs

571ca9a

vercel bot deployed to Preview – live-common-tools October 13, 2022 10:25 View deployment

alexandremgo approved these changes Oct 13, 2022

View reviewed changes

nabil-brn self-requested a review October 17, 2022 16:10

nabil-brn approved these changes Oct 17, 2022

View reviewed changes

ofreyssinet-ledger merged commit b53d648 into develop Oct 18, 2022

ofreyssinet-ledger deleted the support/env-feature-flags branch October 18, 2022 07:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add possibility to override feature flags with environment variable FEATURE_FLAGS #1372

Add possibility to override feature flags with environment variable FEATURE_FLAGS #1372

ofreyssinet-ledger commented Sep 26, 2022 •

edited

Loading

vercel bot commented Sep 26, 2022 •

edited

Loading

changeset-bot bot commented Sep 26, 2022 •

edited

Loading

codecov bot commented Sep 26, 2022 •

edited

Loading

github-actions bot commented Sep 26, 2022 •

edited

Loading

gre left a comment

gre Sep 27, 2022

ofreyssinet-ledger Sep 27, 2022 •

edited

Loading

gre Sep 27, 2022

ofreyssinet-ledger Sep 27, 2022

ofreyssinet-ledger Sep 28, 2022

github-actions bot commented Oct 6, 2022 •

edited

Loading

ofreyssinet-ledger commented Oct 13, 2022 •

edited

Loading

Add possibility to override feature flags with environment variable FEATURE_FLAGS #1372

Add possibility to override feature flags with environment variable FEATURE_FLAGS #1372

Conversation

ofreyssinet-ledger commented Sep 26, 2022 • edited Loading

📝 Description

❓ Context

✅ Checklist

📸 Demo

Compiling LLM with .env file in apps/ledger-live-mobile:

Running LLD like this

🚀 Expectations to reach

vercel bot commented Sep 26, 2022 • edited Loading

changeset-bot bot commented Sep 26, 2022 • edited Loading

🦋 Changeset detected

codecov bot commented Sep 26, 2022 • edited Loading

Codecov Report

github-actions bot commented Sep 26, 2022 • edited Loading

gre left a comment

Choose a reason for hiding this comment

gre Sep 27, 2022

Choose a reason for hiding this comment

ofreyssinet-ledger Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

gre Sep 27, 2022

Choose a reason for hiding this comment

ofreyssinet-ledger Sep 27, 2022

Choose a reason for hiding this comment

ofreyssinet-ledger Sep 28, 2022

Choose a reason for hiding this comment

github-actions bot commented Oct 6, 2022 • edited Loading

ofreyssinet-ledger commented Oct 13, 2022 • edited Loading

ofreyssinet-ledger commented Sep 26, 2022 •

edited

Loading

Compiling LLM with `.env` file in `apps/ledger-live-mobile`:

vercel bot commented Sep 26, 2022 •

edited

Loading

changeset-bot bot commented Sep 26, 2022 •

edited

Loading

codecov bot commented Sep 26, 2022 •

edited

Loading

github-actions bot commented Sep 26, 2022 •

edited

Loading

ofreyssinet-ledger Sep 27, 2022 •

edited

Loading

github-actions bot commented Oct 6, 2022 •

edited

Loading

ofreyssinet-ledger commented Oct 13, 2022 •

edited

Loading