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 01/12] 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 0000000000..b5a3665726 --- /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 e6a722469c..ee1376dd94 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 1973107c31..6c9cb297fa 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 a2c260896d..f1810e9322 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 0000000000..42dcb53a2a --- /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 0000000000..195e0ea275 --- /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 0000000000..720521713e --- /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 0000000000..95235f5b03 --- /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 0000000000..dccf4abc54 --- /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 0000000000..7ce71dafc2 --- /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 0000000000..efcbc13be0 --- /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 a9abb0e49e..ff6565b2bf 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 cac279bc2c..b3a996625f 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 3aecdfab44..467da79b6c 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 02/12] 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 36b58b56bf..628602acf7 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 a248656098..085734a7bb 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 cb143f119f..3fc202a5bf 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 e6a722469c..0000000000 --- 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 0000000000..9c86b13e04 --- /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 1973107c31..2b05c0efd9 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 0000000000..95d06357e8 --- /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 f3da9a0305..bb5365d0e0 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 0000000000..228c0e30b2 --- /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 663a919f94..9299487855 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 0000000000..6087e302dd --- /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 03/12] 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 085734a7bb..ad81b83bf2 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 9c86b13e04..6d2e223cd6 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 95d06357e8..e17ac62cb1 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 62b0c6d0b7..61c54a9789 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 04/12] 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 61c54a9789..62b0c6d0b7 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 05/12] 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 b5a3665726..41300b0f27 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 195e0ea275..670bc41301 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 720521713e..9014b94a4c 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 dccf4abc54..7684b1d468 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 7ce71dafc2..5ef1113be4 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'; From 979219bce6655126a4b6341f5dba92c970e46e94 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 28 Oct 2024 08:58:46 -0700 Subject: [PATCH 06/12] feat: Refine CJS/ESM build configuration for browser SDK. --- packages/sdk/browser/package.json | 25 +++++------- packages/sdk/browser/rollup.config.js | 58 --------------------------- packages/sdk/browser/tsconfig.json | 11 +++-- packages/sdk/browser/tsup.config.ts | 25 ++++++++++++ 4 files changed, 41 insertions(+), 78 deletions(-) delete mode 100644 packages/sdk/browser/rollup.config.js create mode 100644 packages/sdk/browser/tsup.config.ts diff --git a/packages/sdk/browser/package.json b/packages/sdk/browser/package.json index 2b05c0efd9..180d8ec8b2 100644 --- a/packages/sdk/browser/package.json +++ b/packages/sdk/browser/package.json @@ -17,9 +17,14 @@ "sdk" ], "exports": { - "types": "./dist/src/index.d.ts", - "require": "./dist/index.cjs.js", - "import": "./dist/index.es.js" + "require": { + "types": "./dist/index.d.cts", + "default": "./dist/index.cjs" + }, + "import": { + "types": "./dist/esm/index.d.ts", + "default": "./dist/esm/index.js" + } }, "type": "module", "files": [ @@ -27,23 +32,16 @@ ], "scripts": { "clean": "rimraf dist", - "build": "tsc --noEmit && rollup -c rollup.config.js", + "build": "tsup", "lint": "eslint . --ext .ts,.tsx", "prettier": "prettier --write '**/*.@(js|ts|tsx|json|css)' --ignore-path ../../../.prettierignore", "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" - }, "devDependencies": { + "@launchdarkly/js-client-sdk-common": "1.10.0", "@jest/globals": "^29.7.0", - "@rollup/plugin-commonjs": "^25.0.0", - "@rollup/plugin-json": "^6.1.0", - "@rollup/plugin-node-resolve": "^15.0.2", - "@rollup/plugin-terser": "^0.4.3", - "@rollup/plugin-typescript": "^11.1.1", "@trivago/prettier-plugin-sort-imports": "^4.1.1", "@types/jest": "^29.5.11", "@typescript-eslint/eslint-plugin": "^6.20.0", @@ -59,9 +57,8 @@ "jest-environment-jsdom": "^29.7.0", "prettier": "^3.0.0", "rimraf": "^5.0.5", - "rollup": "^3.23.0", - "rollup-plugin-visualizer": "^5.12.0", "ts-jest": "^29.1.1", + "tsup": "^8.3.5", "typedoc": "0.25.0", "typescript": "^5.5.3" } diff --git a/packages/sdk/browser/rollup.config.js b/packages/sdk/browser/rollup.config.js deleted file mode 100644 index a2c260896d..0000000000 --- a/packages/sdk/browser/rollup.config.js +++ /dev/null @@ -1,58 +0,0 @@ -import common from '@rollup/plugin-commonjs'; -import json from '@rollup/plugin-json'; -import resolve from '@rollup/plugin-node-resolve'; -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 terserOpts = { - mangle: { - properties: { - // Mangle class properties which start with an underscore. - regex: /^_/, - // Do not mangle '_meta', because this is part of our JSON - // data model. - reserved: ['_meta'], - }, - }, -}; - -export default [ - { - ...getSharedConfig('es', 'dist/index.es.js'), - plugins: [ - typescript({ - module: 'esnext', - }), - common({ - transformMixedEsModules: true, - esmExternals: true, - }), - resolve(), - terser(terserOpts), - json(), - // The 'sourcemap' option allows using the minified size, not the size before minification. - visualizer({ sourcemap: true }), - ], - }, - { - ...getSharedConfig('cjs', 'dist/index.cjs.js'), - plugins: [typescript(), common(), resolve(), terser(terserOpts), json()], - }, -]; diff --git a/packages/sdk/browser/tsconfig.json b/packages/sdk/browser/tsconfig.json index 9299487855..f8a0b0f1e1 100644 --- a/packages/sdk/browser/tsconfig.json +++ b/packages/sdk/browser/tsconfig.json @@ -8,18 +8,17 @@ "moduleResolution": "node", "noImplicitOverride": true, "resolveJsonModule": true, - // Uses "." so it can load package.json. - "rootDir": ".", + "rootDir": "src", "outDir": "dist", "skipLibCheck": true, - // enables importers to jump to source - "sourceMap": true, + "sourceMap": false, "strict": true, "stripInternal": true, "target": "ES2017", "types": ["node", "jest"], "allowJs": true }, + "include": ["src"], "exclude": [ "vite.config.ts", "__tests__", @@ -27,9 +26,9 @@ "docs", "example", "node_modules", - "contract-tests", "babel.config.js", - "jestSetupFile.ts", + "setup-jest.js", + "rollup.config.js", "**/*.test.ts*" ] } diff --git a/packages/sdk/browser/tsup.config.ts b/packages/sdk/browser/tsup.config.ts new file mode 100644 index 0000000000..1b56227199 --- /dev/null +++ b/packages/sdk/browser/tsup.config.ts @@ -0,0 +1,25 @@ +// It is a dev dependency and the linter doesn't understand. +// eslint-disable-next-line import/no-extraneous-dependencies +import { defineConfig } from 'tsup'; + +export default defineConfig({ + minify: 'terser', + terserOptions: { + mangle: { + properties: { + // Mangle class properties which start with an underscore. + regex: /^_/, + // Do not mangle '_meta', because this is part of our JSON + // data model. + reserved: ['_meta'], + }, + }, + }, + format: ['esm', 'cjs'], + entry: ['src/index.ts'], + splitting: false, + sourcemap: false, + clean: true, + dts: true, + treeshake: true, +}); From 55672bb0d285ed4c18ca247d5a631e41e6de0360 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 28 Oct 2024 11:25:13 -0700 Subject: [PATCH 07/12] --amend --- .../contract-tests/entity/src/makeLogger.ts | 8 ++++---- packages/sdk/browser/package.json | 19 ++++++++++++------- packages/sdk/browser/tsup.config.ts | 3 ++- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/sdk/browser/contract-tests/entity/src/makeLogger.ts b/packages/sdk/browser/contract-tests/entity/src/makeLogger.ts index 6d09061d0d..a8cf9f165d 100644 --- a/packages/sdk/browser/contract-tests/entity/src/makeLogger.ts +++ b/packages/sdk/browser/contract-tests/entity/src/makeLogger.ts @@ -2,19 +2,19 @@ import { LDLogger } from '@launchdarkly/js-client-sdk'; export function makeLogger(tag: string): LDLogger { return { - debug(message, ...args: any[]) { + debug(message: any, ...args: any[]) { // eslint-disable-next-line no-console console.debug(`${new Date().toISOString()} [${tag}]: ${message}`, ...args); }, - info(message, ...args: any[]) { + info(message: any, ...args: any[]) { // eslint-disable-next-line no-console console.info(`${new Date().toISOString()} [${tag}]: ${message}`, ...args); }, - warn(message, ...args: any[]) { + warn(message: any, ...args: any[]) { // eslint-disable-next-line no-console console.warn(`${new Date().toISOString()} [${tag}]: ${message}`, ...args); }, - error(message, ...args: any[]) { + error(message: any, ...args: any[]) { // eslint-disable-next-line no-console console.error(`${new Date().toISOString()} [${tag}]: ${message}`, ...args); }, diff --git a/packages/sdk/browser/package.json b/packages/sdk/browser/package.json index 180d8ec8b2..aa1191e3d5 100644 --- a/packages/sdk/browser/package.json +++ b/packages/sdk/browser/package.json @@ -16,17 +16,20 @@ "feature management", "sdk" ], + "type": "module", + "main": "./dist/index.cjs", + "module": "./dist/index.js", + "types": "./dist/index.d.ts", "exports": { + "import": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js" + }, "require": { "types": "./dist/index.d.cts", - "default": "./dist/index.cjs" - }, - "import": { - "types": "./dist/esm/index.d.ts", - "default": "./dist/esm/index.js" + "require": "./dist/index.cjs" } }, - "type": "module", "files": [ "dist" ], @@ -39,8 +42,10 @@ "coverage": "yarn test --coverage", "check": "yarn prettier && yarn lint && yarn build && yarn test" }, + "dependencies": { + "@launchdarkly/js-client-sdk-common": "1.10.0" + }, "devDependencies": { - "@launchdarkly/js-client-sdk-common": "1.10.0", "@jest/globals": "^29.7.0", "@trivago/prettier-plugin-sort-imports": "^4.1.1", "@types/jest": "^29.5.11", diff --git a/packages/sdk/browser/tsup.config.ts b/packages/sdk/browser/tsup.config.ts index 1b56227199..85d66123ed 100644 --- a/packages/sdk/browser/tsup.config.ts +++ b/packages/sdk/browser/tsup.config.ts @@ -20,6 +20,7 @@ export default defineConfig({ splitting: false, sourcemap: false, clean: true, - dts: true, + noExternal: ['@launchdarkly/js-sdk-common', '@launchdarkly/js-client-sdk-common'], treeshake: true, + dts: true, }); From 87ce8e0e5a53e6f76ab84389017312c7e3385e7e Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 28 Oct 2024 14:59:59 -0700 Subject: [PATCH 08/12] Use esbuild options which removes dependence on rollup and terser. --- packages/sdk/browser/tsup.config.ts | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/sdk/browser/tsup.config.ts b/packages/sdk/browser/tsup.config.ts index 416c90c242..80dfd6c2de 100644 --- a/packages/sdk/browser/tsup.config.ts +++ b/packages/sdk/browser/tsup.config.ts @@ -7,23 +7,21 @@ export default defineConfig({ index: 'src/index.ts', compat: 'src/compat/index.ts' }, - minify: 'terser', - terserOptions: { - mangle: { - properties: { - // Mangle class properties which start with an underscore. - regex: /^_/, - // Do not mangle '_meta', because this is part of our JSON - // data model. - reserved: ['_meta'], - }, - }, - }, + minify: true, format: ['esm', 'cjs'], splitting: false, sourcemap: false, clean: true, noExternal: ['@launchdarkly/js-sdk-common', '@launchdarkly/js-client-sdk-common'], - treeshake: true, dts: true, + metafile: true, + minifyIdentifiers: true, + esbuildOptions(opts) { + // This would normally be `^_(?!meta|_)`, but go doesn't support negative look-ahead assertions, + // so we need to craft something that works without it. + // So start of line followed by a character that isn't followed by m or underscore, but we + // want other things that do start with m, so we need to progressively handle more characters + // of meta with exclusions. + opts.mangleProps = /^_([^m|_]|m[^e]|me[^t]|met[^a])/; + }, }); From cdf80b6fbd1c4c88c9e8b68c3b25a38451092d10 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 28 Oct 2024 15:01:37 -0700 Subject: [PATCH 09/12] Remove unneeded option. --- packages/sdk/browser/tsup.config.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/sdk/browser/tsup.config.ts b/packages/sdk/browser/tsup.config.ts index 80dfd6c2de..4249a69f56 100644 --- a/packages/sdk/browser/tsup.config.ts +++ b/packages/sdk/browser/tsup.config.ts @@ -15,7 +15,6 @@ export default defineConfig({ noExternal: ['@launchdarkly/js-sdk-common', '@launchdarkly/js-client-sdk-common'], dts: true, metafile: true, - minifyIdentifiers: true, esbuildOptions(opts) { // This would normally be `^_(?!meta|_)`, but go doesn't support negative look-ahead assertions, // so we need to craft something that works without it. From 0fbe44c45c24d05c459c5cb64bf67d3b9228c1b4 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 29 Oct 2024 08:46:18 -0700 Subject: [PATCH 10/12] Lint --- packages/sdk/browser/tsup.config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sdk/browser/tsup.config.ts b/packages/sdk/browser/tsup.config.ts index 4249a69f56..17f272124a 100644 --- a/packages/sdk/browser/tsup.config.ts +++ b/packages/sdk/browser/tsup.config.ts @@ -5,7 +5,7 @@ import { defineConfig } from 'tsup'; export default defineConfig({ entry: { index: 'src/index.ts', - compat: 'src/compat/index.ts' + compat: 'src/compat/index.ts', }, minify: true, format: ['esm', 'cjs'], From 4b3beb28453d80511b18bf0f2ba08bc2f5daf955 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 29 Oct 2024 08:48:05 -0700 Subject: [PATCH 11/12] No param re-assign. --- packages/sdk/browser/tsup.config.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/sdk/browser/tsup.config.ts b/packages/sdk/browser/tsup.config.ts index 17f272124a..56a856d876 100644 --- a/packages/sdk/browser/tsup.config.ts +++ b/packages/sdk/browser/tsup.config.ts @@ -21,6 +21,7 @@ export default defineConfig({ // So start of line followed by a character that isn't followed by m or underscore, but we // want other things that do start with m, so we need to progressively handle more characters // of meta with exclusions. + // eslint-disable-next-line no-param-reassign opts.mangleProps = /^_([^m|_]|m[^e]|me[^t]|met[^a])/; }, }); From f7b2254a65ebff687d89dd3945e69a5900e0efa4 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 29 Oct 2024 08:52:39 -0700 Subject: [PATCH 12/12] Change root dir. --- packages/sdk/browser/tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sdk/browser/tsconfig.json b/packages/sdk/browser/tsconfig.json index f8a0b0f1e1..7306c5b0c6 100644 --- a/packages/sdk/browser/tsconfig.json +++ b/packages/sdk/browser/tsconfig.json @@ -8,7 +8,7 @@ "moduleResolution": "node", "noImplicitOverride": true, "resolveJsonModule": true, - "rootDir": "src", + "rootDir": ".", "outDir": "dist", "skipLibCheck": true, "sourceMap": false,