-
Notifications
You must be signed in to change notification settings - Fork 31
feat: allow clients to evaluate bootstrapped flags when not ready #1036
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Introduced `ActiveContextTracker` to manage the current active context and its serialized state. - Updated `LDClientImpl` to utilize the new context tracker for identifying and evaluating flags. - Added logic in `BrowserClientImpl` to read flags from bootstrap data during the initial identification process. This change improves the SDK's ability to handle context and flag management, particularly during initialization.
|
@launchdarkly/browser size report |
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
| this._eventFactoryDefault.unknownFlagEvent(flagKey, defVal, evalContext), | ||
| ); | ||
|
|
||
| this.emitter.emit('error', this._activeContextTracker.getPristineContext(), error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Error events emitted with undefined context during pre-initialization evaluation
When evaluating flags before identification completes (using preset bootstrap data), error events for "flag not found" or "wrong type" are emitted via emitter.emit('error', this._activeContextTracker.getPristineContext(), error) where getPristineContext() returns undefined. Previously, _variationInternal returned early when there was no context, so these error events were never emitted in the pre-initialization state. Error listeners that expect a defined context as the first argument may now receive undefined, potentially causing issues in downstream handlers.
Additional Locations (1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine as the error is probably due to non-existing context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to clarify in the interface that context can be null. Our typing isn't strong for those events, so I would hope there isn't anything expecting context to be always defined.
53906ff to
48334f3
Compare
48334f3 to
1c9cb11
Compare
1c9cb11 to
2d45b52
Compare
packages/shared/sdk-client/src/context/createActiveContextTracker.ts
Outdated
Show resolved
Hide resolved
packages/shared/sdk-client/src/context/createActiveContextTracker.ts
Outdated
Show resolved
Hide resolved
- changed `pristine context` to `unwrapped context` - use closure like a real js developer
| this.logger, | ||
| identifyOptionsWithUpdatedDefaults.bootstrap, | ||
| ); | ||
| this.presetFlags(bootstrapData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Bootstrap data parsed twice causing duplicate log warnings
When identify is called with bootstrap data, readFlagsFromBootstrap is invoked twice with the same data - first in BrowserClientImpl.identifyResult to call presetFlags, then again in BrowserDataManager._finishIdentifyFromBootstrap to call setBootstrap. This causes any warning logs (e.g., "bootstrap data did not include flag metadata") to appear twice, which could confuse users, and unnecessarily parses the same data twice. The bootstrap data could be parsed once and the result reused for both operations.
Related issues
sdk-1653
sdk-1376
sdk-1663
sdk-1681
Describe the solution you've provided
ActiveContextTrackerto manage the current active context and its serialized state.LDClientImplto utilize the new context tracker for identifying and evaluating flags.BrowserClientImplto read flags from bootstrap data during the initial identification process.presetFlagsfunction in client sdk common that allow flagstore to take in contexless data before initializationpresetthe flag store so they can be evaluated before a full context is madeAdditional context
Supersedes #1024
Note
Enable flag evaluation from bootstrap data before identify completes and refactor context handling via ActiveContextTracker with event suppression when no valid context.
BrowserClient.identifyResultandpresetflags for immediate evaluation; track first-identify via_identifyAttempted.packages/sdk/browser/__tests__/BrowserClient.test.ts.ActiveContextTracker(src/context/createActiveContextTracker.ts) and updateLDClientImplto use it for getting/setting context and coordinating identify promises.presetFlagstoLDClientImpl.presetFlagstoFlagManager/DefaultFlagManagerto initialize in-memory flags without persistence.FlagUpdaterto track activeContext(not just key) and adjustinit/initCached/upsertand change callbacks accordingly.Written by Cursor Bugbot for commit b7b4bf8. This will update automatically on new commits. Configure here.