Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/sdk/vue/__tests__/client/composables.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,21 @@ it('throws when used without a provider', () => {

expect(() => mount(Child)).toThrow(/LaunchDarkly client was not found/);
});

it('subscribes to onContextChange on mount and unsubscribes on scope dispose', () => {
const { client, controls } = makeMockClient();
const Child = defineComponent({
setup() {
useBoolVariation('flag', false);
return () => h('div');
},
});

// Provider adds one onContextChange subscriber; the composable adds a second.
const wrapper = mountUnderProvider(client, Child);
expect(controls.contextSubscriberCount()).toBe(2);

// Unmounting disposes both scopes; every onContextChange subscription is removed.
wrapper.unmount();
expect(controls.contextSubscriberCount()).toBe(0);
});
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,10 @@ it('useBoolVariation evaluates once when initialization completes', async () =>
expect(wrapper.text()).toBe('false');
expect(client.boolVariation).not.toHaveBeenCalled();

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

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

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

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

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

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

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

// One identify must produce exactly one evaluation -> one analytics impression.
expect(client.boolVariation as jest.Mock).toHaveBeenCalledTimes(2);
});

it('useBoolVariation evaluates exactly once when ONLY the flag value changes', async () => {
const { client, controls } = makeMockClient();
(client.boolVariation as jest.Mock).mockReturnValue(true);

const Child = defineComponent({
setup() {
const flag = useBoolVariation('my-flag', false);
return () => h('div', String(flag.value));
},
});

mountUnderProvider(client, Child);
(client.boolVariation as jest.Mock).mockClear();

controls.emitChange('my-flag');
await nextTick();

expect(client.boolVariation as jest.Mock).toHaveBeenCalledTimes(1);
});

it('useBoolVariation evaluates exactly once when ONLY the context changes', async () => {
const { client, controls } = makeMockClient();
(client.boolVariation as jest.Mock).mockReturnValue(true);

const Child = defineComponent({
setup() {
const flag = useBoolVariation('my-flag', false);
return () => h('div', String(flag.value));
},
});

mountUnderProvider(client, Child);
(client.boolVariation as jest.Mock).mockClear();

controls.emitContextChange({ kind: 'user', key: 'new-user' });
await nextTick();

expect(client.boolVariation as jest.Mock).toHaveBeenCalledTimes(1);
});

Expand Down
5 changes: 5 additions & 0 deletions packages/sdk/vue/__tests__/client/mockClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export interface MockControls {
emitInitStatus: (r: { status: string; error?: Error }) => void;
emitContextChange: (c: unknown) => void;
subscriberCount: () => number;
contextSubscriberCount: () => number;
}

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

return { client: client as unknown as LDVueClient, controls };
Expand Down
46 changes: 26 additions & 20 deletions packages/sdk/vue/src/client/composables/useVariationCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {
onScopeDispose,
readonly,
ref,
shallowRef,
toValue,
watch,
type InjectionKey,
Expand All @@ -17,9 +16,14 @@ import { injectLDVueInstance } from './useLDClient';
* Shared core for the variation composables. Evaluates a flag and keeps the returned ref in sync.
*
* @remarks
* Re-evaluates when the flag changes (`change:<key>` event), the key changes, the context changes
* (after `identify()`), or the client becomes ready. The base SDK emits `change:<key>` with the
* context, not the new value, so we always call the client rather than reading the event payload.
* Re-evaluates on two independent triggers:
* - the flag's value changing (`change:<key>` event), and
* - the context changing, via {@link LDVueClient.onContextChange}, which fires for every settled
* `start()` outcome (complete, timeout, failed) and for every completed `identify()`.
*
* This means that there is a possible case that we evaluate the flag 2x when switching
* contexts (both triggers fire). This is expected behavior for now as it is more stable
* than trying to consolidate two unrelated signals.
*
* @internal
*/
Expand All @@ -30,7 +34,7 @@ export function useVariationCore<T, R = T>(
injectionKey?: InjectionKey<LDVueInstance>,
notReadyDefault?: (defaultValue: T) => R,
): Readonly<Ref<R>> {
const { client, context, initializedState } = injectLDVueInstance(injectionKey);
const { client } = injectLDVueInstance(injectionKey);

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

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

const flagChanged = shallowRef(0);
const changeHandler = () => {
flagChanged.value += 1;
};
client.on(`change:${currentKey}`, changeHandler);
// Context-driven re-evaluation that will trigger for every settled
// start()/identify() outcome.
const unsubscribeContext = client.onContextChange(update);
Comment thread
joker23 marked this conversation as resolved.
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.

// Batch all synchronous source mutations into one watch run.
const stop = watch([() => toValue(key), context, initializedState, flagChanged], ([newKey]) => {
if (newKey !== currentKey) {
client.off(`change:${currentKey}`, changeHandler);
const stopWatch = watch(
() => toValue(key),
(newKey) => {
client.off(`change:${currentKey}`, update);
currentKey = newKey;
client.on(`change:${currentKey}`, changeHandler);
}
update();
});
client.on(`change:${currentKey}`, update);
update();
},
);

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

return readonly(valueRef) as Readonly<Ref<R>>;
Expand Down
Loading