From 535dff84a3dac8fd85f71455dec9be7ce76915b7 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 15 Dec 2025 17:44:21 -0600 Subject: [PATCH 1/6] refactor: changing Flagstore + FlagUpdater classes to interface - This saves a few bytes --- .../flag-manager/FlagPersistence.test.ts | 20 +-- .../__tests__/flag-manager/FlagStore.test.ts | 6 +- .../flag-manager/FlagUpdater.test.ts | 20 +-- .../src/flag-manager/FlagManager.ts | 8 +- .../src/flag-manager/FlagPersistence.ts | 2 +- .../sdk-client/src/flag-manager/FlagStore.ts | 68 ++++--- .../src/flag-manager/FlagUpdater.ts | 166 +++++++++++------- 7 files changed, 171 insertions(+), 119 deletions(-) diff --git a/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts b/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts index cd31ef46c4..17becee83a 100644 --- a/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts +++ b/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts @@ -2,7 +2,7 @@ import { Context, Crypto, Hasher, LDLogger, Platform, Storage } from '@launchdarkly/js-sdk-common'; import FlagPersistence from '../../src/flag-manager/FlagPersistence'; -import { DefaultFlagStore } from '../../src/flag-manager/FlagStore'; +import { createDefaultFlagStore } from '../../src/flag-manager/FlagStore'; import FlagUpdater from '../../src/flag-manager/FlagUpdater'; import { namespaceForContextData, @@ -14,7 +14,7 @@ const TEST_NAMESPACE = 'TestNamespace'; describe('FlagPersistence tests', () => { test('loadCached returns false when no cache', async () => { - const flagStore = new DefaultFlagStore(); + const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const fpUnderTest = new FlagPersistence( makeMockPlatform(makeMemoryStorage(), makeMockCrypto()), @@ -31,7 +31,7 @@ describe('FlagPersistence tests', () => { }); test('loadCached returns false when corrupt cache', async () => { - const flagStore = new DefaultFlagStore(); + const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const fpUnderTest = new FlagPersistence( makeMockPlatform( @@ -59,7 +59,7 @@ describe('FlagPersistence tests', () => { }); test('loadCached updates FlagUpdater with cached flags', async () => { - const flagStore = new DefaultFlagStore(); + const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const flagUpdater = new FlagUpdater(flagStore, mockLogger); const flagUpdaterSpy = jest.spyOn(flagUpdater, 'initCached'); @@ -87,7 +87,7 @@ describe('FlagPersistence tests', () => { }); test('loadCached migrates pre 10.3.1 cached flags', async () => { - const flagStore = new DefaultFlagStore(); + const flagStore = createDefaultFlagStore(); const memoryStorage = makeMemoryStorage(); const mockLogger = makeMockLogger(); const flagUpdater = new FlagUpdater(flagStore, mockLogger); @@ -118,7 +118,7 @@ describe('FlagPersistence tests', () => { test('init successfully persists flags', async () => { const memoryStorage = makeMemoryStorage(); const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); - const flagStore = new DefaultFlagStore(); + const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const flagUpdater = new FlagUpdater(flagStore, mockLogger); @@ -154,7 +154,7 @@ describe('FlagPersistence tests', () => { test('init prunes cached contexts above max', async () => { const memoryStorage = makeMemoryStorage(); const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); - const flagStore = new DefaultFlagStore(); + const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const flagUpdater = new FlagUpdater(flagStore, mockLogger); @@ -201,7 +201,7 @@ describe('FlagPersistence tests', () => { test('init kicks timestamp', async () => { const memoryStorage = makeMemoryStorage(); const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); - const flagStore = new DefaultFlagStore(); + const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const flagUpdater = new FlagUpdater(flagStore, mockLogger); @@ -234,7 +234,7 @@ describe('FlagPersistence tests', () => { test('upsert updates persistence', async () => { const memoryStorage = makeMemoryStorage(); const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); - const flagStore = new DefaultFlagStore(); + const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const flagUpdater = new FlagUpdater(flagStore, mockLogger); @@ -274,7 +274,7 @@ describe('FlagPersistence tests', () => { test('upsert ignores inactive context', async () => { const memoryStorage = makeMemoryStorage(); const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); - const flagStore = new DefaultFlagStore(); + const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const flagUpdater = new FlagUpdater(flagStore, mockLogger); diff --git a/packages/shared/sdk-client/__tests__/flag-manager/FlagStore.test.ts b/packages/shared/sdk-client/__tests__/flag-manager/FlagStore.test.ts index 8cb3389ecb..2a33ec1e48 100644 --- a/packages/shared/sdk-client/__tests__/flag-manager/FlagStore.test.ts +++ b/packages/shared/sdk-client/__tests__/flag-manager/FlagStore.test.ts @@ -1,10 +1,10 @@ -import { DefaultFlagStore } from '../../src/flag-manager/FlagStore'; +import FlagStore, { createDefaultFlagStore } from '../../src/flag-manager/FlagStore'; describe('given an empty flag store', () => { - let store: DefaultFlagStore; + let store: FlagStore; beforeEach(() => { - store = new DefaultFlagStore(); + store = createDefaultFlagStore(); }); it.each(['unknown', 'toString', 'length'])( diff --git a/packages/shared/sdk-client/__tests__/flag-manager/FlagUpdater.test.ts b/packages/shared/sdk-client/__tests__/flag-manager/FlagUpdater.test.ts index 6bc41a9ff1..9d57536b58 100644 --- a/packages/shared/sdk-client/__tests__/flag-manager/FlagUpdater.test.ts +++ b/packages/shared/sdk-client/__tests__/flag-manager/FlagUpdater.test.ts @@ -1,6 +1,6 @@ import { Context, LDLogger } from '@launchdarkly/js-sdk-common'; -import { DefaultFlagStore } from '../../src/flag-manager/FlagStore'; +import { createDefaultFlagStore } from '../../src/flag-manager/FlagStore'; import FlagUpdater, { FlagsChangeCallback } from '../../src/flag-manager/FlagUpdater'; import { Flag } from '../../src/types'; @@ -27,7 +27,7 @@ function makeMockLogger(): LDLogger { describe('FlagUpdater tests', () => { test('init calls init on underlying flag store', async () => { - const mockStore = new DefaultFlagStore(); + const mockStore = createDefaultFlagStore(); const mockStoreSpy = jest.spyOn(mockStore, 'init'); const mockLogger = makeMockLogger(); @@ -45,7 +45,7 @@ describe('FlagUpdater tests', () => { }); test('triggers callbacks on init', async () => { - const mockStore = new DefaultFlagStore(); + const mockStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const mockCallback: FlagsChangeCallback = jest.fn(); @@ -63,7 +63,7 @@ describe('FlagUpdater tests', () => { }); test('init cached ignores context same as active context', async () => { - const mockStore = new DefaultFlagStore(); + const mockStore = createDefaultFlagStore(); const mockStoreSpy = jest.spyOn(mockStore, 'init'); const mockLogger = makeMockLogger(); @@ -83,7 +83,7 @@ describe('FlagUpdater tests', () => { }); test('upsert ignores inactive context', async () => { - const mockStore = new DefaultFlagStore(); + const mockStore = createDefaultFlagStore(); const mockStoreSpy = jest.spyOn(mockStore, 'init'); const mockLogger = makeMockLogger(); @@ -107,7 +107,7 @@ describe('FlagUpdater tests', () => { }); test('upsert rejects data with old versions', async () => { - const mockStore = new DefaultFlagStore(); + const mockStore = createDefaultFlagStore(); const mockStoreSpy = jest.spyOn(mockStore, 'init'); const mockLogger = makeMockLogger(); @@ -130,7 +130,7 @@ describe('FlagUpdater tests', () => { }); test('upsert updates underlying store', async () => { - const mockStore = new DefaultFlagStore(); + const mockStore = createDefaultFlagStore(); const mockStoreSpyInit = jest.spyOn(mockStore, 'init'); const mockStoreSpyInsertOrUpdate = jest.spyOn(mockStore, 'insertOrUpdate'); const mockLogger = makeMockLogger(); @@ -155,7 +155,7 @@ describe('FlagUpdater tests', () => { }); test('upsert triggers callbacks', async () => { - const mockStore = new DefaultFlagStore(); + const mockStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const mockCallbackA: FlagsChangeCallback = jest.fn(); const mockCallbackB: FlagsChangeCallback = jest.fn(); @@ -184,7 +184,7 @@ describe('FlagUpdater tests', () => { }); test('off removes callback', async () => { - const mockStore = new DefaultFlagStore(); + const mockStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const mockCallback: FlagsChangeCallback = jest.fn(); @@ -217,7 +217,7 @@ describe('FlagUpdater tests', () => { }); test('off can be called many times safely', async () => { - const mockStore = new DefaultFlagStore(); + const mockStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); const mockCallback: FlagsChangeCallback = jest.fn(); diff --git a/packages/shared/sdk-client/src/flag-manager/FlagManager.ts b/packages/shared/sdk-client/src/flag-manager/FlagManager.ts index 7ad670a9ed..094819bf56 100644 --- a/packages/shared/sdk-client/src/flag-manager/FlagManager.ts +++ b/packages/shared/sdk-client/src/flag-manager/FlagManager.ts @@ -2,8 +2,8 @@ import { Context, LDFlagValue, LDLogger, Platform } from '@launchdarkly/js-sdk-c import { namespaceForEnvironment } from '../storage/namespaceUtils'; import FlagPersistence from './FlagPersistence'; -import { DefaultFlagStore } from './FlagStore'; -import FlagUpdater, { FlagsChangeCallback } from './FlagUpdater'; +import { createDefaultFlagStore } from './FlagStore'; +import createFlagUpdater, { FlagsChangeCallback, FlagUpdater } from './FlagUpdater'; import { ItemDescriptor } from './ItemDescriptor'; /** @@ -117,7 +117,7 @@ export interface LDDebugOverride { } export default class DefaultFlagManager implements FlagManager { - private _flagStore = new DefaultFlagStore(); + private _flagStore = createDefaultFlagStore(); private _flagUpdater: FlagUpdater; private _flagPersistencePromise: Promise; private _overrides?: { [key: string]: LDFlagValue }; @@ -136,7 +136,7 @@ export default class DefaultFlagManager implements FlagManager { logger: LDLogger, timeStamper: () => number = () => Date.now(), ) { - this._flagUpdater = new FlagUpdater(this._flagStore, logger); + this._flagUpdater = createFlagUpdater(this._flagStore, logger); this._flagPersistencePromise = this._initPersistence( platform, sdkKey, diff --git a/packages/shared/sdk-client/src/flag-manager/FlagPersistence.ts b/packages/shared/sdk-client/src/flag-manager/FlagPersistence.ts index 3977412d2c..118b35df3b 100644 --- a/packages/shared/sdk-client/src/flag-manager/FlagPersistence.ts +++ b/packages/shared/sdk-client/src/flag-manager/FlagPersistence.ts @@ -4,7 +4,7 @@ import { namespaceForContextData, namespaceForContextIndex } from '../storage/na import { Flags } from '../types'; import ContextIndex from './ContextIndex'; import FlagStore from './FlagStore'; -import FlagUpdater from './FlagUpdater'; +import { FlagUpdater } from './FlagUpdater'; import { ItemDescriptor } from './ItemDescriptor'; /** diff --git a/packages/shared/sdk-client/src/flag-manager/FlagStore.ts b/packages/shared/sdk-client/src/flag-manager/FlagStore.ts index 2108aa6469..d3051a963e 100644 --- a/packages/shared/sdk-client/src/flag-manager/FlagStore.ts +++ b/packages/shared/sdk-client/src/flag-manager/FlagStore.ts @@ -1,43 +1,53 @@ import { ItemDescriptor } from './ItemDescriptor'; /** - * This interface exists for testing purposes + * FlagStore used to store flag data in memory. */ export default interface FlagStore { + /** + * Initializes the flag store with the given flags. + */ init(newFlags: { [key: string]: ItemDescriptor }): void; + /** + * Inserts or updates the flag with the given key and update. + */ insertOrUpdate(key: string, update: ItemDescriptor): void; + /** + * Gets the flag with the given key. + */ get(key: string): ItemDescriptor | undefined; + /** + * Gets all the flags in the flag store. + */ getAll(): { [key: string]: ItemDescriptor }; } /** - * In memory flag store. + * Creates the default implementation of the flag store. */ -export class DefaultFlagStore implements FlagStore { - private _flags: { [key: string]: ItemDescriptor } = {}; - - init(newFlags: { [key: string]: ItemDescriptor }) { - this._flags = Object.entries(newFlags).reduce( - (acc: { [k: string]: ItemDescriptor }, [key, flag]) => { - acc[key] = flag; - return acc; - }, - {}, - ); - } - - insertOrUpdate(key: string, update: ItemDescriptor) { - this._flags[key] = update; - } - - get(key: string): ItemDescriptor | undefined { - if (Object.prototype.hasOwnProperty.call(this._flags, key)) { - return this._flags[key]; - } - return undefined; - } - - getAll(): { [key: string]: ItemDescriptor } { - return this._flags; - } +export function createDefaultFlagStore(): FlagStore { + let flags: { [key: string]: ItemDescriptor } = {}; + return { + init(newFlags: { [key: string]: ItemDescriptor }) { + flags = Object.entries(newFlags).reduce( + (acc: { [k: string]: ItemDescriptor }, [key, flag]) => { + acc[key] = flag; + return acc; + }, + {}, + ); + }, + insertOrUpdate(key: string, update: ItemDescriptor) { + flags[key] = update; + }, + get(key: string): ItemDescriptor | undefined { + if (Object.prototype.hasOwnProperty.call(flags, key)) { + return flags[key]; + } + return undefined; + }, + getAll(): { [key: string]: ItemDescriptor } { + return flags; + }, + }; } diff --git a/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts b/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts index f750e80682..191fb035d4 100644 --- a/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts +++ b/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts @@ -30,76 +30,118 @@ export type FlagsChangeCallback = ( * It handles versions checking to handle out of order flag updates and * also handles flag comparisons for change notification. */ -export default class FlagUpdater { - private _flagStore: FlagStore; - private _logger: LDLogger; - private _activeContext: Context | undefined; - private _changeCallbacks = new Array(); +export interface FlagUpdater { + /** + * Handles the flag changes by calling the change callbacks. + * + * @param keys keys of the flags that changed + * @param type type of change that occurred see {@link FlagChangeType} + */ + handleFlagChanges(keys: string[], type: FlagChangeType): void; - constructor(flagStore: FlagStore, logger: LDLogger) { - this._flagStore = flagStore; - this._logger = logger; - } - - handleFlagChanges(keys: string[], type: FlagChangeType): void { - if (this._activeContext) { - this._changeCallbacks.forEach((callback) => { - try { - callback(this._activeContext!, keys, type); - } catch (err) { - /* intentionally empty */ - } - }); - } else { - this._logger.warn( - 'Received a change event without an active context. Changes will not be propagated.', - ); - } - } - - init(context: Context, newFlags: { [key: string]: ItemDescriptor }) { - this._activeContext = context; - const oldFlags = this._flagStore.getAll(); - this._flagStore.init(newFlags); - const changed = calculateChangedKeys(oldFlags, newFlags); - if (changed.length > 0) { - this.handleFlagChanges(changed, 'init'); - } - } + /** + * Initializes the flag updater with the given context and flags. + * This will be called every time a new context is identified. + * + * @param context the context to initialize the flag updater with + * @param newFlags the flags to initialize the flag updater with + */ + init(context: Context, newFlags: { [key: string]: ItemDescriptor }): void; - initCached(context: Context, newFlags: { [key: string]: ItemDescriptor }) { - if (this._activeContext?.canonicalKey === context.canonicalKey) { - return; - } + initCached(context: Context, newFlags: { [key: string]: ItemDescriptor }): void; - this.init(context, newFlags); - } + /** + * Upserts the flag with the given key and item. + * + * @param context the context to upsert the flag with + * @param key the key of the flag to upsert + * @param item the item to upsert the flag with + * @returns true if the flag was upserted, false otherwise + */ + upsert(context: Context, key: string, item: ItemDescriptor): boolean; - upsert(context: Context, key: string, item: ItemDescriptor): boolean { - if (this._activeContext?.canonicalKey !== context.canonicalKey) { - this._logger.warn('Received an update for an inactive context.'); - return false; - } + /** + * Registers a callback to be called when the flags change. + * + * @param callback the callback to register + */ + on(callback: FlagsChangeCallback): void; - const currentValue = this._flagStore.get(key); - if (currentValue !== undefined && currentValue.version >= item.version) { - // this is an out of order update that can be ignored - return false; - } + /** + * Unregisters a callback to be called when the flags change. + * + * @param callback the callback to unregister + */ + off(callback: FlagsChangeCallback): void; +} - this._flagStore.insertOrUpdate(key, item); - this.handleFlagChanges([key], 'patch'); - return true; - } +export default function createFlagUpdater(_flagStore: FlagStore, _logger: LDLogger): FlagUpdater { + let flagStore: FlagStore = _flagStore; + let logger: LDLogger = _logger; + let activeContext: Context | undefined; + let changeCallbacks = new Array(); - on(callback: FlagsChangeCallback): void { - this._changeCallbacks.push(callback); - } + return { + handleFlagChanges(keys: string[], type: FlagChangeType): void { + if (activeContext) { + changeCallbacks.forEach((callback) => { + try { + callback(activeContext!, keys, type); + } catch (err) { + /* intentionally empty */ + } + }); + } else { + logger.warn( + 'Received a change event without an active context. Changes will not be propagated.', + ); + } + }, - off(callback: FlagsChangeCallback): void { - const index = this._changeCallbacks.indexOf(callback); - if (index > -1) { - this._changeCallbacks.splice(index, 1); + init(context: Context, newFlags: { [key: string]: ItemDescriptor }) { + activeContext = context; + const oldFlags = flagStore.getAll(); + flagStore.init(newFlags); + const changed = calculateChangedKeys(oldFlags, newFlags); + if (changed.length > 0) { + this.handleFlagChanges(changed, 'init'); + } + }, + + initCached(context: Context, newFlags: { [key: string]: ItemDescriptor }) { + if (activeContext?.canonicalKey === context.canonicalKey) { + return; + } + + this.init(context, newFlags); + }, + + upsert(context: Context, key: string, item: ItemDescriptor): boolean { + if (activeContext?.canonicalKey !== context.canonicalKey) { + logger.warn('Received an update for an inactive context.'); + return false; + } + + const currentValue = flagStore.get(key); + if (currentValue !== undefined && currentValue.version >= item.version) { + // this is an out of order update that can be ignored + return false; + } + + flagStore.insertOrUpdate(key, item); + this.handleFlagChanges([key], 'patch'); + return true; + }, + + on(callback: FlagsChangeCallback): void { + changeCallbacks.push(callback); + }, + + off(callback: FlagsChangeCallback): void { + const index = changeCallbacks.indexOf(callback); + if (index > -1) { + changeCallbacks.splice(index, 1); + } } } } From e0449541414176f84c60c8730aec31967812204f Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 22 Dec 2025 17:17:35 -0500 Subject: [PATCH 2/6] feat: add legacy storage key cleanup functionality - Introduced `getLegacyStorageKeys` method to as an optional internal option that could be configured per SDK. - Updated `LDClientImpl` to support cleaning old persistent data based on the new option `cleanOldPersistentData`. - Implemented `getLegacyStorageKeys` in Browser SDK so the feature is fully available there. --- packages/sdk/browser/src/BrowserClient.ts | 5 ++++ .../sdk/browser/src/platform/LocalStorage.ts | 7 ++++++ .../flag-manager/FlagPersistence.test.ts | 20 ++++++++-------- .../flag-manager/FlagUpdater.test.ts | 20 ++++++++-------- .../shared/sdk-client/src/LDClientImpl.ts | 13 +++++++++++ .../shared/sdk-client/src/api/LDOptions.ts | 8 +++++++ .../src/configuration/Configuration.ts | 1 + .../src/configuration/validators.ts | 1 + .../src/flag-manager/FlagUpdater.ts | 23 +++++++++---------- 9 files changed, 66 insertions(+), 32 deletions(-) diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index 96e1904c37..b7c30e5f41 100644 --- a/packages/sdk/browser/src/BrowserClient.ts +++ b/packages/sdk/browser/src/BrowserClient.ts @@ -39,6 +39,7 @@ import { import { LDPlugin } from './LDPlugin'; import validateBrowserOptions, { BrowserOptions, filterToBaseOptionsWithDefaults } from './options'; import BrowserPlatform from './platform/BrowserPlatform'; +import { getAllStorageKeys } from './platform/LocalStorage'; class BrowserClientImpl extends LDClientImpl { private readonly _goalManager?: GoalManager; @@ -137,6 +138,10 @@ class BrowserClientImpl extends LDClientImpl { diagnosticsManager, ), { + getLegacyStorageKeys: () => { + // This logic is derived from https://github.com/launchdarkly/js-sdk-common/blob/main/src/PersistentFlagStore.js + return getAllStorageKeys().filter((key) => key.startsWith(`ld:${clientSideId}:`)); + }, analyticsEventPath: `/events/bulk/${clientSideId}`, diagnosticEventPath: `/events/diagnostic/${clientSideId}`, includeAuthorizationHeader: false, diff --git a/packages/sdk/browser/src/platform/LocalStorage.ts b/packages/sdk/browser/src/platform/LocalStorage.ts index 88748d70c8..788245a5ca 100644 --- a/packages/sdk/browser/src/platform/LocalStorage.ts +++ b/packages/sdk/browser/src/platform/LocalStorage.ts @@ -6,6 +6,13 @@ export function isLocalStorageSupported() { return typeof localStorage !== 'undefined'; } +export function getAllStorageKeys() { + if (!isLocalStorageSupported()) { + return []; + } + return Object.keys(localStorage); +} + /** * Implementation of Storage using localStorage for the browser. * diff --git a/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts b/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts index 17becee83a..2a509f4e1a 100644 --- a/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts +++ b/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts @@ -3,7 +3,7 @@ import { Context, Crypto, Hasher, LDLogger, Platform, Storage } from '@launchdar import FlagPersistence from '../../src/flag-manager/FlagPersistence'; import { createDefaultFlagStore } from '../../src/flag-manager/FlagStore'; -import FlagUpdater from '../../src/flag-manager/FlagUpdater'; +import createFlagUpdater from '../../src/flag-manager/FlagUpdater'; import { namespaceForContextData, namespaceForContextIndex, @@ -21,7 +21,7 @@ describe('FlagPersistence tests', () => { TEST_NAMESPACE, 5, flagStore, - new FlagUpdater(flagStore, mockLogger), + createFlagUpdater(flagStore, mockLogger), mockLogger, ); @@ -41,7 +41,7 @@ describe('FlagPersistence tests', () => { TEST_NAMESPACE, 5, flagStore, - new FlagUpdater(flagStore, mockLogger), + createFlagUpdater(flagStore, mockLogger), mockLogger, ); @@ -61,7 +61,7 @@ describe('FlagPersistence tests', () => { test('loadCached updates FlagUpdater with cached flags', async () => { const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); - const flagUpdater = new FlagUpdater(flagStore, mockLogger); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); const flagUpdaterSpy = jest.spyOn(flagUpdater, 'initCached'); const fpUnderTest = new FlagPersistence( makeMockPlatform(makeMemoryStorage(), makeMockCrypto()), @@ -90,7 +90,7 @@ describe('FlagPersistence tests', () => { const flagStore = createDefaultFlagStore(); const memoryStorage = makeMemoryStorage(); const mockLogger = makeMockLogger(); - const flagUpdater = new FlagUpdater(flagStore, mockLogger); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); const fpUnderTest = new FlagPersistence( makeMockPlatform(memoryStorage, makeMockCrypto()), TEST_NAMESPACE, @@ -120,7 +120,7 @@ describe('FlagPersistence tests', () => { const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); - const flagUpdater = new FlagUpdater(flagStore, mockLogger); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); const fpUnderTest = new FlagPersistence( mockPlatform, @@ -156,7 +156,7 @@ describe('FlagPersistence tests', () => { const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); - const flagUpdater = new FlagUpdater(flagStore, mockLogger); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); const fpUnderTest = new FlagPersistence( mockPlatform, @@ -203,7 +203,7 @@ describe('FlagPersistence tests', () => { const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); - const flagUpdater = new FlagUpdater(flagStore, mockLogger); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); const fpUnderTest = new FlagPersistence( mockPlatform, @@ -236,7 +236,7 @@ describe('FlagPersistence tests', () => { const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); - const flagUpdater = new FlagUpdater(flagStore, mockLogger); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); const fpUnderTest = new FlagPersistence( mockPlatform, @@ -276,7 +276,7 @@ describe('FlagPersistence tests', () => { const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); const flagStore = createDefaultFlagStore(); const mockLogger = makeMockLogger(); - const flagUpdater = new FlagUpdater(flagStore, mockLogger); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); const fpUnderTest = new FlagPersistence( mockPlatform, diff --git a/packages/shared/sdk-client/__tests__/flag-manager/FlagUpdater.test.ts b/packages/shared/sdk-client/__tests__/flag-manager/FlagUpdater.test.ts index 9d57536b58..7d61edf69a 100644 --- a/packages/shared/sdk-client/__tests__/flag-manager/FlagUpdater.test.ts +++ b/packages/shared/sdk-client/__tests__/flag-manager/FlagUpdater.test.ts @@ -1,7 +1,7 @@ import { Context, LDLogger } from '@launchdarkly/js-sdk-common'; import { createDefaultFlagStore } from '../../src/flag-manager/FlagStore'; -import FlagUpdater, { FlagsChangeCallback } from '../../src/flag-manager/FlagUpdater'; +import createFlagUpdater, { FlagsChangeCallback } from '../../src/flag-manager/FlagUpdater'; import { Flag } from '../../src/types'; function makeMockFlag(): Flag { @@ -38,7 +38,7 @@ describe('FlagUpdater tests', () => { flag: makeMockFlag(), }, }; - const updaterUnderTest = new FlagUpdater(mockStore, mockLogger); + const updaterUnderTest = createFlagUpdater(mockStore, mockLogger); updaterUnderTest.init(context, flags); expect(mockStoreSpy).toHaveBeenCalledTimes(1); expect(mockStoreSpy).toHaveBeenLastCalledWith(flags); @@ -56,7 +56,7 @@ describe('FlagUpdater tests', () => { flag: makeMockFlag(), }, }; - const updaterUnderTest = new FlagUpdater(mockStore, mockLogger); + const updaterUnderTest = createFlagUpdater(mockStore, mockLogger); updaterUnderTest.on(mockCallback); updaterUnderTest.init(context, flags); expect(mockCallback).toHaveBeenCalledTimes(1); @@ -75,7 +75,7 @@ describe('FlagUpdater tests', () => { flag: makeMockFlag(), }, }; - const updaterUnderTest = new FlagUpdater(mockStore, mockLogger); + const updaterUnderTest = createFlagUpdater(mockStore, mockLogger); updaterUnderTest.init(activeContext, flags); expect(mockStoreSpy).toHaveBeenCalledTimes(1); updaterUnderTest.initCached(sameContext, flags); @@ -95,7 +95,7 @@ describe('FlagUpdater tests', () => { flag: makeMockFlag(), }, }; - const updaterUnderTest = new FlagUpdater(mockStore, mockLogger); + const updaterUnderTest = createFlagUpdater(mockStore, mockLogger); updaterUnderTest.init(activeContext, flags); expect(mockStoreSpy).toHaveBeenCalledTimes(1); @@ -118,7 +118,7 @@ describe('FlagUpdater tests', () => { flag: makeMockFlag(), }, }; - const updaterUnderTest = new FlagUpdater(mockStore, mockLogger); + const updaterUnderTest = createFlagUpdater(mockStore, mockLogger); updaterUnderTest.init(context, flags); expect(mockStoreSpy).toHaveBeenCalledTimes(1); @@ -142,7 +142,7 @@ describe('FlagUpdater tests', () => { flag: makeMockFlag(), }, }; - const updaterUnderTest = new FlagUpdater(mockStore, mockLogger); + const updaterUnderTest = createFlagUpdater(mockStore, mockLogger); updaterUnderTest.init(context, flags); expect(mockStoreSpyInit).toHaveBeenCalledTimes(1); @@ -167,7 +167,7 @@ describe('FlagUpdater tests', () => { flag: makeMockFlag(), }, }; - const updaterUnderTest = new FlagUpdater(mockStore, mockLogger); + const updaterUnderTest = createFlagUpdater(mockStore, mockLogger); updaterUnderTest.init(context, flags); // register the callbacks @@ -195,7 +195,7 @@ describe('FlagUpdater tests', () => { flag: makeMockFlag(), }, }; - const updaterUnderTest = new FlagUpdater(mockStore, mockLogger); + const updaterUnderTest = createFlagUpdater(mockStore, mockLogger); updaterUnderTest.init(context, flags); // register the callback @@ -228,7 +228,7 @@ describe('FlagUpdater tests', () => { flag: makeMockFlag(), }, }; - const updaterUnderTest = new FlagUpdater(mockStore, mockLogger); + const updaterUnderTest = createFlagUpdater(mockStore, mockLogger); updaterUnderTest.init(context, flags); updaterUnderTest.off(mockCallback); updaterUnderTest.on(mockCallback); diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index 1dc13c0ecf..ded2f096ab 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -164,6 +164,19 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { if (this._inspectorManager.hasInspectors()) { this._hookRunner.addHook(getInspectorHook(this._inspectorManager)); } + + if (options.cleanOldPersistentData && internalOptions?.getLegacyStorageKeys) { + this.logger.debug('Cleaning old persistent data.'); + Promise.all( + internalOptions.getLegacyStorageKeys().map((key) => this.platform.storage?.clear(key)), + ) + .catch((error) => { + this.logger.error(`Error cleaning old persistent data: ${error}`); + }) + .finally(() => { + this.logger.debug('Cleaned old persistent data.'); + }); + } } allFlags(): LDFlagSet { diff --git a/packages/shared/sdk-client/src/api/LDOptions.ts b/packages/shared/sdk-client/src/api/LDOptions.ts index 5ce6b647d3..d5f1e107ad 100644 --- a/packages/shared/sdk-client/src/api/LDOptions.ts +++ b/packages/shared/sdk-client/src/api/LDOptions.ts @@ -271,4 +271,12 @@ export interface LDOptions { * let us know through a github issue or support. */ inspectors?: LDInspection[]; + + /** + * Whether to clean old persistent data. If set to true, the SDK will check to see if old + * there are any persistent data that is generated by an older version and remove it. + * + * @defaultValue false. + */ + cleanOldPersistentData?: boolean; } diff --git a/packages/shared/sdk-client/src/configuration/Configuration.ts b/packages/shared/sdk-client/src/configuration/Configuration.ts index a421a889ab..ead620ac35 100644 --- a/packages/shared/sdk-client/src/configuration/Configuration.ts +++ b/packages/shared/sdk-client/src/configuration/Configuration.ts @@ -22,6 +22,7 @@ export interface LDClientInternalOptions extends internal.LDInternalOptions { trackEventModifier?: (event: internal.InputCustomEvent) => internal.InputCustomEvent; getImplementationHooks: (environmentMetadata: LDPluginEnvironmentMetadata) => Hook[]; credentialType: 'clientSideId' | 'mobileKey'; + getLegacyStorageKeys?: () => string[]; } export interface Configuration { diff --git a/packages/shared/sdk-client/src/configuration/validators.ts b/packages/shared/sdk-client/src/configuration/validators.ts index fba69b4382..b8a89de6c3 100644 --- a/packages/shared/sdk-client/src/configuration/validators.ts +++ b/packages/shared/sdk-client/src/configuration/validators.ts @@ -34,6 +34,7 @@ const validators: Record = { payloadFilterKey: TypeValidators.stringMatchingRegex(/^[a-zA-Z0-9](\w|\.|-)*$/), hooks: TypeValidators.createTypeArray('Hook[]', {}), inspectors: TypeValidators.createTypeArray('LDInspection', {}), + cleanOldPersistentData: TypeValidators.Boolean, }; export default validators; diff --git a/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts b/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts index 191fb035d4..da6cd5b8e0 100644 --- a/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts +++ b/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts @@ -76,10 +76,10 @@ export interface FlagUpdater { } export default function createFlagUpdater(_flagStore: FlagStore, _logger: LDLogger): FlagUpdater { - let flagStore: FlagStore = _flagStore; - let logger: LDLogger = _logger; + const flagStore: FlagStore = _flagStore; + const logger: LDLogger = _logger; let activeContext: Context | undefined; - let changeCallbacks = new Array(); + const changeCallbacks = new Array(); return { handleFlagChanges(keys: string[], type: FlagChangeType): void { @@ -107,41 +107,40 @@ export default function createFlagUpdater(_flagStore: FlagStore, _logger: LDLogg this.handleFlagChanges(changed, 'init'); } }, - initCached(context: Context, newFlags: { [key: string]: ItemDescriptor }) { if (activeContext?.canonicalKey === context.canonicalKey) { return; } - + this.init(context, newFlags); }, - + upsert(context: Context, key: string, item: ItemDescriptor): boolean { if (activeContext?.canonicalKey !== context.canonicalKey) { logger.warn('Received an update for an inactive context.'); return false; } - + const currentValue = flagStore.get(key); if (currentValue !== undefined && currentValue.version >= item.version) { // this is an out of order update that can be ignored return false; } - + flagStore.insertOrUpdate(key, item); this.handleFlagChanges([key], 'patch'); return true; }, - + on(callback: FlagsChangeCallback): void { changeCallbacks.push(callback); }, - + off(callback: FlagsChangeCallback): void { const index = changeCallbacks.indexOf(callback); if (index > -1) { changeCallbacks.splice(index, 1); } - } - } + }, + }; } From 0775c605c1067ced7da57aba48524dc2f0e5a52f Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 22 Dec 2025 17:31:40 -0500 Subject: [PATCH 3/6] test: adding unit tests for legacy data clean up - Added a test to verify that old persistent data is cleaned while retaining current keys. --- .../__tests__/LDClientImpl.storage.test.ts | 42 +++++++++++++++++++ .../shared/sdk-client/src/LDClientImpl.ts | 3 +- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/packages/shared/sdk-client/__tests__/LDClientImpl.storage.test.ts b/packages/shared/sdk-client/__tests__/LDClientImpl.storage.test.ts index e46d06a534..192aa6aae2 100644 --- a/packages/shared/sdk-client/__tests__/LDClientImpl.storage.test.ts +++ b/packages/shared/sdk-client/__tests__/LDClientImpl.storage.test.ts @@ -651,4 +651,46 @@ describe('sdk-client storage', () => { expect(flagsInStorage['does-not-exist']).toMatchObject({ ...deleteResponse, deleted: true }); expect(emitter.emit).toHaveBeenCalledWith('change', context, ['does-not-exist']); }); + + it('cleans old persistent data while leaving current keys untouched', async () => { + const storage: Record = { + legacyKey1: 'legacy-key-1', + legacyKey2: 'legacy-key-2', + currentKey1: flagStorageKey, + currentKey2: indexStorageKey, + otherKey: 'some-other-key', + }; + + // Set up storage with legacy keys, current keys, and other keys + mockPlatform.storage.get.mockImplementation((storageKey: string) => { + return storage[storageKey]; + }); + + const legacyKeys = ['legacyKey1', 'legacyKey2']; + const internalOptions = { + getLegacyStorageKeys: () => legacyKeys, + getImplementationHooks: () => [], + credentialType: 'clientSideId' as const, + }; + + ldc = new LDClientImpl( + testSdkKey, + AutoEnvAttributes.Disabled, + mockPlatform, + { + logger, + sendEvents: false, + cleanOldPersistentData: true, + }, + makeTestDataManagerFactory(testSdkKey, mockPlatform), + internalOptions, + ); + + expect(mockPlatform.storage.clear).toHaveBeenCalledWith('legacyKey1'); + expect(mockPlatform.storage.clear).toHaveBeenCalledWith('legacyKey2'); + expect(mockPlatform.storage.clear).toHaveBeenCalledTimes(2); + expect(mockPlatform.storage.clear).not.toHaveBeenCalledWith('currentKey1'); + expect(mockPlatform.storage.clear).not.toHaveBeenCalledWith('currentKey2'); + expect(mockPlatform.storage.clear).not.toHaveBeenCalledWith('otherKey'); + }); }); diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index ded2f096ab..d836928a66 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -165,7 +165,8 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { this._hookRunner.addHook(getInspectorHook(this._inspectorManager)); } - if (options.cleanOldPersistentData && internalOptions?.getLegacyStorageKeys) { + if (options.cleanOldPersistentData && internalOptions?.getLegacyStorageKeys && this.platform.storage) { + // NOTE: we are letting this fail silently because it's not critical and we don't want to block the client from initializing. this.logger.debug('Cleaning old persistent data.'); Promise.all( internalOptions.getLegacyStorageKeys().map((key) => this.platform.storage?.clear(key)), From 88d769bc4a68fb093fb252bcf911c9e2f5e4f37a Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 22 Dec 2025 17:36:24 -0500 Subject: [PATCH 4/6] docs: add note on "override" changes in LDClientImpl - Added a comment in `LDClientImpl.ts` to clarify that "override" changes are not being tracked as they are currently used only for debugging and not persisted. This may change in the future. --- packages/shared/sdk-client/src/LDClientImpl.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index d836928a66..f9b4840605 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -646,6 +646,10 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { }; } }); + + // NOTE: we are not tracking "override" changes because, at the time of writing, + // these changes are only used for debugging purposes and are not persisted. This + // may change in the future. if (type === 'init') { this._inspectorManager.onFlagsChanged(details); } else if (type === 'patch') { From 6b67e8696871d82c73a3f221729b91f03c90788a Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 22 Dec 2025 17:43:40 -0500 Subject: [PATCH 5/6] chore: fix lint issues --- packages/sdk/browser/src/BrowserClient.ts | 7 ++--- .../__tests__/LDClientImpl.storage.test.ts | 6 ++-- .../shared/sdk-client/src/LDClientImpl.ts | 30 ++++++++++++------- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index b7c30e5f41..6191ad434a 100644 --- a/packages/sdk/browser/src/BrowserClient.ts +++ b/packages/sdk/browser/src/BrowserClient.ts @@ -138,10 +138,9 @@ class BrowserClientImpl extends LDClientImpl { diagnosticsManager, ), { - getLegacyStorageKeys: () => { - // This logic is derived from https://github.com/launchdarkly/js-sdk-common/blob/main/src/PersistentFlagStore.js - return getAllStorageKeys().filter((key) => key.startsWith(`ld:${clientSideId}:`)); - }, + // This logic is derived from https://github.com/launchdarkly/js-sdk-common/blob/main/src/PersistentFlagStore.js + getLegacyStorageKeys: () => + getAllStorageKeys().filter((key) => key.startsWith(`ld:${clientSideId}:`)), analyticsEventPath: `/events/bulk/${clientSideId}`, diagnosticEventPath: `/events/diagnostic/${clientSideId}`, includeAuthorizationHeader: false, diff --git a/packages/shared/sdk-client/__tests__/LDClientImpl.storage.test.ts b/packages/shared/sdk-client/__tests__/LDClientImpl.storage.test.ts index 192aa6aae2..8b1668bcb7 100644 --- a/packages/shared/sdk-client/__tests__/LDClientImpl.storage.test.ts +++ b/packages/shared/sdk-client/__tests__/LDClientImpl.storage.test.ts @@ -662,9 +662,7 @@ describe('sdk-client storage', () => { }; // Set up storage with legacy keys, current keys, and other keys - mockPlatform.storage.get.mockImplementation((storageKey: string) => { - return storage[storageKey]; - }); + mockPlatform.storage.get.mockImplementation((storageKey: string) => storage[storageKey]); const legacyKeys = ['legacyKey1', 'legacyKey2']; const internalOptions = { @@ -686,6 +684,8 @@ describe('sdk-client storage', () => { internalOptions, ); + await jest.runAllTimersAsync(); + expect(mockPlatform.storage.clear).toHaveBeenCalledWith('legacyKey1'); expect(mockPlatform.storage.clear).toHaveBeenCalledWith('legacyKey2'); expect(mockPlatform.storage.clear).toHaveBeenCalledTimes(2); diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index f9b4840605..df187b4c7d 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -165,18 +165,26 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { this._hookRunner.addHook(getInspectorHook(this._inspectorManager)); } - if (options.cleanOldPersistentData && internalOptions?.getLegacyStorageKeys && this.platform.storage) { + if ( + options.cleanOldPersistentData && + internalOptions?.getLegacyStorageKeys && + this.platform.storage + ) { // NOTE: we are letting this fail silently because it's not critical and we don't want to block the client from initializing. - this.logger.debug('Cleaning old persistent data.'); - Promise.all( - internalOptions.getLegacyStorageKeys().map((key) => this.platform.storage?.clear(key)), - ) - .catch((error) => { - this.logger.error(`Error cleaning old persistent data: ${error}`); - }) - .finally(() => { - this.logger.debug('Cleaned old persistent data.'); - }); + try { + this.logger.debug('Cleaning old persistent data.'); + Promise.all( + internalOptions.getLegacyStorageKeys().map((key) => this.platform.storage?.clear(key)), + ) + .catch((error) => { + this.logger.error(`Error cleaning old persistent data: ${error}`); + }) + .finally(() => { + this.logger.debug('Cleaned old persistent data.'); + }); + } catch (error) { + this.logger.error(`Error cleaning old persistent data: ${error}`); + } } } From 8289e33a5b30740ae876efae1c06717685e0db7f Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Tue, 6 Jan 2026 10:57:39 -0600 Subject: [PATCH 6/6] chore: addressing PR comments --- packages/sdk/browser/src/platform/LocalStorage.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/sdk/browser/src/platform/LocalStorage.ts b/packages/sdk/browser/src/platform/LocalStorage.ts index 788245a5ca..8b07bd0d4a 100644 --- a/packages/sdk/browser/src/platform/LocalStorage.ts +++ b/packages/sdk/browser/src/platform/LocalStorage.ts @@ -6,11 +6,21 @@ export function isLocalStorageSupported() { return typeof localStorage !== 'undefined'; } -export function getAllStorageKeys() { +export function getAllStorageKeys(): string[] { if (!isLocalStorageSupported()) { return []; } - return Object.keys(localStorage); + + const keys: string[] = []; + + for (let i = 0; i < localStorage.length; i += 1) { + const key = localStorage.key(i); + if (key) { + keys.push(key); + } + } + + return keys; } /**