From 44f6e9d041a3472fc30efd13a0765499057356eb Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 14 Nov 2025 14:30:46 -0800 Subject: [PATCH] feat!: Only support identify with result. --- .../__tests__/BrowserClient.plugins.test.ts | 12 ++-- .../browser/__tests__/BrowserClient.test.ts | 55 +++++++--------- .../compat/LDClientCompatImpl.test.ts | 32 ++++----- packages/sdk/browser/src/BrowserClient.ts | 66 +++++++++++++++++-- packages/sdk/browser/src/LDClient.ts | 36 +--------- .../browser/src/compat/LDClientCompatImpl.ts | 7 +- packages/sdk/browser/src/index.ts | 4 +- 7 files changed, 114 insertions(+), 98 deletions(-) diff --git a/packages/sdk/browser/__tests__/BrowserClient.plugins.test.ts b/packages/sdk/browser/__tests__/BrowserClient.plugins.test.ts index ca03faf6ed..bc4c8e4e1e 100644 --- a/packages/sdk/browser/__tests__/BrowserClient.plugins.test.ts +++ b/packages/sdk/browser/__tests__/BrowserClient.plugins.test.ts @@ -6,7 +6,7 @@ import { LDLogger, } from '@launchdarkly/js-client-sdk-common'; -import { BrowserClient } from '../src/BrowserClient'; +import { makeClient } from '../src/BrowserClient'; import { LDPlugin } from '../src/LDPlugin'; import { BrowserOptions } from '../src/options'; import { makeBasicPlatform } from './BrowserClient.mocks'; @@ -41,7 +41,7 @@ it('registers plugins and executes hooks during initialization', async () => { const platform = makeBasicPlatform(); - const client = new BrowserClient( + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { @@ -132,7 +132,7 @@ it('registers multiple plugins and executes all hooks', async () => { const platform = makeBasicPlatform(); - const client = new BrowserClient( + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { @@ -198,8 +198,7 @@ it('passes correct environmentMetadata to plugin getHooks and register functions const platform = makeBasicPlatform(options); - // eslint-disable-next-line no-new - new BrowserClient( + makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { @@ -280,8 +279,7 @@ it('passes correct environmentMetadata without optional fields', async () => { const platform = makeBasicPlatform(); - // eslint-disable-next-line no-new - new BrowserClient( + makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { diff --git a/packages/sdk/browser/__tests__/BrowserClient.test.ts b/packages/sdk/browser/__tests__/BrowserClient.test.ts index c29ef97bd3..0d06abde70 100644 --- a/packages/sdk/browser/__tests__/BrowserClient.test.ts +++ b/packages/sdk/browser/__tests__/BrowserClient.test.ts @@ -4,7 +4,7 @@ import { LDSingleKindContext, } from '@launchdarkly/js-client-sdk-common'; -import { BrowserClient } from '../src/BrowserClient'; +import { makeClient } from '../src/BrowserClient'; import { makeBasicPlatform } from './BrowserClient.mocks'; import { goodBootstrapDataWithReasons } from './testBootstrapData'; @@ -27,7 +27,7 @@ describe('given a mock platform for a BrowserClient', () => { }); it('includes urls in custom events', async () => { - const client = new BrowserClient( + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { @@ -56,7 +56,7 @@ describe('given a mock platform for a BrowserClient', () => { }); it('can filter URLs in custom events', async () => { - const client = new BrowserClient( + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { @@ -90,7 +90,7 @@ describe('given a mock platform for a BrowserClient', () => { }); it('can filter URLs in click events', async () => { - const client = new BrowserClient( + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { @@ -133,7 +133,7 @@ describe('given a mock platform for a BrowserClient', () => { }); it('can filter URLs in pageview events', async () => { - const client = new BrowserClient( + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { @@ -163,7 +163,7 @@ describe('given a mock platform for a BrowserClient', () => { }); it('can use bootstrap data', async () => { - const client = new BrowserClient( + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { @@ -189,17 +189,17 @@ describe('given a mock platform for a BrowserClient', () => { }); }); - it('can shed intermediate identifyResult calls', async () => { - const client = new BrowserClient( + it('can shed intermediate identify calls', async () => { + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { streaming: false, logger, diagnosticOptOut: true, sendEvents: false, fetchGoals: false }, platform, ); - const promise1 = client.identifyResult({ key: 'user-key-1', kind: 'user' }); - const promise2 = client.identifyResult({ key: 'user-key-2', kind: 'user' }); - const promise3 = client.identifyResult({ key: 'user-key-3', kind: 'user' }); + const promise1 = client.identify({ key: 'user-key-1', kind: 'user' }); + const promise2 = client.identify({ key: 'user-key-2', kind: 'user' }); + const promise3 = client.identify({ key: 'user-key-3', kind: 'user' }); const [result1, result2, result3] = await Promise.all([promise1, promise2, promise3]); @@ -212,7 +212,7 @@ describe('given a mock platform for a BrowserClient', () => { it('calls beforeIdentify in order', async () => { const order: string[] = []; - const client = new BrowserClient( + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { @@ -250,7 +250,7 @@ describe('given a mock platform for a BrowserClient', () => { it('completes identify calls in order', async () => { const order: string[] = []; - const client = new BrowserClient( + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { @@ -292,7 +292,7 @@ describe('given a mock platform for a BrowserClient', () => { it('completes awaited identify calls in order without shedding', async () => { const order: string[] = []; - const client = new BrowserClient( + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { @@ -323,9 +323,9 @@ describe('given a mock platform for a BrowserClient', () => { platform, ); - const result1 = await client.identifyResult({ key: 'user-key-1', kind: 'user' }); - const result2 = await client.identifyResult({ key: 'user-key-2', kind: 'user' }); - const result3 = await client.identifyResult({ key: 'user-key-3', kind: 'user' }); + const result1 = await client.identify({ key: 'user-key-1', kind: 'user' }); + const result2 = await client.identify({ key: 'user-key-2', kind: 'user' }); + const result3 = await client.identify({ key: 'user-key-3', kind: 'user' }); expect(result1.status).toEqual('completed'); expect(result2.status).toEqual('completed'); @@ -335,8 +335,8 @@ describe('given a mock platform for a BrowserClient', () => { expect(order).toEqual(['user-key-1', 'user-key-2', 'user-key-3']); }); - it('can shed intermediate identify calls', async () => { - const client = new BrowserClient( + it('can shed intermediate identify calls without waiting for results', async () => { + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { streaming: false, logger, diagnosticOptOut: true, sendEvents: false, fetchGoals: false }, @@ -354,25 +354,16 @@ describe('given a mock platform for a BrowserClient', () => { }); it('it does not shed non-shedable identify calls', async () => { - const client = new BrowserClient( + const client = makeClient( 'client-side-id', AutoEnvAttributes.Disabled, { streaming: false, logger, diagnosticOptOut: true, sendEvents: false, fetchGoals: false }, platform, ); - const promise1 = client.identifyResult( - { key: 'user-key-1', kind: 'user' }, - { sheddable: false }, - ); - const promise2 = client.identifyResult( - { key: 'user-key-2', kind: 'user' }, - { sheddable: false }, - ); - const promise3 = client.identifyResult( - { key: 'user-key-3', kind: 'user' }, - { sheddable: false }, - ); + const promise1 = client.identify({ key: 'user-key-1', kind: 'user' }, { sheddable: false }); + const promise2 = client.identify({ key: 'user-key-2', kind: 'user' }, { sheddable: false }); + const promise3 = client.identify({ key: 'user-key-3', kind: 'user' }, { sheddable: false }); const [result1, result2, result3] = await Promise.all([promise1, promise2, promise3]); diff --git a/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts b/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts index 41300b0f27..a213b9e8d0 100644 --- a/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts +++ b/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts @@ -2,12 +2,14 @@ import { jest } from '@jest/globals'; import { LDContext, LDFlagSet } from '@launchdarkly/js-client-sdk-common'; -import { BrowserClient } from '../../src/BrowserClient'; +// Import after mocking +import { makeClient } from '../../src/BrowserClient'; import LDClientCompatImpl from '../../src/compat/LDClientCompatImpl'; import { LDOptions } from '../../src/compat/LDCompatOptions'; +import { LDClient } from '../../src/LDClient'; // @ts-ignore -const mockBrowserClient: jest.MockedObject = { +const mockBrowserClient: jest.MockedObject = { identify: jest.fn(), allFlags: jest.fn(), close: jest.fn(), @@ -15,7 +17,6 @@ const mockBrowserClient: jest.MockedObject = { setStreaming: jest.fn(), on: jest.fn(), off: jest.fn(), - sdkKey: 'test-sdk-key', variation: jest.fn(), variationDetail: jest.fn(), boolVariation: jest.fn(), @@ -39,17 +40,18 @@ const mockBrowserClient: jest.MockedObject = { jest.mock('../../src/BrowserClient', () => ({ __esModule: true, - BrowserClient: jest.fn(() => mockBrowserClient), + makeClient: jest.fn(), })); +const mockMakeClient = makeClient as jest.MockedFunction; + afterEach(() => { - jest.resetAllMocks(); + jest.clearAllMocks(); }); beforeEach(() => { - // TypesScript doesn't understand that the BrowserClient is a mock. - // @ts-ignore - BrowserClient.mockImplementation(() => mockBrowserClient); + // Restore the mock implementation after clearing + mockMakeClient.mockReturnValue(mockBrowserClient); }); describe('given a LDClientCompatImpl client with mocked browser client that is immediately ready', () => { @@ -57,13 +59,13 @@ describe('given a LDClientCompatImpl client with mocked browser client that is i beforeEach(() => { jest.useFakeTimers(); - mockBrowserClient.identify.mockImplementation(() => Promise.resolve()); + mockBrowserClient.identify.mockImplementation(() => Promise.resolve({ status: 'completed' })); client = new LDClientCompatImpl('env-key', { kind: 'user', key: 'user-key' }); }); it('should resolve waitForInitialization when the client is already initialized', async () => { jest.advanceTimersToNextTimer(); - mockBrowserClient.identify.mockResolvedValue(undefined); + mockBrowserClient.identify.mockResolvedValue({ status: 'completed' }); await expect(client.waitForInitialization()).resolves.toBeUndefined(); expect(mockBrowserClient.identify).toHaveBeenCalledWith( @@ -92,7 +94,7 @@ describe('given a LDClientCompatImpl client with mocked browser client that init const context: LDContext = { kind: 'user', key: 'new-user' }; const mockFlags: LDFlagSet = { flag1: true, flag2: false }; - mockBrowserClient.identify.mockResolvedValue(); + mockBrowserClient.identify.mockResolvedValue({ status: 'completed' }); mockBrowserClient.allFlags.mockReturnValue(mockFlags); const result = await client.identify(context); @@ -107,7 +109,7 @@ describe('given a LDClientCompatImpl client with mocked browser client that init const mockFlags: LDFlagSet = { flag1: true, flag2: false }; mockBrowserClient.allFlags.mockReturnValue(mockFlags); - mockBrowserClient.identify.mockImplementation(() => Promise.resolve()); + mockBrowserClient.identify.mockImplementation(() => Promise.resolve({ status: 'completed' })); // Starting advancing the timers for the nest call. The wrapped promises // do not resolve sychronously. jest.advanceTimersToNextTimerAsync(); @@ -161,7 +163,7 @@ describe('given a LDClientCompatImpl client with mocked browser client that init it('should resolve waitForInitialization when the client is initialized', async () => { jest.advanceTimersToNextTimer(); - mockBrowserClient.identify.mockResolvedValue(undefined); + mockBrowserClient.identify.mockResolvedValue({ status: 'completed' }); await expect(client.waitForInitialization()).resolves.toBeUndefined(); expect(mockBrowserClient.identify).toHaveBeenCalledWith( @@ -172,7 +174,7 @@ describe('given a LDClientCompatImpl client with mocked browser client that init it('should resolve second waitForInitialization immediately', async () => { jest.advanceTimersToNextTimer(); - mockBrowserClient.identify.mockResolvedValue(undefined); + mockBrowserClient.identify.mockResolvedValue({ status: 'completed' }); await expect(client.waitForInitialization()).resolves.toBeUndefined(); await expect(client.waitForInitialization()).resolves.toBeUndefined(); @@ -184,7 +186,7 @@ describe('given a LDClientCompatImpl client with mocked browser client that init it('should resolve waitUntilReady immediately if the client is already initialized', async () => { jest.advanceTimersToNextTimer(); - mockBrowserClient.identify.mockResolvedValue(undefined); + mockBrowserClient.identify.mockResolvedValue({ status: 'completed' }); await expect(client.waitUntilReady()).resolves.toBeUndefined(); expect(mockBrowserClient.identify).toHaveBeenCalledWith( diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index 5f27e85504..abb9ceb4c2 100644 --- a/packages/sdk/browser/src/BrowserClient.ts +++ b/packages/sdk/browser/src/BrowserClient.ts @@ -5,11 +5,13 @@ import { Configuration, Encoding, FlagManager, + Hook, internal, LDClientImpl, LDContext, LDEmitter, LDEmitterEventName, + LDFlagValue, LDHeaders, LDIdentifyResult, LDPluginEnvironmentMetadata, @@ -23,11 +25,13 @@ import { registerStateDetection } from './BrowserStateDetector'; import GoalManager from './goals/GoalManager'; import { Goal, isClick } from './goals/Goals'; import { LDClient } from './LDClient'; +import { LDPlugin } from './LDPlugin'; import validateBrowserOptions, { BrowserOptions, filterToBaseOptionsWithDefaults } from './options'; import BrowserPlatform from './platform/BrowserPlatform'; -export class BrowserClient extends LDClientImpl implements LDClient { +class BrowserClientImpl extends LDClientImpl { private readonly _goalManager?: GoalManager; + private readonly _plugins?: LDPlugin[]; constructor( clientSideId: string, @@ -132,6 +136,8 @@ export class BrowserClient extends LDClientImpl implements LDClient { this.setEventSendingEnabled(true, false); + this._plugins = validatedBrowserOptions.plugins; + if (validatedBrowserOptions.fetchGoals) { this._goalManager = new GoalManager( clientSideId, @@ -180,11 +186,14 @@ export class BrowserClient extends LDClientImpl implements LDClient { registerStateDetection(() => this.flush()); } } + } + + registerPlugins(client: LDClient): void { internal.safeRegisterPlugins( - logger, + this.logger, this.environmentMetadata, - this, - validatedBrowserOptions.plugins, + client, + this._plugins || [], ); } @@ -231,3 +240,52 @@ export class BrowserClient extends LDClientImpl implements LDClient { this._updateAutomaticStreamingState(); } } + +export function makeClient( + clientSideId: string, + autoEnvAttributes: AutoEnvAttributes, + options: BrowserOptions = {}, + overridePlatform?: Platform, +): LDClient { + const impl = new BrowserClientImpl(clientSideId, autoEnvAttributes, options, overridePlatform); + + // Return a PIMPL style implementation. This decouples the interface from the interface of the implementation. + // In the future we should consider updating the common SDK code to not use inheritance and instead compose + // the leaf-implementation. + // The purpose for this in the short-term is to have a signature for identify that is different than the class implementation. + // Using an object with PIMPL here also allows us to completely hide the underlying implementation, where with a class + // it is trivial to access what should be protected (or even private) fields. + const client: LDClient = { + variation: (key: string, defaultValue?: LDFlagValue) => impl.variation(key, defaultValue), + variationDetail: (key: string, defaultValue?: LDFlagValue) => + impl.variationDetail(key, defaultValue), + boolVariation: (key: string, defaultValue: boolean) => impl.boolVariation(key, defaultValue), + boolVariationDetail: (key: string, defaultValue: boolean) => + impl.boolVariationDetail(key, defaultValue), + numberVariation: (key: string, defaultValue: number) => impl.numberVariation(key, defaultValue), + numberVariationDetail: (key: string, defaultValue: number) => + impl.numberVariationDetail(key, defaultValue), + stringVariation: (key: string, defaultValue: string) => impl.stringVariation(key, defaultValue), + stringVariationDetail: (key: string, defaultValue: string) => + impl.stringVariationDetail(key, defaultValue), + jsonVariation: (key: string, defaultValue: unknown) => impl.jsonVariation(key, defaultValue), + jsonVariationDetail: (key: string, defaultValue: unknown) => + impl.jsonVariationDetail(key, defaultValue), + track: (key: string, data?: any, metricValue?: number) => impl.track(key, data, metricValue), + on: (key: LDEmitterEventName, callback: (...args: any[]) => void) => impl.on(key, callback), + off: (key: LDEmitterEventName, callback: (...args: any[]) => void) => impl.off(key, callback), + flush: () => impl.flush(), + setStreaming: (streaming?: boolean) => impl.setStreaming(streaming), + identify: (pristineContext: LDContext, identifyOptions?: LDIdentifyOptions) => + impl.identifyResult(pristineContext, identifyOptions), + getContext: () => impl.getContext(), + close: () => impl.close(), + allFlags: () => impl.allFlags(), + addHook: (hook: Hook) => impl.addHook(hook), + logger: impl.logger, + }; + + impl.registerPlugins(client); + + return client; +} diff --git a/packages/sdk/browser/src/LDClient.ts b/packages/sdk/browser/src/LDClient.ts index e0ae2c1681..6c4f515eef 100644 --- a/packages/sdk/browser/src/LDClient.ts +++ b/packages/sdk/browser/src/LDClient.ts @@ -37,40 +37,6 @@ export type LDClient = Omit< */ setStreaming(streaming?: boolean): void; - /** - * Identifies a context to LaunchDarkly and returns a promise which resolves to an object containing the result of - * the identify operation. - * - * Unlike the server-side SDKs, the client-side JavaScript SDKs maintain a current context state, - * which is set when you call `identify()`. - * - * Changing the current context also causes all feature flag values to be reloaded. Until that has - * finished, calls to {@link variation} will still return flag values for the previous context. You can - * await the Promise to determine when the new flag values are available. - * - * This function will shed intermediate identify operations by default. For example, if you call identify 3 times in - * a row, without waiting for the previous one to complete, the middle call to identify may be discarded. To disable - * this set `sheddable` to `false` in the `identifyOptions` parameter. - * - * @param context - * The LDContext object. - * @param identifyOptions - * Optional configuration. Please see {@link LDIdentifyOptions}. - * @returns - * A Promise which resolves when the flag values for the specified - * context are available. It rejects when: - * - * 1. The context is unspecified or has no key. - * - * 2. The identify timeout is exceeded. In client SDKs this defaults to 5s. - * You can customize this timeout with {@link LDIdentifyOptions | identifyOptions}. - * - * 3. A network error is encountered during initialization. - * - * @ignore Implementation Note: Browser implementation has different options. - */ - identify(context: LDContext, identifyOptions?: LDIdentifyOptions): Promise; - /** * Identifies a context to LaunchDarkly and returns a promise which resolves to an object containing the result of * the identify operation. @@ -96,7 +62,7 @@ export type LDClient = Omit< * * @ignore Implementation Note: Browser implementation has different options. */ - identifyResult( + identify( pristineContext: LDContext, identifyOptions?: LDIdentifyOptions, ): Promise; diff --git a/packages/sdk/browser/src/compat/LDClientCompatImpl.ts b/packages/sdk/browser/src/compat/LDClientCompatImpl.ts index 9014b94a4c..632b9731ef 100644 --- a/packages/sdk/browser/src/compat/LDClientCompatImpl.ts +++ b/packages/sdk/browser/src/compat/LDClientCompatImpl.ts @@ -12,14 +12,15 @@ import { LDTimeoutError, } from '@launchdarkly/js-client-sdk-common'; -import { BrowserClient } from '../BrowserClient'; +import { makeClient } from '../BrowserClient'; +import { LDClient as BrowserLDClient } from '../LDClient'; import { LDClient } from './LDClientCompat'; import { LDOptions } from './LDCompatOptions'; import LDEmitterCompat, { CompatEventName } from './LDEmitterCompat'; import { wrapPromiseCallback } from './wrapPromiseCallback'; export default class LDClientCompatImpl implements LDClient { - private _client: BrowserClient; + private _client: BrowserLDClient; public readonly logger: LDLogger; private _initResolve?: () => void; @@ -41,7 +42,7 @@ export default class LDClientCompatImpl implements LDClient { const cleanedOptions = { ...options }; delete cleanedOptions.bootstrap; delete cleanedOptions.hash; - this._client = new BrowserClient(envKey, AutoEnvAttributes.Disabled, options); + this._client = makeClient(envKey, AutoEnvAttributes.Disabled, options); this._emitter = new LDEmitterCompat(this._client); this.logger = this._client.logger; this._initIdentify(context, bootstrap, hash); diff --git a/packages/sdk/browser/src/index.ts b/packages/sdk/browser/src/index.ts index 9485aea0a4..a4d63959f3 100644 --- a/packages/sdk/browser/src/index.ts +++ b/packages/sdk/browser/src/index.ts @@ -12,7 +12,7 @@ */ import { AutoEnvAttributes } from '@launchdarkly/js-client-sdk-common'; -import { BrowserClient } from './BrowserClient'; +import { makeClient } from './BrowserClient'; import { LDClient } from './LDClient'; import { BrowserOptions as LDOptions } from './options'; @@ -38,5 +38,5 @@ export type { LDPlugin } from './LDPlugin'; */ export function initialize(clientSideId: string, options?: LDOptions): LDClient { // AutoEnvAttributes are not supported yet in the browser SDK. - return new BrowserClient(clientSideId, AutoEnvAttributes.Disabled, options); + return makeClient(clientSideId, AutoEnvAttributes.Disabled, options); }