From 5856f644b59acb6cea372537909c7d4a5f59048d Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Tue, 14 Jul 2026 16:12:39 -0400 Subject: [PATCH] chore: aligning vue composables to trigger eval events --- .../vue/__tests__/client/composables.test.ts | 18 ++++++ .../client/composables/useVariation.test.ts | 57 +++++++++++++++++-- .../sdk/vue/__tests__/client/mockClient.ts | 5 ++ .../client/composables/useVariationCore.ts | 46 ++++++++------- 4 files changed, 102 insertions(+), 24 deletions(-) diff --git a/packages/sdk/vue/__tests__/client/composables.test.ts b/packages/sdk/vue/__tests__/client/composables.test.ts index c129b6722e..0e2b7ed783 100644 --- a/packages/sdk/vue/__tests__/client/composables.test.ts +++ b/packages/sdk/vue/__tests__/client/composables.test.ts @@ -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); +}); diff --git a/packages/sdk/vue/__tests__/client/composables/useVariation.test.ts b/packages/sdk/vue/__tests__/client/composables/useVariation.test.ts index 31a825ed8b..77de4631cc 100644 --- a/packages/sdk/vue/__tests__/client/composables/useVariation.test.ts +++ b/packages/sdk/vue/__tests__/client/composables/useVariation.test.ts @@ -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); @@ -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); @@ -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); @@ -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: 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: (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); }); diff --git a/packages/sdk/vue/__tests__/client/mockClient.ts b/packages/sdk/vue/__tests__/client/mockClient.ts index e695d48d27..66642c083d 100644 --- a/packages/sdk/vue/__tests__/client/mockClient.ts +++ b/packages/sdk/vue/__tests__/client/mockClient.ts @@ -9,6 +9,7 @@ export interface MockControls { emitInitStatus: (r: { status: string; error?: Error }) => void; emitContextChange: (c: unknown) => void; subscriberCount: () => number; + contextSubscriberCount: () => number; } /** @@ -84,6 +85,7 @@ export function makeMockClient(initial?: { setBool: (v: boolean) => { boolValue = v; }, + // mirrors the real client: change: 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, @@ -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 }; diff --git a/packages/sdk/vue/src/client/composables/useVariationCore.ts b/packages/sdk/vue/src/client/composables/useVariationCore.ts index 9b75ee4da4..a40524d2c1 100644 --- a/packages/sdk/vue/src/client/composables/useVariationCore.ts +++ b/packages/sdk/vue/src/client/composables/useVariationCore.ts @@ -2,7 +2,6 @@ import { onScopeDispose, readonly, ref, - shallowRef, toValue, watch, type InjectionKey, @@ -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:` event), the key changes, the context changes - * (after `identify()`), or the client becomes ready. The base SDK emits `change:` 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:` 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 */ @@ -30,7 +34,7 @@ export function useVariationCore( injectionKey?: InjectionKey, notReadyDefault?: (defaultValue: T) => R, ): Readonly> { - const { client, context, initializedState } = injectLDVueInstance(injectionKey); + const { client } = injectLDVueInstance(injectionKey); const evaluateValue = (): R => { if (client.isReady()) { @@ -45,26 +49,28 @@ export function useVariationCore( }; 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); - // 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>;