From 1abe883a384ba81dcb4ebd8f23608f6375a93132 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 14 Apr 2025 15:44:16 -0700 Subject: [PATCH] fix: Handle default flush interval for browser SDK. --- .../sdk/browser/__tests__/options.test.ts | 29 ++++++++++++++----- packages/sdk/browser/src/BrowserClient.ts | 9 ++++-- packages/sdk/browser/src/options.ts | 21 ++++++++------ 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/packages/sdk/browser/__tests__/options.test.ts b/packages/sdk/browser/__tests__/options.test.ts index cb1d84a67f..d8aefc63b1 100644 --- a/packages/sdk/browser/__tests__/options.test.ts +++ b/packages/sdk/browser/__tests__/options.test.ts @@ -2,7 +2,7 @@ import { jest } from '@jest/globals'; import { LDLogger } from '@launchdarkly/js-client-sdk-common'; -import validateOptions, { filterToBaseOptions } from '../src/options'; +import validateBrowserOptions, { filterToBaseOptionsWithDefaults } from '../src/options'; let logger: LDLogger; @@ -16,7 +16,7 @@ beforeEach(() => { }); it('logs no warnings when all configuration is valid', () => { - validateOptions( + validateBrowserOptions( { fetchGoals: true, eventUrlTransformer: (url: string) => url, @@ -31,7 +31,7 @@ it('logs no warnings when all configuration is valid', () => { }); it('warns for invalid configuration', () => { - validateOptions( + validateBrowserOptions( { // @ts-ignore fetchGoals: 'yes', @@ -50,8 +50,8 @@ it('warns for invalid configuration', () => { ); }); -it('applies default options', () => { - const opts = validateOptions({}, logger); +it('applies default browser-specific options', () => { + const opts = validateBrowserOptions({}, logger); expect(opts.fetchGoals).toBe(true); expect(opts.eventUrlTransformer).toBeDefined(); @@ -69,9 +69,24 @@ it('filters to base options', () => { eventUrlTransformer: (url: string) => url, }; - const baseOpts = filterToBaseOptions(opts); + const baseOpts = filterToBaseOptionsWithDefaults(opts); expect(baseOpts.debug).toBe(false); - expect(Object.keys(baseOpts).length).toEqual(1); + expect(Object.keys(baseOpts).length).toEqual(2); expect(baseOpts).not.toHaveProperty('fetchGoals'); expect(baseOpts).not.toHaveProperty('eventUrlTransformer'); + expect(baseOpts.flushInterval).toEqual(2); +}); + +it('applies default overrides to common config flushInterval', () => { + const opts = {}; + const result = filterToBaseOptionsWithDefaults(opts); + expect(result.flushInterval).toEqual(2); +}); + +it('does not override common config flushInterval if it is set', () => { + const opts = { + flushInterval: 15, + }; + const result = filterToBaseOptionsWithDefaults(opts); + expect(result.flushInterval).toEqual(15); }); diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index 0ef3490e99..1585bdee24 100644 --- a/packages/sdk/browser/src/BrowserClient.ts +++ b/packages/sdk/browser/src/BrowserClient.ts @@ -21,7 +21,7 @@ import { BrowserIdentifyOptions as LDIdentifyOptions } from './BrowserIdentifyOp import { registerStateDetection } from './BrowserStateDetector'; import GoalManager from './goals/GoalManager'; import { Goal, isClick } from './goals/Goals'; -import validateOptions, { BrowserOptions, filterToBaseOptions } from './options'; +import validateBrowserOptions, { BrowserOptions, filterToBaseOptionsWithDefaults } from './options'; import BrowserPlatform from './platform/BrowserPlatform'; /** @@ -116,13 +116,16 @@ export class BrowserClient extends LDClientImpl implements LDClient { const baseUrl = options.baseUri ?? 'https://clientsdk.launchdarkly.com'; const platform = overridePlatform ?? new BrowserPlatform(logger); - const validatedBrowserOptions = validateOptions(options, logger); + // Only the browser-specific options are in validatedBrowserOptions. + const validatedBrowserOptions = validateBrowserOptions(options, logger); + // The base options are in baseOptionsWithDefaults. + const baseOptionsWithDefaults = filterToBaseOptionsWithDefaults({ ...options, logger }); const { eventUrlTransformer } = validatedBrowserOptions; super( clientSideId, autoEnvAttributes, platform, - filterToBaseOptions({ ...options, logger }), + baseOptionsWithDefaults, ( flagManager: FlagManager, configuration: Configuration, diff --git a/packages/sdk/browser/src/options.ts b/packages/sdk/browser/src/options.ts index 89e53a4c82..afe1bbf66f 100644 --- a/packages/sdk/browser/src/options.ts +++ b/packages/sdk/browser/src/options.ts @@ -73,8 +73,14 @@ const validators: { [Property in keyof BrowserOptions]: TypeValidator | undefine streaming: TypeValidators.Boolean, }; -export function filterToBaseOptions(opts: BrowserOptions): LDOptionsBase { - const baseOptions: LDOptionsBase = { ...opts }; +function withBrowserDefaults(opts: BrowserOptions): BrowserOptions { + const output = { ...opts }; + output.flushInterval ??= DEFAULT_FLUSH_INTERVAL_SECONDS; + return output; +} + +export function filterToBaseOptionsWithDefaults(opts: BrowserOptions): LDOptionsBase { + const baseOptions: LDOptionsBase = withBrowserDefaults(opts); // Remove any browser specific configuration keys so we don't get warnings from // the base implementation for unknown configuration. @@ -84,14 +90,11 @@ export function filterToBaseOptions(opts: BrowserOptions): LDOptionsBase { return baseOptions; } -function applyBrowserDefaults(opts: BrowserOptions) { - // eslint-disable-next-line no-param-reassign - opts.flushInterval ??= DEFAULT_FLUSH_INTERVAL_SECONDS; -} - -export default function validateOptions(opts: BrowserOptions, logger: LDLogger): ValidatedOptions { +export default function validateBrowserOptions( + opts: BrowserOptions, + logger: LDLogger, +): ValidatedOptions { const output: ValidatedOptions = { ...optDefaults }; - applyBrowserDefaults(output); Object.entries(validators).forEach((entry) => { const [key, validator] = entry as [keyof BrowserOptions, TypeValidator];