feat: expose original feature name on flags, fix analytics casing

flagsmith-core.ts

@@ -68,6 +68,10 @@ const FLAGSMITH_CONFIG_ANALYTICS_KEY = "flagsmith_value_";
 const FLAGSMITH_FLAG_ANALYTICS_KEY = "flagsmith_enabled_";
 const FLAGSMITH_TRAIT_ANALYTICS_KEY = "flagsmith_trait_";
 
+// Flags are stored lower-cased with spaces replaced, so that callers can look a flag
+// up by any casing. The feature's original name is kept on the flag itself as `name`.
+const normalizeFlagKey = (key: string) => key.toLowerCase().replace(/ /g, '_');
+
 const Flagsmith = class {
     _trigger?:(()=>void)|null= null
     _triggerLoadingState?:(()=>void)|null= null
@@ -120,8 +124,9 @@ const Flagsmith = class {
             traits = traits || [];
             features.forEach(feature => {
                 const experiment = feature.metadata?.experiment;
-                flags[feature.feature.name.toLowerCase().replace(/ /g, '_')] = {
+                flags[normalizeFlagKey(feature.feature.name)] = {
                     id: feature.feature.id,
+                    name: feature.feature.name,
                     enabled: feature.enabled,
                     value: feature.feature_state_value,
                     ...(feature.variant ? { variant: feature.variant } : {}),
@@ -694,14 +699,14 @@ const Flagsmith = class {
     }
 
     getValue = (key: string, options?: GetValueOptions, skipAnalytics?: boolean) => {
-        const flag = this.flags && this.flags[key.toLowerCase().replace(/ /g, '_')];
+        const flag = this.flags && this.flags[normalizeFlagKey(key)];
         let res = null;
         if (flag) {
             res = flag.value;
         }
 
         if (!options?.skipAnalytics && !skipAnalytics) {
-            this.evaluateFlag(key, "VALUE");
+            this.evaluateFlag(key, "VALUE", flag);
         }
 
         if (res === null && typeof options?.fallback !== 'undefined') {
@@ -818,21 +823,21 @@ const Flagsmith = class {
     hasFeature = (key: string, options?: HasFeatureOptions) => {
         // Support legacy skipAnalytics boolean parameter
         const usingNewOptions = typeof options === 'object'
-        const flag = this.flags && this.flags[key.toLowerCase().replace(/ /g, '_')];
+        const flag = this.flags && this.flags[normalizeFlagKey(key)];
         let res = false;
         if (!flag && usingNewOptions && typeof options.fallback !== 'undefined') {
             res = options?.fallback
         } else if (flag && flag.enabled) {
             res = true;
         }
         if ((usingNewOptions && !options.skipAnalytics) || !options) {
-            this.evaluateFlag(key, "ENABLED");
+            this.evaluateFlag(key, "ENABLED", flag);
         }
         if(this.sentryClient) {
           try {
               this.sentryClient.getIntegrationByName(
                   "FeatureFlags",
-              )?.addFeatureFlag?.(key, res);
+              )?.addFeatureFlag?.(flag?.name || normalizeFlagKey(key), res);
           } catch (e) {
               console.error(e)
           }
@@ -947,15 +952,20 @@ const Flagsmith = class {
         }
     }
 
-    private evaluateFlag =(key: string, method: 'VALUE' | 'ENABLED') => {
+    private evaluateFlag =(key: string, method: 'VALUE' | 'ENABLED', flag?: IFlagsmithFeature | null) => {
+        // The API resolves analytics by exact feature name, so always report the
+        // feature's own name rather than whatever casing the caller happened to use.
+        // Without this, getValue('My Flag') and getValue('my_flag') report as two
+        // separate features and neither resolves unless it matches the name exactly.
+        const analyticsKey = flag?.name || normalizeFlagKey(key);
         if (this.datadogRum) {
             if (!this.datadogRum!.client!.addFeatureFlagEvaluation) {
                 console.error('Flagsmith: Your datadog RUM client does not support the function addFeatureFlagEvaluation, please update it.');
             } else {
                 if (method === 'VALUE') {
-                    this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_CONFIG_ANALYTICS_KEY + key, this.getValue(key, {}, true));
+                    this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_CONFIG_ANALYTICS_KEY + analyticsKey, this.getValue(key, {}, true));
                 } else {
-                    this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_FLAG_ANALYTICS_KEY + key, this.hasFeature(key, true));
+                    this.datadogRum!.client!.addFeatureFlagEvaluation(FLAGSMITH_FLAG_ANALYTICS_KEY + analyticsKey, this.hasFeature(key, true));
                 }
             }
         }
@@ -965,10 +975,10 @@ const Flagsmith = class {
             if (!this.evaluationEvent[this.evaluationContext.environment.apiKey]) {
                 this.evaluationEvent[this.evaluationContext.environment.apiKey] = {};
             }
-            if (this.evaluationEvent[this.evaluationContext.environment.apiKey][key] === undefined) {
-                this.evaluationEvent[this.evaluationContext.environment.apiKey][key] = 0;
+            if (this.evaluationEvent[this.evaluationContext.environment.apiKey][analyticsKey] === undefined) {
+                this.evaluationEvent[this.evaluationContext.environment.apiKey][analyticsKey] = 0;
             }
-            this.evaluationEvent[this.evaluationContext.environment.apiKey][key] += 1;
+            this.evaluationEvent[this.evaluationContext.environment.apiKey][analyticsKey] += 1;
         }
 
         this.updateEventStorage();
@@ -1007,8 +1017,11 @@ const Flagsmith = class {
             this.log(`Flagsmith: trackExposureEvent called for "${featureName}" without an identity; call identify() (optionally with transient: true) or pass opts.identifier. No exposure recorded.`);
             return;
         }
+        // As with evaluateFlag, report the feature's own name so that exposures for the
+        // same flag aggregate regardless of the casing the caller used.
+        const key = normalizeFlagKey(featureName);
         this.eventProcessor.trackExposureEvent({
-            featureName,
+            featureName: this.flags?.[key]?.name || key,
             identifier,
             value: opts?.value ?? null,
             traits: resolveTraitValues(opts?.traits ?? this.evaluationContext.identity?.traits),
@@ -1019,7 +1032,7 @@ const Flagsmith = class {
     flushEvents = (): Promise<void> => this.eventProcessor ? this.eventProcessor.flush() : Promise.resolve();
 
     getExperimentFlag = (featureName: string): IFlagsmithFeature | null => {
-        const key = featureName.toLowerCase().replace(/ /g, '_');
+        const key = normalizeFlagKey(featureName);
         const flag = (this.flags && this.flags[key]) || null;
         // When events are disabled this degrades to a plain flag read.
         if (!this.eventProcessor) return flag;

test/feature-name-casing.test.ts

@@ -0,0 +1,105 @@
+import { IInitConfig } from '../types';
+import { delay, getFlagsmith } from './test-constants';
+
+const mixedCaseFlags = [
+    {
+        feature: { id: 1, name: 'MyFeatureFlag', type: 'STANDARD' },
+        enabled: true,
+        feature_state_value: 'on',
+    },
+    {
+        feature: { id: 2, name: 'another_flag', type: 'STANDARD' },
+        enabled: false,
+        feature_state_value: null,
+    },
+];
+
+const instances: any[] = [];
+
+afterEach(() => {
+    // enableAnalytics starts an interval that nothing public tears down.
+    instances.forEach((flagsmith) => clearInterval(flagsmith.analyticsInterval));
+    instances.length = 0;
+});
+
+async function initWithMixedCaseFlags(config: Partial<IInitConfig> = {}) {
+    const { flagsmith, initConfig, mockFetch } = getFlagsmith(config);
+    mockFetch.mockImplementation(async (url: string) => {
+        if (url.includes('analytics/flags')) {
+            return { status: 200, text: () => Promise.resolve('{}') };
+        }
+        if (url.includes('/flags/')) {
+            return { status: 200, text: () => Promise.resolve(JSON.stringify(mixedCaseFlags)) };
+        }
+        throw new Error('Please mock the call to ' + url);
+    });
+    await flagsmith.init(initConfig);
+    instances.push(flagsmith);
+    return { flagsmith, mockFetch };
+}
+
+const postedAnalytics = (mockFetch: jest.Mock) => {
+    const call = mockFetch.mock.calls.find(([url]: [string]) => url.includes('analytics/flags'));
+    return call ? JSON.parse(call[1].body) : null;
+};
+
+const byName = (flags: Record<string, any>) =>
+    Object.fromEntries(Object.entries(flags).map(([key, flag]) => [flag.name ?? key, flag]));
+
+describe('feature name casing', () => {
+    test('exposes the original feature name on each flag', async () => {
+        const { flagsmith } = await initWithMixedCaseFlags();
+        expect(flagsmith.getAllFlags()).toEqual({
+            myfeatureflag: { id: 1, name: 'MyFeatureFlag', enabled: true, value: 'on' },
+            another_flag: { id: 2, name: 'another_flag', enabled: false, value: null },
+        });
+    });
+
+    test('getAllFlags can be re-keyed by original feature name', async () => {
+        const { flagsmith } = await initWithMixedCaseFlags();
+        const flagsByName = byName(flagsmith.getAllFlags());
+        expect(Object.keys(flagsByName).sort()).toEqual(['MyFeatureFlag', 'another_flag']);
+        expect(flagsByName.MyFeatureFlag.value).toBe('on');
+    });
+
+    test('lookups still resolve regardless of the casing the caller uses', async () => {
+        const { flagsmith } = await initWithMixedCaseFlags();
+        expect(flagsmith.getValue('MyFeatureFlag')).toBe('on');
+        expect(flagsmith.getValue('myfeatureflag')).toBe('on');
+        expect(flagsmith.hasFeature('MYFEATUREFLAG')).toBe(true);
+    });
+
+    test('falls back to the flag key when name is absent, as with defaultFlags', async () => {
+        const { flagsmith, initConfig } = getFlagsmith({
+            preventFetch: true,
+            defaultFlags: { my_default: { enabled: true, value: 1 } },
+        });
+        await flagsmith.init(initConfig);
+        expect(byName(flagsmith.getAllFlags()).my_default).toEqual({ enabled: true, value: 1 });
+    });
+
+    test('records analytics against the canonical feature name, whatever casing the caller used', async () => {
+        const { flagsmith, mockFetch } = await initWithMixedCaseFlags({ enableAnalytics: true });
+        await delay(1); // evaluationEvent is restored from storage asynchronously
+
+        flagsmith.getValue('myfeatureflag');
+        flagsmith.getValue('MyFeatureFlag');
+        flagsmith.hasFeature('MYFEATUREFLAG');
+
+        // @ts-ignore internal, normally driven by an interval
+        await flagsmith.analyticsFlags();
+        expect(postedAnalytics(mockFetch)).toEqual({ MyFeatureFlag: 3 });
+    });
+
+    test('buckets unknown flags under the normalised key rather than the caller casing', async () => {
+        const { flagsmith, mockFetch } = await initWithMixedCaseFlags({ enableAnalytics: true });
+        await delay(1);
+
+        flagsmith.getValue('Does Not Exist');
+        flagsmith.getValue('does_not_exist');
+
+        // @ts-ignore internal, normally driven by an interval
+        await flagsmith.analyticsFlags();
+        expect(postedAnalytics(mockFetch)).toEqual({ does_not_exist: 2 });
+    });
+});

test/react.test.tsx

@@ -10,7 +10,7 @@ import {
     identityState,
     testIdentity,
 } from './test-constants'
-import removeIds from './test-utils/remove-ids'
+import toRenderedFlags from './test-utils/to-rendered-flags'
 
 const FlagsmithPage: FC<Record<string, never>> = () => {
     const flags = useFlags(Object.keys(defaultState.flags))
@@ -74,7 +74,7 @@ describe('FlagsmithProvider', () => {
                 error: null,
                 source: 'SERVER',
             })
-            expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(removeIds(defaultState.flags))
+            expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(toRenderedFlags(defaultState.flags))
         })
     })
     it('fetches and renders flags for an identified user', async () => {
@@ -94,7 +94,7 @@ describe('FlagsmithProvider', () => {
                 error: null,
                 source: 'SERVER',
             })
-            expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(removeIds(identityState.flags))
+            expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(toRenderedFlags(identityState.flags))
         })
     })
     it('renders cached flags', async () => {
@@ -124,7 +124,7 @@ describe('FlagsmithProvider', () => {
                 error: null,
                 source: 'CACHE',
             })
-            expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(removeIds(defaultState.flags))
+            expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(toRenderedFlags(defaultState.flags))
         })
     })
 
@@ -159,7 +159,7 @@ describe('FlagsmithProvider', () => {
                 error: null,
                 source: 'CACHE',
             })
-            expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(removeIds(defaultState.flags))
+            expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(toRenderedFlags(defaultState.flags))
         })
     })
 
@@ -183,7 +183,7 @@ describe('FlagsmithProvider', () => {
                 error: null,
                 source: 'DEFAULT_FLAGS',
             })
-            expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(removeIds(defaultState.flags))
+            expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(toRenderedFlags(defaultState.flags))
         })
     })
     it('reports a loaded state when hydrated from serverState with no options', async () => {
@@ -195,7 +195,7 @@ describe('FlagsmithProvider', () => {
         )
 
         // Flags are available from serverState immediately.
-        expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(removeIds(defaultState.flags))
+        expect(JSON.parse(screen.getByTestId('flags').innerHTML)).toEqual(toRenderedFlags(defaultState.flags))
 
         // Loading state must not be stuck on isLoading/isFetching=true.
         await waitFor(() => {

test/test-constants.ts

@@ -14,13 +14,14 @@ export const defaultState = {
     flags: {
         hero: {
             id: 1804,
+            name: 'hero',
             enabled: true,
             value: 'https://s3-us-west-2.amazonaws.com/com.uppercut.hero-images/assets/0466/comps/466_03314.jpg',
         },
-        font_size: { id: 6149, enabled: true, value: 16 },
-        json_value: { id: 80317, enabled: true, value: '{"title":"Hello World"}' },
-        number_value: { id: 80318, enabled: true, value: 1 },
-        off_value: { id: 80319, enabled: false, value: null },
+        font_size: { id: 6149, name: 'font_size', enabled: true, value: 16 },
+        json_value: { id: 80317, name: 'json_value', enabled: true, value: '{"title":"Hello World"}' },
+        number_value: { id: 80318, name: 'number_value', enabled: true, value: 1 },
+        off_value: { id: 80319, name: 'off_value', enabled: false, value: null },
     },
 };
 
@@ -42,13 +43,14 @@ export const identityState = {
     flags: {
         hero: {
             id: 1804,
+            name: 'hero',
             enabled: true,
             value: 'https://s3-us-west-2.amazonaws.com/com.uppercut.hero-images/assets/0466/comps/466_03314.jpg'
         },
-        font_size: { id: 6149, enabled: true, value: 16 },
-        json_value: { id: 80317, enabled: true, value: '{"title":"Hello World"}' },
-        number_value: { id: 80318, enabled: true, value: 1 },
-        off_value: { id: 80319, enabled: false, value: null },
+        font_size: { id: 6149, name: 'font_size', enabled: true, value: 16 },
+        json_value: { id: 80317, name: 'json_value', enabled: true, value: '{"title":"Hello World"}' },
+        number_value: { id: 80318, name: 'number_value', enabled: true, value: 1 },
+        off_value: { id: 80319, name: 'off_value', enabled: false, value: null },
     },
 };
 export const defaultStateAlt = {

test/test-utils/to-rendered-flags.ts

@@ -0,0 +1,22 @@
+// `useFlags` projects a subset of each stored flag (see UseFlagsReturn in react.tsx):
+// `id` and `name` are internal to the flag store and are not rendered. Strip them so
+// state fixtures can be compared against what the hook actually returns.
+const NOT_PROJECTED_BY_USE_FLAGS = ['id', 'name'];
+
+export default function toRenderedFlags(obj: Record<string, any>) {
+    if (typeof obj !== 'object' || obj === null) {
+        return obj;
+    }
+
+    const newObj: Record<string, any> = {};
+
+    for (const key in obj) {
+        if (Object.prototype.hasOwnProperty.call(obj, key)) {
+            if (!NOT_PROJECTED_BY_USE_FLAGS.includes(key)) {
+                newObj[key] = toRenderedFlags(obj[key]);
+            }
+        }
+    }
+
+    return newObj;
+}

types.d.ts

@@ -20,6 +20,21 @@ export interface IFlagsmithExperiment {
 
 export interface IFlagsmithFeature<Value = IFlagsmithValue> {
     id?: number;
+    /**
+     * The feature name exactly as it is defined in Flagsmith, preserving its original
+     * casing. Flag keys in {@link IFlags} are lower-cased, so use this to rebuild a map
+     * keyed by the real feature names:
+     *
+     * ```ts
+     * const flagsByName = Object.fromEntries(
+     *     Object.entries(flagsmith.getAllFlags()).map(([key, flag]) => [flag.name ?? key, flag])
+     * );
+     * ```
+     *
+     * Absent on flags supplied via `defaultFlags` and on flags restored from a cache
+     * written by an older version of this SDK, hence the `?? key` fallback.
+     */
+    name?: string;
     enabled: boolean;
     value: Value;
     variant?: string;

utils/version.ts

@@ -1,2 +1,2 @@
 // Auto-generated by write-version.js
-export const SDK_VERSION = "11.0.0";
+export const SDK_VERSION = "12.4.0";