Skip to content

Commit 1f6a8b1

Browse files
committed
chore: aligning vue composables to trigger eval events
1 parent ce99fe0 commit 1f6a8b1

4 files changed

Lines changed: 102 additions & 24 deletions

File tree

packages/sdk/vue/__tests__/client/composables.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,21 @@ it('throws when used without a provider', () => {
143143

144144
expect(() => mount(Child)).toThrow(/LaunchDarkly client was not found/);
145145
});
146+
147+
it('subscribes to onContextChange on mount and unsubscribes on scope dispose', () => {
148+
const { client, controls } = makeMockClient();
149+
const Child = defineComponent({
150+
setup() {
151+
useBoolVariation('flag', false);
152+
return () => h('div');
153+
},
154+
});
155+
156+
// Provider adds one onContextChange subscriber; the composable adds a second.
157+
const wrapper = mountUnderProvider(client, Child);
158+
expect(controls.contextSubscriberCount()).toBe(2);
159+
160+
// Unmounting disposes both scopes; every onContextChange subscription is removed.
161+
wrapper.unmount();
162+
expect(controls.contextSubscriberCount()).toBe(0);
163+
});

packages/sdk/vue/__tests__/client/composables/useVariation.test.ts

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,10 @@ it('useBoolVariation evaluates once when initialization completes', async () =>
137137
expect(wrapper.text()).toBe('false');
138138
expect(client.boolVariation).not.toHaveBeenCalled();
139139

140+
// start() resolving to complete flips the client to ready and notifies context
141+
// subscribers; the composable re-evaluates via its onContextChange subscription.
140142
controls.emitInitStatus({ status: 'complete' });
143+
controls.emitContextChange({ kind: 'user', key: 'context-key' });
141144
await nextTick();
142145

143146
expect(client.boolVariation).toHaveBeenCalledTimes(1);
@@ -158,7 +161,12 @@ it('useBoolVariation evaluates when initialization fails (client returns default
158161
mountUnderProvider(client, Child);
159162
expect(client.boolVariation).not.toHaveBeenCalled();
160163

164+
// A failed start() still resolves (failure is a resolved result, not a rejection), so the
165+
// base client notifies context subscribers exactly once. The composable must re-evaluate
166+
// exactly once via onContextChange, not a second time via any init-status subscription
167+
// (SDK-2640 double-eval-on-failure guard).
161168
controls.emitInitStatus({ status: 'failed', error: new Error('network error') });
169+
controls.emitContextChange({ kind: 'user', key: 'context-key' });
162170
await nextTick();
163171

164172
expect(client.boolVariation).toHaveBeenCalledTimes(1);
@@ -184,7 +192,7 @@ it('useBoolVariation re-evaluates when context changes after identify', async ()
184192
expect((client.boolVariation as jest.Mock).mock.calls.length).toBe(callsBefore + 1);
185193
});
186194

187-
it('useBoolVariation evaluates only once per identify (no duplicate analytics impression)', async () => {
195+
it('useBoolVariation evaluates twice when a single identify changes BOTH context and the flag value', async () => {
188196
const { client, controls } = makeMockClient();
189197
(client.boolVariation as jest.Mock).mockReturnValue(true);
190198

@@ -198,13 +206,54 @@ it('useBoolVariation evaluates only once per identify (no duplicate analytics im
198206
mountUnderProvider(client, Child);
199207
(client.boolVariation as jest.Mock).mockClear();
200208

201-
// A single identify() in the base SDK emits change:<key> synchronously AND notifies
202-
// context subscribers, so model both for one identify.
209+
// A single identify() that changes both the context and the watched flag's value fires
210+
// change:<key> (flag-value trigger) and notifies context subscribers (context trigger).
211+
// Each fires update() once -> two evaluations. Accepted react-parity cost (SDK-2194): the
212+
// flagChanged counter that used to batch these into one flush is gone.
203213
controls.emitChange('my-flag');
204214
controls.emitContextChange({ kind: 'user', key: 'new-user' });
205215
await nextTick();
206216

207-
// One identify must produce exactly one evaluation -> one analytics impression.
217+
expect(client.boolVariation as jest.Mock).toHaveBeenCalledTimes(2);
218+
});
219+
220+
it('useBoolVariation evaluates exactly once when ONLY the flag value changes', async () => {
221+
const { client, controls } = makeMockClient();
222+
(client.boolVariation as jest.Mock).mockReturnValue(true);
223+
224+
const Child = defineComponent({
225+
setup() {
226+
const flag = useBoolVariation('my-flag', false);
227+
return () => h('div', String(flag.value));
228+
},
229+
});
230+
231+
mountUnderProvider(client, Child);
232+
(client.boolVariation as jest.Mock).mockClear();
233+
234+
controls.emitChange('my-flag');
235+
await nextTick();
236+
237+
expect(client.boolVariation as jest.Mock).toHaveBeenCalledTimes(1);
238+
});
239+
240+
it('useBoolVariation evaluates exactly once when ONLY the context changes', async () => {
241+
const { client, controls } = makeMockClient();
242+
(client.boolVariation as jest.Mock).mockReturnValue(true);
243+
244+
const Child = defineComponent({
245+
setup() {
246+
const flag = useBoolVariation('my-flag', false);
247+
return () => h('div', String(flag.value));
248+
},
249+
});
250+
251+
mountUnderProvider(client, Child);
252+
(client.boolVariation as jest.Mock).mockClear();
253+
254+
controls.emitContextChange({ kind: 'user', key: 'new-user' });
255+
await nextTick();
256+
208257
expect(client.boolVariation as jest.Mock).toHaveBeenCalledTimes(1);
209258
});
210259

packages/sdk/vue/__tests__/client/mockClient.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export interface MockControls {
99
emitInitStatus: (r: { status: string; error?: Error }) => void;
1010
emitContextChange: (c: unknown) => void;
1111
subscriberCount: () => number;
12+
contextSubscriberCount: () => number;
1213
}
1314

1415
/**
@@ -84,6 +85,7 @@ export function makeMockClient(initial?: {
8485
setBool: (v: boolean) => {
8586
boolValue = v;
8687
},
88+
// mirrors the real client: change:<key> events carry the context, not the new flag value
8789
emitChange: (key: string) =>
8890
handlers.get(`change:${key}`)?.forEach((h) => h({ kind: 'user', key: 'context-key' })),
8991
handlerCount: (event: string) => handlers.get(event)?.length ?? 0,
@@ -95,6 +97,9 @@ export function makeMockClient(initial?: {
9597
},
9698
emitContextChange: (c: unknown) => contextSubs.forEach((cb) => cb(c)),
9799
subscriberCount: () => initStatusSubs.size + contextSubs.size,
100+
// separate from subscriberCount so composable tests can assert on context subs alone,
101+
// without init-status subs (which the composable doesn't use) muddying the count
102+
contextSubscriberCount: () => contextSubs.size,
98103
};
99104

100105
return { client: client as unknown as LDVueClient, controls };

packages/sdk/vue/src/client/composables/useVariationCore.ts

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import {
22
onScopeDispose,
33
readonly,
44
ref,
5-
shallowRef,
65
toValue,
76
watch,
87
type InjectionKey,
@@ -17,9 +16,14 @@ import { injectLDVueInstance } from './useLDClient';
1716
* Shared core for the variation composables. Evaluates a flag and keeps the returned ref in sync.
1817
*
1918
* @remarks
20-
* Re-evaluates when the flag changes (`change:<key>` event), the key changes, the context changes
21-
* (after `identify()`), or the client becomes ready. The base SDK emits `change:<key>` with the
22-
* context, not the new value, so we always call the client rather than reading the event payload.
19+
* Re-evaluates on two independent triggers:
20+
* - the flag's value changing (`change:<key>` event), and
21+
* - the context changing, via {@link LDVueClient.onContextChange}, which fires for every settled
22+
* `start()` outcome (complete, timeout, failed) and for every completed `identify()`.
23+
*
24+
* This means that there is a possible case that we evaluate the flag 2x when switching
25+
* contexts (both triggers fire). This is expected behavior for now as it is more stable
26+
* than trying to consolidate two unrelated signals.
2327
*
2428
* @internal
2529
*/
@@ -30,7 +34,7 @@ export function useVariationCore<T, R = T>(
3034
injectionKey?: InjectionKey<LDVueInstance>,
3135
notReadyDefault?: (defaultValue: T) => R,
3236
): Readonly<Ref<R>> {
33-
const { client, context, initializedState } = injectLDVueInstance(injectionKey);
37+
const { client } = injectLDVueInstance(injectionKey);
3438

3539
const evaluateValue = (): R => {
3640
if (client.isReady()) {
@@ -45,26 +49,28 @@ export function useVariationCore<T, R = T>(
4549
};
4650

4751
let currentKey = toValue(key);
52+
client.on(`change:${currentKey}`, update);
4853

49-
const flagChanged = shallowRef(0);
50-
const changeHandler = () => {
51-
flagChanged.value += 1;
52-
};
53-
client.on(`change:${currentKey}`, changeHandler);
54+
// Context-driven re-evaluation that will trigger for every settled
55+
// start()/identify() outcome.
56+
const unsubscribeContext = client.onContextChange(update);
5457

55-
// Batch all synchronous source mutations into one watch run.
56-
const stop = watch([() => toValue(key), context, initializedState, flagChanged], ([newKey]) => {
57-
if (newKey !== currentKey) {
58-
client.off(`change:${currentKey}`, changeHandler);
58+
const stopWatch = watch(
59+
() => toValue(key),
60+
(newKey) => {
61+
client.off(`change:${currentKey}`, update);
5962
currentKey = newKey;
60-
client.on(`change:${currentKey}`, changeHandler);
61-
}
62-
update();
63-
});
63+
client.on(`change:${currentKey}`, update);
64+
update();
65+
},
66+
);
6467

6568
onScopeDispose(() => {
66-
client.off(`change:${currentKey}`, changeHandler);
67-
stop();
69+
// client.on/onContextChange are plain event-emitter subscriptions, invisible to Vue's
70+
// reactivity system, so they leak on unmount unless torn down explicitly here.
71+
client.off(`change:${currentKey}`, update);
72+
unsubscribeContext();
73+
stopWatch();
6874
});
6975

7076
return readonly(valueRef) as Readonly<Ref<R>>;

0 commit comments

Comments
 (0)