fix: Handle default flush interval for browser SDK. (#822)

The browser SDK was incorrectly handling its default flush interval
configuration.

There are two layers to defaults for the browser SDK. The first is
defaults which differ from the base configuration defaults, second is
defaults for browser specific configuration.

The base defaults were being applied to the browser specific
configuration, which meant those changes were being lost and instead the
base default was being used instead.

Fixes: SDK-1199

Author: kinyoklion

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);
 });

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,

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];