From ae4de36be488edb5182545463e4eb1c2bb62f524 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 22 Oct 2024 15:44:04 -0700 Subject: [PATCH 1/5] feat: Add a module for increased backward compatibility. --- .../compat/LDClientCompatImpl.test.ts | 149 +++++++++++ packages/sdk/browser/jest.config.js | 1 + packages/sdk/browser/package.json | 13 +- packages/sdk/browser/rollup.config.js | 28 +-- packages/sdk/browser/setup-jest.js | 23 ++ .../sdk/browser/src/compat/LDClientCompat.ts | 122 +++++++++ .../browser/src/compat/LDClientCompatImpl.ts | 232 ++++++++++++++++++ .../sdk/browser/src/compat/LDCompatOptions.ts | 19 ++ .../sdk/browser/src/compat/LDEmitterCompat.ts | 77 ++++++ packages/sdk/browser/src/compat/index.ts | 50 ++++ .../browser/src/compat/wrapPromiseCallback.ts | 40 +++ .../shared/sdk-client/src/LDClientImpl.ts | 28 ++- .../shared/sdk-client/src/api/LDClient.ts | 3 +- .../sdk-client/src/api/LDIdentifyOptions.ts | 13 + 14 files changed, 769 insertions(+), 29 deletions(-) create mode 100644 packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts create mode 100644 packages/sdk/browser/setup-jest.js create mode 100644 packages/sdk/browser/src/compat/LDClientCompat.ts create mode 100644 packages/sdk/browser/src/compat/LDClientCompatImpl.ts create mode 100644 packages/sdk/browser/src/compat/LDCompatOptions.ts create mode 100644 packages/sdk/browser/src/compat/LDEmitterCompat.ts create mode 100644 packages/sdk/browser/src/compat/index.ts create mode 100644 packages/sdk/browser/src/compat/wrapPromiseCallback.ts diff --git a/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts b/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts new file mode 100644 index 000000000..b5a366572 --- /dev/null +++ b/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts @@ -0,0 +1,149 @@ +import { jest } from '@jest/globals'; + +import { LDContext, LDFlagSet, LDLogger } from '@launchdarkly/js-client-sdk-common'; + +const mockBrowserClient = { + identify: jest.fn(), + allFlags: jest.fn(), + close: jest.fn(), + flush: jest.fn(), + _emitter: jest.fn(() => ({ + emit: jest.fn(), + on: jest.fn(), + off: jest.fn(), + })), +}; + +jest.unstable_mockModule('../../src/BrowserClient', () => ({ + __esModule: true, + BrowserClient: jest.fn(), +})); + +const { default: LDClientCompatImpl } = await import('../../src/compat/LDClientCompatImpl'); + +describe('given a LDClientCompatImpl client with mocked browser client', () => { + // @ts-ignore + let client: LDClientCompatImpl; + // let mockBrowserClient: jest.Mocked; + let mockLogger: LDLogger; + + beforeEach(() => { + // mockBrowserClient = { + // identify: jest.fn(), + // allFlags: jest.fn(), + // close: jest.fn(), + // flush: jest.fn(), + // } as unknown as jest.Mocked; + + // (BrowserClient as jest.MockedClass).mockImplementation( + // () => mockBrowserClient, + // ); + mockLogger = { + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + }; + client = new LDClientCompatImpl( + 'env-key', + { kind: 'user', key: 'user-key' }, + { logger: mockLogger }, + ); + }); + + it('should return a promise from identify when no callback is provided', async () => { + const context: LDContext = { kind: 'user', key: 'new-user' }; + const mockFlags: LDFlagSet = { flag1: true, flag2: false }; + // @ts-ignore + mockBrowserClient.identify.mockResolvedValue(undefined); + mockBrowserClient.allFlags.mockReturnValue(mockFlags); + + const result = await client.identify(context); + + expect(mockBrowserClient.identify).toHaveBeenCalledWith(context, { hash: undefined }); + expect(result).toEqual(mockFlags); + }); + + it('should call the callback when provided to identify', (done) => { + const context: LDContext = { kind: 'user', key: 'new-user' }; + const mockFlags: LDFlagSet = { flag1: true, flag2: false }; + // @ts-ignore + mockBrowserClient.identify.mockResolvedValue(undefined); + mockBrowserClient.allFlags.mockReturnValue(mockFlags); + + // @ts-ignore + client.identify(context, undefined, (err, flags) => { + expect(err).toBeNull(); + expect(flags).toEqual(mockFlags); + done(); + }); + }); + + it('should return a promise from close when no callback is provided', async () => { + // @ts-ignore + mockBrowserClient.close.mockResolvedValue(); + + await expect(client.close()).resolves.toBeUndefined(); + expect(mockBrowserClient.close).toHaveBeenCalled(); + }); + + it('should call the callback when provided to close', (done) => { + // @ts-ignore + mockBrowserClient.close.mockResolvedValue(); + + client.close(() => { + expect(mockBrowserClient.close).toHaveBeenCalled(); + done(); + }); + }); + + it('should return a promise from flush when no callback is provided', async () => { + // @ts-ignore + mockBrowserClient.flush.mockResolvedValue({ result: true }); + + await expect(client.flush()).resolves.toBeUndefined(); + expect(mockBrowserClient.flush).toHaveBeenCalled(); + }); + + it('should call the callback when provided to flush', (done) => { + // @ts-ignore + mockBrowserClient.flush.mockResolvedValue({ result: true }); + + client.flush(() => { + expect(mockBrowserClient.flush).toHaveBeenCalled(); + done(); + }); + }); + + // it('should resolve immediately if the client is already initialized', async () => { + // mockBrowserClient.waitForInitialization.mockResolvedValue(undefined); + + // await expect(client.waitForInitialization()).resolves.toBeUndefined(); + // expect(mockBrowserClient.waitForInitialization).toHaveBeenCalledWith({ noTimeout: true }); + // }); + + // it('should log a warning when no timeout is specified for waitForInitialization', async () => { + // mockBrowserClient.waitForInitialization.mockResolvedValue(undefined); + + // await client.waitForInitialization(); + + // expect(mockLogger.warn).toHaveBeenCalledWith( + // expect.stringContaining('The waitForInitialization function was called without a timeout specified.') + // ); + // }); + + // it('should apply a timeout when specified for waitForInitialization', async () => { + // mockBrowserClient.waitForInitialization.mockResolvedValue(undefined); + + // await client.waitForInitialization(5); + + // expect(mockBrowserClient.waitForInitialization).toHaveBeenCalledWith({ timeout: 5 }); + // }); + + // it('should reject with a timeout error when initialization takes too long', async () => { + // mockBrowserClient.waitForInitialization.mockRejectedValue(new Error('Timeout')); + + // await expect(client.waitForInitialization(1)).rejects.toThrow('Timeout'); + // expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('waitForInitialization timed out')); + // }); +}); diff --git a/packages/sdk/browser/jest.config.js b/packages/sdk/browser/jest.config.js index e6a722469..ee1376dd9 100644 --- a/packages/sdk/browser/jest.config.js +++ b/packages/sdk/browser/jest.config.js @@ -5,4 +5,5 @@ export default { testEnvironment: 'jest-environment-jsdom', testPathIgnorePatterns: ['./dist', './src'], testMatch: ['**.test.ts'], + setupFiles: ['./setup-jest.js'], }; diff --git a/packages/sdk/browser/package.json b/packages/sdk/browser/package.json index 1973107c3..6c9cb297f 100644 --- a/packages/sdk/browser/package.json +++ b/packages/sdk/browser/package.json @@ -17,9 +17,16 @@ "sdk" ], "exports": { - "types": "./dist/src/index.d.ts", - "require": "./dist/index.cjs.js", - "import": "./dist/index.es.js" + ".": { + "types": "./dist/src/index.d.ts", + "require": "./dist/index.cjs.js", + "import": "./dist/index.es.js" + }, + "./compat": { + "types": "./dist/src/compat/index.d.ts", + "require": "./dist/compat.cjs.js", + "import": "./dist/compat.es.js" + } }, "type": "module", "files": [ diff --git a/packages/sdk/browser/rollup.config.js b/packages/sdk/browser/rollup.config.js index a2c260896..f1810e932 100644 --- a/packages/sdk/browser/rollup.config.js +++ b/packages/sdk/browser/rollup.config.js @@ -5,19 +5,17 @@ import terser from '@rollup/plugin-terser'; import typescript from '@rollup/plugin-typescript'; import { visualizer } from 'rollup-plugin-visualizer'; -const getSharedConfig = (format, file) => ({ - input: 'src/index.ts', - output: [ - { - format: format, - sourcemap: true, - file: file, - }, - ], - onwarn: (warning) => { - if (warning.code !== 'CIRCULAR_DEPENDENCY') { - console.error(`(!) ${warning.message}`); - } +const getSharedConfig = (format, extension) => ({ + input: { + index: 'src/index.ts', + compat: 'src/compat/index.ts' + }, + output: + { + format: format, + sourcemap: true, + dir: 'dist', + entryFileNames: '[name]' + extension }, }); @@ -35,7 +33,7 @@ const terserOpts = { export default [ { - ...getSharedConfig('es', 'dist/index.es.js'), + ...getSharedConfig('es', '.es.js'), plugins: [ typescript({ module: 'esnext', @@ -52,7 +50,7 @@ export default [ ], }, { - ...getSharedConfig('cjs', 'dist/index.cjs.js'), + ...getSharedConfig('cjs', '.cjs.js'), plugins: [typescript(), common(), resolve(), terser(terserOpts), json()], }, ]; diff --git a/packages/sdk/browser/setup-jest.js b/packages/sdk/browser/setup-jest.js new file mode 100644 index 000000000..42dcb53a2 --- /dev/null +++ b/packages/sdk/browser/setup-jest.js @@ -0,0 +1,23 @@ +import { TextEncoder, TextDecoder } from 'node:util'; +import * as crypto from 'crypto'; + +global.TextEncoder = TextEncoder; + +Object.assign(window, { TextDecoder, TextEncoder }); + +Object.defineProperty(global.self, "crypto", { + value: { + getRandomValues: (arr) => crypto.randomBytes(arr.length), + subtle: { + digest: (algorithm, data) => { + return new Promise((resolve, reject) => + resolve( + crypto.createHash(algorithm.toLowerCase().replace("-", "")) + .update(data) + .digest() + ) + ); + }, + }, + }, +}); diff --git a/packages/sdk/browser/src/compat/LDClientCompat.ts b/packages/sdk/browser/src/compat/LDClientCompat.ts new file mode 100644 index 000000000..195e0ea27 --- /dev/null +++ b/packages/sdk/browser/src/compat/LDClientCompat.ts @@ -0,0 +1,122 @@ +import { LDContext, LDFlagSet } from '@launchdarkly/js-client-sdk-common'; +import { LDClient as LDCLientBrowser } from '../BrowserClient'; + +/** + * Compatibility interface. This interface extends the base LDCLient interface with functions + * that improve backwards compatibility. + * + * If starting a new project please import the root package instead of `/compat`. + * + * In the `launchdarkly-js-client-sdk@3.x` package a number of functions had the return typings + * incorrect. Any function which optionally returned a promise based on a callback had incorrect + * typings. Those have been corrected in this implementation. + */ +export interface LDClient extends Omit { + /** + * Identifies a context to LaunchDarkly. + * + * Unlike the server-side SDKs, the client-side JavaScript SDKs maintain a current context state, + * which is set at initialization time. You only need to call `identify()` if the context has changed + * since then. + * + * 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 + * use a callback or a Promise to determine when the new flag values are available. + * + * @param context + * The context properties. Must contain at least the `key` property. + * @param hash + * The signed context key if you are using [Secure Mode](https://docs.launchdarkly.com/sdk/features/secure-mode#configuring-secure-mode-in-the-javascript-client-side-sdk). + * @param onDone + * A function which will be called as soon as the flag values for the new context are available, + * with two parameters: an error value (if any), and an {@link LDFlagSet} containing the new values + * (which can also be obtained by calling {@link variation}). If the callback is omitted, you will + * receive a Promise instead. + * @returns + * If you provided a callback, then nothing. Otherwise, a Promise which resolve once the flag + * values for the new context are available, providing an {@link LDFlagSet} containing the new values + * (which can also be obtained by calling {@link variation}). + */ + identify( + context: LDContext, + hash?: string, + onDone?: (err: Error | null, flags: LDFlagSet | null) => void + ): Promise | undefined; + + /** + * Returns a Promise that tracks the client's initialization state. + * + * The Promise will be resolved if the client successfully initializes, or rejected if client + * initialization has irrevocably failed (for instance, if it detects that the SDK key is invalid). + * + * ``` + * // using Promise then() and catch() handlers + * client.waitForInitialization(5).then(() => { + * doSomethingWithSuccessfullyInitializedClient(); + * }).catch(err => { + * doSomethingForFailedStartup(err); + * }); + * + * // using async/await + * try { + * await client.waitForInitialization(5); + * doSomethingWithSuccessfullyInitializedClient(); + * } catch (err) { + * doSomethingForFailedStartup(err); + * } + * ``` + * + * It is important that you handle the rejection case; otherwise it will become an unhandled Promise + * rejection, which is a serious error on some platforms. The Promise is not created unless you + * request it, so if you never call `waitForInitialization()` then you do not have to worry about + * unhandled rejections. + * + * Note that you can also use event listeners ({@link on}) for the same purpose: the event `"initialized"` + * indicates success, and `"failed"` indicates failure. + * + * @param timeout + * The amount of time, in seconds, to wait for initialization before rejecting the promise. + * Using a large timeout is not recommended. If you use a large timeout and await it, then + * any network delays will cause your application to wait a long time before + * continuing execution. + * + * If no timeout is specified, then the returned promise will only be resolved when the client + * successfully initializes or initialization fails. + * + * @returns + * A Promise that will be resolved if the client initializes successfully, or rejected if it + * fails or the specified timeout elapses. + */ + waitForInitialization(timeout?: number): Promise; + + /** + * Shuts down the client and releases its resources, after delivering any pending analytics + * events. + * + * @param onDone + * A function which will be called when the operation completes. If omitted, you + * will receive a Promise instead. + * + * @returns + * If you provided a callback, then nothing. Otherwise, a Promise which resolves once + * closing is finished. It will never be rejected. + */ + close(onDone?: () => void): Promise | undefined; + + /** + * Flushes all pending analytics events. + * + * Normally, batches of events are delivered in the background at intervals determined by the + * `flushInterval` property of {@link LDOptions}. Calling `flush()` triggers an immediate delivery. + * + * @param onDone + * A function which will be called when the flush completes. If omitted, you + * will receive a Promise instead. + * + * @returns + * If you provided a callback, then nothing. Otherwise, a Promise which resolves once + * flushing is finished. Note that the Promise will be rejected if the HTTP request + * fails, so be sure to attach a rejection handler to it. + */ + flush(onDone?: () => void): Promise | undefined; +} diff --git a/packages/sdk/browser/src/compat/LDClientCompatImpl.ts b/packages/sdk/browser/src/compat/LDClientCompatImpl.ts new file mode 100644 index 000000000..720521713 --- /dev/null +++ b/packages/sdk/browser/src/compat/LDClientCompatImpl.ts @@ -0,0 +1,232 @@ +// TODO may or may not need this. +import { + AutoEnvAttributes, + cancelableTimedPromise, + Hook, + LDContext, + LDEmitterEventName, + LDEvaluationDetail, + LDEvaluationDetailTyped, + LDFlagSet, + LDFlagValue, + LDLogger, + LDTimeoutError, +} from '@launchdarkly/js-client-sdk-common'; + +import { BrowserClient } from '../BrowserClient'; +import { LDClient } from './LDClientCompat'; +import { LDOptions } from './LDCompatOptions'; +import LDEmitterCompat from './LDEmitterCompat'; +import { wrapPromiseCallback } from './wrapPromiseCallback'; + +export default class LDClientCompatImpl implements LDClient { + private _client: BrowserClient; + public readonly logger: LDLogger; + + private _initResolve?: () => void; + + private _initReject?: (err: Error) => void; + + private _rejectionReason: Error | undefined; + + private _initializedPromise?: Promise; + + private _initState: 'success' | 'failure' | 'initializing' = 'initializing'; + + private _emitter: LDEmitterCompat; + + constructor(envKey: string, context: LDContext, options?: LDOptions) { + const bootstrap = options?.bootstrap; + const hash = options?.hash; + + const cleanedOptions = { ...options }; + delete cleanedOptions.bootstrap; + delete cleanedOptions.hash; + this._client = new BrowserClient(envKey, AutoEnvAttributes.Disabled, options); + this._emitter = new LDEmitterCompat(this._client); + this.logger = this._client.logger; + this._initIdentify(context, bootstrap, hash); + } + + private async _initIdentify( + context: LDContext, + bootstrap?: LDFlagSet, + hash?: string, + ): Promise { + try { + await this._client.identify(context, { noTimeout: true, bootstrap, hash }); + this._initState = 'success'; + this._initResolve?.(); + this._emitter.emit('initialized'); + } catch (err) { + this._rejectionReason = err as Error; + this._initState = 'failure'; + this._initReject?.(err as Error); + this._emitter.emit('failed', err); + } + // Ready will always be emitted in addition to either 'initialized' or 'failed'. + this._emitter.emit('ready'); + } + + allFlags(): LDFlagSet { + return this._client.allFlags(); + } + + boolVariation(key: string, defaultValue: boolean): boolean { + return this._client.boolVariation(key, defaultValue); + } + + boolVariationDetail(key: string, defaultValue: boolean): LDEvaluationDetailTyped { + return this._client.boolVariationDetail(key, defaultValue); + } + + jsonVariation(key: string, defaultValue: unknown): unknown { + return this._client.jsonVariation(key, defaultValue); + } + + jsonVariationDetail(key: string, defaultValue: unknown): LDEvaluationDetailTyped { + return this._client.jsonVariationDetail(key, defaultValue); + } + + numberVariation(key: string, defaultValue: number): number { + return this._client.numberVariation(key, defaultValue); + } + + numberVariationDetail(key: string, defaultValue: number): LDEvaluationDetailTyped { + return this._client.numberVariationDetail(key, defaultValue); + } + + off(key: LDEmitterEventName, callback: (...args: any[]) => void): void { + this._emitter.off(key, callback); + } + + on(key: LDEmitterEventName, callback: (...args: any[]) => void): void { + this._emitter.on(key, callback); + } + + stringVariation(key: string, defaultValue: string): string { + return this._client.stringVariation(key, defaultValue); + } + + stringVariationDetail(key: string, defaultValue: string): LDEvaluationDetailTyped { + return this._client.stringVariationDetail(key, defaultValue); + } + + track(key: string, data?: any, metricValue?: number): void { + this._client.track(key, data, metricValue); + } + + variation(key: string, defaultValue?: LDFlagValue) { + return this._client.variation(key, defaultValue); + } + + variationDetail(key: string, defaultValue?: LDFlagValue): LDEvaluationDetail { + return this._client.variationDetail(key, defaultValue); + } + + addHook(hook: Hook): void { + this._client.addHook(hook); + } + + setStreaming(streaming?: boolean): void { + this._client.setStreaming(streaming); + } + + identify( + context: LDContext, + hash?: string, + onDone?: (err: Error | null, flags: LDFlagSet | null) => void, + ): Promise | undefined { + return wrapPromiseCallback( + this._client.identify(context, { hash }).then(() => this.allFlags()), + onDone, + ) as Promise | undefined; + // The typing here is a little funny. The wrapPromiseCallback can technically return + // `Promise`, but in the case where it would resolve to undefined we are not + // actually using the promise, because it means a callback was specified. + } + + close(onDone?: () => void): Promise | undefined { + return wrapPromiseCallback(this._client.close().then(), onDone); + } + + flush(onDone?: () => void): Promise | undefined { + // The .then() is to strip the return value making a void promise. + return wrapPromiseCallback(this._client.flush().then(), onDone); + } + + getContext(): LDContext | undefined { + return this._client.getContext(); + } + + waitForInitialization(timeout?: number): Promise { + // An initialization promise is only created if someone is going to use that promise. + // If we always created an initialization promise, and there was no call waitForInitialization + // by the time the promise was rejected, then that would result in an unhandled promise + // rejection. + + // It waitForInitialization was previously called, then we can use that promise even if it has + // been resolved or rejected. + if (this._initializedPromise) { + return this._promiseWithTimeout(this._initializedPromise, timeout); + } + + switch (this._initState) { + case 'success': + return Promise.resolve(); + case 'failure': + return Promise.reject(this._rejectionReason); + case 'initializing': + // Continue with the existing logic for creating and handling the promise + break; + default: + throw new Error( + 'Unexpected initialization state. This represents an implementation error in the SDK.', + ); + } + + if (timeout === undefined) { + this.logger?.warn( + 'The waitForInitialization function was called without a timeout specified.' + + ' In a future version a default timeout will be applied.', + ); + } + + if (!this._initializedPromise) { + this._initializedPromise = new Promise((resolve, reject) => { + this._initResolve = resolve; + this._initReject = reject; + }); + } + + return this._promiseWithTimeout(this._initializedPromise, timeout); + } + + /** + * Apply a timeout promise to a base promise. This is for use with waitForInitialization. + * Currently it returns a LDClient. In the future it should return a status. + * + * The client isn't always the expected type of the consumer. It returns an LDClient interface + * which is less capable than, for example, the node client interface. + * + * @param basePromise The promise to race against a timeout. + * @param timeout The timeout in seconds. + * @param logger A logger to log when the timeout expires. + * @returns + */ + private _promiseWithTimeout(basePromise: Promise, timeout?: number): Promise { + if (timeout) { + const cancelableTimeout = cancelableTimedPromise(timeout, 'waitForInitialization'); + return Promise.race([ + basePromise.then(() => cancelableTimeout.cancel()), + cancelableTimeout.promise, + ]).catch((reason) => { + if (reason instanceof LDTimeoutError) { + this.logger?.error(reason.message); + } + throw reason; + }); + } + return basePromise; + } +} diff --git a/packages/sdk/browser/src/compat/LDCompatOptions.ts b/packages/sdk/browser/src/compat/LDCompatOptions.ts new file mode 100644 index 000000000..95235f5b0 --- /dev/null +++ b/packages/sdk/browser/src/compat/LDCompatOptions.ts @@ -0,0 +1,19 @@ +import { LDFlagSet } from '@launchdarkly/js-client-sdk-common'; + +import { BrowserOptions } from '../options'; + +export interface LDOptions extends BrowserOptions { + /** + * The initial set of flags to use until the remote set is retrieved. + * + * For more information, refer to the + * [SDK Reference Guide](https://docs.launchdarkly.com/sdk/features/bootstrapping#javascript). + */ + bootstrap?: LDFlagSet; + + /** + * The signed canonical context key, for the initial context, if you are using + * [Secure Mode](https://docs.launchdarkly.com/sdk/features/secure-mode#configuring-secure-mode-in-the-javascript-client-side-sdk). + */ + hash?: string; +} diff --git a/packages/sdk/browser/src/compat/LDEmitterCompat.ts b/packages/sdk/browser/src/compat/LDEmitterCompat.ts new file mode 100644 index 000000000..dccf4abc5 --- /dev/null +++ b/packages/sdk/browser/src/compat/LDEmitterCompat.ts @@ -0,0 +1,77 @@ +import { LDLogger } from '@launchdarkly/js-sdk-common'; +import { LDEmitterEventName } from '@launchdarkly/js-client-sdk-common'; +import { LDClient } from '../BrowserClient'; + +type CompatOnlyEvents = 'ready' | 'failed' | 'initialized'; +export type CompatEventName = LDEmitterEventName | CompatOnlyEvents; + +const COMPAT_EVENTS: string[] = ['ready', 'failed', 'initialized']; + +export default class LDEmitterCompat { + private _listeners: Map = new Map(); + + constructor(private readonly _client: LDClient) { } + + on(name: CompatEventName, listener: Function) { + if (COMPAT_EVENTS.includes(name)) { + if (!this._listeners.has(name)) { + this._listeners.set(name, [listener]); + } else { + this._listeners.get(name)?.push(listener); + } + } else { + this._client.on(name, listener as (...args: any[]) => void) + } + } + + /** + * Unsubscribe one or all events. + * + * @param name + * @param listener Optional. If unspecified, all listeners for the event will be removed. + */ + off(name: CompatEventName, listener?: Function) { + if (COMPAT_EVENTS.includes(name)) { + const existingListeners = this._listeners.get(name); + if (!existingListeners) { + return; + } + + if (listener) { + const updated = existingListeners.filter((fn) => fn !== listener); + if (updated.length === 0) { + this._listeners.delete(name); + } else { + this._listeners.set(name, updated); + } + return; + } + + // listener was not specified, so remove them all for that event + this._listeners.delete(name); + } else { + this._client.off(name, listener as (...args: any[]) => void) + } + } + + private _invokeListener(listener: Function, name: CompatEventName, ...detail: any[]) { + try { + listener(...detail); + } catch (err) { + this._client.logger.error(`Encountered error invoking handler for "${name}", detail: "${err}"`); + } + } + + emit(name: CompatEventName, ...detail: any[]) { + const listeners = this._listeners.get(name); + listeners?.forEach((listener) => this._invokeListener(listener, name, ...detail)); + } + + eventNames(): string[] { + return [...this._listeners.keys()]; + } + + listenerCount(name: CompatEventName): number { + return this._listeners.get(name)?.length ?? 0; + } +} diff --git a/packages/sdk/browser/src/compat/index.ts b/packages/sdk/browser/src/compat/index.ts new file mode 100644 index 000000000..7ce71dafc --- /dev/null +++ b/packages/sdk/browser/src/compat/index.ts @@ -0,0 +1,50 @@ +/** + * This module provides a compatibility layer which emulates the interface used + * in the launchdarkly-js-client 3.x package. + * + * Some code changes may still be required, for example {@link LDOptions} removes + * support for some previously available options. + */ +import { AutoEnvAttributes } from '@launchdarkly/js-client-sdk-common'; +import { LDContext, LDOptions } from '..'; +import { LDClient } from './LDClientCompat'; +import LDClientCompatImpl from './LDClientCompatImpl'; + +/** + * Creates an instance of the LaunchDarkly client. This version of initialization is for + * improved backwards compatibility. In general the `initialize` function from the root packge + * should be used instead of the one in the `/compat` module. + * + * The client will begin attempting to connect to LaunchDarkly as soon as it is created. To + * determine when it is ready to use, call {@link LDClient.waitForInitialization}, or register an + * event listener for the `"ready"` event using {@link LDClient.on}. + * + * Example: + * import { initialize } from '@launchdarkly/js-client-sdk/compat'; + * const client = initialize(envKey, context, options); + * + * Note: The `compat` module minimizes compatibility breaks, but not all functionality is directly + * equivalent to the previous version. + * + * LDOptions are where the primary differences are. By default the new SDK implementation will + * generally use localStorage to cache flags. This can be disabled by setting the + * `maxCachedContexts` to 0. + * + * This does allow combinations that were not possible before. For insance an initial context + * could be identified using bootstrap, and a second context without bootstrap, and the second + * context could cache flags in local storage. For more control the primary module can be used + * instead of this `compat` module (for instance bootstrap can be provided per identify call in + * the primary module). + * + * @param envKey + * The environment ID. + * @param context + * The initial context properties. These can be changed later with {@link LDClient.identify}. + * @param options + * Optional configuration settings. + * @return + * The new client instance. + */ +export function initialize(envKey: string, context: LDContext, options?: LDOptions): LDClient { + return new LDClientCompatImpl(envKey, context, options); +} diff --git a/packages/sdk/browser/src/compat/wrapPromiseCallback.ts b/packages/sdk/browser/src/compat/wrapPromiseCallback.ts new file mode 100644 index 000000000..efcbc13be --- /dev/null +++ b/packages/sdk/browser/src/compat/wrapPromiseCallback.ts @@ -0,0 +1,40 @@ +/** + * Wrap a promise to invoke an optional callback upon resolution or rejection. + * + * This function assumes the callback follows the Node.js callback type: (err, value) => void + * + * If a callback is provided: + * - if the promise is resolved, invoke the callback with (null, value) + * - if the promise is rejected, invoke the callback with (error, null) + * + * @param {Promise} promise + * @param {Function} callback + * @returns Promise | undefined + */ +export function wrapPromiseCallback( + promise: Promise, + callback?: (err: Error | null, res: T | null) => void, +): Promise | undefined { + const ret = promise.then( + (value) => { + if (callback) { + setTimeout(() => { + callback(null, value); + }, 0); + } + return value; + }, + (error) => { + if (callback) { + setTimeout(() => { + callback(error, null); + }, 0); + } else { + return Promise.reject(error); + } + return undefined; + }, + ); + + return !callback ? ret : undefined; +} diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index a9abb0e49..ff6565b2b 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -39,13 +39,14 @@ import LDEmitter, { EventName } from './LDEmitter'; const { ClientMessages, ErrorKinds } = internal; +const DEFAULT_IDENIFY_TIMEOUT_SECONDS = 5; + export default class LDClientImpl implements LDClient { private readonly _config: Configuration; private _uncheckedContext?: LDContext; private _checkedContext?: Context; private readonly _diagnosticsManager?: internal.DiagnosticsManager; private _eventProcessor?: internal.EventProcessor; - private _identifyTimeout: number = 5; readonly logger: LDLogger; private _updateProcessor?: subsystem.LDStreamProcessor; @@ -181,7 +182,10 @@ export default class LDClientImpl implements LDClient { return this._checkedContext; } - private _createIdentifyPromise(timeout: number): { + private _createIdentifyPromise( + timeout: number, + noTimeout: boolean, + ): { identifyPromise: Promise; identifyResolve: () => void; identifyReject: (err: Error) => void; @@ -189,13 +193,17 @@ export default class LDClientImpl implements LDClient { let res: any; let rej: any; - const slow = new Promise((resolve, reject) => { + const basePromise = new Promise((resolve, reject) => { res = resolve; rej = reject; }); + if (noTimeout) { + return { identifyPromise: basePromise, identifyResolve: res, identifyReject: rej }; + } + const timed = timedPromise(timeout, 'identify'); - const raced = Promise.race([timed, slow]).catch((e) => { + const raced = Promise.race([timed, basePromise]).catch((e) => { if (e.message.includes('timed out')) { this.logger.error(`identify error: ${e}`); } @@ -221,11 +229,12 @@ export default class LDClientImpl implements LDClient { * 3. A network error is encountered during initialization. */ async identify(pristineContext: LDContext, identifyOptions?: LDIdentifyOptions): Promise { - if (identifyOptions?.timeout) { - this._identifyTimeout = identifyOptions.timeout; - } + const identifyTimeout = identifyOptions?.timeout ?? DEFAULT_IDENIFY_TIMEOUT_SECONDS; + const noTimeout = identifyOptions?.timeout === undefined && identifyOptions?.noTimeout === true; - if (this._identifyTimeout > this._highTimeoutThreshold) { + // When noTimeout is specified, and a timeout is not secified, then this condition cannot + // be encountered. (Our default would need to be greater) + if (identifyTimeout > this._highTimeoutThreshold) { this.logger.warn( 'The identify function was called with a timeout greater than ' + `${this._highTimeoutThreshold} seconds. We recommend a timeout of less than ` + @@ -250,7 +259,8 @@ export default class LDClientImpl implements LDClient { this._eventProcessor?.sendEvent(this._eventFactoryDefault.identifyEvent(this._checkedContext)); const { identifyPromise, identifyResolve, identifyReject } = this._createIdentifyPromise( - this._identifyTimeout, + identifyTimeout, + noTimeout, ); this.logger.debug(`Identifying ${JSON.stringify(this._checkedContext)}`); diff --git a/packages/shared/sdk-client/src/api/LDClient.ts b/packages/shared/sdk-client/src/api/LDClient.ts index cac279bc2..b3a996625 100644 --- a/packages/shared/sdk-client/src/api/LDClient.ts +++ b/packages/shared/sdk-client/src/api/LDClient.ts @@ -58,8 +58,7 @@ export interface LDClient { /** * Shuts down the client and releases its resources, after delivering any pending analytics - * events. After the client is closed, all calls to {@link variation} will return default values, - * and it will not make any requests to LaunchDarkly. + * events. */ close(): Promise; diff --git a/packages/shared/sdk-client/src/api/LDIdentifyOptions.ts b/packages/shared/sdk-client/src/api/LDIdentifyOptions.ts index 3aecdfab4..467da79b6 100644 --- a/packages/shared/sdk-client/src/api/LDIdentifyOptions.ts +++ b/packages/shared/sdk-client/src/api/LDIdentifyOptions.ts @@ -20,4 +20,17 @@ export interface LDIdentifyOptions { * Defaults to false. */ waitForNetworkResults?: boolean; + + /** + * When set to true, and timeout is not set, this indicates that the identify operation will + * not have any timeout. In typical usage, where an application awaits the promise, a timeout + * is important because identify can potentially take indefinite time depending on network + * conditions. If your application specifically does not block any operations pending the promise + * resolution, then you can use this opton to explicitly indicate that. + * + * If you set this to true, and you do not set a timeout, and you block aspects of operation of + * your application, then those aspects can be blocked indefinitely. Generally this option will + * not be required. + */ + noTimeout?: boolean; } From 0349691e24517cb75ff66643761e2b2a4b407cc7 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 25 Oct 2024 09:41:47 -0700 Subject: [PATCH 2/5] chore: Change testing to use cjs. --- .../__tests__/BrowserDataManager.test.ts | 3 -- .../platform/BrowserEncoding.test.ts | 4 --- .../__tests__/platform/BrowserHasher.test.ts | 4 --- packages/sdk/browser/jest.config.js | 8 ----- packages/sdk/browser/jest.config.json | 26 ++++++++++++++ packages/sdk/browser/package.json | 7 ++-- packages/sdk/browser/setup-jest.js | 26 ++++++++++++++ packages/sdk/browser/src/goals/GoalTracker.ts | 3 +- .../browser/src/vendor/escapeStringRegexp.ts | 34 +++++++++++++++++++ packages/sdk/browser/tsconfig.json | 1 - packages/sdk/browser/tsconfig.test.json | 27 +++++++++++++++ 11 files changed, 117 insertions(+), 26 deletions(-) delete mode 100644 packages/sdk/browser/jest.config.js create mode 100644 packages/sdk/browser/jest.config.json create mode 100644 packages/sdk/browser/setup-jest.js create mode 100644 packages/sdk/browser/src/vendor/escapeStringRegexp.ts create mode 100644 packages/sdk/browser/tsconfig.test.json diff --git a/packages/sdk/browser/__tests__/BrowserDataManager.test.ts b/packages/sdk/browser/__tests__/BrowserDataManager.test.ts index 36b58b56b..628602acf 100644 --- a/packages/sdk/browser/__tests__/BrowserDataManager.test.ts +++ b/packages/sdk/browser/__tests__/BrowserDataManager.test.ts @@ -1,5 +1,4 @@ import { jest } from '@jest/globals'; -import { TextEncoder } from 'node:util'; import { ApplicationTags, @@ -26,8 +25,6 @@ import LocalStorage from '../src/platform/LocalStorage'; import { MockHasher } from './MockHasher'; import { goodBootstrapData } from './testBootstrapData'; -global.TextEncoder = TextEncoder; - function mockResponse(value: string, statusCode: number) { const response: Response = { headers: { diff --git a/packages/sdk/browser/__tests__/platform/BrowserEncoding.test.ts b/packages/sdk/browser/__tests__/platform/BrowserEncoding.test.ts index a24865609..085734a7b 100644 --- a/packages/sdk/browser/__tests__/platform/BrowserEncoding.test.ts +++ b/packages/sdk/browser/__tests__/platform/BrowserEncoding.test.ts @@ -1,9 +1,5 @@ -// TextEncoder should be part of jsdom, but it is not. So we can import it from node in the tests. -import { TextEncoder } from 'node:util'; - import BrowserEncoding from '../../src/platform/BrowserEncoding'; -global.TextEncoder = TextEncoder; it('can base64 a basic ASCII string', () => { const encoding = new BrowserEncoding(); diff --git a/packages/sdk/browser/__tests__/platform/BrowserHasher.test.ts b/packages/sdk/browser/__tests__/platform/BrowserHasher.test.ts index cb143f119..3fc202a5b 100644 --- a/packages/sdk/browser/__tests__/platform/BrowserHasher.test.ts +++ b/packages/sdk/browser/__tests__/platform/BrowserHasher.test.ts @@ -1,11 +1,7 @@ -// TextEncoder should be part of jsdom, but it is not. So we can import it from node in the tests. import { webcrypto } from 'node:crypto'; -import { TextEncoder } from 'node:util'; import BrowserHasher from '../../src/platform/BrowserHasher'; -global.TextEncoder = TextEncoder; - // Crypto is injectable as it is also not correctly available with the combination of node and jsdom. /** diff --git a/packages/sdk/browser/jest.config.js b/packages/sdk/browser/jest.config.js deleted file mode 100644 index e6a722469..000000000 --- a/packages/sdk/browser/jest.config.js +++ /dev/null @@ -1,8 +0,0 @@ -export default { - extensionsToTreatAsEsm: ['.ts'], - verbose: true, - preset: 'ts-jest/presets/default-esm', - testEnvironment: 'jest-environment-jsdom', - testPathIgnorePatterns: ['./dist', './src'], - testMatch: ['**.test.ts'], -}; diff --git a/packages/sdk/browser/jest.config.json b/packages/sdk/browser/jest.config.json new file mode 100644 index 000000000..9c86b13e0 --- /dev/null +++ b/packages/sdk/browser/jest.config.json @@ -0,0 +1,26 @@ +{ + "verbose": true, + "testEnvironment": "jest-environment-jsdom", + "testPathIgnorePatterns": [ + "./dist", + "./src" + ], + "testMatch": [ + "**.test.ts" + ], + "setupFiles": [ + "./setup-jest.js" + ], + "transform": { + "^.+\\.ts$": [ + "ts-jest", + { + "tsConfig": "tsconfig.test.json" + } + ], + "^.+.tsx?$": [ + "ts-jest", + {} + ] + } +} diff --git a/packages/sdk/browser/package.json b/packages/sdk/browser/package.json index 1973107c3..2b05c0efd 100644 --- a/packages/sdk/browser/package.json +++ b/packages/sdk/browser/package.json @@ -30,14 +30,12 @@ "build": "tsc --noEmit && rollup -c rollup.config.js", "lint": "eslint . --ext .ts,.tsx", "prettier": "prettier --write '**/*.@(js|ts|tsx|json|css)' --ignore-path ../../../.prettierignore", - "test": "NODE_OPTIONS=--experimental-vm-modules npx jest --runInBand", + "test": "npx jest --runInBand", "coverage": "yarn test --coverage", "check": "yarn prettier && yarn lint && yarn build && yarn test" }, "dependencies": { - "@launchdarkly/js-client-sdk-common": "1.10.0", - "escape-string-regexp": "^5.0.0", - "rollup-plugin-visualizer": "^5.12.0" + "@launchdarkly/js-client-sdk-common": "1.10.0" }, "devDependencies": { "@jest/globals": "^29.7.0", @@ -62,6 +60,7 @@ "prettier": "^3.0.0", "rimraf": "^5.0.5", "rollup": "^3.23.0", + "rollup-plugin-visualizer": "^5.12.0", "ts-jest": "^29.1.1", "typedoc": "0.25.0", "typescript": "^5.5.3" diff --git a/packages/sdk/browser/setup-jest.js b/packages/sdk/browser/setup-jest.js new file mode 100644 index 000000000..95d06357e --- /dev/null +++ b/packages/sdk/browser/setup-jest.js @@ -0,0 +1,26 @@ +const { TextEncoder, TextDecoder } = require('node:util'); +const crypto = require('node:crypto'); + +global.TextEncoder = TextEncoder; + +Object.assign(window, { TextDecoder, TextEncoder }); + +// Based on: +// https://stackoverflow.com/a/71750830 + +Object.defineProperty(global.self, "crypto", { + value: { + getRandomValues: (arr) => crypto.randomBytes(arr.length), + subtle: { + digest: (algorithm, data) => { + return new Promise((resolve) => + resolve( + crypto.createHash(algorithm.toLowerCase().replace("-", "")) + .update(data) + .digest() + ) + ); + }, + }, + }, +}); diff --git a/packages/sdk/browser/src/goals/GoalTracker.ts b/packages/sdk/browser/src/goals/GoalTracker.ts index f3da9a030..bb5365d0e 100644 --- a/packages/sdk/browser/src/goals/GoalTracker.ts +++ b/packages/sdk/browser/src/goals/GoalTracker.ts @@ -1,5 +1,3 @@ -import escapeStringRegexp from 'escape-string-regexp'; - import { addDocumentEventListener, getHref, @@ -7,6 +5,7 @@ import { getLocationSearch, querySelectorAll, } from '../BrowserApi'; +import escapeStringRegexp from '../vendor/escapeStringRegexp'; import { ClickGoal, Goal, Matcher } from './Goals'; type EventHandler = (goal: Goal) => void; diff --git a/packages/sdk/browser/src/vendor/escapeStringRegexp.ts b/packages/sdk/browser/src/vendor/escapeStringRegexp.ts new file mode 100644 index 000000000..228c0e30b --- /dev/null +++ b/packages/sdk/browser/src/vendor/escapeStringRegexp.ts @@ -0,0 +1,34 @@ +// From here: https://github.com/sindresorhus/escape-string-regexp + +// This is vendored to reduce the complexity of the built and test setup. +// The NPM package for escape-string-regexp is ESM only, and that introduces +// complexity to the jest configuration which works best/easiest with CJS. + +/** + * MIT License + * + * Copyright (c) Sindre Sorhus (https://sindresorhus.com) + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + * + */ + +/** + * Escape regular expression especial characters. + * + * @param string The regular expression to escape. + * @returns The escaped expression. + */ +export default function escapeStringRegexp(string: string) { + if (typeof string !== 'string') { + throw new TypeError('Expected a string'); + } + + // Escape characters with special meaning either inside or outside character sets. + // Use a simple backslash escape when it’s always valid, and a `\xnn` escape when the simpler form would be disallowed by Unicode patterns’ stricter grammar. + return string.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d'); +} diff --git a/packages/sdk/browser/tsconfig.json b/packages/sdk/browser/tsconfig.json index 663a919f9..929948785 100644 --- a/packages/sdk/browser/tsconfig.json +++ b/packages/sdk/browser/tsconfig.json @@ -29,7 +29,6 @@ "node_modules", "contract-tests", "babel.config.js", - "jest.config.js", "jestSetupFile.ts", "**/*.test.ts*" ] diff --git a/packages/sdk/browser/tsconfig.test.json b/packages/sdk/browser/tsconfig.test.json new file mode 100644 index 000000000..6087e302d --- /dev/null +++ b/packages/sdk/browser/tsconfig.test.json @@ -0,0 +1,27 @@ +{ + "compilerOptions": { + "rootDir": "src", + "outDir": "dist", + "lib": ["es6", "DOM"], + "module": "CommonJS", + "strict": true, + "noImplicitOverride": true, + "sourceMap": true, + "declaration": true, + "declarationMap": true, + "stripInternal": true + }, + "exclude": [ + "vite.config.ts", + "__tests__", + "dist", + "docs", + "example", + "node_modules", + "contract-tests", + "babel.config.js", + "jest.config.js", + "jestSetupFile.ts", + "**/*.test.ts*" + ] +} From 421b0b5771cb009e8788a59bc21ddee16ad21435 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 25 Oct 2024 10:16:08 -0700 Subject: [PATCH 3/5] Linting. --- .../__tests__/platform/BrowserEncoding.test.ts | 1 - packages/sdk/browser/jest.config.json | 18 ++++-------------- packages/sdk/browser/setup-jest.js | 8 +++----- packages/sdk/cloudflare/jsr.json | 12 ++---------- 4 files changed, 9 insertions(+), 30 deletions(-) diff --git a/packages/sdk/browser/__tests__/platform/BrowserEncoding.test.ts b/packages/sdk/browser/__tests__/platform/BrowserEncoding.test.ts index 085734a7b..ad81b83bf 100644 --- a/packages/sdk/browser/__tests__/platform/BrowserEncoding.test.ts +++ b/packages/sdk/browser/__tests__/platform/BrowserEncoding.test.ts @@ -1,6 +1,5 @@ import BrowserEncoding from '../../src/platform/BrowserEncoding'; - it('can base64 a basic ASCII string', () => { const encoding = new BrowserEncoding(); expect(encoding.btoa('toaster')).toEqual('dG9hc3Rlcg=='); diff --git a/packages/sdk/browser/jest.config.json b/packages/sdk/browser/jest.config.json index 9c86b13e0..6d2e223cd 100644 --- a/packages/sdk/browser/jest.config.json +++ b/packages/sdk/browser/jest.config.json @@ -1,16 +1,9 @@ { "verbose": true, "testEnvironment": "jest-environment-jsdom", - "testPathIgnorePatterns": [ - "./dist", - "./src" - ], - "testMatch": [ - "**.test.ts" - ], - "setupFiles": [ - "./setup-jest.js" - ], + "testPathIgnorePatterns": ["./dist", "./src"], + "testMatch": ["**.test.ts"], + "setupFiles": ["./setup-jest.js"], "transform": { "^.+\\.ts$": [ "ts-jest", @@ -18,9 +11,6 @@ "tsConfig": "tsconfig.test.json" } ], - "^.+.tsx?$": [ - "ts-jest", - {} - ] + "^.+.tsx?$": ["ts-jest", {}] } } diff --git a/packages/sdk/browser/setup-jest.js b/packages/sdk/browser/setup-jest.js index 95d06357e..e17ac62cb 100644 --- a/packages/sdk/browser/setup-jest.js +++ b/packages/sdk/browser/setup-jest.js @@ -8,17 +8,15 @@ Object.assign(window, { TextDecoder, TextEncoder }); // Based on: // https://stackoverflow.com/a/71750830 -Object.defineProperty(global.self, "crypto", { +Object.defineProperty(global.self, 'crypto', { value: { getRandomValues: (arr) => crypto.randomBytes(arr.length), subtle: { digest: (algorithm, data) => { return new Promise((resolve) => resolve( - crypto.createHash(algorithm.toLowerCase().replace("-", "")) - .update(data) - .digest() - ) + crypto.createHash(algorithm.toLowerCase().replace('-', '')).update(data).digest(), + ), ); }, }, diff --git a/packages/sdk/cloudflare/jsr.json b/packages/sdk/cloudflare/jsr.json index 62b0c6d0b..61c54a978 100644 --- a/packages/sdk/cloudflare/jsr.json +++ b/packages/sdk/cloudflare/jsr.json @@ -3,15 +3,7 @@ "version": "2.6.0", "exports": "./src/index.ts", "publish": { - "include": [ - "LICENSE", - "README.md", - "package.json", - "jsr.json", - "src/**/*.ts" - ], - "exclude": [ - "src/**/*.test.ts" - ] + "include": ["LICENSE", "README.md", "package.json", "jsr.json", "src/**/*.ts"], + "exclude": ["src/**/*.test.ts"] } } From f96c2110385bfb0dd76c0c21536bf4ff74d4445d Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 25 Oct 2024 10:18:15 -0700 Subject: [PATCH 4/5] Remove JSR file from PR. --- packages/sdk/cloudflare/jsr.json | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/sdk/cloudflare/jsr.json b/packages/sdk/cloudflare/jsr.json index 61c54a978..62b0c6d0b 100644 --- a/packages/sdk/cloudflare/jsr.json +++ b/packages/sdk/cloudflare/jsr.json @@ -3,7 +3,15 @@ "version": "2.6.0", "exports": "./src/index.ts", "publish": { - "include": ["LICENSE", "README.md", "package.json", "jsr.json", "src/**/*.ts"], - "exclude": ["src/**/*.test.ts"] + "include": [ + "LICENSE", + "README.md", + "package.json", + "jsr.json", + "src/**/*.ts" + ], + "exclude": [ + "src/**/*.test.ts" + ] } } From aadb06b309df9851989206f5ce51ccc244e5caf6 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 25 Oct 2024 14:12:54 -0700 Subject: [PATCH 5/5] Finish tests. --- .../compat/LDClientCompatImpl.test.ts | 501 +++++++++++++++--- .../sdk/browser/src/compat/LDClientCompat.ts | 84 +-- .../browser/src/compat/LDClientCompatImpl.ts | 20 +- .../sdk/browser/src/compat/LDEmitterCompat.ts | 12 +- packages/sdk/browser/src/compat/index.ts | 1 - 5 files changed, 512 insertions(+), 106 deletions(-) diff --git a/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts b/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts index b5a366572..41300b0f2 100644 --- a/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts +++ b/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts @@ -1,61 +1,98 @@ import { jest } from '@jest/globals'; -import { LDContext, LDFlagSet, LDLogger } from '@launchdarkly/js-client-sdk-common'; +import { LDContext, LDFlagSet } from '@launchdarkly/js-client-sdk-common'; -const mockBrowserClient = { +import { BrowserClient } from '../../src/BrowserClient'; +import LDClientCompatImpl from '../../src/compat/LDClientCompatImpl'; +import { LDOptions } from '../../src/compat/LDCompatOptions'; + +// @ts-ignore +const mockBrowserClient: jest.MockedObject = { identify: jest.fn(), allFlags: jest.fn(), close: jest.fn(), flush: jest.fn(), - _emitter: jest.fn(() => ({ - emit: jest.fn(), - on: jest.fn(), - off: jest.fn(), - })), + setStreaming: jest.fn(), + on: jest.fn(), + off: jest.fn(), + sdkKey: 'test-sdk-key', + variation: jest.fn(), + variationDetail: jest.fn(), + boolVariation: jest.fn(), + boolVariationDetail: jest.fn(), + numberVariation: jest.fn(), + numberVariationDetail: jest.fn(), + stringVariation: jest.fn(), + stringVariationDetail: jest.fn(), + jsonVariation: jest.fn(), + jsonVariationDetail: jest.fn(), + track: jest.fn(), + addHook: jest.fn(), + logger: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, + getContext: jest.fn(), }; -jest.unstable_mockModule('../../src/BrowserClient', () => ({ +jest.mock('../../src/BrowserClient', () => ({ __esModule: true, - BrowserClient: jest.fn(), + BrowserClient: jest.fn(() => mockBrowserClient), })); -const { default: LDClientCompatImpl } = await import('../../src/compat/LDClientCompatImpl'); +afterEach(() => { + jest.resetAllMocks(); +}); -describe('given a LDClientCompatImpl client with mocked browser client', () => { +beforeEach(() => { + // TypesScript doesn't understand that the BrowserClient is a mock. // @ts-ignore + BrowserClient.mockImplementation(() => mockBrowserClient); +}); + +describe('given a LDClientCompatImpl client with mocked browser client that is immediately ready', () => { let client: LDClientCompatImpl; - // let mockBrowserClient: jest.Mocked; - let mockLogger: LDLogger; beforeEach(() => { - // mockBrowserClient = { - // identify: jest.fn(), - // allFlags: jest.fn(), - // close: jest.fn(), - // flush: jest.fn(), - // } as unknown as jest.Mocked; - - // (BrowserClient as jest.MockedClass).mockImplementation( - // () => mockBrowserClient, - // ); - mockLogger = { - error: jest.fn(), - warn: jest.fn(), - info: jest.fn(), - debug: jest.fn(), - }; - client = new LDClientCompatImpl( - 'env-key', + jest.useFakeTimers(); + mockBrowserClient.identify.mockImplementation(() => Promise.resolve()); + 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); + + await expect(client.waitForInitialization()).resolves.toBeUndefined(); + expect(mockBrowserClient.identify).toHaveBeenCalledWith( { kind: 'user', key: 'user-key' }, - { logger: mockLogger }, + { bootstrap: undefined, hash: undefined, noTimeout: true }, + ); + }); +}); + +describe('given a LDClientCompatImpl client with mocked browser client that initializes after a delay', () => { + let client: LDClientCompatImpl; + + beforeEach(() => { + jest.useFakeTimers(); + mockBrowserClient.identify.mockImplementation( + () => + new Promise((r) => { + setTimeout(r, 100); + }), ); + client = new LDClientCompatImpl('env-key', { kind: 'user', key: 'user-key' }); }); it('should return a promise from identify when no callback is provided', async () => { + jest.advanceTimersToNextTimer(); const context: LDContext = { kind: 'user', key: 'new-user' }; const mockFlags: LDFlagSet = { flag1: true, flag2: false }; - // @ts-ignore - mockBrowserClient.identify.mockResolvedValue(undefined); + + mockBrowserClient.identify.mockResolvedValue(); mockBrowserClient.allFlags.mockReturnValue(mockFlags); const result = await client.identify(context); @@ -65,13 +102,16 @@ describe('given a LDClientCompatImpl client with mocked browser client', () => { }); it('should call the callback when provided to identify', (done) => { + jest.advanceTimersToNextTimer(); const context: LDContext = { kind: 'user', key: 'new-user' }; const mockFlags: LDFlagSet = { flag1: true, flag2: false }; - // @ts-ignore - mockBrowserClient.identify.mockResolvedValue(undefined); + mockBrowserClient.allFlags.mockReturnValue(mockFlags); + mockBrowserClient.identify.mockImplementation(() => Promise.resolve()); + // Starting advancing the timers for the nest call. The wrapped promises + // do not resolve sychronously. + jest.advanceTimersToNextTimerAsync(); - // @ts-ignore client.identify(context, undefined, (err, flags) => { expect(err).toBeNull(); expect(flags).toEqual(mockFlags); @@ -80,7 +120,7 @@ describe('given a LDClientCompatImpl client with mocked browser client', () => { }); it('should return a promise from close when no callback is provided', async () => { - // @ts-ignore + jest.advanceTimersToNextTimer(); mockBrowserClient.close.mockResolvedValue(); await expect(client.close()).resolves.toBeUndefined(); @@ -88,7 +128,7 @@ describe('given a LDClientCompatImpl client with mocked browser client', () => { }); it('should call the callback when provided to close', (done) => { - // @ts-ignore + jest.advanceTimersToNextTimer(); mockBrowserClient.close.mockResolvedValue(); client.close(() => { @@ -98,7 +138,7 @@ describe('given a LDClientCompatImpl client with mocked browser client', () => { }); it('should return a promise from flush when no callback is provided', async () => { - // @ts-ignore + jest.advanceTimersToNextTimer(); mockBrowserClient.flush.mockResolvedValue({ result: true }); await expect(client.flush()).resolves.toBeUndefined(); @@ -106,44 +146,379 @@ describe('given a LDClientCompatImpl client with mocked browser client', () => { }); it('should call the callback when provided to flush', (done) => { - // @ts-ignore + jest.advanceTimersToNextTimer(); mockBrowserClient.flush.mockResolvedValue({ result: true }); + // Starting advancing the timers for the nest call. The wrapped promises + // do not resolve sychronously. + jest.advanceTimersToNextTimerAsync(); + jest.advanceTimersToNextTimerAsync(); client.flush(() => { expect(mockBrowserClient.flush).toHaveBeenCalled(); done(); }); }); - // it('should resolve immediately if the client is already initialized', async () => { - // mockBrowserClient.waitForInitialization.mockResolvedValue(undefined); + it('should resolve waitForInitialization when the client is initialized', async () => { + jest.advanceTimersToNextTimer(); + mockBrowserClient.identify.mockResolvedValue(undefined); - // await expect(client.waitForInitialization()).resolves.toBeUndefined(); - // expect(mockBrowserClient.waitForInitialization).toHaveBeenCalledWith({ noTimeout: true }); - // }); + await expect(client.waitForInitialization()).resolves.toBeUndefined(); + expect(mockBrowserClient.identify).toHaveBeenCalledWith( + { kind: 'user', key: 'user-key' }, + { bootstrap: undefined, hash: undefined, noTimeout: true }, + ); + }); - // it('should log a warning when no timeout is specified for waitForInitialization', async () => { - // mockBrowserClient.waitForInitialization.mockResolvedValue(undefined); + it('should resolve second waitForInitialization immediately', async () => { + jest.advanceTimersToNextTimer(); + mockBrowserClient.identify.mockResolvedValue(undefined); - // await client.waitForInitialization(); + await expect(client.waitForInitialization()).resolves.toBeUndefined(); + await expect(client.waitForInitialization()).resolves.toBeUndefined(); + expect(mockBrowserClient.identify).toHaveBeenCalledWith( + { kind: 'user', key: 'user-key' }, + { bootstrap: undefined, hash: undefined, noTimeout: true }, + ); + }); - // expect(mockLogger.warn).toHaveBeenCalledWith( - // expect.stringContaining('The waitForInitialization function was called without a timeout specified.') - // ); - // }); + it('should resolve waitUntilReady immediately if the client is already initialized', async () => { + jest.advanceTimersToNextTimer(); + mockBrowserClient.identify.mockResolvedValue(undefined); - // it('should apply a timeout when specified for waitForInitialization', async () => { - // mockBrowserClient.waitForInitialization.mockResolvedValue(undefined); + await expect(client.waitUntilReady()).resolves.toBeUndefined(); + expect(mockBrowserClient.identify).toHaveBeenCalledWith( + { kind: 'user', key: 'user-key' }, + { bootstrap: undefined, hash: undefined, noTimeout: true }, + ); + }); + + it('should log a warning when no timeout is specified for waitForInitialization', async () => { + jest.advanceTimersToNextTimerAsync(); + await client.waitForInitialization(); - // await client.waitForInitialization(5); + expect(mockBrowserClient.logger.warn).toHaveBeenCalledWith( + expect.stringContaining( + 'The waitForInitialization function was called without a timeout specified.', + ), + ); + }); + + it('should apply a timeout when specified for waitForInitialization', async () => { + jest.useRealTimers(); + await expect(async () => client.waitForInitialization(0.25)).rejects.toThrow(); + }); + + it('should pass through allFlags call', () => { + const mockFlags = { flag1: true, flag2: false }; + mockBrowserClient.allFlags.mockReturnValue(mockFlags); - // expect(mockBrowserClient.waitForInitialization).toHaveBeenCalledWith({ timeout: 5 }); - // }); + const result = client.allFlags(); - // it('should reject with a timeout error when initialization takes too long', async () => { - // mockBrowserClient.waitForInitialization.mockRejectedValue(new Error('Timeout')); + expect(result).toEqual(mockFlags); + expect(mockBrowserClient.allFlags).toHaveBeenCalled(); + }); + + it('should pass through variation call', () => { + const flagKey = 'test-flag'; + const defaultValue = false; + mockBrowserClient.variation.mockReturnValue(true); + + const result = client.variation(flagKey, defaultValue); + + expect(result).toBe(true); + expect(mockBrowserClient.variation).toHaveBeenCalledWith(flagKey, defaultValue); + }); + + it('should pass through variationDetail call', () => { + const flagKey = 'test-flag'; + const defaultValue = 'default'; + const mockDetail = { value: 'test', variationIndex: 1, reason: { kind: 'OFF' } }; + mockBrowserClient.variationDetail.mockReturnValue(mockDetail); + + const result = client.variationDetail(flagKey, defaultValue); + + expect(result).toEqual(mockDetail); + expect(mockBrowserClient.variationDetail).toHaveBeenCalledWith(flagKey, defaultValue); + }); + + it('should pass through boolVariation call', () => { + const flagKey = 'bool-flag'; + const defaultValue = false; + mockBrowserClient.boolVariation.mockReturnValue(true); + + const result = client.boolVariation(flagKey, defaultValue); + + expect(result).toBe(true); + expect(mockBrowserClient.boolVariation).toHaveBeenCalledWith(flagKey, defaultValue); + }); + + it('should pass through boolVariationDetail call', () => { + const flagKey = 'bool-flag'; + const defaultValue = false; + const mockDetail = { value: true, variationIndex: 1, reason: { kind: 'OFF' } }; + mockBrowserClient.boolVariationDetail.mockReturnValue(mockDetail); + + const result = client.boolVariationDetail(flagKey, defaultValue); + + expect(result).toEqual(mockDetail); + expect(mockBrowserClient.boolVariationDetail).toHaveBeenCalledWith(flagKey, defaultValue); + }); + + it('should pass through stringVariation call', () => { + const flagKey = 'string-flag'; + const defaultValue = 'default'; + mockBrowserClient.stringVariation.mockReturnValue('test'); + + const result = client.stringVariation(flagKey, defaultValue); + + expect(result).toBe('test'); + expect(mockBrowserClient.stringVariation).toHaveBeenCalledWith(flagKey, defaultValue); + }); + + it('should pass through stringVariationDetail call', () => { + const flagKey = 'string-flag'; + const defaultValue = 'default'; + const mockDetail = { value: 'test', variationIndex: 1, reason: { kind: 'OFF' } }; + mockBrowserClient.stringVariationDetail.mockReturnValue(mockDetail); + + const result = client.stringVariationDetail(flagKey, defaultValue); + + expect(result).toEqual(mockDetail); + expect(mockBrowserClient.stringVariationDetail).toHaveBeenCalledWith(flagKey, defaultValue); + }); + + it('should pass through numberVariation call', () => { + const flagKey = 'number-flag'; + const defaultValue = 0; + mockBrowserClient.numberVariation.mockReturnValue(42); + + const result = client.numberVariation(flagKey, defaultValue); + + expect(result).toBe(42); + expect(mockBrowserClient.numberVariation).toHaveBeenCalledWith(flagKey, defaultValue); + }); + + it('should pass through numberVariationDetail call', () => { + const flagKey = 'number-flag'; + const defaultValue = 0; + const mockDetail = { value: 42, variationIndex: 1, reason: { kind: 'OFF' } }; + mockBrowserClient.numberVariationDetail.mockReturnValue(mockDetail); + + const result = client.numberVariationDetail(flagKey, defaultValue); + + expect(result).toEqual(mockDetail); + expect(mockBrowserClient.numberVariationDetail).toHaveBeenCalledWith(flagKey, defaultValue); + }); + + it('should pass through jsonVariation call', () => { + const flagKey = 'json-flag'; + const defaultValue = { default: true }; + const mockJson = { test: 'value' }; + mockBrowserClient.jsonVariation.mockReturnValue(mockJson); + + const result = client.jsonVariation(flagKey, defaultValue); + + expect(result).toEqual(mockJson); + expect(mockBrowserClient.jsonVariation).toHaveBeenCalledWith(flagKey, defaultValue); + }); - // await expect(client.waitForInitialization(1)).rejects.toThrow('Timeout'); - // expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('waitForInitialization timed out')); - // }); + it('should pass through jsonVariationDetail call', () => { + const flagKey = 'json-flag'; + const defaultValue = { default: true }; + const mockDetail = { value: { test: 'value' }, variationIndex: 1, reason: { kind: 'OFF' } }; + mockBrowserClient.jsonVariationDetail.mockReturnValue(mockDetail); + + const result = client.jsonVariationDetail(flagKey, defaultValue); + + expect(result).toEqual(mockDetail); + expect(mockBrowserClient.jsonVariationDetail).toHaveBeenCalledWith(flagKey, defaultValue); + }); + + it('should pass through track call', () => { + const eventName = 'test-event'; + const data = { key: 'value' }; + const metricValue = 1.5; + + client.track(eventName, data, metricValue); + + expect(mockBrowserClient.track).toHaveBeenCalledWith(eventName, data, metricValue); + }); + + it('should pass through getContext call', () => { + const mockContext = { kind: 'user', key: 'user-key' }; + mockBrowserClient.getContext.mockReturnValue(mockContext); + + const result = client.getContext(); + + expect(result).toEqual(mockContext); + expect(mockBrowserClient.getContext).toHaveBeenCalled(); + }); + + it('should pass through setStreaming call', () => { + const streamingEnabled = true; + + client.setStreaming(streamingEnabled); + + expect(mockBrowserClient.setStreaming).toHaveBeenCalledWith(streamingEnabled); + }); + + it('should emit ready and initialized events', async () => { + const readyListener = jest.fn(); + const initializedListener = jest.fn(); + + client.on('ready', readyListener); + client.on('initialized', initializedListener); + + jest.advanceTimersToNextTimerAsync(); + await client.waitForInitialization(); + + expect(readyListener).toHaveBeenCalledTimes(1); + expect(initializedListener).toHaveBeenCalledTimes(1); + }); + + it('it unregisters ready andinitialized handlers handlers', async () => { + const readyListener = jest.fn(); + const initializedListener = jest.fn(); + + client.on('ready', readyListener); + client.on('initialized', initializedListener); + + client.off('ready', readyListener); + client.off('initialized', initializedListener); + + jest.advanceTimersToNextTimerAsync(); + await client.waitForInitialization(); + + expect(readyListener).not.toHaveBeenCalled(); + expect(initializedListener).not.toHaveBeenCalled(); + }); + + it('forwards addHook calls to BrowserClient', () => { + const testHook = { + getMetadata: () => ({ name: 'Test Hook' }), + }; + + client.addHook(testHook); + + expect(mockBrowserClient.addHook).toHaveBeenCalledWith(testHook); + }); +}); + +it('forwards bootstrap and hash to BrowserClient identify call', async () => { + mockBrowserClient.identify.mockImplementation( + () => + new Promise((r) => { + setTimeout(r, 100); + }), + ); + const bootstrapData = { flagKey: { version: 1, variation: 0, value: true } }; + const options: LDOptions = { + bootstrap: bootstrapData, + hash: 'someHash', + }; + const context: LDContext = { kind: 'user', key: 'user-key' }; + + // We are testing side-effects, ignore we are not assigning the client. + // eslint-disable-next-line no-new + new LDClientCompatImpl('env-key', context, options); + + expect(mockBrowserClient.identify).toHaveBeenCalledWith(context, { + bootstrap: bootstrapData, + hash: 'someHash', + noTimeout: true, + }); +}); + +describe('given a LDClientCompatImpl client with mocked browser client which fails to initialize', () => { + let client: LDClientCompatImpl; + + beforeEach(() => { + jest.useFakeTimers(); + mockBrowserClient.identify.mockImplementation( + () => + new Promise((r, reject) => { + setTimeout(() => reject(new Error('Identify failed')), 100); + }), + ); + client = new LDClientCompatImpl('env-key', { kind: 'user', key: 'user-key' }); + }); + + it('should handle rejection of initial identification before waitForInitialization is called', async () => { + await jest.advanceTimersToNextTimer(); + + await expect(client.waitForInitialization()).rejects.toThrow('Identify failed'); + + expect(mockBrowserClient.identify).toHaveBeenCalledWith( + { kind: 'user', key: 'user-key' }, + { bootstrap: undefined, hash: undefined, noTimeout: true }, + ); + }); + + it('should handle rejection of initial identification after waitForInitialization is called', async () => { + const makeAssertion = () => + expect(client.waitForInitialization()).rejects.toThrow('Identify failed'); + const promise = makeAssertion(); + jest.advanceTimersToNextTimer(); + await promise; + + expect(mockBrowserClient.identify).toHaveBeenCalledWith( + { kind: 'user', key: 'user-key' }, + { bootstrap: undefined, hash: undefined, noTimeout: true }, + ); + }); + + it('should handle rejection of initial identification before waitUntilReady is called', async () => { + await jest.advanceTimersToNextTimer(); + + await expect(client.waitUntilReady()).resolves.toBeUndefined(); + + expect(mockBrowserClient.identify).toHaveBeenCalledWith( + { kind: 'user', key: 'user-key' }, + { bootstrap: undefined, hash: undefined, noTimeout: true }, + ); + }); + + it('should handle rejection of initial identification after waitUntilReady is called', async () => { + const makeAssertion = () => expect(client.waitUntilReady()).resolves.toBeUndefined(); + const promise = makeAssertion(); + jest.advanceTimersToNextTimer(); + await promise; + + expect(mockBrowserClient.identify).toHaveBeenCalledWith( + { kind: 'user', key: 'user-key' }, + { bootstrap: undefined, hash: undefined, noTimeout: true }, + ); + }); + + it('should emit failed and ready events', async () => { + const readyListener = jest.fn(); + const failedListener = jest.fn(); + + client.on('ready', readyListener); + client.on('failed', failedListener); + + jest.advanceTimersToNextTimerAsync(); + await expect(client.waitForInitialization()).rejects.toThrow('Identify failed'); + + expect(readyListener).toHaveBeenCalledTimes(1); + expect(failedListener).toHaveBeenCalledTimes(1); + }); + + it('it unregisters failed handlers', async () => { + const readyListener = jest.fn(); + const failedListener = jest.fn(); + + client.on('ready', readyListener); + client.on('failed', failedListener); + + client.off('ready', readyListener); + client.off('failed', failedListener); + + jest.advanceTimersToNextTimerAsync(); + await expect(client.waitForInitialization()).rejects.toThrow('Identify failed'); + + expect(readyListener).not.toHaveBeenCalled(); + expect(failedListener).not.toHaveBeenCalled(); + }); }); diff --git a/packages/sdk/browser/src/compat/LDClientCompat.ts b/packages/sdk/browser/src/compat/LDClientCompat.ts index 195e0ea27..670bc4130 100644 --- a/packages/sdk/browser/src/compat/LDClientCompat.ts +++ b/packages/sdk/browser/src/compat/LDClientCompat.ts @@ -1,4 +1,5 @@ import { LDContext, LDFlagSet } from '@launchdarkly/js-client-sdk-common'; + import { LDClient as LDCLientBrowser } from '../BrowserClient'; /** @@ -13,34 +14,34 @@ import { LDClient as LDCLientBrowser } from '../BrowserClient'; */ export interface LDClient extends Omit { /** - * Identifies a context to LaunchDarkly. - * - * Unlike the server-side SDKs, the client-side JavaScript SDKs maintain a current context state, - * which is set at initialization time. You only need to call `identify()` if the context has changed - * since then. - * - * 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 - * use a callback or a Promise to determine when the new flag values are available. - * - * @param context - * The context properties. Must contain at least the `key` property. - * @param hash - * The signed context key if you are using [Secure Mode](https://docs.launchdarkly.com/sdk/features/secure-mode#configuring-secure-mode-in-the-javascript-client-side-sdk). - * @param onDone - * A function which will be called as soon as the flag values for the new context are available, - * with two parameters: an error value (if any), and an {@link LDFlagSet} containing the new values - * (which can also be obtained by calling {@link variation}). If the callback is omitted, you will - * receive a Promise instead. - * @returns - * If you provided a callback, then nothing. Otherwise, a Promise which resolve once the flag - * values for the new context are available, providing an {@link LDFlagSet} containing the new values - * (which can also be obtained by calling {@link variation}). - */ + * Identifies a context to LaunchDarkly. + * + * Unlike the server-side SDKs, the client-side JavaScript SDKs maintain a current context state, + * which is set at initialization time. You only need to call `identify()` if the context has changed + * since then. + * + * 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 + * use a callback or a Promise to determine when the new flag values are available. + * + * @param context + * The context properties. Must contain at least the `key` property. + * @param hash + * The signed context key if you are using [Secure Mode](https://docs.launchdarkly.com/sdk/features/secure-mode#configuring-secure-mode-in-the-javascript-client-side-sdk). + * @param onDone + * A function which will be called as soon as the flag values for the new context are available, + * with two parameters: an error value (if any), and an {@link LDFlagSet} containing the new values + * (which can also be obtained by calling {@link variation}). If the callback is omitted, you will + * receive a Promise instead. + * @returns + * If you provided a callback, then nothing. Otherwise, a Promise which resolve once the flag + * values for the new context are available, providing an {@link LDFlagSet} containing the new values + * (which can also be obtained by calling {@link variation}). + */ identify( context: LDContext, hash?: string, - onDone?: (err: Error | null, flags: LDFlagSet | null) => void + onDone?: (err: Error | null, flags: LDFlagSet | null) => void, ): Promise | undefined; /** @@ -50,13 +51,6 @@ export interface LDClient extends Omit { - * doSomethingWithSuccessfullyInitializedClient(); - * }).catch(err => { - * doSomethingForFailedStartup(err); - * }); - * * // using async/await * try { * await client.waitForInitialization(5); @@ -89,6 +83,32 @@ export interface LDClient extends Omit; + /** + * Returns a Promise that tracks the client's initialization state. + * + * The returned Promise will be resolved once the client has either successfully initialized + * or failed to initialize (e.g. due to an invalid environment key or a server error). It will + * never be rejected. + * + * ``` + * // using async/await + * await client.waitUntilReady(); + * doSomethingWithClient(); + * ``` + * + * If you want to distinguish between these success and failure conditions, use + * {@link waitForInitialization} instead. + * + * If you prefer to use event listeners ({@link on}) rather than Promises, you can listen on the + * client for a `"ready"` event, which will be fired in either case. + * + * @returns + * A Promise that will be resolved once the client is no longer trying to initialize. + * @deprecated Please use {@link waitForInitialization} instead. This method will always + * cause a warning to be logged because it is implemented via waitForInitialization. + */ + waitUntilReady(): Promise; + /** * Shuts down the client and releases its resources, after delivering any pending analytics * events. diff --git a/packages/sdk/browser/src/compat/LDClientCompatImpl.ts b/packages/sdk/browser/src/compat/LDClientCompatImpl.ts index 720521713..9014b94a4 100644 --- a/packages/sdk/browser/src/compat/LDClientCompatImpl.ts +++ b/packages/sdk/browser/src/compat/LDClientCompatImpl.ts @@ -4,7 +4,6 @@ import { cancelableTimedPromise, Hook, LDContext, - LDEmitterEventName, LDEvaluationDetail, LDEvaluationDetailTyped, LDFlagSet, @@ -16,7 +15,7 @@ import { import { BrowserClient } from '../BrowserClient'; import { LDClient } from './LDClientCompat'; import { LDOptions } from './LDCompatOptions'; -import LDEmitterCompat from './LDEmitterCompat'; +import LDEmitterCompat, { CompatEventName } from './LDEmitterCompat'; import { wrapPromiseCallback } from './wrapPromiseCallback'; export default class LDClientCompatImpl implements LDClient { @@ -96,11 +95,11 @@ export default class LDClientCompatImpl implements LDClient { return this._client.numberVariationDetail(key, defaultValue); } - off(key: LDEmitterEventName, callback: (...args: any[]) => void): void { + off(key: CompatEventName, callback: (...args: any[]) => void): void { this._emitter.off(key, callback); } - on(key: LDEmitterEventName, callback: (...args: any[]) => void): void { + on(key: CompatEventName, callback: (...args: any[]) => void): void { this._emitter.on(key, callback); } @@ -152,7 +151,10 @@ export default class LDClientCompatImpl implements LDClient { flush(onDone?: () => void): Promise | undefined { // The .then() is to strip the return value making a void promise. - return wrapPromiseCallback(this._client.flush().then(), onDone); + return wrapPromiseCallback( + this._client.flush().then(() => undefined), + onDone, + ); } getContext(): LDContext | undefined { @@ -202,6 +204,14 @@ export default class LDClientCompatImpl implements LDClient { return this._promiseWithTimeout(this._initializedPromise, timeout); } + async waitUntilReady(): Promise { + try { + await this.waitForInitialization(); + } catch { + // We do not care about the error. + } + } + /** * Apply a timeout promise to a base promise. This is for use with waitForInitialization. * Currently it returns a LDClient. In the future it should return a status. diff --git a/packages/sdk/browser/src/compat/LDEmitterCompat.ts b/packages/sdk/browser/src/compat/LDEmitterCompat.ts index dccf4abc5..7684b1d46 100644 --- a/packages/sdk/browser/src/compat/LDEmitterCompat.ts +++ b/packages/sdk/browser/src/compat/LDEmitterCompat.ts @@ -1,5 +1,5 @@ -import { LDLogger } from '@launchdarkly/js-sdk-common'; import { LDEmitterEventName } from '@launchdarkly/js-client-sdk-common'; + import { LDClient } from '../BrowserClient'; type CompatOnlyEvents = 'ready' | 'failed' | 'initialized'; @@ -10,7 +10,7 @@ const COMPAT_EVENTS: string[] = ['ready', 'failed', 'initialized']; export default class LDEmitterCompat { private _listeners: Map = new Map(); - constructor(private readonly _client: LDClient) { } + constructor(private readonly _client: LDClient) {} on(name: CompatEventName, listener: Function) { if (COMPAT_EVENTS.includes(name)) { @@ -20,7 +20,7 @@ export default class LDEmitterCompat { this._listeners.get(name)?.push(listener); } } else { - this._client.on(name, listener as (...args: any[]) => void) + this._client.on(name, listener as (...args: any[]) => void); } } @@ -50,7 +50,7 @@ export default class LDEmitterCompat { // listener was not specified, so remove them all for that event this._listeners.delete(name); } else { - this._client.off(name, listener as (...args: any[]) => void) + this._client.off(name, listener as (...args: any[]) => void); } } @@ -58,7 +58,9 @@ export default class LDEmitterCompat { try { listener(...detail); } catch (err) { - this._client.logger.error(`Encountered error invoking handler for "${name}", detail: "${err}"`); + this._client.logger.error( + `Encountered error invoking handler for "${name}", detail: "${err}"`, + ); } } diff --git a/packages/sdk/browser/src/compat/index.ts b/packages/sdk/browser/src/compat/index.ts index 7ce71dafc..5ef1113be 100644 --- a/packages/sdk/browser/src/compat/index.ts +++ b/packages/sdk/browser/src/compat/index.ts @@ -5,7 +5,6 @@ * Some code changes may still be required, for example {@link LDOptions} removes * support for some previously available options. */ -import { AutoEnvAttributes } from '@launchdarkly/js-client-sdk-common'; import { LDContext, LDOptions } from '..'; import { LDClient } from './LDClientCompat'; import LDClientCompatImpl from './LDClientCompatImpl';