chore(vue-sdk): add variation composables#1773
Conversation
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk size report |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d469468. Configure here.
…t.ts Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| let currentKey = toValue(key); | ||
| const changeHandler = () => { | ||
| update(); | ||
| }; | ||
| client.on(`change:${currentKey}`, changeHandler); | ||
|
|
||
| // Re-subscribe when the key changes; re-evaluate when the key, context, or readiness changes. | ||
| // Both the change-event handler and the context watch may call evaluate() on a single identify(). | ||
| const stop = watch([() => toValue(key), context, initializedState], ([newKey]) => { | ||
| if (newKey !== currentKey) { | ||
| client.off(`change:${currentKey}`, changeHandler); | ||
| currentKey = newKey; | ||
| client.on(`change:${currentKey}`, changeHandler); | ||
| } | ||
| update(); | ||
| }); |
There was a problem hiding this comment.
🔴 Flag evaluation fires twice on a single identify, causing duplicate analytics impressions
The flag is evaluated a second time by the context watcher (update() at packages/sdk/vue/src/client/composables/useVariationCore.ts:62) after it was already evaluated synchronously by the change-event handler, so a single identify produces two evaluation calls instead of one.
Impact: Each identify that changes a flag value generates a redundant analytics impression, inflating metrics.
Duplicate evaluation mechanism during identify()
When identify() is called in the base SDK, it emits both change:<key> (synchronously) and a context-change notification. In useVariationCore, these are handled by two independent paths:
-
The
changeHandler(packages/sdk/vue/src/client/composables/useVariationCore.ts:49-51) is registered viaclient.on('change:<key>', ...)at line 52. It fires synchronously whenchange:<key>is emitted, callingupdate()→evaluateValue()→client.boolVariation()(or whichever typed variant). -
The Vue
watch(packages/sdk/vue/src/client/composables/useVariationCore.ts:56-63) monitors[key, context, initializedState]. WhenonContextChangeupdatescontext.value(packages/sdk/vue/src/client/provider/LDVueContext.ts:49), the watcher is scheduled. On the next tick it fires and callsupdate()→evaluateValue()→client.boolVariation()again.
There is no debounce, guard flag, or coalescing mechanism between these two paths. The comment at line 54-55 acknowledges the overlap but no code prevents the duplicate.
The test at packages/sdk/vue/__tests__/client/composables/useVariation.test.ts:187-209 expects exactly 1 call (toHaveBeenCalledTimes(1)) after emitting both events, but the implementation produces 2 calls.
Prompt for agents
In useVariationCore.ts, the changeHandler (line 49-51, triggered by client.on('change:<key>')) and the Vue watch callback (line 56-63, triggered by context/initializedState changes) both call update() independently. When identify() fires both events, evaluate() is called twice, producing duplicate analytics impressions.
To fix this, introduce a microtask-level coalescing mechanism for the update() call. For example, replace the direct update() calls in both the changeHandler and the watcher with a scheduleUpdate() function that uses a simple dirty flag and queueMicrotask (or Vue's nextTick) to batch multiple triggers into a single evaluation:
let updatePending = false;
const scheduleUpdate = () => {
if (!updatePending) {
updatePending = true;
queueMicrotask(() => {
updatePending = false;
update();
});
}
};
Then call scheduleUpdate() instead of update() in both the changeHandler and the watch callback. This ensures that even when both events fire in the same tick, only one evaluation occurs.
Alternatively, if synchronous update from the changeHandler is desirable for streaming updates (where no context change follows), you could add a per-tick guard that skips the evaluation in the watcher if the changeHandler already ran during the same microtask.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
I think this comment is stale.
joker23
force-pushed
the
skz/sdk-2194/migrate-vue-sdk-variation-composables
branch
from
044db00 to
dd1d9ce
Compare
2 participants
Note
Medium Risk
New public API on the flag evaluation path affects when impressions fire and how apps render before init; behavior is heavily tested but mistakes could skew analytics or UI gating.
Overview
Adds the Vue SDK’s reactive flag evaluation layer: typed composables for bool/string/number/JSON flags plus matching
*VariationDetailhelpers, all built on a shareduseVariationCorethat wires into the existing provider injection.useVariationCorekeeps a readonly ref in sync by calling the client when ready (otherwise defaults or syntheticCLIENT_NOT_READYdetail), listening onchange:<key>, and re-running when the flag key, context afteridentify(), or initialization state changes. A batchedwatchis meant to collapse simultaneouschange:<key>and context updates into one evaluation per identify (guarded by tests for duplicate impressions). Listeners are swapped when the key is reactive and removed on scope dispose.Public exports move from the prior scaffold TODO to
composables/index. Tests cover provider/plugin integration, lifecycle, and typed behavior;makeMockClientnow tracks handlers in arrays with correctoffand setsreadyfrom init status.Reviewed by Cursor Bugbot for commit dd1d9ce. Bugbot is set up for automated code reviews on this repo. Configure here.