diff --git a/flagsmith-core.ts b/flagsmith-core.ts index 0512de32..fc5fce60 100644 --- a/flagsmith-core.ts +++ b/flagsmith-core.ts @@ -68,6 +68,8 @@ const FLAGSMITH_CONFIG_ANALYTICS_KEY = "flagsmith_value_"; const FLAGSMITH_FLAG_ANALYTICS_KEY = "flagsmith_enabled_"; const FLAGSMITH_TRAIT_ANALYTICS_KEY = "flagsmith_trait_"; +const normalizeFlagKey = (key: string) => key.toLowerCase().replace(/ /g, '_'); + const Flagsmith = class { _trigger?:(()=>void)|null= null _triggerLoadingState?:(()=>void)|null= null @@ -120,8 +122,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,7 +697,7 @@ 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; @@ -818,7 +821,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 @@ -1019,7 +1022,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..d55eade3 --- /dev/null +++ b/test/feature-name-casing.test.ts @@ -0,0 +1,47 @@ +import { IInitConfig } from '../types'; +import { 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, + }, +]; + +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); + return { flagsmith, mockFetch }; +} + +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('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); + }); +}); 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;