Repository: netlify/cli
Pull request #5792 (2023-06-12 20:02:18)

feat: add support for feature flags
🎉 Thanks for submitting a pull request! 🎉

#### Summary

Fixes https://github.com/netlify/pod-dev-foundations/issues/519

Along with [bitballoon](https://github.com/netlify/bitballoon/pull/15495) and [open-api](https://github.com/netlify/open-api/pull/466) changes, this completes the addition of feature flag support in the cli. 

Currently feature_flags are enabled on staging, so you can test this PR by running the cli with `NETLIFY_API_URL` set to staging, and using the `isFeatureFlagEnabled` function in any command.

By passing `feature_flags: 'cli'` to the `api.getSite` call, we are requesting a list of feature flags that are prefixed with `cli`. No other feature flags will be available.

---

For us to review and ship your PR efficiently, please perform the following steps:

- [ ] Open a [bug/issue](https://github.com/netlify/cli/issues/new/choose) before writing your code 🧑‍💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you\`re fixing a typo or something that\`s on fire 🔥 (e.g. incident related), you can skip this step.
- [ ] Read the [contribution guidelines](../CONTRIBUTING.md) 📖. This ensures your code follows our style guide and
      passes our tests.
- [ ] Update or add tests (if any source code was changed or added) 🧪
- [ ] Update or add documentation (if features were changed or added) 📝
- [ ] Make sure the status checks below are successful ✅

**A picture of a cute animal (not mandatory, but encouraged)**
<img width="330" alt="Screenshot 2023-06-12 at 4 01 51 PM" src="https://github.com/netlify/cli/assets/4700602/5777c45f-192a-4f10-9dc6-b2242b142afd">


github-actions[bot]: ## 📊 Benchmark results

*Comparing with 125512c77312a79b350028222e0775efd1744a78*

- **Dependency count**: 1,222 (no change)
- **Package size**: 232 MB ⬇️ **0.01% decrease** vs. 125512c77312a79b350028222e0775efd1744a78
<!--delta:post-package-size@{}-->
eduardoboucas: > By passing `feature_flags: 'cli'` to the `api.getSite` call, we are requesting a list of feature flags that are prefixed with `cli`.

This isn't currently part of this PR. Will you add it, or does that live somewhere else?
sarahetter: @eduardoboucas Thanks for the review!

> > By passing `feature_flags: 'cli'` to the `api.getSite` call, we are requesting a list of feature flags that are prefixed with `cli`.
> 
> This isn't currently part of this PR. Will you add it, or does that live somewhere else?

Ah I was referring to what I'm doing on line 16 in feature-flags.mjs - `const siteInfo = await api.getSite({ site_id: siteId, feature_flags: 'cli' })`

I thought the `feature_flags: 'cli'` bit was requesting `cli` prefixed flags from bitballoon? Or am I mistaken?
eduardoboucas: > Ah I was referring to what I'm doing on line 16 in feature-flags.mjs - `const siteInfo = await api.getSite({ site_id: siteId, feature_flags: 'cli' })`
> 
> I thought the `feature_flags: 'cli'` bit was requesting `cli` prefixed flags from bitballoon? Or am I mistaken?

Doh, you're right! I missed this somehow, sorry.
danez: > By passing feature_flags: 'cli' to the api.getSite call, we are requesting a list of feature flags that are prefixed with cli

prefixed? in like `cli-my-ff`? Couldn't we use the CLI environment from devcycle? Because if they need to be prefixed they won't help in enabling feature flags that are used in edge-bundler order zip-it-and-ship-it.

why are we doing separate `getSite` call and do not use the ones we already do on every CLI invocation? Wasn't the idea to smuggle feature_flags into `getSite` so we do not have a separate API call?

Sorry I might be a little late with these questions.


sarahetter: > prefixed? in like cli-my-ff? 

These decisions were made in the bitballoon PR, I'm not sure about the answer. 

> why are we doing a separate getSite call and do not use the ones we already do on every CLI invocation?

I thought about doing this, but it meant making changes in @netlify/config (allowing passing of a `featureFlagPrefix` but felt that it meant adding complication to another repo, as well as more difficult testing. I can switch to this if we think it's the better solution


eduardoboucas: > why are we doing a separate `getSite` call and do not use the ones we already do on every CLI invocation? Wasn't the idea to smuggle feature_flags into `getSite` so we do not have a separate API call?

Yeah, that's a good point and it was indeed the intention.

@sarahetter if we could make that happen without too much complexity, I think it's worth a try.

Patch:
diff --git a/src/commands/base-command.mjs b/src/commands/base-command.mjs
index 7101dd5b..2e131564 100644
--- a/src/commands/base-command.mjs
+++ b/src/commands/base-command.mjs
@@ -527,6 +527,7 @@ export default class BaseCommand extends Command {
         pathPrefix,
         scheme,
         offline,
+        siteFeatureFlagPrefix: 'cli',
       })
     } catch (error_) {
       const isUserError = error_.customErrorInfo !== undefined && error_.customErrorInfo.type === 'resolveConfig'
diff --git a/src/utils/feature-flags.mjs b/src/utils/feature-flags.mjs
new file mode 100644
index 00000000..827741c5
--- /dev/null
+++ b/src/utils/feature-flags.mjs
@@ -0,0 +1,19 @@
+/**
+ * Allows us to check if a feature flag is enabled for a site.
+ * Due to versioning of the cli, and the desire to remove flags from
+ * our feature flag service when they should always evaluate to true,
+ * we can't just look for the presense of {featureFlagName: true}, as
+ * the absense of a flag should also evaluate to the flag being enabled.
+ * Instead, we return that the feature flag is enabled if it isn't
+ * specifically set to false in the response
+ * @param {*} siteInfo
+ * @param {string} flagName
+ *
+ * @returns {boolean}
+ */
+export const isFeatureFlagEnabled = (flagName, siteInfo) => {
+  if (siteInfo.feature_flags && siteInfo.feature_flags[flagName] !== false) {
+    return true
+  }
+  return false
+}


Test patch:
diff --git a/tests/unit/utils/feature-flags.test.mjs b/tests/unit/utils/feature-flags.test.mjs
new file mode 100644
index 00000000..54923da2
--- /dev/null
+++ b/tests/unit/utils/feature-flags.test.mjs
@@ -0,0 +1,83 @@
+import { describe, expect, test } from 'vitest'
+
+import { isFeatureFlagEnabled } from '../../../src/utils/feature-flags.mjs'
+
+describe('isFeatureFlagEnabled', () => {
+  test('should return true if feature flag is not present', async () => {
+    const siteInfo = {
+      feature_flags: {
+        cool_new_feature: true,
+        amazing_feature: false,
+      },
+    }
+
+    const result = isFeatureFlagEnabled('netlify_feature', siteInfo)
+
+    expect(result).toBe(true)
+  })
+
+  test('should return true if feature flag is true', async () => {
+    const siteInfo = {
+      feature_flags: {
+        cool_new_feature: true,
+        amazing_feature: false,
+      },
+    }
+
+    const result = isFeatureFlagEnabled('cool_new_feature', siteInfo)
+
+    expect(result).toBe(true)
+  })
+
+  test('should return true if feature flag is a string', async () => {
+    const siteInfo = {
+      feature_flags: {
+        cool_new_feature: 'my string',
+        amazing_feature: false,
+      },
+    }
+
+    const result = isFeatureFlagEnabled('cool_new_feature', siteInfo)
+
+    expect(result).toBe(true)
+  })
+
+  test('should return true if feature flag is a number', async () => {
+    const siteInfo = {
+      feature_flags: {
+        cool_new_feature: 42,
+        amazing_feature: false,
+      },
+    }
+
+    const result = isFeatureFlagEnabled('cool_new_feature', siteInfo)
+
+    expect(result).toBe(true)
+  })
+
+  test('should return true if feature flag is an object', async () => {
+    const siteInfo = {
+      feature_flags: {
+        cool_new_feature: { key: 'value' },
+        amazing_feature: false,
+      },
+    }
+
+    const result = isFeatureFlagEnabled('cool_new_feature', siteInfo)
+
+    expect(result).toBe(true)
+  })
+
+  test('should return false if feature flag is false', async () => {
+    const siteInfo = {
+      feature_flags: {
+        cool_new_feature: true,
+        amazing_feature: false,
+      },
+    }
+
+    const result = isFeatureFlagEnabled('amazing_feature', siteInfo)
+
+    expect(result).toBe(false)
+  })
+})