Skip to content

Commit 4551809

Browse files
committed
fix: re-fire useExperiment exposure on identity change; tighten reserved event guard
1 parent 7f15e1e commit 4551809

5 files changed

Lines changed: 42 additions & 5 deletions

File tree

event-processor.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ export class EventProcessor {
113113
};
114114

115115
start() {
116+
// stop() clears any existing timer and flushes; the flush is a no-op
117+
// when the buffer is empty, which is the usual case on (re)start.
116118
this.stop();
117119
if (this.flushInterval > 0) {
118120
this.timer = setInterval(this.flush, this.flushInterval);
@@ -125,6 +127,9 @@ export class EventProcessor {
125127
clearInterval(this.timer);
126128
this.timer = null;
127129
}
130+
// Best-effort, fire-and-forget: this flush is not awaited, so on rapid
131+
// re-init buffered events may be lost. Callers needing a guarantee
132+
// (SSR/teardown) should await flushEvents() instead.
128133
this.flush();
129134
}
130135

flagsmith-core.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -961,8 +961,8 @@ const Flagsmith = class {
961961
metadata?: Record<string, unknown>;
962962
}) => {
963963
const processor = this.requireEventProcessor();
964-
if (event.startsWith('$') && event !== FLAG_EXPOSURE_EVENT) {
965-
throw new Error(`Flagsmith: event name "${event}" uses the reserved "$" prefix but is not a known system event.`);
964+
if (event.startsWith('$')) {
965+
throw new Error(`Flagsmith: event names starting with "$" are reserved; use trackExposureEvent to record "${FLAG_EXPOSURE_EVENT}".`);
966966
}
967967
processor.trackEvent({
968968
event,

react.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,10 @@ const getRenderKey = (flagsmith: IFlagsmith, flags: string[], traits: string[] =
8686

8787
const getExperimentRenderKey = (flagsmith: IFlagsmith | null, key: string): string => {
8888
const flag = flagsmith?.getAllFlags()?.[key]
89-
return `${flag?.value}${flag?.enabled}`
89+
const identifier = flagsmith?.getContext().identity?.identifier ?? null
90+
// Identity is part of the key so that switching identity re-renders (and
91+
// re-fires the exposure) even when the resolved value is unchanged.
92+
return `${identifier}|${flag?.value}|${flag?.enabled}`
9093
}
9194

9295
export function useFlagsmithLoading() {
@@ -227,20 +230,20 @@ export function useExperiment(featureName: string): IFlagsmithFeature | null {
227230
}, [flagsmith, key])
228231

229232
const flag = (flagsmith?.getAllFlags()?.[key] as IFlagsmithFeature | undefined) ?? null
233+
const identifier = flagsmith?.getContext().identity?.identifier ?? null
230234

231235
useEffect(() => {
232236
if (!flagsmith?.eventsEnabled || !flag) {
233237
return
234238
}
235-
const identifier = flagsmith.getContext().identity?.identifier ?? null
236239
const exposureKey = `${key}:${identifier}:${flag.value}`
237240
if (lastExposureKey.current === exposureKey) {
238241
return
239242
}
240243
lastExposureKey.current = exposureKey
241244
flagsmith.getExperimentFlag(featureName)
242245
// eslint-disable-next-line react-hooks/exhaustive-deps
243-
}, [flagsmith, featureName, key, flag?.value, flag?.enabled])
246+
}, [flagsmith, featureName, key, identifier, flag?.value, flag?.enabled])
244247

245248
return flag
246249
}

test/events.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ describe('events gate', () => {
4343
});
4444
await flagsmith.init(initConfig);
4545
expect(() => flagsmith.trackEvent('$custom')).toThrow(/reserved/i);
46+
expect(() => flagsmith.trackEvent('$flag_exposure')).toThrow(/reserved/i);
4647
});
4748
});
4849

test/react-events.test.tsx

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,34 @@ describe('useExperiment', () => {
116116
expect(values).toEqual(['16', '20'])
117117
})
118118

119+
test('fires a fresh exposure when identity changes even if the value is unchanged', async () => {
120+
const { flagsmith, initConfig, mockFetch } = getFlagsmith(eventsConfig({ identity: testIdentity }))
121+
render(
122+
<FlagsmithProvider flagsmith={flagsmith} options={initConfig}>
123+
<Probe feature="font_size" />
124+
</FlagsmithProvider>
125+
)
126+
127+
await waitFor(() => {
128+
expect(JSON.parse(screen.getByTestId('exp').innerHTML)?.value).toBe(16)
129+
})
130+
131+
// A different identity that resolves the SAME font_size value (16).
132+
getMockFetchWithValue(mockFetch, {
133+
flags: [
134+
{ enabled: true, feature_state_value: 16, feature: { id: 6149, name: 'font_size' } },
135+
],
136+
traits: [],
137+
})
138+
await flagsmith.identify('other_identity')
139+
140+
await waitFor(async () => {
141+
await flagsmith.flushEvents()
142+
expect(exposures(mockFetch)).toHaveLength(2)
143+
})
144+
expect(exposures(mockFetch).map((e: any) => e.identifier)).toEqual([testIdentity, 'other_identity'])
145+
})
146+
119147
test('renders the flag but fires no exposure when events are disabled', async () => {
120148
const { flagsmith, initConfig, mockFetch } = getFlagsmith({ identity: testIdentity })
121149
render(

0 commit comments

Comments
 (0)