From 8e3120e8d9b8b3d04d9cede52d3d256756eb001f Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 29 Oct 2024 08:33:25 -0700 Subject: [PATCH 1/2] chore: Enable polling report contract tests. (#639) --- .../sdk/browser/contract-tests/entity/src/ClientEntity.ts | 2 +- packages/sdk/browser/contract-tests/suppressions.txt | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts b/packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts index 6d50584d0..f05a587fe 100644 --- a/packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts +++ b/packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts @@ -18,7 +18,7 @@ function makeSdkConfig(options: SDKConfigParams, tag: string) { const cf: LDOptions = { withReasons: options.clientSide.evaluationReasons, logger: makeLogger(`${tag}.sdk`), - // useReport: options.clientSide.useReport, + useReport: options.clientSide.useReport, }; if (options.serviceEndpoints) { diff --git a/packages/sdk/browser/contract-tests/suppressions.txt b/packages/sdk/browser/contract-tests/suppressions.txt index 6410dd06f..30a7f7365 100644 --- a/packages/sdk/browser/contract-tests/suppressions.txt +++ b/packages/sdk/browser/contract-tests/suppressions.txt @@ -4,9 +4,3 @@ streaming/requests/URL path is computed correctly/no environment filter/base URI streaming/requests/context properties/single kind minimal/REPORT streaming/requests/context properties/single kind with all attributes/REPORT streaming/requests/context properties/multi-kind/REPORT -polling/requests/method and headers/REPORT/http -polling/requests/URL path is computed correctly/no environment filter/base URI has no trailing slash/REPORT -polling/requests/URL path is computed correctly/no environment filter/base URI has a trailing slash/REPORT -polling/requests/context properties/single kind minimal/REPORT -polling/requests/context properties/single kind with all attributes/REPORT -polling/requests/context properties/multi-kind/REPORT From 44a223730fed10fbd75e8de7c87c63570774fe96 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 29 Oct 2024 08:42:29 -0700 Subject: [PATCH 2/2] feat: Add a module for increased backward compatibility. (#637) --- .../compat/LDClientCompatImpl.test.ts | 524 ++++++++++++++++++ packages/sdk/browser/package.json | 13 +- packages/sdk/browser/rollup.config.js | 28 +- .../sdk/browser/src/compat/LDClientCompat.ts | 142 +++++ .../browser/src/compat/LDClientCompatImpl.ts | 242 ++++++++ .../sdk/browser/src/compat/LDCompatOptions.ts | 19 + .../sdk/browser/src/compat/LDEmitterCompat.ts | 79 +++ packages/sdk/browser/src/compat/index.ts | 49 ++ .../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 + 12 files changed, 1151 insertions(+), 29 deletions(-) create mode 100644 packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts create mode 100644 packages/sdk/browser/src/compat/LDClientCompat.ts create mode 100644 packages/sdk/browser/src/compat/LDClientCompatImpl.ts create mode 100644 packages/sdk/browser/src/compat/LDCompatOptions.ts create mode 100644 packages/sdk/browser/src/compat/LDEmitterCompat.ts create mode 100644 packages/sdk/browser/src/compat/index.ts create mode 100644 packages/sdk/browser/src/compat/wrapPromiseCallback.ts diff --git a/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts b/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts new file mode 100644 index 000000000..41300b0f2 --- /dev/null +++ b/packages/sdk/browser/__tests__/compat/LDClientCompatImpl.test.ts @@ -0,0 +1,524 @@ +import { jest } from '@jest/globals'; + +import { LDContext, LDFlagSet } from '@launchdarkly/js-client-sdk-common'; + +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(), + 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.mock('../../src/BrowserClient', () => ({ + __esModule: true, + BrowserClient: jest.fn(() => mockBrowserClient), +})); + +afterEach(() => { + jest.resetAllMocks(); +}); + +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; + + beforeEach(() => { + 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' }, + { 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 }; + + mockBrowserClient.identify.mockResolvedValue(); + 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) => { + jest.advanceTimersToNextTimer(); + const context: LDContext = { kind: 'user', key: 'new-user' }; + const mockFlags: LDFlagSet = { flag1: true, flag2: false }; + + 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(); + + 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 () => { + jest.advanceTimersToNextTimer(); + mockBrowserClient.close.mockResolvedValue(); + + await expect(client.close()).resolves.toBeUndefined(); + expect(mockBrowserClient.close).toHaveBeenCalled(); + }); + + it('should call the callback when provided to close', (done) => { + jest.advanceTimersToNextTimer(); + mockBrowserClient.close.mockResolvedValue(); + + client.close(() => { + expect(mockBrowserClient.close).toHaveBeenCalled(); + done(); + }); + }); + + it('should return a promise from flush when no callback is provided', async () => { + jest.advanceTimersToNextTimer(); + 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) => { + 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 waitForInitialization when the client is initialized', async () => { + jest.advanceTimersToNextTimer(); + mockBrowserClient.identify.mockResolvedValue(undefined); + + await expect(client.waitForInitialization()).resolves.toBeUndefined(); + expect(mockBrowserClient.identify).toHaveBeenCalledWith( + { kind: 'user', key: 'user-key' }, + { bootstrap: undefined, hash: undefined, noTimeout: true }, + ); + }); + + it('should resolve second waitForInitialization immediately', async () => { + jest.advanceTimersToNextTimer(); + mockBrowserClient.identify.mockResolvedValue(undefined); + + 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 }, + ); + }); + + it('should resolve waitUntilReady immediately if the client is already initialized', async () => { + jest.advanceTimersToNextTimer(); + mockBrowserClient.identify.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(); + + 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); + + const result = client.allFlags(); + + 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); + }); + + 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/package.json b/packages/sdk/browser/package.json index 2b05c0efd..d28c12c3e 100644 --- a/packages/sdk/browser/package.json +++ b/packages/sdk/browser/package.json @@ -17,9 +17,16 @@ "sdk" ], "exports": { - "types": "./dist/src/index.d.ts", - "require": "./dist/index.cjs.js", - "import": "./dist/index.es.js" + ".": { + "types": "./dist/src/index.d.ts", + "require": "./dist/index.cjs.js", + "import": "./dist/index.es.js" + }, + "./compat": { + "types": "./dist/src/compat/index.d.ts", + "require": "./dist/compat.cjs.js", + "import": "./dist/compat.es.js" + } }, "type": "module", "files": [ diff --git a/packages/sdk/browser/rollup.config.js b/packages/sdk/browser/rollup.config.js index a2c260896..f1810e932 100644 --- a/packages/sdk/browser/rollup.config.js +++ b/packages/sdk/browser/rollup.config.js @@ -5,19 +5,17 @@ import terser from '@rollup/plugin-terser'; import typescript from '@rollup/plugin-typescript'; import { visualizer } from 'rollup-plugin-visualizer'; -const getSharedConfig = (format, file) => ({ - input: 'src/index.ts', - output: [ - { - format: format, - sourcemap: true, - file: file, - }, - ], - onwarn: (warning) => { - if (warning.code !== 'CIRCULAR_DEPENDENCY') { - console.error(`(!) ${warning.message}`); - } +const getSharedConfig = (format, extension) => ({ + input: { + index: 'src/index.ts', + compat: 'src/compat/index.ts' + }, + output: + { + format: format, + sourcemap: true, + dir: 'dist', + entryFileNames: '[name]' + extension }, }); @@ -35,7 +33,7 @@ const terserOpts = { export default [ { - ...getSharedConfig('es', 'dist/index.es.js'), + ...getSharedConfig('es', '.es.js'), plugins: [ typescript({ module: 'esnext', @@ -52,7 +50,7 @@ export default [ ], }, { - ...getSharedConfig('cjs', 'dist/index.cjs.js'), + ...getSharedConfig('cjs', '.cjs.js'), plugins: [typescript(), common(), resolve(), terser(terserOpts), json()], }, ]; diff --git a/packages/sdk/browser/src/compat/LDClientCompat.ts b/packages/sdk/browser/src/compat/LDClientCompat.ts new file mode 100644 index 000000000..670bc4130 --- /dev/null +++ b/packages/sdk/browser/src/compat/LDClientCompat.ts @@ -0,0 +1,142 @@ +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 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; + + /** + * 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. + * + * @param onDone + * A function which will be called when the operation completes. If omitted, you + * will receive a Promise instead. + * + * @returns + * If you provided a callback, then nothing. Otherwise, a Promise which resolves once + * closing is finished. It will never be rejected. + */ + close(onDone?: () => void): Promise | undefined; + + /** + * Flushes all pending analytics events. + * + * Normally, batches of events are delivered in the background at intervals determined by the + * `flushInterval` property of {@link LDOptions}. Calling `flush()` triggers an immediate delivery. + * + * @param onDone + * A function which will be called when the flush completes. If omitted, you + * will receive a Promise instead. + * + * @returns + * If you provided a callback, then nothing. Otherwise, a Promise which resolves once + * flushing is finished. Note that the Promise will be rejected if the HTTP request + * fails, so be sure to attach a rejection handler to it. + */ + flush(onDone?: () => void): Promise | undefined; +} diff --git a/packages/sdk/browser/src/compat/LDClientCompatImpl.ts b/packages/sdk/browser/src/compat/LDClientCompatImpl.ts new file mode 100644 index 000000000..9014b94a4 --- /dev/null +++ b/packages/sdk/browser/src/compat/LDClientCompatImpl.ts @@ -0,0 +1,242 @@ +// TODO may or may not need this. +import { + AutoEnvAttributes, + cancelableTimedPromise, + Hook, + LDContext, + 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, { CompatEventName } 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: CompatEventName, callback: (...args: any[]) => void): void { + this._emitter.off(key, callback); + } + + on(key: CompatEventName, 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(() => undefined), + 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); + } + + 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. + * + * The client isn't always the expected type of the consumer. It returns an LDClient interface + * which is less capable than, for example, the node client interface. + * + * @param basePromise The promise to race against a timeout. + * @param timeout The timeout in seconds. + * @param logger A logger to log when the timeout expires. + * @returns + */ + private _promiseWithTimeout(basePromise: Promise, timeout?: number): Promise { + if (timeout) { + const cancelableTimeout = cancelableTimedPromise(timeout, 'waitForInitialization'); + return Promise.race([ + basePromise.then(() => cancelableTimeout.cancel()), + cancelableTimeout.promise, + ]).catch((reason) => { + if (reason instanceof LDTimeoutError) { + this.logger?.error(reason.message); + } + throw reason; + }); + } + return basePromise; + } +} diff --git a/packages/sdk/browser/src/compat/LDCompatOptions.ts b/packages/sdk/browser/src/compat/LDCompatOptions.ts new file mode 100644 index 000000000..95235f5b0 --- /dev/null +++ b/packages/sdk/browser/src/compat/LDCompatOptions.ts @@ -0,0 +1,19 @@ +import { LDFlagSet } from '@launchdarkly/js-client-sdk-common'; + +import { BrowserOptions } from '../options'; + +export interface LDOptions extends BrowserOptions { + /** + * The initial set of flags to use until the remote set is retrieved. + * + * For more information, refer to the + * [SDK Reference Guide](https://docs.launchdarkly.com/sdk/features/bootstrapping#javascript). + */ + bootstrap?: LDFlagSet; + + /** + * The signed canonical context key, for the initial context, if you are using + * [Secure Mode](https://docs.launchdarkly.com/sdk/features/secure-mode#configuring-secure-mode-in-the-javascript-client-side-sdk). + */ + hash?: string; +} diff --git a/packages/sdk/browser/src/compat/LDEmitterCompat.ts b/packages/sdk/browser/src/compat/LDEmitterCompat.ts new file mode 100644 index 000000000..7684b1d46 --- /dev/null +++ b/packages/sdk/browser/src/compat/LDEmitterCompat.ts @@ -0,0 +1,79 @@ +import { LDEmitterEventName } from '@launchdarkly/js-client-sdk-common'; + +import { LDClient } from '../BrowserClient'; + +type CompatOnlyEvents = 'ready' | 'failed' | 'initialized'; +export type CompatEventName = LDEmitterEventName | CompatOnlyEvents; + +const COMPAT_EVENTS: string[] = ['ready', 'failed', 'initialized']; + +export default class LDEmitterCompat { + private _listeners: Map = new Map(); + + constructor(private readonly _client: LDClient) {} + + on(name: CompatEventName, listener: Function) { + if (COMPAT_EVENTS.includes(name)) { + if (!this._listeners.has(name)) { + this._listeners.set(name, [listener]); + } else { + this._listeners.get(name)?.push(listener); + } + } else { + this._client.on(name, listener as (...args: any[]) => void); + } + } + + /** + * Unsubscribe one or all events. + * + * @param name + * @param listener Optional. If unspecified, all listeners for the event will be removed. + */ + off(name: CompatEventName, listener?: Function) { + if (COMPAT_EVENTS.includes(name)) { + const existingListeners = this._listeners.get(name); + if (!existingListeners) { + return; + } + + if (listener) { + const updated = existingListeners.filter((fn) => fn !== listener); + if (updated.length === 0) { + this._listeners.delete(name); + } else { + this._listeners.set(name, updated); + } + return; + } + + // listener was not specified, so remove them all for that event + this._listeners.delete(name); + } else { + this._client.off(name, listener as (...args: any[]) => void); + } + } + + private _invokeListener(listener: Function, name: CompatEventName, ...detail: any[]) { + try { + listener(...detail); + } catch (err) { + this._client.logger.error( + `Encountered error invoking handler for "${name}", detail: "${err}"`, + ); + } + } + + emit(name: CompatEventName, ...detail: any[]) { + const listeners = this._listeners.get(name); + listeners?.forEach((listener) => this._invokeListener(listener, name, ...detail)); + } + + eventNames(): string[] { + return [...this._listeners.keys()]; + } + + listenerCount(name: CompatEventName): number { + return this._listeners.get(name)?.length ?? 0; + } +} diff --git a/packages/sdk/browser/src/compat/index.ts b/packages/sdk/browser/src/compat/index.ts new file mode 100644 index 000000000..5ef1113be --- /dev/null +++ b/packages/sdk/browser/src/compat/index.ts @@ -0,0 +1,49 @@ +/** + * 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 { LDContext, LDOptions } from '..'; +import { LDClient } from './LDClientCompat'; +import LDClientCompatImpl from './LDClientCompatImpl'; + +/** + * Creates an instance of the LaunchDarkly client. This version of initialization is for + * improved backwards compatibility. In general the `initialize` function from the root packge + * should be used instead of the one in the `/compat` module. + * + * The client will begin attempting to connect to LaunchDarkly as soon as it is created. To + * determine when it is ready to use, call {@link LDClient.waitForInitialization}, or register an + * event listener for the `"ready"` event using {@link LDClient.on}. + * + * Example: + * import { initialize } from '@launchdarkly/js-client-sdk/compat'; + * const client = initialize(envKey, context, options); + * + * Note: The `compat` module minimizes compatibility breaks, but not all functionality is directly + * equivalent to the previous version. + * + * LDOptions are where the primary differences are. By default the new SDK implementation will + * generally use localStorage to cache flags. This can be disabled by setting the + * `maxCachedContexts` to 0. + * + * This does allow combinations that were not possible before. For insance an initial context + * could be identified using bootstrap, and a second context without bootstrap, and the second + * context could cache flags in local storage. For more control the primary module can be used + * instead of this `compat` module (for instance bootstrap can be provided per identify call in + * the primary module). + * + * @param envKey + * The environment ID. + * @param context + * The initial context properties. These can be changed later with {@link LDClient.identify}. + * @param options + * Optional configuration settings. + * @return + * The new client instance. + */ +export function initialize(envKey: string, context: LDContext, options?: LDOptions): LDClient { + return new LDClientCompatImpl(envKey, context, options); +} diff --git a/packages/sdk/browser/src/compat/wrapPromiseCallback.ts b/packages/sdk/browser/src/compat/wrapPromiseCallback.ts new file mode 100644 index 000000000..efcbc13be --- /dev/null +++ b/packages/sdk/browser/src/compat/wrapPromiseCallback.ts @@ -0,0 +1,40 @@ +/** + * Wrap a promise to invoke an optional callback upon resolution or rejection. + * + * This function assumes the callback follows the Node.js callback type: (err, value) => void + * + * If a callback is provided: + * - if the promise is resolved, invoke the callback with (null, value) + * - if the promise is rejected, invoke the callback with (error, null) + * + * @param {Promise} promise + * @param {Function} callback + * @returns Promise | undefined + */ +export function wrapPromiseCallback( + promise: Promise, + callback?: (err: Error | null, res: T | null) => void, +): Promise | undefined { + const ret = promise.then( + (value) => { + if (callback) { + setTimeout(() => { + callback(null, value); + }, 0); + } + return value; + }, + (error) => { + if (callback) { + setTimeout(() => { + callback(error, null); + }, 0); + } else { + return Promise.reject(error); + } + return undefined; + }, + ); + + return !callback ? ret : undefined; +} diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index a9abb0e49..ff6565b2b 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -39,13 +39,14 @@ import LDEmitter, { EventName } from './LDEmitter'; const { ClientMessages, ErrorKinds } = internal; +const DEFAULT_IDENIFY_TIMEOUT_SECONDS = 5; + export default class LDClientImpl implements LDClient { private readonly _config: Configuration; private _uncheckedContext?: LDContext; private _checkedContext?: Context; private readonly _diagnosticsManager?: internal.DiagnosticsManager; private _eventProcessor?: internal.EventProcessor; - private _identifyTimeout: number = 5; readonly logger: LDLogger; private _updateProcessor?: subsystem.LDStreamProcessor; @@ -181,7 +182,10 @@ export default class LDClientImpl implements LDClient { return this._checkedContext; } - private _createIdentifyPromise(timeout: number): { + private _createIdentifyPromise( + timeout: number, + noTimeout: boolean, + ): { identifyPromise: Promise; identifyResolve: () => void; identifyReject: (err: Error) => void; @@ -189,13 +193,17 @@ export default class LDClientImpl implements LDClient { let res: any; let rej: any; - const slow = new Promise((resolve, reject) => { + const basePromise = new Promise((resolve, reject) => { res = resolve; rej = reject; }); + if (noTimeout) { + return { identifyPromise: basePromise, identifyResolve: res, identifyReject: rej }; + } + const timed = timedPromise(timeout, 'identify'); - const raced = Promise.race([timed, slow]).catch((e) => { + const raced = Promise.race([timed, basePromise]).catch((e) => { if (e.message.includes('timed out')) { this.logger.error(`identify error: ${e}`); } @@ -221,11 +229,12 @@ export default class LDClientImpl implements LDClient { * 3. A network error is encountered during initialization. */ async identify(pristineContext: LDContext, identifyOptions?: LDIdentifyOptions): Promise { - if (identifyOptions?.timeout) { - this._identifyTimeout = identifyOptions.timeout; - } + const identifyTimeout = identifyOptions?.timeout ?? DEFAULT_IDENIFY_TIMEOUT_SECONDS; + const noTimeout = identifyOptions?.timeout === undefined && identifyOptions?.noTimeout === true; - if (this._identifyTimeout > this._highTimeoutThreshold) { + // When noTimeout is specified, and a timeout is not secified, then this condition cannot + // be encountered. (Our default would need to be greater) + if (identifyTimeout > this._highTimeoutThreshold) { this.logger.warn( 'The identify function was called with a timeout greater than ' + `${this._highTimeoutThreshold} seconds. We recommend a timeout of less than ` + @@ -250,7 +259,8 @@ export default class LDClientImpl implements LDClient { this._eventProcessor?.sendEvent(this._eventFactoryDefault.identifyEvent(this._checkedContext)); const { identifyPromise, identifyResolve, identifyReject } = this._createIdentifyPromise( - this._identifyTimeout, + identifyTimeout, + noTimeout, ); this.logger.debug(`Identifying ${JSON.stringify(this._checkedContext)}`); diff --git a/packages/shared/sdk-client/src/api/LDClient.ts b/packages/shared/sdk-client/src/api/LDClient.ts index cac279bc2..b3a996625 100644 --- a/packages/shared/sdk-client/src/api/LDClient.ts +++ b/packages/shared/sdk-client/src/api/LDClient.ts @@ -58,8 +58,7 @@ export interface LDClient { /** * Shuts down the client and releases its resources, after delivering any pending analytics - * events. After the client is closed, all calls to {@link variation} will return default values, - * and it will not make any requests to LaunchDarkly. + * events. */ close(): Promise; diff --git a/packages/shared/sdk-client/src/api/LDIdentifyOptions.ts b/packages/shared/sdk-client/src/api/LDIdentifyOptions.ts index 3aecdfab4..467da79b6 100644 --- a/packages/shared/sdk-client/src/api/LDIdentifyOptions.ts +++ b/packages/shared/sdk-client/src/api/LDIdentifyOptions.ts @@ -20,4 +20,17 @@ export interface LDIdentifyOptions { * Defaults to false. */ waitForNetworkResults?: boolean; + + /** + * When set to true, and timeout is not set, this indicates that the identify operation will + * not have any timeout. In typical usage, where an application awaits the promise, a timeout + * is important because identify can potentially take indefinite time depending on network + * conditions. If your application specifically does not block any operations pending the promise + * resolution, then you can use this opton to explicitly indicate that. + * + * If you set this to true, and you do not set a timeout, and you block aspects of operation of + * your application, then those aspects can be blocked indefinitely. Generally this option will + * not be required. + */ + noTimeout?: boolean; }