From 2588b6c16bfef72b0a15deeae2bc3a6fcce366f0 Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Tue, 6 Oct 2026 09:16:19 +0200 Subject: [PATCH] feat: expose original feature name on flags, fix analytics casing Flag keys in `getAllFlags()` are lower-cased with spaces replaced, and the feature's original name was discarded at ingest. Consumers who want a map keyed by their real feature names - e.g. to hold every flag in app state rather than reading them one at a time - had no way to recover it. Each flag now carries the feature's own name as `name`: const flagsByName = Object.fromEntries( Object.entries(flagsmith.getAllFlags()).map(([key, flag]) => [flag.name ?? key, flag]) ) This is purely additive - flag keys are unchanged. `name` is absent on flags supplied via `defaultFlags` and on flags restored from a cache written by an older SDK, hence the `?? key` fallback. Also fixes analytics being recorded against the caller's casing rather than the feature's name. The API resolves analytics with a case-sensitive match on Feature.name, so getValue('MyFlag') and getValue('myflag') were reported as two separate features and neither resolved unless it matched exactly - meaning flags in projects that don't use lower-case names never registered as in use. evaluateFlag, the Sentry addFeatureFlag call and trackExposureEvent now all report the resolved flag's name. Co-Authored-By: Claude Opus 5 (1M context) --- flagsmith-core.ts | 41 +++++++---- test/feature-name-casing.test.ts | 105 +++++++++++++++++++++++++++ test/react.test.tsx | 14 ++-- test/test-constants.ts | 18 +++-- test/test-utils/remove-ids.ts | 17 ----- test/test-utils/to-rendered-flags.ts | 22 ++++++ types.d.ts | 15 ++++ utils/version.ts | 2 +- 8 files changed, 187 insertions(+), 47 deletions(-) create mode 100644 test/feature-name-casing.test.ts delete mode 100644 test/test-utils/remove-ids.ts create mode 100644 test/test-utils/to-rendered-flags.ts diff --git a/flagsmith-core.ts b/flagsmith-core.ts index 0512de32..c2af152c 100644 --- a/flagsmith-core.ts +++ b/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,7 +823,7 @@ 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 @@ -826,13 +831,13 @@ const Flagsmith = class { 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 => 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; diff --git a/test/feature-name-casing.test.ts b/test/feature-name-casing.test.ts new file mode 100644 index 00000000..de8240ac --- /dev/null +++ b/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 = {}) { + 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) => + 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 }); + }); +}); diff --git a/test/react.test.tsx b/test/react.test.tsx index f67d1e65..0237a8e8 100644 --- a/test/react.test.tsx +++ b/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> = () => { 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(() => { diff --git a/test/test-constants.ts b/test/test-constants.ts index 7c91dfcf..bb5d6fbf 100644 --- a/test/test-constants.ts +++ b/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 = { diff --git a/test/test-utils/remove-ids.ts b/test/test-utils/remove-ids.ts deleted file mode 100644 index c9d11fb8..00000000 --- a/test/test-utils/remove-ids.ts +++ /dev/null @@ -1,17 +0,0 @@ -export default function removeIds(obj: Record) { - if (typeof obj !== 'object' || obj === null) { - return obj; - } - - const newObj:Record = {}; - - for (const key in obj) { - if (Object.prototype.hasOwnProperty.call(obj, key)) { - if (key !== 'id') { - newObj[key] = removeIds(obj[key]); - } - } - } - - return newObj; -} diff --git a/test/test-utils/to-rendered-flags.ts b/test/test-utils/to-rendered-flags.ts new file mode 100644 index 00000000..c0b1231e --- /dev/null +++ b/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) { + if (typeof obj !== 'object' || obj === null) { + return obj; + } + + const newObj: Record = {}; + + 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; +} diff --git a/types.d.ts b/types.d.ts index 88ee8f57..0fc3318d 100644 --- a/types.d.ts +++ b/types.d.ts @@ -20,6 +20,21 @@ export interface IFlagsmithExperiment { export interface IFlagsmithFeature { 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; diff --git a/utils/version.ts b/utils/version.ts index d22ac37c..93d83540 100644 --- a/utils/version.ts +++ b/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";