Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions packages/sdk/svelte/__tests__/lib/client/SvelteLDClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import { EventEmitter } from 'node:events';
import { get } from 'svelte/store';
import { afterEach, beforeEach, describe, expect, it, Mock, vi } from 'vitest';

import { initialize, LDClient } from '@launchdarkly/js-client-sdk/compat';
import { createClient, LDClient } from '@launchdarkly/js-client-sdk';

import { LD } from '../../../src/lib/client/SvelteLDClient';

vi.mock('@launchdarkly/js-client-sdk/compat', { spy: true });
vi.mock('@launchdarkly/js-client-sdk', { spy: true });

const clientSideID = 'test-client-side-id';
const rawFlags = { 'test-flag': true, 'another-test-flag': 'flag-value' };
Expand All @@ -21,6 +21,7 @@ const mockLDClient = {
allFlags: vi.fn().mockReturnValue(rawFlags),
variation: vi.fn((_, defaultValue) => defaultValue),
identify: vi.fn(),
start: vi.fn(),
};

describe('launchDarkly', () => {
Expand All @@ -40,8 +41,7 @@ describe('launchDarkly', () => {
const ld = LD;

beforeEach(() => {
// mocks the initialize function to return the mockLDClient
(initialize as Mock<typeof initialize>).mockReturnValue(
(createClient as Mock<typeof createClient>).mockReturnValue(
mockLDClient as unknown as LDClient,
);
});
Expand All @@ -66,23 +66,23 @@ describe('launchDarkly', () => {
ld.initialize(clientSideID, mockContext);

expect(get(initializing)).toBe(true); // should be true before the ready event is emitted
mockLDEventEmitter.emit('ready');
mockLDEventEmitter.emit('initialized');

expect(get(initializing)).toBe(false);
});

it('should initialize the LaunchDarkly SDK instance', () => {
ld.initialize(clientSideID, mockContext);

expect(initialize).toHaveBeenCalledWith('test-client-side-id', mockContext);
expect(createClient).toHaveBeenCalledWith('test-client-side-id', mockContext, undefined);
});

it('should register function that gets flag values when client is ready', () => {
const newFlags = { ...rawFlags, 'new-flag': true };
const allFlagsSpy = vi.spyOn(mockLDClient, 'allFlags').mockReturnValue(newFlags);

ld.initialize(clientSideID, mockContext);
mockLDEventEmitter.emit('ready');
mockLDEventEmitter.emit('initialized');

expect(allFlagsSpy).toHaveBeenCalledOnce();
expect(allFlagsSpy).toHaveReturnedWith(newFlags);
Expand All @@ -104,8 +104,7 @@ describe('launchDarkly', () => {
const ld = LD;

beforeEach(() => {
// mocks the initialize function to return the mockLDClient
(initialize as Mock<typeof initialize>).mockReturnValue(
Copy link

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'.

Fix in Cursor Fix in Web

(createClient as Mock<typeof createClient>).mockReturnValue(
mockLDClient as unknown as LDClient,
);
});
Expand Down Expand Up @@ -166,8 +165,7 @@ describe('launchDarkly', () => {
const ld = LD;

beforeEach(() => {
// mocks the initialize function to return the mockLDClient
(initialize as Mock<typeof initialize>).mockReturnValue(
(createClient as Mock<typeof createClient>).mockReturnValue(
mockLDClient as unknown as LDClient,
);
});
Expand All @@ -191,8 +189,7 @@ describe('launchDarkly', () => {
const ld = LD;

beforeEach(() => {
// mocks the initialize function to return the mockLDClient
(initialize as Mock<typeof initialize>).mockReturnValue(
(createClient as Mock<typeof createClient>).mockReturnValue(
mockLDClient as unknown as LDClient,
);
});
Expand Down
23 changes: 13 additions & 10 deletions packages/sdk/svelte/src/lib/client/SvelteLDClient.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { derived, type Readable, readonly, writable, type Writable } from 'svelte/store';

import type { LDFlagSet } from '@launchdarkly/js-client-sdk';
import {
initialize,
createClient as createClientSdk,
type LDClient,
type LDContext,
type LDFlagSet,
type LDFlagValue,
} from '@launchdarkly/js-client-sdk/compat';
type LDOptions,
} from '@launchdarkly/js-client-sdk';

export type { LDContext, LDFlagValue };

Expand Down Expand Up @@ -62,7 +63,7 @@ function toFlagsProxy(client: LDClient, flags: LDFlags): LDFlags {
* Creates a LaunchDarkly instance.
* @returns {Object} The LaunchDarkly instance object.
*/
function createLD() {
function init() {
let coreLdClient: LDClient | undefined;
const loading = writable(true);
const flagsWritable = writable<LDFlags>({});
Expand All @@ -73,21 +74,23 @@ function createLD() {
* @param {LDContext} context - The user context.
* @returns {Object} An object with the initialization status store.
*/
function LDInitialize(clientId: LDClientID, context: LDContext) {
coreLdClient = initialize(clientId, context);
coreLdClient!.on('ready', () => {
function initialize(clientId: LDClientID, context: LDContext, options?: LDOptions) {
coreLdClient = createClientSdk(clientId, context, options);
coreLdClient.on('initialized', () => {
Copy link

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.

Fix in Cursor Fix in Web

loading.set(false);
const rawFlags = coreLdClient!.allFlags();
const allFlags = toFlagsProxy(coreLdClient!, rawFlags);
flagsWritable.set(allFlags);
});

coreLdClient!.on('change', () => {
coreLdClient.on('change', () => {
const rawFlags = coreLdClient!.allFlags();
const allFlags = toFlagsProxy(coreLdClient!, rawFlags);
flagsWritable.set(allFlags);
});

coreLdClient.start();

Copy link

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.

Fix in Cursor Fix in Web

return {
initializing: loading,
};
Expand Down Expand Up @@ -125,12 +128,12 @@ function createLD() {
return {
identify,
flags: readonly(flagsWritable),
initialize: LDInitialize,
initialize,
initializing: readonly(loading),
watch,
useFlag,
};
}

/** The LaunchDarkly instance */
export const LD = createLD();
export const LD = init();