Skip to content

Commit 77864cb

Browse files
authored
feat(js-client-sdk): add ability to customize storage impl (#1404)
This PR will enable browser sdk to take in a custom storage implementation. Additional details: - added common implementation for client sdks to eventually to make this feature common to all js client sdks - custom storage implementations will be wrapped to ensure they don't throw <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Optional API with safe wrapper; default persistence path is unchanged when `storage` is not set. > > **Overview** > Adds an optional **`storage`** setting on the browser (and React client) SDK so apps can plug in their own persistence instead of **`window.localStorage`**, while keeping today’s default when the option is omitted. > > Shared client code introduces the **`LDStorage`** contract (`get` / `set` / `clear`) and **`createSafeStorage`**, which adapts custom implementations to the internal storage API: failures are logged, reads return **`null`**, and writes are dropped so a bad implementation cannot take down the host app. The browser client validates **`storage`**, wraps it with **`createSafeStorage`**, and passes it into **`BrowserPlatform`**, which uses the override when provided and otherwise still chooses **`LocalStorage`** only when supported. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a1f4117. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 3fc712c commit 77864cb

13 files changed

Lines changed: 406 additions & 6 deletions

File tree

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { AutoEnvAttributes, LDLogger } from '@launchdarkly/js-client-sdk-common';
2+
3+
import { makeClient } from '../src/BrowserClient';
4+
5+
const mockPlatformConstructor = jest.fn();
6+
7+
jest.mock('../src/platform/BrowserPlatform', () => ({
8+
__esModule: true,
9+
default: jest.fn().mockImplementation((logger, options, storage) => {
10+
mockPlatformConstructor(logger, options, storage);
11+
// eslint-disable-next-line global-require
12+
const { makeBasicPlatform } = require('./BrowserClient.mocks');
13+
const platform = makeBasicPlatform(options);
14+
// Surface the storage the client wired in so we can assert on it, while
15+
// keeping a working mock platform for the rest of construction.
16+
if (storage) {
17+
platform.storage = storage;
18+
}
19+
return platform;
20+
}),
21+
}));
22+
23+
let logger: LDLogger;
24+
25+
beforeEach(() => {
26+
jest.clearAllMocks();
27+
logger = { debug: jest.fn(), info: jest.fn(), warn: jest.fn(), error: jest.fn() };
28+
});
29+
30+
function getWiredStorage() {
31+
return mockPlatformConstructor.mock.calls[0][2];
32+
}
33+
34+
it('wraps and passes a custom storage through to the platform', async () => {
35+
const myStorage = {
36+
get: jest.fn(async () => 'cached'),
37+
set: jest.fn(async () => {}),
38+
clear: jest.fn(async () => {}),
39+
};
40+
41+
makeClient(
42+
'client-side-id',
43+
{ key: 'user-key', kind: 'user' },
44+
AutoEnvAttributes.Disabled,
45+
{ streaming: false, logger, diagnosticOptOut: true, storage: myStorage },
46+
);
47+
48+
const wired = getWiredStorage();
49+
expect(wired).toBeDefined();
50+
51+
// The wired storage delegates to the user's implementation.
52+
await expect(wired.get('k')).resolves.toEqual('cached');
53+
await wired.set('k', 'v');
54+
await wired.clear('k');
55+
expect(myStorage.get).toHaveBeenCalledWith('k');
56+
expect(myStorage.set).toHaveBeenCalledWith('k', 'v');
57+
expect(myStorage.clear).toHaveBeenCalledWith('k');
58+
expect(logger.warn).not.toHaveBeenCalled();
59+
});
60+
61+
it('does not pass a storage override when none is configured', () => {
62+
makeClient(
63+
'client-side-id',
64+
{ key: 'user-key', kind: 'user' },
65+
AutoEnvAttributes.Disabled,
66+
{ streaming: false, logger, diagnosticOptOut: true },
67+
);
68+
69+
expect(getWiredStorage()).toBeUndefined();
70+
});
71+
72+
it('does not let a throwing custom storage escape the wrapper', async () => {
73+
const throwingStorage = {
74+
get: jest.fn(async () => {
75+
throw new Error('boom');
76+
}),
77+
set: jest.fn(async () => {
78+
throw new Error('boom');
79+
}),
80+
clear: jest.fn(async () => {
81+
throw new Error('boom');
82+
}),
83+
};
84+
85+
makeClient(
86+
'client-side-id',
87+
{ key: 'user-key', kind: 'user' },
88+
AutoEnvAttributes.Disabled,
89+
{ streaming: false, logger, diagnosticOptOut: true, storage: throwingStorage },
90+
);
91+
92+
const wired = getWiredStorage();
93+
await expect(wired.get('k')).resolves.toBeNull();
94+
await expect(wired.set('k', 'v')).resolves.toBeUndefined();
95+
await expect(wired.clear('k')).resolves.toBeUndefined();
96+
expect(logger.error).toHaveBeenCalledTimes(3);
97+
});

packages/sdk/browser/__tests__/options.test.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { jest } from '@jest/globals';
22

3-
import { LDLogger } from '@launchdarkly/js-client-sdk-common';
3+
import { LDLogger, LDStorage } from '@launchdarkly/js-client-sdk-common';
44

55
import validateBrowserOptions, { filterToBaseOptionsWithDefaults } from '../src/options';
66

@@ -16,10 +16,23 @@ beforeEach(() => {
1616
});
1717

1818
it('logs no warnings when all configuration is valid', () => {
19+
const storage: LDStorage = {
20+
get(_key: string): Promise<string | null> {
21+
throw new Error('Function not implemented.');
22+
},
23+
set(_key: string, _value: string): Promise<void> {
24+
throw new Error('Function not implemented.');
25+
},
26+
clear(_key: string): Promise<void> {
27+
throw new Error('Function not implemented.');
28+
},
29+
};
30+
1931
validateBrowserOptions(
2032
{
2133
fetchGoals: true,
2234
eventUrlTransformer: (url: string) => url,
35+
storage,
2336
},
2437
logger,
2538
);
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { LDLogger, Storage } from '@launchdarkly/js-client-sdk-common';
2+
3+
import BrowserPlatform from '../../src/platform/BrowserPlatform';
4+
import LocalStorage, { isLocalStorageSupported } from '../../src/platform/LocalStorage';
5+
6+
jest.mock('../../src/platform/LocalStorage', () => {
7+
const actual = jest.requireActual('../../src/platform/LocalStorage');
8+
return {
9+
__esModule: true,
10+
default: jest.fn(),
11+
isLocalStorageSupported: jest.fn(() => true),
12+
getAllStorageKeys: actual.getAllStorageKeys,
13+
};
14+
});
15+
16+
const mockLocalStorage = LocalStorage as jest.MockedClass<typeof LocalStorage>;
17+
const mockIsSupported = isLocalStorageSupported as jest.MockedFunction<typeof isLocalStorageSupported>;
18+
19+
let logger: LDLogger;
20+
21+
beforeEach(() => {
22+
jest.clearAllMocks();
23+
mockIsSupported.mockReturnValue(true);
24+
logger = { debug: jest.fn(), info: jest.fn(), warn: jest.fn(), error: jest.fn() };
25+
});
26+
27+
function makeCustomStorage(): Storage {
28+
return { get: jest.fn(), set: jest.fn(), clear: jest.fn() };
29+
}
30+
31+
it('uses the provided storage override and does not construct LocalStorage', () => {
32+
const custom = makeCustomStorage();
33+
const platform = new BrowserPlatform(logger, {}, custom);
34+
35+
expect(platform.storage).toBe(custom);
36+
expect(mockLocalStorage).not.toHaveBeenCalled();
37+
});
38+
39+
it('falls back to LocalStorage when no override is provided and localStorage is supported', () => {
40+
const platform = new BrowserPlatform(logger, {});
41+
42+
expect(mockLocalStorage).toHaveBeenCalledTimes(1);
43+
expect(platform.storage).toBe(mockLocalStorage.mock.instances[0]);
44+
});
45+
46+
it('has no storage when no override is provided and localStorage is unsupported', () => {
47+
mockIsSupported.mockReturnValue(false);
48+
49+
const platform = new BrowserPlatform(logger, {});
50+
51+
expect(platform.storage).toBeUndefined();
52+
expect(mockLocalStorage).not.toHaveBeenCalled();
53+
});
54+
55+
it('uses the provided storage override even when localStorage is unsupported', () => {
56+
mockIsSupported.mockReturnValue(false);
57+
const custom = makeCustomStorage();
58+
59+
const platform = new BrowserPlatform(logger, {}, custom);
60+
61+
expect(platform.storage).toBe(custom);
62+
expect(mockLocalStorage).not.toHaveBeenCalled();
63+
});

packages/sdk/browser/src/BrowserClient.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
Configuration,
88
createDefaultSourceFactoryProvider,
99
createFDv2DataManagerBase,
10+
createSafeStorage,
1011
FDv2ConnectionMode,
1112
FlagManager,
1213
Hook,
@@ -71,9 +72,12 @@ class BrowserClientImpl extends LDClientImpl {
7172
// TODO: Use the already-configured baseUri from the SDK config. SDK-560
7273
const baseUrl = options.baseUri ?? 'https://clientsdk.launchdarkly.com';
7374

74-
const platform = overridePlatform ?? new BrowserPlatform(logger, options);
7575
// Only the browser-specific options are in validatedBrowserOptions.
7676
const validatedBrowserOptions = validateBrowserOptions(options, logger);
77+
const safeStorage = validatedBrowserOptions.storage
78+
? createSafeStorage(validatedBrowserOptions.storage, logger)
79+
: undefined;
80+
const platform = overridePlatform ?? new BrowserPlatform(logger, options, safeStorage);
7781
// The base options are in baseOptionsWithDefaults.
7882
const baseOptionsWithDefaults = filterToBaseOptionsWithDefaults({ ...options, logger });
7983
const { eventUrlTransformer } = validatedBrowserOptions;

packages/sdk/browser/src/common.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export type {
3131
LDInspection,
3232
LDLogger,
3333
LDLogLevel,
34+
LDStorage,
3435
LDMultiKindContext,
3536
LDSingleKindContext,
3637
TrackSeriesContext,

packages/sdk/browser/src/options.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {
22
LDClientDataSystemOptions,
33
LDLogger,
44
LDOptions as LDOptionsBase,
5+
LDStorage,
56
ManualModeSwitching,
67
OptionMessages,
78
TypeValidator,
@@ -96,6 +97,23 @@ export interface BrowserOptions extends Omit<LDOptionsBase, 'initialConnectionMo
9697
* A list of plugins to be used with the SDK.
9798
*/
9899
plugins?: LDPlugin[];
100+
101+
/**
102+
* Custom storage implementation.
103+
*
104+
* Storage is used for caching flag values for a context as well as persisting
105+
* generated identifiers.
106+
*
107+
* By default the browser SDK uses `window.localStorage`.
108+
*
109+
* @remarks
110+
* If there is an issue with the storage implementation, then generated keys
111+
* and context caches may not be persisted. This will cause the SDK to generate
112+
* new keys and context caches on every startup. Implementations should not
113+
* throw; if one does, the SDK logs the error and degrades gracefully rather
114+
* than crashing the application.
115+
*/
116+
storage?: LDStorage;
99117
}
100118

101119
export interface ValidatedOptions {
@@ -104,20 +122,23 @@ export interface ValidatedOptions {
104122
streaming?: boolean;
105123
automaticBackgroundHandling?: boolean;
106124
plugins: LDPlugin[];
125+
storage?: LDStorage;
107126
}
108127

109128
const optDefaults = {
110129
fetchGoals: true,
111130
eventUrlTransformer: (url: string) => url,
112131
streaming: undefined,
113132
plugins: [],
133+
storage: undefined,
114134
};
115135

116136
const validators: { [Property in keyof BrowserOptions]: TypeValidator | undefined } = {
117137
fetchGoals: TypeValidators.Boolean,
118138
eventUrlTransformer: TypeValidators.Function,
119139
streaming: TypeValidators.Boolean,
120140
plugins: TypeValidators.createTypeArray('LDPlugin', {}),
141+
storage: TypeValidators.Object,
121142
};
122143

123144
function withBrowserDefaults(opts: BrowserOptions): BrowserOptions {

packages/sdk/browser/src/platform/BrowserPlatform.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,8 @@ export default class BrowserPlatform implements Platform {
2323
requests: Requests = new BrowserRequests();
2424
storage?: Storage;
2525

26-
constructor(logger: LDLogger, options: BrowserOptions) {
27-
if (isLocalStorageSupported()) {
28-
this.storage = new LocalStorage(logger);
29-
}
26+
constructor(logger: LDLogger, options: BrowserOptions, storage?: Storage) {
27+
this.storage = storage ?? (isLocalStorageSupported() ? new LocalStorage(logger) : undefined);
3028
this.info = new BrowserInfo(options);
3129
}
3230
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import type { LDStorage } from '../../src/client';
2+
import type { LDReactClientOptions } from '../../src/client/LDOptions';
3+
4+
it('accepts a custom storage override on the react client options', () => {
5+
const storage: LDStorage = {
6+
get: async () => null,
7+
set: async () => {},
8+
clear: async () => {},
9+
};
10+
11+
const options: LDReactClientOptions = { storage };
12+
13+
expect(options.storage).toBe(storage);
14+
});

0 commit comments

Comments
 (0)