chore: aligning vue composables to trigger eval events#1804
Conversation
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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}
| client.on(`change:${currentKey}`, changeHandler); | ||
| // Context-driven re-evaluation that will trigger for every settled | ||
| // start()/identify() outcome. | ||
| const unsubscribeContext = client.onContextChange(update); |
There was a problem hiding this comment.
🔴 Flag values stay stuck on defaults when client startup unexpectedly rejects
The composable now relies solely on the context-change subscription to trigger re-evaluation after startup (client.onContextChange(update) at packages/sdk/vue/src/client/composables/useVariationCore.ts:56), but the client's rejection handler never notifies context subscribers, so the flag value is never refreshed.
Impact: When the underlying SDK's start() promise rejects (rather than resolving with a failure status), the UI permanently displays the hard-coded default value even though the client is marked as ready.
Mechanism: rejection handler omits context-subscriber notification
The Vue client wrapper in packages/sdk/vue/src/client/LDVueClient.ts:108-121 has a rejection handler for baseClient.start(). In this handler, initializationState is set to 'failed' (line 111) and initStatusSubscribers are notified (lines 113-119), but notifyContextSubscribers() is never called — unlike the success handler at packages/sdk/vue/src/client/LDVueClient.ts:92-107 which calls it on line 97.
The old useVariationCore code watched the reactive initializedState ref (which the provider updates via onInitializationStatusChange — see packages/sdk/vue/src/client/provider/LDVueContext.ts:44-47). This meant ANY initialization state change, including the rejection path, would trigger re-evaluation.
The new code only subscribes to client.onContextChange, which is never fired in the rejection path. So update() is never called, and valueRef retains the notReadyDefault or defaultValue.
The test at packages/sdk/vue/__tests__/client/composables/useVariation.test.ts:168-169 masks this by manually emitting both emitInitStatus and emitContextChange, but the real client only emits context-change notifications in the success handler.
Prompt for agents
The composable in useVariationCore.ts now relies solely on client.onContextChange to re-evaluate flags after startup. However, the LDVueClient wrapper's rejection handler for start() (LDVueClient.ts lines 108-121) notifies initStatusSubscribers but does NOT call notifyContextSubscribers(). This means the composable never re-evaluates when start() rejects.
Two possible fixes:
1. In LDVueClient.ts, add a call to notifyContextSubscribers() in the rejection handler (after line 112), mirroring what the success handler does on line 97. This ensures context subscribers are always notified regardless of how start() settles.
2. In useVariationCore.ts, additionally subscribe to onInitializationStatusChange to handle the failure case, similar to how the old code watched the initializedState ref.
Option 1 is simpler and more consistent — the rejection handler should behave the same as the success handler with respect to context notifications, since isReady() returns true in both cases.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Note
Medium Risk
Changes when flag evaluations run (including possible double impressions on identify) and removes init-state watching from variation composables, which can affect analytics and readiness edge cases.
Overview
Vue variation composables (
useVariationCore) now re-sync flag refs from two direct client subscriptions—change:<key>andonContextChange—instead of a single batchedwatchon providercontext,initializedState, and aflagChangedcounter.Lifecycle:
onContextChangeis registered on mount and torn down inonScopeDisposealongsidechange:<key>listeners (explicit cleanup because these are outside Vue reactivity). Key changes still swapchange:<key>handlers via a narrowwatchon the flag key only.Behavior change: When one
identify()updates both context and the watched flag, the composable may call the client twice (one eval per signal); tests document this as accepted React-parity cost after removing batching. Init complete/failure is expected to drive re-eval once viaonContextChange, not a separate init-status path (SDK-2640). Tests andmakeMockClientaddcontextSubscriberCountand split scenarios for flag-only vs context-only vs combined updates.Reviewed by Cursor Bugbot for commit 1f6a8b1. Bugbot is set up for automated code reviews on this repo. Configure here.