-
Notifications
You must be signed in to change notification settings - Fork 31
chore: update svelte sdk to use browser 4.x #1042
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
base: main
Are you sure you want to change the base?
Conversation
|
@launchdarkly/browser size report |
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
| coreLdClient!.on('ready', () => { | ||
| function initialize(clientId: LDClientID, context: LDContext, options?: LDOptions) { | ||
| coreLdClient = createClientSdk(clientId, context, options); | ||
| coreLdClient.on('initialized', () => { |
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: Initialization failure leaves loading state stuck forever
The migration from the compat SDK's 'ready' event to the non-compat SDK's 'initialized' event changes error handling behavior. In the compat SDK, 'ready' always fires regardless of success or failure. In the non-compat SDK, 'initialized' only fires on successful initialization. If initialization fails (network issues, invalid credentials, etc.), the loading store will never be set to false, leaving the initializing state permanently true. This could cause UI components to remain in a loading state indefinitely. The code needs to also handle the 'error' event or use an alternative approach to detect when initialization completes regardless of outcome.
| }); | ||
|
|
||
| coreLdClient.start(); | ||
|
|
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: identify method return type changed breaking existing code
The migration changes the return type of the identify function. The compat SDK's identify method returned Promise<LDFlagSet> (the actual flag values), while the non-compat SDK's identify returns Promise<LDIdentifyResult> (a status object like { status: 'completed' }). Any existing code that relied on the return value of LD.identify() to access flag values will now silently receive a status object instead, causing undefined values when accessing flag keys.
- removes the browser compat dependency on svelte sdk
b3c84c1 to
bcde547
Compare
|
|
||
| beforeEach(() => { | ||
| // mocks the initialize function to return the mockLDClient | ||
| (initialize as Mock<typeof initialize>).mockReturnValue( |
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: Test emits wrong event after API migration
The migration from the compat API to browser 4.x changed the initialization event from 'ready' to 'initialized', but one test case was missed. Line 134 still emits mockLDEventEmitter.emit('ready') while the implementation now listens for 'initialized'. This causes the test to not properly trigger the initialization callback, though it may pass coincidentally due to state leaking from other tests that correctly emit 'initialized'.
sdk-1703
sdk-1691
Note
Switches the Svelte SDK to the v4 browser API, replacing compat
initializewithcreateClient/start, using theinitializedevent, and updating types/tests.packages/sdk/svelte/src/lib/client/SvelteLDClient.ts):@launchdarkly/js-client-sdkand usecreateClient+start().initialized(wasready) andchangeevents to manage loading and flags.initialize(clientId, context, options?)now acceptsLDOptionsand returns{ initializing }.createLDtoinit; exportLD = init().LDFlagSet,LDFlagValue,LDContext,LDOptionsfrom core SDK.packages/sdk/svelte/__tests__/lib/client/SvelteLDClient.test.ts):@launchdarkly/js-client-sdkandcreateClient; expectcreateClient(clientId, context, undefined)andinitializedevent.startto mock client; update assertions for event name changes and behavior.Written by Cursor Bugbot for commit bcde547. This will update automatically on new commits. Configure here.