refactor: flag resolver should use stricter types (#2571)

Adding stricter types to `FlagResolver` can possibly help improve our DX - Help us prevent errors like typos, guide us to correctly add a flag when needed, and warn us of stray checks whenever we do a clean up at a later stage.
Unleash · Dec 20, 2022 · 7ce5b3d · 7ce5b3d
1 parent 4b519ea
commit 7ce5b3d
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 96 deletions.
diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap
@@ -67,6 +67,7 @@ exports[`should create default config 1`] = `
       "isEnabled": [Function],
     },
     "flags": {
+      "E": false,
       "ENABLE_DARK_MODE_SUPPORT": false,
       "anonymiseEventLog": false,
       "batchMetrics": false,
@@ -83,6 +84,7 @@ exports[`should create default config 1`] = `
   },
   "flagResolver": FlagResolver {
     "experiments": {
+      "E": false,
       "ENABLE_DARK_MODE_SUPPORT": false,
       "anonymiseEventLog": false,
       "batchMetrics": false,

diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts
@@ -1,70 +1,61 @@
 import { parseEnvVarBoolean } from '../util';
 
-export type IFlags = Partial<Record<string, boolean>>;
+export type IFlags = Partial<typeof flags>;
+export type IFlagKey = keyof IFlags;
 
-export const defaultExperimentalOptions = {
-    flags: {
-        ENABLE_DARK_MODE_SUPPORT: false,
-        anonymiseEventLog: false,
-        embedProxy: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_EMBED_PROXY,
-            true,
-        ),
-        changeRequests: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_CHANGE_REQUESTS,
-            false,
-        ),
-        embedProxyFrontend: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_EMBED_PROXY_FRONTEND,
-            true,
-        ),
-        batchMetrics: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_BATCH_METRICS,
-            false,
-        ),
-        responseTimeWithAppName: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_RESPONSE_TIME_WITH_APP_NAME,
-            false,
-        ),
-        proxyReturnAllToggles: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_PROXY_RETURN_ALL_TOGGLES,
-            false,
-        ),
-        variantsPerEnvironment: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_VARIANTS_PER_ENVIRONMENT,
-            false,
-        ),
-        favorites: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_FAVORITES,
-            false,
-        ),
-        maintenance: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_MAINTENANCE,
-            false,
-        ),
-        networkView: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_NETWORK_VIEW,
-            false,
-        ),
-    },
+const flags = {
+    E: false,
+    ENABLE_DARK_MODE_SUPPORT: false,
+    anonymiseEventLog: false,
+    embedProxy: parseEnvVarBoolean(
+        process.env.UNLEASH_EXPERIMENTAL_EMBED_PROXY,
+        true,
+    ),
+    changeRequests: parseEnvVarBoolean(
+        process.env.UNLEASH_EXPERIMENTAL_CHANGE_REQUESTS,
+        false,
+    ),
+    embedProxyFrontend: parseEnvVarBoolean(
+        process.env.UNLEASH_EXPERIMENTAL_EMBED_PROXY_FRONTEND,
+        true,
+    ),
+    batchMetrics: parseEnvVarBoolean(
+        process.env.UNLEASH_EXPERIMENTAL_BATCH_METRICS,
+        false,
+    ),
+    responseTimeWithAppName: parseEnvVarBoolean(
+        process.env.UNLEASH_EXPERIMENTAL_RESPONSE_TIME_WITH_APP_NAME,
+        false,
+    ),
+    proxyReturnAllToggles: parseEnvVarBoolean(
+        process.env.UNLEASH_EXPERIMENTAL_PROXY_RETURN_ALL_TOGGLES,
+        false,
+    ),
+    variantsPerEnvironment: parseEnvVarBoolean(
+        process.env.UNLEASH_EXPERIMENTAL_VARIANTS_PER_ENVIRONMENT,
+        false,
+    ),
+    favorites: parseEnvVarBoolean(
+        process.env.UNLEASH_EXPERIMENTAL_FAVORITES,
+        false,
+    ),
+    networkView: parseEnvVarBoolean(
+        process.env.UNLEASH_EXPERIMENTAL_NETWORK_VIEW,
+        false,
+    ),
+    maintenance: parseEnvVarBoolean(
+        process.env.UNLEASH_EXPERIMENTAL_MAINTENANCE,
+        false,
+    ),
+};
+
+export const defaultExperimentalOptions: IExperimentalOptions = {
+    flags,
     externalResolver: { isEnabled: (): boolean => false },
 };
 
 export interface IExperimentalOptions {
-    flags: {
-        [key: string]: boolean;
-        ENABLE_DARK_MODE_SUPPORT?: boolean;
-        embedProxy?: boolean;
-        embedProxyFrontend?: boolean;
-        batchMetrics?: boolean;
-        anonymiseEventLog?: boolean;
-        changeRequests?: boolean;
-        proxyReturnAllToggles?: boolean;
-        variantsPerEnvironment?: boolean;
-        favorites?: boolean;
-        networkView?: boolean;
-        maintenance?: boolean;
-    };
+    flags: IFlags;
     externalResolver: IExternalFlagResolver;
 }
 
@@ -74,9 +65,9 @@ export interface IFlagContext {
 
 export interface IFlagResolver {
     getAll: (context?: IFlagContext) => IFlags;
-    isEnabled: (expName: string, context?: IFlagContext) => boolean;
+    isEnabled: (expName: IFlagKey, context?: IFlagContext) => boolean;
 }
 
 export interface IExternalFlagResolver {
-    isEnabled: (flagName: string, context?: IFlagContext) => boolean;
+    isEnabled: (flagName: IFlagKey, context?: IFlagContext) => boolean;
 }
diff --git a/src/lib/util/flag-resolver.test.ts b/src/lib/util/flag-resolver.test.ts
@@ -1,5 +1,6 @@
-import { defaultExperimentalOptions } from '../types/experimental';
+import { defaultExperimentalOptions, IFlagKey } from '../types/experimental';
 import FlagResolver from './flag-resolver';
+import { IExperimentalOptions } from '../types/experimental';
 
 test('should produce empty exposed flags', () => {
     const resolver = new FlagResolver(defaultExperimentalOptions);
@@ -14,8 +15,9 @@ test('should produce UI flags with extra dynamic flags', () => {
         ...defaultExperimentalOptions,
         flags: { extraFlag: false },
     };
-    const resolver = new FlagResolver(config);
-    const result = resolver.getAll();
+
+    const resolver = new FlagResolver(config as IExperimentalOptions);
+    const result = resolver.getAll() as typeof config.flags;
 
     expect(result.extraFlag).toBe(false);
 });
@@ -29,14 +31,14 @@ test('should use external resolver for dynamic flags', () => {
         },
     };
 
-    const resolver = new FlagResolver({
-        flags: {
-            extraFlag: false,
-        },
+    const config = {
+        flags: { extraFlag: false },
         externalResolver,
-    });
+    };
 
-    const result = resolver.getAll();
+    const resolver = new FlagResolver(config as IExperimentalOptions);
+
+    const result = resolver.getAll() as typeof config.flags;
 
     expect(result.extraFlag).toBe(true);
 });
@@ -48,15 +50,14 @@ test('should not use external resolver for enabled experiments', () => {
         },
     };
 
-    const resolver = new FlagResolver({
-        flags: {
-            should_be_enabled: true,
-            extraFlag: false,
-        },
+    const config = {
+        flags: { should_be_enabled: true, extraFlag: false },
         externalResolver,
-    });
+    };
 
-    const result = resolver.getAll();
+    const resolver = new FlagResolver(config as IExperimentalOptions);
+
+    const result = resolver.getAll() as typeof config.flags;
 
     expect(result.should_be_enabled).toBe(true);
 });
@@ -67,16 +68,16 @@ test('should load experimental flags', () => {
             return false;
         },
     };
-    const resolver = new FlagResolver({
-        flags: {
-            extraFlag: false,
-            someFlag: true,
-        },
+
+    const config = {
+        flags: { extraFlag: false, someFlag: true },
         externalResolver,
-    });
+    };
 
-    expect(resolver.isEnabled('someFlag')).toBe(true);
-    expect(resolver.isEnabled('extraFlag')).toBe(false);
+    const resolver = new FlagResolver(config as IExperimentalOptions);
+
+    expect(resolver.isEnabled('someFlag' as IFlagKey)).toBe(true);
+    expect(resolver.isEnabled('extraFlag' as IFlagKey)).toBe(false);
 });
 
 test('should load experimental flags from external provider', () => {
@@ -88,14 +89,13 @@ test('should load experimental flags from external provider', () => {
         },
     };
 
-    const resolver = new FlagResolver({
-        flags: {
-            extraFlag: false,
-            someFlag: true,
-        },
+    const config = {
+        flags: { extraFlag: false, someFlag: true },
         externalResolver,
-    });
+    };
+
+    const resolver = new FlagResolver(config as IExperimentalOptions);
 
-    expect(resolver.isEnabled('someFlag')).toBe(true);
-    expect(resolver.isEnabled('extraFlag')).toBe(true);
+    expect(resolver.isEnabled('someFlag' as IFlagKey)).toBe(true);
+    expect(resolver.isEnabled('extraFlag' as IFlagKey)).toBe(true);
 });
diff --git a/src/lib/util/flag-resolver.ts b/src/lib/util/flag-resolver.ts
@@ -4,6 +4,7 @@ import {
     IFlagContext,
     IFlags,
     IFlagResolver,
+    IFlagKey,
 } from '../types/experimental';
 export default class FlagResolver implements IFlagResolver {
     private experiments: IFlags;
@@ -18,7 +19,7 @@ export default class FlagResolver implements IFlagResolver {
     getAll(context?: IFlagContext): IFlags {
         const flags: IFlags = { ...this.experiments };
 
-        Object.keys(flags).forEach((flagName) => {
+        Object.keys(flags).forEach((flagName: IFlagKey) => {
             if (!this.experiments[flagName])
                 flags[flagName] = this.externalResolver.isEnabled(
                     flagName,
@@ -29,7 +30,7 @@ export default class FlagResolver implements IFlagResolver {
         return flags;
     }
 
-    isEnabled(expName: string, context?: IFlagContext): boolean {
+    isEnabled(expName: IFlagKey, context?: IFlagContext): boolean {
         if (this.experiments[expName]) {
             return true;
         }