From c14f108d4d2c2718e9df6d1a89d4c6e51fce5524 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Mon, 12 Aug 2024 09:58:44 +0100
Subject: [PATCH] Respond to PR comments.

---
 packages/backend-core/src/features/index.ts | 53 ++++++++++-----------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/packages/backend-core/src/features/index.ts b/packages/backend-core/src/features/index.ts
index cce407bfb0..25c9b260d8 100644
--- a/packages/backend-core/src/features/index.ts
+++ b/packages/backend-core/src/features/index.ts
@@ -83,22 +83,18 @@ class NumberFlag extends Flag<number> {
 }
 
 export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
-  private readonly flags: T
-
-  constructor(flags: T) {
-    this.flags = flags
-  }
+  constructor(private readonly flagSchema: T) {}
 
   defaults(): FlagValues<T> {
-    return Object.keys(this.flags).reduce((acc, key) => {
+    return Object.keys(this.flagSchema).reduce((acc, key) => {
       const typedKey = key as keyof T
-      acc[typedKey] = this.flags[key].defaultValue
+      acc[typedKey] = this.flagSchema[key].defaultValue
       return acc
     }, {} as FlagValues<T>)
   }
 
   isFlagName(name: string | number | symbol): name is keyof T {
-    return this.flags[name as keyof T] !== undefined
+    return this.flagSchema[name as keyof T] !== undefined
   }
 
   async get<K extends keyof T>(
@@ -120,9 +116,7 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
   async fetch(ctx?: UserCtx): Promise<FlagValues<T>> {
     return await tracer.trace("features.fetch", async span => {
       const tags: Record<string, any> = {}
-
-      const flags = this.defaults()
-
+      const flagValues = this.defaults()
       const currentTenantId = context.getTenantId()
       const specificallySetFalse = new Set<string>()
 
@@ -146,12 +140,13 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
             throw new Error(`Feature: ${feature} is not an allowed option`)
           }
 
-          if (typeof flags[feature] !== "boolean") {
+          if (typeof flagValues[feature] !== "boolean") {
             throw new Error(`Feature: ${feature} is not a boolean`)
           }
 
-          // @ts-ignore
-          flags[feature] = value
+          // @ts-expect-error - TS does not like you writing into a generic type,
+          // but we know that it's okay in this case because it's just an object.
+          flagValues[feature] = value
           tags[`flags.${feature}.source`] = "environment"
         }
       }
@@ -159,19 +154,22 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
       const license = ctx?.user?.license
       if (license) {
         for (const feature of license.features) {
-          const flag = this.flags[feature]
-          if (!flag) {
+          if (!this.isFlagName(feature)) {
             continue
           }
 
-          if (flags[feature] === true || specificallySetFalse.has(feature)) {
+          if (
+            flagValues[feature] === true ||
+            specificallySetFalse.has(feature)
+          ) {
             // If the flag is already set to through environment variables, we
             // don't want to override it back to false here.
             continue
           }
 
-          // @ts-ignore
-          flags[feature] = true
+          // @ts-expect-error - TS does not like you writing into a generic type,
+          // but we know that it's okay in this case because it's just an object.
+          flagValues[feature] = true
           tags[`flags.${feature}.source`] = "license"
         }
       }
@@ -180,25 +178,26 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
       if (posthog && identity?.type === IdentityType.USER) {
         const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id)
         for (const [name, value] of Object.entries(posthogFlags.featureFlags)) {
-          const flag = this.flags[name]
-          if (!flag) {
+          if (!this.isFlagName(name)) {
             // We don't want an unexpected PostHog flag to break the app, so we
             // just log it and continue.
             console.warn(`Unexpected posthog flag "${name}": ${value}`)
             continue
           }
 
-          if (flags[name] === true || specificallySetFalse.has(name)) {
+          if (flagValues[name] === true || specificallySetFalse.has(name)) {
             // If the flag is already set to through environment variables, we
             // don't want to override it back to false here.
             continue
           }
 
           const payload = posthogFlags.featureFlagPayloads?.[name]
-
+          const flag = this.flagSchema[name]
           try {
-            // @ts-ignore
-            flags[name] = flag.parse(payload || value)
+            // @ts-expect-error - TS does not like you writing into a generic
+            // type, but we know that it's okay in this case because it's just
+            // an object.
+            flagValues[name] = flag.parse(payload || value)
             tags[`flags.${name}.source`] = "posthog"
           } catch (err) {
             // We don't want an invalid PostHog flag to break the app, so we just
@@ -208,12 +207,12 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
         }
       }
 
-      for (const [key, value] of Object.entries(flags)) {
+      for (const [key, value] of Object.entries(flagValues)) {
         tags[`flags.${key}.value`] = value
       }
       span?.addTags(tags)
 
-      return flags
+      return flagValues
     })
   }
 }