From 18d1fd839194434883687df9b7ab6d8b16e64da2 Mon Sep 17 00:00:00 2001 From: Kevin Elko Date: Wed, 13 Nov 2024 10:44:20 -0500 Subject: [PATCH 01/19] Add config option for templateId --- packages/remote-config/src/api.ts | 40 ++++++++++++++++--- .../src/client/remote_config_fetch_client.ts | 1 + packages/remote-config/src/errors.ts | 2 + packages/remote-config/src/public_types.ts | 12 ++++++ packages/remote-config/src/register.ts | 14 ++----- 5 files changed, 52 insertions(+), 17 deletions(-) diff --git a/packages/remote-config/src/api.ts b/packages/remote-config/src/api.ts index 607d4944d26..2744d2d5439 100644 --- a/packages/remote-config/src/api.ts +++ b/packages/remote-config/src/api.ts @@ -16,19 +16,21 @@ */ import { _getProvider, FirebaseApp, getApp } from '@firebase/app'; +import { deepEqual } from '@firebase/util'; import { CustomSignals, LogLevel as RemoteConfigLogLevel, RemoteConfig, - Value + Value, + RemoteConfigOptions } from './public_types'; -import { RemoteConfigAbortSignal } from './client/remote_config_fetch_client'; +import { RemoteConfigAbortSignal, FetchResponse } from './client/remote_config_fetch_client'; import { RC_COMPONENT_NAME, RC_CUSTOM_SIGNAL_KEY_MAX_LENGTH, RC_CUSTOM_SIGNAL_VALUE_MAX_LENGTH } from './constants'; -import { ErrorCode, hasErrorCode } from './errors'; +import { ERROR_FACTORY, ErrorCode, hasErrorCode } from './errors'; import { RemoteConfig as RemoteConfigImpl } from './remote_config'; import { Value as ValueImpl } from './value'; import { LogLevel as FirebaseLogLevel } from '@firebase/logger'; @@ -41,9 +43,17 @@ import { getModularInstance } from '@firebase/util'; * * @public */ -export function getRemoteConfig(app: FirebaseApp = getApp()): RemoteConfig { +export function getRemoteConfig(app: FirebaseApp = getApp(), options: RemoteConfigOptions = {}): RemoteConfig { app = getModularInstance(app); const rcProvider = _getProvider(app, RC_COMPONENT_NAME); + if (rcProvider.isInitialized()) { + const initialOptions = rcProvider.getOptions() as RemoteConfigOptions; + if (deepEqual(initialOptions, options)) { + return rcProvider.getImmediate(); + } + throw ERROR_FACTORY.create(ErrorCode.ALREADY_INITIALIZED); + } + rcProvider.initialize({ options }); return rcProvider.getImmediate(); } @@ -143,6 +153,24 @@ export async function fetchConfig(remoteConfig: RemoteConfig): Promise { } } +/** + * Manually hydrates the config state without making an async fetch request. + * @param remoteConfig - The {@link RemoteConfig} instance. + * @param fetchResponse - The fetchResponse containing the config values and eTag + * with which to hydrate the internal state. + */ +export async function setConfigState(remoteConfig: RemoteConfig, fetchResponse: FetchResponse) { + const rc = getModularInstance(remoteConfig) as RemoteConfigImpl; + await Promise.all([ + rc._storage.setLastSuccessfulFetchResponse(fetchResponse), + rc._storageCache.setLastSuccessfulFetchTimestampMillis(Date.now()), + rc._storageCache.setLastFetchStatus('success'), + // TODO - maybe we just call activate() here? + rc._storage.setActiveConfigEtag(fetchResponse.eTag || ''), + rc._storageCache.setActiveConfig(fetchResponse.config || {}), + ]); +} + /** * Gets all config. * @@ -223,7 +251,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { if (!rc._isInitializationComplete) { rc._logger.debug( `A value was requested for key "${key}" before SDK initialization completed.` + - ' Await on ensureInitialized if the intent was to get a previously activated value.' + ' Await on ensureInitialized if the intent was to get a previously activated value.' ); } const activeConfig = rc._storageCache.getActiveConfig(); @@ -234,7 +262,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { } rc._logger.debug( `Returning static value for key "${key}".` + - ' Define a default or remote value if this is unintentional.' + ' Define a default or remote value if this is unintentional.' ); return new ValueImpl('static'); } diff --git a/packages/remote-config/src/client/remote_config_fetch_client.ts b/packages/remote-config/src/client/remote_config_fetch_client.ts index 71ea66d5e50..4b15bc2a2cb 100644 --- a/packages/remote-config/src/client/remote_config_fetch_client.ts +++ b/packages/remote-config/src/client/remote_config_fetch_client.ts @@ -115,6 +115,7 @@ export interface FetchRequest { *

Modeled after the native {@link Response} interface, but simplified for Remote Config's * use case. */ +// TODO - should we move this public_types.ts? export interface FetchResponse { /** * The HTTP status, which is useful for differentiating success responses with data from diff --git a/packages/remote-config/src/errors.ts b/packages/remote-config/src/errors.ts index 762eeb899ee..446bd2c6e7a 100644 --- a/packages/remote-config/src/errors.ts +++ b/packages/remote-config/src/errors.ts @@ -18,6 +18,7 @@ import { ErrorFactory, FirebaseError } from '@firebase/util'; export const enum ErrorCode { + ALREADY_INITIALIZED = 'already-initialized', REGISTRATION_WINDOW = 'registration-window', REGISTRATION_PROJECT_ID = 'registration-project-id', REGISTRATION_API_KEY = 'registration-api-key', @@ -36,6 +37,7 @@ export const enum ErrorCode { } const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = { + [ErrorCode.ALREADY_INITIALIZED]: 'Remote Config already initialized', [ErrorCode.REGISTRATION_WINDOW]: 'Undefined window object. This SDK only supports usage in a browser environment.', [ErrorCode.REGISTRATION_PROJECT_ID]: diff --git a/packages/remote-config/src/public_types.ts b/packages/remote-config/src/public_types.ts index 365d5e5905f..1e0b882dd26 100644 --- a/packages/remote-config/src/public_types.ts +++ b/packages/remote-config/src/public_types.ts @@ -17,6 +17,18 @@ import { FirebaseApp } from '@firebase/app'; +/** + * Options for Remote Config initialization. + * + * @public + */ +export interface RemoteConfigOptions { + /** + * The ID of the template to use. If not provided, defaults to "firebase" + */ + templateId?: string; +} + /** * The Firebase Remote Config service interface. * diff --git a/packages/remote-config/src/register.ts b/packages/remote-config/src/register.ts index ff83e761888..e65a48d01ad 100644 --- a/packages/remote-config/src/register.ts +++ b/packages/remote-config/src/register.ts @@ -27,7 +27,7 @@ import { InstanceFactoryOptions } from '@firebase/component'; import { Logger, LogLevel as FirebaseLogLevel } from '@firebase/logger'; -import { RemoteConfig } from './public_types'; +import { RemoteConfig, RemoteConfigOptions } from './public_types'; import { name as packageName, version } from '../package.json'; import { ensureInitialized } from './api'; import { CachingClient } from './client/caching_client'; @@ -57,7 +57,7 @@ export function registerRemoteConfig(): void { function remoteConfigFactory( container: ComponentContainer, - { instanceIdentifier: namespace }: InstanceFactoryOptions + { options }: { options?: RemoteConfigOptions } ): RemoteConfig { /* Dependencies */ // getImmediate for FirebaseApp will always succeed @@ -67,14 +67,6 @@ export function registerRemoteConfig(): void { .getProvider('installations-internal') .getImmediate(); - // Guards against the SDK being used in non-browser environments. - if (typeof window === 'undefined') { - throw ERROR_FACTORY.create(ErrorCode.REGISTRATION_WINDOW); - } - // Guards against the SDK being used when indexedDB is not available. - if (!isIndexedDBAvailable()) { - throw ERROR_FACTORY.create(ErrorCode.INDEXED_DB_UNAVAILABLE); - } // Normalizes optional inputs. const { projectId, apiKey, appId } = app.options; if (!projectId) { @@ -86,7 +78,7 @@ export function registerRemoteConfig(): void { if (!appId) { throw ERROR_FACTORY.create(ErrorCode.REGISTRATION_APP_ID); } - namespace = namespace || 'firebase'; + const namespace = options?.templateId || 'firebase'; const storage = new Storage(appId, app.name, namespace); const storageCache = new StorageCache(storage); From fce5596c9850b48c9134b5121e42d80a28bd46d8 Mon Sep 17 00:00:00 2001 From: Kevin Elko Date: Thu, 21 Nov 2024 10:49:29 -0500 Subject: [PATCH 02/19] Update API to accept initial config as a part of getRemoteConfig options --- packages/remote-config/src/api.ts | 36 +++++++++---------- .../src/client/remote_config_fetch_client.ts | 1 - packages/remote-config/src/public_types.ts | 8 ++++- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/packages/remote-config/src/api.ts b/packages/remote-config/src/api.ts index 2744d2d5439..dab08d49987 100644 --- a/packages/remote-config/src/api.ts +++ b/packages/remote-config/src/api.ts @@ -54,7 +54,23 @@ export function getRemoteConfig(app: FirebaseApp = getApp(), options: RemoteConf throw ERROR_FACTORY.create(ErrorCode.ALREADY_INITIALIZED); } rcProvider.initialize({ options }); - return rcProvider.getImmediate(); + const rc = rcProvider.getImmediate() as RemoteConfigImpl; + + if (options.initialFetchResponse) { + // + rc._initializePromise = Promise.all([ + rc._storage.setLastSuccessfulFetchResponse(options.initialFetchResponse), + rc._storage.setActiveConfigEtag(options.initialFetchResponse?.eTag || ''), + rc._storageCache.setLastSuccessfulFetchTimestampMillis(Date.now()), + rc._storageCache.setLastFetchStatus('success'), + rc._storageCache.setActiveConfig(options.initialFetchResponse?.config || {}) + ]).then(); + // The storageCache methods above set their in-memory fields sycnhronously, so it's + // safe to declare our initialization complete at this point. + rc._isInitializationComplete = true; + } + + return rc; } /** @@ -153,24 +169,6 @@ export async function fetchConfig(remoteConfig: RemoteConfig): Promise { } } -/** - * Manually hydrates the config state without making an async fetch request. - * @param remoteConfig - The {@link RemoteConfig} instance. - * @param fetchResponse - The fetchResponse containing the config values and eTag - * with which to hydrate the internal state. - */ -export async function setConfigState(remoteConfig: RemoteConfig, fetchResponse: FetchResponse) { - const rc = getModularInstance(remoteConfig) as RemoteConfigImpl; - await Promise.all([ - rc._storage.setLastSuccessfulFetchResponse(fetchResponse), - rc._storageCache.setLastSuccessfulFetchTimestampMillis(Date.now()), - rc._storageCache.setLastFetchStatus('success'), - // TODO - maybe we just call activate() here? - rc._storage.setActiveConfigEtag(fetchResponse.eTag || ''), - rc._storageCache.setActiveConfig(fetchResponse.config || {}), - ]); -} - /** * Gets all config. * diff --git a/packages/remote-config/src/client/remote_config_fetch_client.ts b/packages/remote-config/src/client/remote_config_fetch_client.ts index 4b15bc2a2cb..71ea66d5e50 100644 --- a/packages/remote-config/src/client/remote_config_fetch_client.ts +++ b/packages/remote-config/src/client/remote_config_fetch_client.ts @@ -115,7 +115,6 @@ export interface FetchRequest { *

Modeled after the native {@link Response} interface, but simplified for Remote Config's * use case. */ -// TODO - should we move this public_types.ts? export interface FetchResponse { /** * The HTTP status, which is useful for differentiating success responses with data from diff --git a/packages/remote-config/src/public_types.ts b/packages/remote-config/src/public_types.ts index 1e0b882dd26..cfad9d7fd48 100644 --- a/packages/remote-config/src/public_types.ts +++ b/packages/remote-config/src/public_types.ts @@ -16,6 +16,7 @@ */ import { FirebaseApp } from '@firebase/app'; +import { FetchResponse } from './client/remote_config_fetch_client'; /** * Options for Remote Config initialization. @@ -24,9 +25,14 @@ import { FirebaseApp } from '@firebase/app'; */ export interface RemoteConfigOptions { /** - * The ID of the template to use. If not provided, defaults to "firebase" + * The ID of the template to use. If not provided, defaults to "firebase". */ templateId?: string; + + /** + * Hydrates the state with an initial fetch response. + */ + initialFetchResponse?: FetchResponse; } /** From da21142f4fa71b83f65bc924a88ff8b3011236fc Mon Sep 17 00:00:00 2001 From: Kevin Elko Date: Thu, 21 Nov 2024 10:58:17 -0500 Subject: [PATCH 03/19] split storage into indexeddeb storage and in-memory storage implementations --- packages/remote-config/src/api.ts | 3 +- packages/remote-config/src/register.ts | 4 +- packages/remote-config/src/storage/storage.ts | 53 ++++++++++++++----- 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/packages/remote-config/src/api.ts b/packages/remote-config/src/api.ts index dab08d49987..cbd2f164595 100644 --- a/packages/remote-config/src/api.ts +++ b/packages/remote-config/src/api.ts @@ -57,7 +57,8 @@ export function getRemoteConfig(app: FirebaseApp = getApp(), options: RemoteConf const rc = rcProvider.getImmediate() as RemoteConfigImpl; if (options.initialFetchResponse) { - // + // We use these initial writes as the initialization promise since they will hydrate the same + // fields that storageCache.loadFromStorage would set. rc._initializePromise = Promise.all([ rc._storage.setLastSuccessfulFetchResponse(options.initialFetchResponse), rc._storage.setActiveConfigEtag(options.initialFetchResponse?.eTag || ''), diff --git a/packages/remote-config/src/register.ts b/packages/remote-config/src/register.ts index e65a48d01ad..446e1b997b3 100644 --- a/packages/remote-config/src/register.ts +++ b/packages/remote-config/src/register.ts @@ -36,7 +36,7 @@ import { RetryingClient } from './client/retrying_client'; import { RC_COMPONENT_NAME } from './constants'; import { ErrorCode, ERROR_FACTORY } from './errors'; import { RemoteConfig as RemoteConfigImpl } from './remote_config'; -import { Storage } from './storage/storage'; +import { IndexedDbStorage, InMemoryStorage, Storage } from './storage/storage'; import { StorageCache } from './storage/storage_cache'; // This needs to be in the same file that calls `getProvider()` on the component // or it will get tree-shaken out. @@ -80,7 +80,7 @@ export function registerRemoteConfig(): void { } const namespace = options?.templateId || 'firebase'; - const storage = new Storage(appId, app.name, namespace); + const storage = isIndexedDBAvailable() ? new IndexedDbStorage(appId, app.name, namespace) : new InMemoryStorage(); const storageCache = new StorageCache(storage); const logger = new Logger(packageName); diff --git a/packages/remote-config/src/storage/storage.ts b/packages/remote-config/src/storage/storage.ts index 52e660f1fdb..f0c13214369 100644 --- a/packages/remote-config/src/storage/storage.ts +++ b/packages/remote-config/src/storage/storage.ts @@ -113,19 +113,7 @@ export function openDatabase(): Promise { /** * Abstracts data persistence. */ -export class Storage { - /** - * @param appId enables storage segmentation by app (ID + name). - * @param appName enables storage segmentation by app (ID + name). - * @param namespace enables storage segmentation by namespace. - */ - constructor( - private readonly appId: string, - private readonly appName: string, - private readonly namespace: string, - private readonly openDbPromise = openDatabase() - ) {} - +export abstract class Storage { getLastFetchStatus(): Promise { return this.get('last_fetch_status'); } @@ -187,6 +175,27 @@ export class Storage { return this.get('custom_signals'); } + abstract setCustomSignals(customSignals: CustomSignals): Promise; + abstract get(key: ProjectNamespaceKeyFieldValue): Promise; + abstract set(key: ProjectNamespaceKeyFieldValue, value: T): Promise; + abstract delete(key: ProjectNamespaceKeyFieldValue): Promise; +} + +export class IndexedDbStorage extends Storage { + /** + * @param appId enables storage segmentation by app (ID + name). + * @param appName enables storage segmentation by app (ID + name). + * @param namespace enables storage segmentation by namespace. + */ + constructor( + private readonly appId: string, + private readonly appName: string, + private readonly namespace: string, + private readonly openDbPromise = openDatabase() + ) { + super(); + } + async setCustomSignals(customSignals: CustomSignals): Promise { const db = await this.openDbPromise; const transaction = db.transaction([APP_NAMESPACE_STORE], 'readwrite'); @@ -344,3 +353,21 @@ export class Storage { return [this.appId, this.appName, this.namespace, key].join(); } } + +export class InMemoryStorage extends Storage { + private db: { [key: string]: any } = {} + + async get(key: ProjectNamespaceKeyFieldValue): Promise { + return Promise.resolve(this.db[key] as T); + } + + async set(key: ProjectNamespaceKeyFieldValue, value: T): Promise { + this.db[key] = value; + return Promise.resolve(undefined); + } + + async delete(key: ProjectNamespaceKeyFieldValue): Promise { + this.db[key] = undefined; + return Promise.resolve(); + } +} From 11c72523a47054322688299b966954f50be84f42 Mon Sep 17 00:00:00 2001 From: Kevin Elko Date: Thu, 21 Nov 2024 11:25:26 -0500 Subject: [PATCH 04/19] split out storage impls --- packages/remote-config/src/api.ts | 3 +-- packages/remote-config/src/register.ts | 7 ++++--- packages/remote-config/src/storage/storage.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/remote-config/src/api.ts b/packages/remote-config/src/api.ts index cbd2f164595..28f269df311 100644 --- a/packages/remote-config/src/api.ts +++ b/packages/remote-config/src/api.ts @@ -16,7 +16,7 @@ */ import { _getProvider, FirebaseApp, getApp } from '@firebase/app'; -import { deepEqual } from '@firebase/util'; +import { deepEqual, getModularInstance } from '@firebase/util'; import { CustomSignals, LogLevel as RemoteConfigLogLevel, @@ -34,7 +34,6 @@ import { ERROR_FACTORY, ErrorCode, hasErrorCode } from './errors'; import { RemoteConfig as RemoteConfigImpl } from './remote_config'; import { Value as ValueImpl } from './value'; import { LogLevel as FirebaseLogLevel } from '@firebase/logger'; -import { getModularInstance } from '@firebase/util'; /** * diff --git a/packages/remote-config/src/register.ts b/packages/remote-config/src/register.ts index 446e1b997b3..14426621f12 100644 --- a/packages/remote-config/src/register.ts +++ b/packages/remote-config/src/register.ts @@ -24,7 +24,6 @@ import { Component, ComponentType, ComponentContainer, - InstanceFactoryOptions } from '@firebase/component'; import { Logger, LogLevel as FirebaseLogLevel } from '@firebase/logger'; import { RemoteConfig, RemoteConfigOptions } from './public_types'; @@ -36,7 +35,7 @@ import { RetryingClient } from './client/retrying_client'; import { RC_COMPONENT_NAME } from './constants'; import { ErrorCode, ERROR_FACTORY } from './errors'; import { RemoteConfig as RemoteConfigImpl } from './remote_config'; -import { IndexedDbStorage, InMemoryStorage, Storage } from './storage/storage'; +import { IndexedDbStorage, InMemoryStorage } from './storage/storage'; import { StorageCache } from './storage/storage_cache'; // This needs to be in the same file that calls `getProvider()` on the component // or it will get tree-shaken out. @@ -80,7 +79,9 @@ export function registerRemoteConfig(): void { } const namespace = options?.templateId || 'firebase'; - const storage = isIndexedDBAvailable() ? new IndexedDbStorage(appId, app.name, namespace) : new InMemoryStorage(); + const storage = isIndexedDBAvailable() ? + new IndexedDbStorage(appId, app.name, namespace) : + new InMemoryStorage(); const storageCache = new StorageCache(storage); const logger = new Logger(packageName); diff --git a/packages/remote-config/src/storage/storage.ts b/packages/remote-config/src/storage/storage.ts index f0c13214369..629942e9322 100644 --- a/packages/remote-config/src/storage/storage.ts +++ b/packages/remote-config/src/storage/storage.ts @@ -355,7 +355,7 @@ export class IndexedDbStorage extends Storage { } export class InMemoryStorage extends Storage { - private db: { [key: string]: any } = {} + private db: { [key: string]: unknown } = {}; async get(key: ProjectNamespaceKeyFieldValue): Promise { return Promise.resolve(this.db[key] as T); From d1f6bfd09fd3b9da3a7025a09a576fc08072f079 Mon Sep 17 00:00:00 2001 From: kjelko Date: Tue, 10 Dec 2024 16:39:27 -0500 Subject: [PATCH 05/19] Initial pass at adding some tests --- packages/remote-config/test/api.test.ts | 126 ++++++++++++ .../test/storage/storage.test.ts | 188 ++++++++++-------- 2 files changed, 226 insertions(+), 88 deletions(-) create mode 100644 packages/remote-config/test/api.test.ts diff --git a/packages/remote-config/test/api.test.ts b/packages/remote-config/test/api.test.ts new file mode 100644 index 00000000000..bceea184120 --- /dev/null +++ b/packages/remote-config/test/api.test.ts @@ -0,0 +1,126 @@ +import { expect } from 'chai'; +import { ensureInitialized, fetchAndActivate, getRemoteConfig, getString } from '../src/index'; +import '../test/setup'; +import { deleteApp, FirebaseApp, initializeApp, _addOrOverwriteComponent } from '@firebase/app'; +import * as sinon from 'sinon'; +import { FetchResponse } from '../src/client/remote_config_fetch_client'; +import { + Component, + ComponentType +} from '@firebase/component'; +import { FirebaseInstallations } from '@firebase/installations-types'; +import { + openDatabase, + APP_NAMESPACE_STORE, +} from '../src/storage/storage'; + +const fakeFirebaseConfig = { + apiKey: 'api-key', + authDomain: 'project-id.firebaseapp.com', + databaseURL: 'https://project-id.firebaseio.com', + projectId: 'project-id', + storageBucket: 'project-id.appspot.com', + messagingSenderId: 'sender-id', + appId: '1:111:web:a1234' +}; + +async function clearDatabase(): Promise { + const db = await openDatabase(); + db.transaction([APP_NAMESPACE_STORE], 'readwrite') + .objectStore(APP_NAMESPACE_STORE) + .clear(); +} + +describe('Remote Config API', () => { + let app: FirebaseApp; + const STUB_FETCH_RESPONSE: FetchResponse = { + status: 200, + eTag: 'asdf', + config: { 'foobar': 'hello world' }, + }; + let fetchStub: sinon.SinonStub; + + beforeEach(() => { + fetchStub = sinon.stub(window, 'fetch'); + app = initializeApp(fakeFirebaseConfig); + _addOrOverwriteComponent( + app, + new Component( + 'installations-internal', + () => { + return { + getId: () => Promise.resolve('fis-id'), + getToken: () => Promise.resolve('fis-token'), + } as any as FirebaseInstallations; + }, + ComponentType.PUBLIC + ) as any, + ); + }); + + afterEach(async () => { + fetchStub.restore(); + await clearDatabase(); + await deleteApp(app); + }); + + function setFetchResponse(response: FetchResponse = { status: 200 }): void { + fetchStub.returns(Promise.resolve({ + ok: response.status === 200, + status: response.status, + headers: new Headers({ ETag: response.eTag || '' }), + json: () => + Promise.resolve({ + entries: response.config, + state: 'OK' + }) + } as Response)); + } + + it('allows multiple initializations if options are same', () => { + const rc = getRemoteConfig(app, { templateId: 'altTemplate' }); + const rc2 = getRemoteConfig(app, { templateId: 'altTemplate' }); + expect(rc).to.equal(rc2); + }); + + it('throws an error if options are different', () => { + getRemoteConfig(app); + expect(() => { + getRemoteConfig(app, { templateId: 'altTemplate' }); + }).to.throw(/Remote Config already initialized/); + }); + + it('makes a fetch call', async () => { + const rc = getRemoteConfig(app); + setFetchResponse(STUB_FETCH_RESPONSE); + await fetchAndActivate(rc); + await ensureInitialized(rc); + expect(getString(rc, 'foobar')).to.equal('hello world'); + }); + + it('calls fetch with default templateId', async () => { + const rc = getRemoteConfig(app); + setFetchResponse(); + await fetchAndActivate(rc); + await ensureInitialized(rc); + expect(fetchStub).to.be.calledOnceWith( + 'https://firebaseremoteconfig.googleapis.com/v1/projects/project-id/namespaces/firebase:fetch?key=api-key', + sinon.match.object + ); + }); + + it('calls fetch with alternate templateId', async () => { + const rc = getRemoteConfig(app, { templateId: 'altTemplate' }); + setFetchResponse(); + await fetchAndActivate(rc); + expect(fetchStub).to.be.calledOnceWith( + 'https://firebaseremoteconfig.googleapis.com/v1/projects/project-id/namespaces/altTemplate:fetch?key=api-key', + sinon.match.object); + }); + + it('hydrates with initialFetchResponse', async () => { + const rc = getRemoteConfig(app, { initialFetchResponse: STUB_FETCH_RESPONSE }); + await ensureInitialized(rc); + expect(getString(rc, 'foobar')).to.equal('hello world'); + }); +}); \ No newline at end of file diff --git a/packages/remote-config/test/storage/storage.test.ts b/packages/remote-config/test/storage/storage.test.ts index 7a865107791..f19a7f76048 100644 --- a/packages/remote-config/test/storage/storage.test.ts +++ b/packages/remote-config/test/storage/storage.test.ts @@ -18,10 +18,11 @@ import '../setup'; import { expect } from 'chai'; import { - Storage, ThrottleMetadata, openDatabase, - APP_NAMESPACE_STORE + APP_NAMESPACE_STORE, + IndexedDbStorage, + InMemoryStorage } from '../../src/storage/storage'; import { FetchResponse } from '../../src/client/remote_config_fetch_client'; @@ -33,142 +34,153 @@ async function clearDatabase(): Promise { .clear(); } -describe('Storage', () => { - const storage = new Storage('appId', 'appName', 'namespace'); +describe('IndexedDbStorage', () => { + + const indexedDbTestCase = { + storage: new IndexedDbStorage('appId', 'appName', 'namespace'), + name: 'IndexedDbStorage', + }; + + const inMemoryStorage = { + storage: new InMemoryStorage(), + name: 'InMemoryStorage', + }; beforeEach(async () => { await clearDatabase(); }); - it('constructs a composite key', async () => { + it(`${indexedDbTestCase.name} constructs a composite key`, async () => { // This is defensive, but the cost of accidentally changing the key composition is high. - expect(storage.createCompositeKey('throttle_metadata')).to.eq( + expect(indexedDbTestCase.storage.createCompositeKey('throttle_metadata')).to.eq( 'appId,appName,namespace,throttle_metadata' ); }); - it('sets and gets last fetch attempt status', async () => { - const expectedStatus = 'success'; + for (const { name, storage } of [indexedDbTestCase, inMemoryStorage]) { + it(`${name} sets and gets last fetch attempt status`, async () => { + const expectedStatus = 'success'; - await storage.setLastFetchStatus(expectedStatus); + await storage.setLastFetchStatus(expectedStatus); - const actualStatus = await storage.getLastFetchStatus(); + const actualStatus = await storage.getLastFetchStatus(); - expect(actualStatus).to.deep.eq(expectedStatus); - }); + expect(actualStatus).to.deep.eq(expectedStatus); + }); - it('sets and gets last fetch success timestamp', async () => { - const lastSuccessfulFetchTimestampMillis = 123; + it(`${name} sets and gets last fetch success timestamp`, async () => { + const lastSuccessfulFetchTimestampMillis = 123; - await storage.setLastSuccessfulFetchTimestampMillis( - lastSuccessfulFetchTimestampMillis - ); + await storage.setLastSuccessfulFetchTimestampMillis( + lastSuccessfulFetchTimestampMillis + ); - const actualMetadata = - await storage.getLastSuccessfulFetchTimestampMillis(); + const actualMetadata = + await storage.getLastSuccessfulFetchTimestampMillis(); - expect(actualMetadata).to.deep.eq(lastSuccessfulFetchTimestampMillis); - }); + expect(actualMetadata).to.deep.eq(lastSuccessfulFetchTimestampMillis); + }); - it('sets and gets last successful fetch response', async () => { - const lastSuccessfulFetchResponse = { status: 200 } as FetchResponse; + it(`${name} sets and gets last successful fetch response`, async () => { + const lastSuccessfulFetchResponse = { status: 200 } as FetchResponse; - await storage.setLastSuccessfulFetchResponse(lastSuccessfulFetchResponse); + await storage.setLastSuccessfulFetchResponse(lastSuccessfulFetchResponse); - const actualConfig = await storage.getLastSuccessfulFetchResponse(); + const actualConfig = await storage.getLastSuccessfulFetchResponse(); - expect(actualConfig).to.deep.eq(lastSuccessfulFetchResponse); - }); + expect(actualConfig).to.deep.eq(lastSuccessfulFetchResponse); + }); - it('sets and gets active config', async () => { - const expectedConfig = { key: 'value' }; + it(`${name} sets and gets active config`, async () => { + const expectedConfig = { key: 'value' }; - await storage.setActiveConfig(expectedConfig); + await storage.setActiveConfig(expectedConfig); - const storedConfig = await storage.getActiveConfig(); + const storedConfig = await storage.getActiveConfig(); - expect(storedConfig).to.deep.eq(expectedConfig); - }); + expect(storedConfig).to.deep.eq(expectedConfig); + }); - it('sets and gets active config etag', async () => { - const expectedEtag = 'etag'; + it(`${name} sets and gets active config etag`, async () => { + const expectedEtag = 'etag'; - await storage.setActiveConfigEtag(expectedEtag); + await storage.setActiveConfigEtag(expectedEtag); - const storedConfigEtag = await storage.getActiveConfigEtag(); + const storedConfigEtag = await storage.getActiveConfigEtag(); - expect(storedConfigEtag).to.deep.eq(expectedEtag); - }); + expect(storedConfigEtag).to.deep.eq(expectedEtag); + }); - it('sets, gets and deletes throttle metadata', async () => { - const expectedMetadata = { - throttleEndTimeMillis: 1 - } as ThrottleMetadata; + it(`${name} sets, gets and deletes throttle metadata`, async () => { + const expectedMetadata = { + throttleEndTimeMillis: 1 + } as ThrottleMetadata; - await storage.setThrottleMetadata(expectedMetadata); + await storage.setThrottleMetadata(expectedMetadata); - let actualMetadata = await storage.getThrottleMetadata(); + let actualMetadata = await storage.getThrottleMetadata(); - expect(actualMetadata).to.deep.eq(expectedMetadata); + expect(actualMetadata).to.deep.eq(expectedMetadata); - await storage.deleteThrottleMetadata(); + await storage.deleteThrottleMetadata(); - actualMetadata = await storage.getThrottleMetadata(); + actualMetadata = await storage.getThrottleMetadata(); - expect(actualMetadata).to.be.undefined; - }); + expect(actualMetadata).to.be.undefined; + }); - it('sets and gets custom signals', async () => { - const customSignals = { key: 'value', key1: 'value1', key2: 1 }; - const customSignalsInStorage = { - key: 'value', - key1: 'value1', - key2: '1' - }; + it('sets and gets custom signals', async () => { + const customSignals = { key: 'value', key1: 'value1', key2: 1 }; + const customSignalsInStorage = { + key: 'value', + key1: 'value1', + key2: '1' + }; - await storage.setCustomSignals(customSignals); + await storage.setCustomSignals(customSignals); - const storedCustomSignals = await storage.getCustomSignals(); + const storedCustomSignals = await storage.getCustomSignals(); - expect(storedCustomSignals).to.deep.eq(customSignalsInStorage); - }); + expect(storedCustomSignals).to.deep.eq(customSignalsInStorage); + }); - it('upserts custom signals when key is present in storage', async () => { - const customSignals = { key: 'value', key1: 'value1' }; - const updatedSignals = { key: 'value', key1: 'value2' }; + it('upserts custom signals when key is present in storage', async () => { + const customSignals = { key: 'value', key1: 'value1' }; + const updatedSignals = { key: 'value', key1: 'value2' }; - await storage.setCustomSignals(customSignals); + await storage.setCustomSignals(customSignals); - await storage.setCustomSignals({ key1: 'value2' }); + await storage.setCustomSignals({ key1: 'value2' }); - const storedCustomSignals = await storage.getCustomSignals(); + const storedCustomSignals = await storage.getCustomSignals(); - expect(storedCustomSignals).to.deep.eq(updatedSignals); - }); + expect(storedCustomSignals).to.deep.eq(updatedSignals); + }); - it('deletes custom signal when value supplied is null', async () => { - const customSignals = { key: 'value', key1: 'value1' }; - const updatedSignals = { key: 'value' }; + it('deletes custom signal when value supplied is null', async () => { + const customSignals = { key: 'value', key1: 'value1' }; + const updatedSignals = { key: 'value' }; - await storage.setCustomSignals(customSignals); + await storage.setCustomSignals(customSignals); - await storage.setCustomSignals({ key1: null }); + await storage.setCustomSignals({ key1: null }); - const storedCustomSignals = await storage.getCustomSignals(); + const storedCustomSignals = await storage.getCustomSignals(); - expect(storedCustomSignals).to.deep.eq(updatedSignals); - }); + expect(storedCustomSignals).to.deep.eq(updatedSignals); + }); - it('throws an error when supplied with excess custom signals', async () => { - const customSignals: { [key: string]: string } = {}; - for (let i = 0; i < 101; i++) { - customSignals[`key${i}`] = `value${i}`; - } + it('throws an error when supplied with excess custom signals', async () => { + const customSignals: { [key: string]: string } = {}; + for (let i = 0; i < 101; i++) { + customSignals[`key${i}`] = `value${i}`; + } - await expect( - storage.setCustomSignals(customSignals) - ).to.eventually.be.rejectedWith( - 'Remote Config: Setting more than 100 custom signals is not supported.' - ); - }); + await expect( + storage.setCustomSignals(customSignals) + ).to.eventually.be.rejectedWith( + 'Remote Config: Setting more than 100 custom signals is not supported.' + ); + }); + } }); From 2bbe42f75deb128b4c6fc3d97b8be5ebc0768994 Mon Sep 17 00:00:00 2001 From: kjelko Date: Wed, 8 Jan 2025 15:10:38 -0500 Subject: [PATCH 06/19] rename and clean up a few things --- packages/remote-config/src/storage/storage.ts | 12 ++++++++---- packages/remote-config/test/api.test.ts | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/remote-config/src/storage/storage.ts b/packages/remote-config/src/storage/storage.ts index 629942e9322..e9fbf51b119 100644 --- a/packages/remote-config/src/storage/storage.ts +++ b/packages/remote-config/src/storage/storage.ts @@ -355,19 +355,23 @@ export class IndexedDbStorage extends Storage { } export class InMemoryStorage extends Storage { - private db: { [key: string]: unknown } = {}; + private storage: { [key: string]: unknown } = {}; async get(key: ProjectNamespaceKeyFieldValue): Promise { - return Promise.resolve(this.db[key] as T); + return Promise.resolve(this.storage[key] as T); } async set(key: ProjectNamespaceKeyFieldValue, value: T): Promise { - this.db[key] = value; + this.storage[key] = value; return Promise.resolve(undefined); } async delete(key: ProjectNamespaceKeyFieldValue): Promise { - this.db[key] = undefined; + this.storage[key] = undefined; return Promise.resolve(); } + + async setCustomSignals(customSignals: CustomSignals): Promise { + return Promise.resolve({}); + } } diff --git a/packages/remote-config/test/api.test.ts b/packages/remote-config/test/api.test.ts index bceea184120..0a30a947a1b 100644 --- a/packages/remote-config/test/api.test.ts +++ b/packages/remote-config/test/api.test.ts @@ -123,4 +123,4 @@ describe('Remote Config API', () => { await ensureInitialized(rc); expect(getString(rc, 'foobar')).to.equal('hello world'); }); -}); \ No newline at end of file +}); From d145636dd9feaead07ec2ba5bfe898db89f73c87 Mon Sep 17 00:00:00 2001 From: kjelko Date: Wed, 8 Jan 2025 16:12:46 -0500 Subject: [PATCH 07/19] clean up after merging main --- packages/remote-config/src/api.ts | 2 +- packages/remote-config/src/storage/storage.ts | 30 ++- .../test/storage/storage.test.ts | 185 +++++++++--------- 3 files changed, 127 insertions(+), 90 deletions(-) diff --git a/packages/remote-config/src/api.ts b/packages/remote-config/src/api.ts index 28f269df311..9cb27d8cb24 100644 --- a/packages/remote-config/src/api.ts +++ b/packages/remote-config/src/api.ts @@ -24,7 +24,7 @@ import { Value, RemoteConfigOptions } from './public_types'; -import { RemoteConfigAbortSignal, FetchResponse } from './client/remote_config_fetch_client'; +import { RemoteConfigAbortSignal } from './client/remote_config_fetch_client'; import { RC_COMPONENT_NAME, RC_CUSTOM_SIGNAL_KEY_MAX_LENGTH, diff --git a/packages/remote-config/src/storage/storage.ts b/packages/remote-config/src/storage/storage.ts index e9fbf51b119..f0aa8ad8913 100644 --- a/packages/remote-config/src/storage/storage.ts +++ b/packages/remote-config/src/storage/storage.ts @@ -372,6 +372,34 @@ export class InMemoryStorage extends Storage { } async setCustomSignals(customSignals: CustomSignals): Promise { - return Promise.resolve({}); + const combinedSignals = { + ...this.storage['custom_signals'] as CustomSignals, + ...customSignals, + }; + + const updatedSignals = Object.fromEntries( + Object.entries(combinedSignals) + .filter(([_, v]) => v !== null) + .map(([k, v]) => { + // Stringify numbers to store a map of string keys and values which can be sent + // as-is in a fetch call. + if (typeof v === 'number') { + return [k, v.toString()]; + } + return [k, v]; + }) + ); + + if ( + Object.keys(updatedSignals).length > RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS + ) { + throw ERROR_FACTORY.create(ErrorCode.CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS, { + maxSignals: RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS + }); + } + + this.storage['custom_signals'] = updatedSignals; + + return Promise.resolve(this.storage['custom_signals'] as CustomSignals); } } diff --git a/packages/remote-config/test/storage/storage.test.ts b/packages/remote-config/test/storage/storage.test.ts index f19a7f76048..393036b2a73 100644 --- a/packages/remote-config/test/storage/storage.test.ts +++ b/packages/remote-config/test/storage/storage.test.ts @@ -22,7 +22,8 @@ import { openDatabase, APP_NAMESPACE_STORE, IndexedDbStorage, - InMemoryStorage + InMemoryStorage, + Storage } from '../../src/storage/storage'; import { FetchResponse } from '../../src/client/remote_config_fetch_client'; @@ -34,15 +35,15 @@ async function clearDatabase(): Promise { .clear(); } -describe('IndexedDbStorage', () => { +describe('Storage', () => { const indexedDbTestCase = { - storage: new IndexedDbStorage('appId', 'appName', 'namespace'), + getStorage: () => new IndexedDbStorage('appId', 'appName', 'namespace'), name: 'IndexedDbStorage', }; const inMemoryStorage = { - storage: new InMemoryStorage(), + getStorage: () => new InMemoryStorage(), name: 'InMemoryStorage', }; @@ -52,135 +53,143 @@ describe('IndexedDbStorage', () => { it(`${indexedDbTestCase.name} constructs a composite key`, async () => { // This is defensive, but the cost of accidentally changing the key composition is high. - expect(indexedDbTestCase.storage.createCompositeKey('throttle_metadata')).to.eq( + expect(indexedDbTestCase.getStorage().createCompositeKey('throttle_metadata')).to.eq( 'appId,appName,namespace,throttle_metadata' ); }); - for (const { name, storage } of [indexedDbTestCase, inMemoryStorage]) { - it(`${name} sets and gets last fetch attempt status`, async () => { - const expectedStatus = 'success'; + for (const { name, getStorage } of [indexedDbTestCase, inMemoryStorage]) { + describe(name, () => { + let storage: Storage; - await storage.setLastFetchStatus(expectedStatus); + beforeEach(() => { + storage = getStorage(); + }); - const actualStatus = await storage.getLastFetchStatus(); + it(`sets and gets last fetch attempt status`, async () => { + const expectedStatus = 'success'; - expect(actualStatus).to.deep.eq(expectedStatus); - }); + await storage.setLastFetchStatus(expectedStatus); - it(`${name} sets and gets last fetch success timestamp`, async () => { - const lastSuccessfulFetchTimestampMillis = 123; + const actualStatus = await storage.getLastFetchStatus(); - await storage.setLastSuccessfulFetchTimestampMillis( - lastSuccessfulFetchTimestampMillis - ); + expect(actualStatus).to.deep.eq(expectedStatus); + }); - const actualMetadata = - await storage.getLastSuccessfulFetchTimestampMillis(); + it(`sets and gets last fetch success timestamp`, async () => { + const lastSuccessfulFetchTimestampMillis = 123; - expect(actualMetadata).to.deep.eq(lastSuccessfulFetchTimestampMillis); - }); + await storage.setLastSuccessfulFetchTimestampMillis( + lastSuccessfulFetchTimestampMillis + ); - it(`${name} sets and gets last successful fetch response`, async () => { - const lastSuccessfulFetchResponse = { status: 200 } as FetchResponse; + const actualMetadata = + await storage.getLastSuccessfulFetchTimestampMillis(); - await storage.setLastSuccessfulFetchResponse(lastSuccessfulFetchResponse); + expect(actualMetadata).to.deep.eq(lastSuccessfulFetchTimestampMillis); + }); - const actualConfig = await storage.getLastSuccessfulFetchResponse(); + it(`sets and gets last successful fetch response`, async () => { + const lastSuccessfulFetchResponse = { status: 200 } as FetchResponse; - expect(actualConfig).to.deep.eq(lastSuccessfulFetchResponse); - }); + await storage.setLastSuccessfulFetchResponse(lastSuccessfulFetchResponse); - it(`${name} sets and gets active config`, async () => { - const expectedConfig = { key: 'value' }; + const actualConfig = await storage.getLastSuccessfulFetchResponse(); - await storage.setActiveConfig(expectedConfig); + expect(actualConfig).to.deep.eq(lastSuccessfulFetchResponse); + }); - const storedConfig = await storage.getActiveConfig(); + it(`sets and gets active config`, async () => { + const expectedConfig = { key: 'value' }; - expect(storedConfig).to.deep.eq(expectedConfig); - }); + await storage.setActiveConfig(expectedConfig); - it(`${name} sets and gets active config etag`, async () => { - const expectedEtag = 'etag'; + const storedConfig = await storage.getActiveConfig(); - await storage.setActiveConfigEtag(expectedEtag); + expect(storedConfig).to.deep.eq(expectedConfig); + }); - const storedConfigEtag = await storage.getActiveConfigEtag(); + it(`sets and gets active config etag`, async () => { + const expectedEtag = 'etag'; - expect(storedConfigEtag).to.deep.eq(expectedEtag); - }); + await storage.setActiveConfigEtag(expectedEtag); - it(`${name} sets, gets and deletes throttle metadata`, async () => { - const expectedMetadata = { - throttleEndTimeMillis: 1 - } as ThrottleMetadata; + const storedConfigEtag = await storage.getActiveConfigEtag(); - await storage.setThrottleMetadata(expectedMetadata); + expect(storedConfigEtag).to.deep.eq(expectedEtag); + }); - let actualMetadata = await storage.getThrottleMetadata(); + it(`sets, gets and deletes throttle metadata`, async () => { + const expectedMetadata = { + throttleEndTimeMillis: 1 + } as ThrottleMetadata; - expect(actualMetadata).to.deep.eq(expectedMetadata); + await storage.setThrottleMetadata(expectedMetadata); - await storage.deleteThrottleMetadata(); + let actualMetadata = await storage.getThrottleMetadata(); - actualMetadata = await storage.getThrottleMetadata(); + expect(actualMetadata).to.deep.eq(expectedMetadata); - expect(actualMetadata).to.be.undefined; - }); + await storage.deleteThrottleMetadata(); - it('sets and gets custom signals', async () => { - const customSignals = { key: 'value', key1: 'value1', key2: 1 }; - const customSignalsInStorage = { - key: 'value', - key1: 'value1', - key2: '1' - }; + actualMetadata = await storage.getThrottleMetadata(); - await storage.setCustomSignals(customSignals); + expect(actualMetadata).to.be.undefined; + }); - const storedCustomSignals = await storage.getCustomSignals(); + it(`sets and gets custom signals`, async () => { + const customSignals = { key: 'value', key1: 'value1', key2: 1 }; + const customSignalsInStorage = { + key: 'value', + key1: 'value1', + key2: '1' + }; - expect(storedCustomSignals).to.deep.eq(customSignalsInStorage); - }); + await storage.setCustomSignals(customSignals); - it('upserts custom signals when key is present in storage', async () => { - const customSignals = { key: 'value', key1: 'value1' }; - const updatedSignals = { key: 'value', key1: 'value2' }; + const storedCustomSignals = await storage.getCustomSignals(); - await storage.setCustomSignals(customSignals); + expect(storedCustomSignals).to.deep.eq(customSignalsInStorage); + }); - await storage.setCustomSignals({ key1: 'value2' }); + it(`upserts custom signals when key is present in storage`, async () => { + const customSignals = { key: 'value', key1: 'value1' }; + const updatedSignals = { key: 'value', key1: 'value2' }; - const storedCustomSignals = await storage.getCustomSignals(); + await storage.setCustomSignals(customSignals); - expect(storedCustomSignals).to.deep.eq(updatedSignals); - }); + await storage.setCustomSignals({ key1: 'value2' }); - it('deletes custom signal when value supplied is null', async () => { - const customSignals = { key: 'value', key1: 'value1' }; - const updatedSignals = { key: 'value' }; + const storedCustomSignals = await storage.getCustomSignals(); - await storage.setCustomSignals(customSignals); + expect(storedCustomSignals).to.deep.eq(updatedSignals); + }); - await storage.setCustomSignals({ key1: null }); + it(`deletes custom signal when value supplied is null`, async () => { + const customSignals = { key: 'value', key1: 'value1' }; + const updatedSignals = { key: 'value' }; - const storedCustomSignals = await storage.getCustomSignals(); + await storage.setCustomSignals(customSignals); - expect(storedCustomSignals).to.deep.eq(updatedSignals); - }); + await storage.setCustomSignals({ key1: null }); + + const storedCustomSignals = await storage.getCustomSignals(); + + expect(storedCustomSignals).to.deep.eq(updatedSignals); + }); + + it(`throws an error when supplied with excess custom signals`, async () => { + const customSignals: { [key: string]: string } = {}; + for (let i = 0; i < 101; i++) { + customSignals[`key${i}`] = `value${i}`; + } - it('throws an error when supplied with excess custom signals', async () => { - const customSignals: { [key: string]: string } = {}; - for (let i = 0; i < 101; i++) { - customSignals[`key${i}`] = `value${i}`; - } - - await expect( - storage.setCustomSignals(customSignals) - ).to.eventually.be.rejectedWith( - 'Remote Config: Setting more than 100 custom signals is not supported.' - ); + await expect( + storage.setCustomSignals(customSignals) + ).to.eventually.be.rejectedWith( + 'Remote Config: Setting more than 100 custom signals is not supported.' + ); + }); }); } }); From 2255b799fb824a17402ad5b2ddee9e1c648a5762 Mon Sep 17 00:00:00 2001 From: kjelko Date: Thu, 9 Jan 2025 10:36:06 -0500 Subject: [PATCH 08/19] Run docgen --- common/api-review/remote-config.api.md | 21 +++++- docs-devsite/_toc.yaml | 6 ++ docs-devsite/remote-config.fetchresponse.md | 67 +++++++++++++++++++ ...emote-config.firebaseremoteconfigobject.md | 19 ++++++ docs-devsite/remote-config.md | 10 ++- .../remote-config.remoteconfigoptions.md | 46 +++++++++++++ packages/remote-config/src/public_types.ts | 2 + .../test/storage/storage.test.ts | 20 +++--- 8 files changed, 177 insertions(+), 14 deletions(-) create mode 100644 docs-devsite/remote-config.fetchresponse.md create mode 100644 docs-devsite/remote-config.firebaseremoteconfigobject.md create mode 100644 docs-devsite/remote-config.remoteconfigoptions.md diff --git a/common/api-review/remote-config.api.md b/common/api-review/remote-config.api.md index bf6cf4761de..213335929dd 100644 --- a/common/api-review/remote-config.api.md +++ b/common/api-review/remote-config.api.md @@ -24,9 +24,22 @@ export function fetchAndActivate(remoteConfig: RemoteConfig): Promise; // @public export function fetchConfig(remoteConfig: RemoteConfig): Promise; +// @public +export interface FetchResponse { + config?: FirebaseRemoteConfigObject; + eTag?: string; + status: number; +} + // @public export type FetchStatus = 'no-fetch-yet' | 'success' | 'failure' | 'throttle'; +// @public +export interface FirebaseRemoteConfigObject { + // (undocumented) + [key: string]: string; +} + // @public export function getAll(remoteConfig: RemoteConfig): Record; @@ -37,7 +50,7 @@ export function getBoolean(remoteConfig: RemoteConfig, key: string): boolean; export function getNumber(remoteConfig: RemoteConfig, key: string): number; // @public (undocumented) -export function getRemoteConfig(app?: FirebaseApp): RemoteConfig; +export function getRemoteConfig(app?: FirebaseApp, options?: RemoteConfigOptions): RemoteConfig; // @public export function getString(remoteConfig: RemoteConfig, key: string): string; @@ -62,6 +75,12 @@ export interface RemoteConfig { settings: RemoteConfigSettings; } +// @public +export interface RemoteConfigOptions { + initialFetchResponse?: FetchResponse; + templateId?: string; +} + // @public export interface RemoteConfigSettings { fetchTimeoutMillis: number; diff --git a/docs-devsite/_toc.yaml b/docs-devsite/_toc.yaml index 4ab67bcd6ef..bf0318389ba 100644 --- a/docs-devsite/_toc.yaml +++ b/docs-devsite/_toc.yaml @@ -430,8 +430,14 @@ toc: section: - title: CustomSignals path: /docs/reference/js/remote-config.customsignals.md + - title: FetchResponse + path: /docs/reference/js/remote-config.fetchresponse.md + - title: FirebaseRemoteConfigObject + path: /docs/reference/js/remote-config.firebaseremoteconfigobject.md - title: RemoteConfig path: /docs/reference/js/remote-config.remoteconfig.md + - title: RemoteConfigOptions + path: /docs/reference/js/remote-config.remoteconfigoptions.md - title: RemoteConfigSettings path: /docs/reference/js/remote-config.remoteconfigsettings.md - title: Value diff --git a/docs-devsite/remote-config.fetchresponse.md b/docs-devsite/remote-config.fetchresponse.md new file mode 100644 index 00000000000..222cd9cb37c --- /dev/null +++ b/docs-devsite/remote-config.fetchresponse.md @@ -0,0 +1,67 @@ +Project: /docs/reference/js/_project.yaml +Book: /docs/reference/_book.yaml +page_type: reference + +{% comment %} +DO NOT EDIT THIS FILE! +This is generated by the JS SDK team, and any local changes will be +overwritten. Changes should be made in the source code at +https://github.com/firebase/firebase-js-sdk +{% endcomment %} + +# FetchResponse interface +Defines a successful response (200 or 304). + +

Modeled after the native interface, but simplified for Remote Config's use case. + +Signature: + +```typescript +export interface FetchResponse +``` + +## Properties + +| Property | Type | Description | +| --- | --- | --- | +| [config](./remote-config.fetchresponse.md#fetchresponseconfig) | [FirebaseRemoteConfigObject](./remote-config.firebaseremoteconfigobject.md#firebaseremoteconfigobject_interface) | Defines the map of parameters returned as "entries" in the fetch response body.

Only defined for 200 responses. | +| [eTag](./remote-config.fetchresponse.md#fetchresponseetag) | string | Defines the ETag response header value.

Only defined for 200 and 304 responses. | +| [status](./remote-config.fetchresponse.md#fetchresponsestatus) | number | The HTTP status, which is useful for differentiating success responses with data from those without.

is modeled after the native interface, so HTTP status is first-class.

Disambiguation: the fetch response returns a legacy "state" value that is redundant with the HTTP status code. The former is normalized into the latter. | + +## FetchResponse.config + +Defines the map of parameters returned as "entries" in the fetch response body. + +

Only defined for 200 responses. + +Signature: + +```typescript +config?: FirebaseRemoteConfigObject; +``` + +## FetchResponse.eTag + +Defines the ETag response header value. + +

Only defined for 200 and 304 responses. + +Signature: + +```typescript +eTag?: string; +``` + +## FetchResponse.status + +The HTTP status, which is useful for differentiating success responses with data from those without. + +

is modeled after the native interface, so HTTP status is first-class. + +

Disambiguation: the fetch response returns a legacy "state" value that is redundant with the HTTP status code. The former is normalized into the latter. + +Signature: + +```typescript +status: number; +``` diff --git a/docs-devsite/remote-config.firebaseremoteconfigobject.md b/docs-devsite/remote-config.firebaseremoteconfigobject.md new file mode 100644 index 00000000000..e7d89e5ec56 --- /dev/null +++ b/docs-devsite/remote-config.firebaseremoteconfigobject.md @@ -0,0 +1,19 @@ +Project: /docs/reference/js/_project.yaml +Book: /docs/reference/_book.yaml +page_type: reference + +{% comment %} +DO NOT EDIT THIS FILE! +This is generated by the JS SDK team, and any local changes will be +overwritten. Changes should be made in the source code at +https://github.com/firebase/firebase-js-sdk +{% endcomment %} + +# FirebaseRemoteConfigObject interface +Defines a self-descriptive reference for config key-value pairs. + +Signature: + +```typescript +export interface FirebaseRemoteConfigObject +``` diff --git a/docs-devsite/remote-config.md b/docs-devsite/remote-config.md index 40319453a3f..bcab3ce1e7f 100644 --- a/docs-devsite/remote-config.md +++ b/docs-devsite/remote-config.md @@ -17,7 +17,7 @@ The Firebase Remote Config Web SDK. This SDK does not work in a Node.js environm | Function | Description | | --- | --- | | function(app, ...) | -| [getRemoteConfig(app)](./remote-config.md#getremoteconfig_cf608e1) | | +| [getRemoteConfig(app, options)](./remote-config.md#getremoteconfig_61d368f) | | | function(remoteConfig, ...) | | [activate(remoteConfig)](./remote-config.md#activate_722a192) | Makes the last fetched config available to the getters. | | [ensureInitialized(remoteConfig)](./remote-config.md#ensureinitialized_722a192) | Ensures the last activated config are available to the getters. | @@ -38,7 +38,10 @@ The Firebase Remote Config Web SDK. This SDK does not work in a Node.js environm | Interface | Description | | --- | --- | | [CustomSignals](./remote-config.customsignals.md#customsignals_interface) | Defines the type for representing custom signals and their values.

The values in CustomSignals must be one of the following types:

| +| [FetchResponse](./remote-config.fetchresponse.md#fetchresponse_interface) | Defines a successful response (200 or 304).

Modeled after the native interface, but simplified for Remote Config's use case. | +| [FirebaseRemoteConfigObject](./remote-config.firebaseremoteconfigobject.md#firebaseremoteconfigobject_interface) | Defines a self-descriptive reference for config key-value pairs. | | [RemoteConfig](./remote-config.remoteconfig.md#remoteconfig_interface) | The Firebase Remote Config service interface. | +| [RemoteConfigOptions](./remote-config.remoteconfigoptions.md#remoteconfigoptions_interface) | Options for Remote Config initialization. | | [RemoteConfigSettings](./remote-config.remoteconfigsettings.md#remoteconfigsettings_interface) | Defines configuration options for the Remote Config SDK. | | [Value](./remote-config.value.md#value_interface) | Wraps a value with metadata and type-safe getters. | @@ -52,12 +55,12 @@ The Firebase Remote Config Web SDK. This SDK does not work in a Node.js environm ## function(app, ...) -### getRemoteConfig(app) {:#getremoteconfig_cf608e1} +### getRemoteConfig(app, options) {:#getremoteconfig_61d368f} Signature: ```typescript -export declare function getRemoteConfig(app?: FirebaseApp): RemoteConfig; +export declare function getRemoteConfig(app?: FirebaseApp, options?: RemoteConfigOptions): RemoteConfig; ``` #### Parameters @@ -65,6 +68,7 @@ export declare function getRemoteConfig(app?: FirebaseApp): RemoteConfig; | Parameter | Type | Description | | --- | --- | --- | | app | [FirebaseApp](./app.firebaseapp.md#firebaseapp_interface) | The [FirebaseApp](./app.firebaseapp.md#firebaseapp_interface) instance. | +| options | [RemoteConfigOptions](./remote-config.remoteconfigoptions.md#remoteconfigoptions_interface) | | Returns: diff --git a/docs-devsite/remote-config.remoteconfigoptions.md b/docs-devsite/remote-config.remoteconfigoptions.md new file mode 100644 index 00000000000..7caa96fa73b --- /dev/null +++ b/docs-devsite/remote-config.remoteconfigoptions.md @@ -0,0 +1,46 @@ +Project: /docs/reference/js/_project.yaml +Book: /docs/reference/_book.yaml +page_type: reference + +{% comment %} +DO NOT EDIT THIS FILE! +This is generated by the JS SDK team, and any local changes will be +overwritten. Changes should be made in the source code at +https://github.com/firebase/firebase-js-sdk +{% endcomment %} + +# RemoteConfigOptions interface +Options for Remote Config initialization. + +Signature: + +```typescript +export interface RemoteConfigOptions +``` + +## Properties + +| Property | Type | Description | +| --- | --- | --- | +| [initialFetchResponse](./remote-config.remoteconfigoptions.md#remoteconfigoptionsinitialfetchresponse) | [FetchResponse](./remote-config.fetchresponse.md#fetchresponse_interface) | Hydrates the state with an initial fetch response. | +| [templateId](./remote-config.remoteconfigoptions.md#remoteconfigoptionstemplateid) | string | The ID of the template to use. If not provided, defaults to "firebase". | + +## RemoteConfigOptions.initialFetchResponse + +Hydrates the state with an initial fetch response. + +Signature: + +```typescript +initialFetchResponse?: FetchResponse; +``` + +## RemoteConfigOptions.templateId + +The ID of the template to use. If not provided, defaults to "firebase". + +Signature: + +```typescript +templateId?: string; +``` diff --git a/packages/remote-config/src/public_types.ts b/packages/remote-config/src/public_types.ts index cfad9d7fd48..514971ed05e 100644 --- a/packages/remote-config/src/public_types.ts +++ b/packages/remote-config/src/public_types.ts @@ -18,6 +18,8 @@ import { FirebaseApp } from '@firebase/app'; import { FetchResponse } from './client/remote_config_fetch_client'; +export { FetchResponse, FirebaseRemoteConfigObject } from './client/remote_config_fetch_client'; + /** * Options for Remote Config initialization. * diff --git a/packages/remote-config/test/storage/storage.test.ts b/packages/remote-config/test/storage/storage.test.ts index 393036b2a73..cdf09e897b3 100644 --- a/packages/remote-config/test/storage/storage.test.ts +++ b/packages/remote-config/test/storage/storage.test.ts @@ -66,7 +66,7 @@ describe('Storage', () => { storage = getStorage(); }); - it(`sets and gets last fetch attempt status`, async () => { + it('sets and gets last fetch attempt status', async () => { const expectedStatus = 'success'; await storage.setLastFetchStatus(expectedStatus); @@ -76,7 +76,7 @@ describe('Storage', () => { expect(actualStatus).to.deep.eq(expectedStatus); }); - it(`sets and gets last fetch success timestamp`, async () => { + it('sets and gets last fetch success timestamp', async () => { const lastSuccessfulFetchTimestampMillis = 123; await storage.setLastSuccessfulFetchTimestampMillis( @@ -89,7 +89,7 @@ describe('Storage', () => { expect(actualMetadata).to.deep.eq(lastSuccessfulFetchTimestampMillis); }); - it(`sets and gets last successful fetch response`, async () => { + it('sets and gets last successful fetch response', async () => { const lastSuccessfulFetchResponse = { status: 200 } as FetchResponse; await storage.setLastSuccessfulFetchResponse(lastSuccessfulFetchResponse); @@ -99,7 +99,7 @@ describe('Storage', () => { expect(actualConfig).to.deep.eq(lastSuccessfulFetchResponse); }); - it(`sets and gets active config`, async () => { + it('sets and gets active config', async () => { const expectedConfig = { key: 'value' }; await storage.setActiveConfig(expectedConfig); @@ -109,7 +109,7 @@ describe('Storage', () => { expect(storedConfig).to.deep.eq(expectedConfig); }); - it(`sets and gets active config etag`, async () => { + it('sets and gets active config etag', async () => { const expectedEtag = 'etag'; await storage.setActiveConfigEtag(expectedEtag); @@ -119,7 +119,7 @@ describe('Storage', () => { expect(storedConfigEtag).to.deep.eq(expectedEtag); }); - it(`sets, gets and deletes throttle metadata`, async () => { + it('sets, gets and deletes throttle metadata', async () => { const expectedMetadata = { throttleEndTimeMillis: 1 } as ThrottleMetadata; @@ -137,7 +137,7 @@ describe('Storage', () => { expect(actualMetadata).to.be.undefined; }); - it(`sets and gets custom signals`, async () => { + it('sets and gets custom signals', async () => { const customSignals = { key: 'value', key1: 'value1', key2: 1 }; const customSignalsInStorage = { key: 'value', @@ -152,7 +152,7 @@ describe('Storage', () => { expect(storedCustomSignals).to.deep.eq(customSignalsInStorage); }); - it(`upserts custom signals when key is present in storage`, async () => { + it('upserts custom signals when key is present in storage', async () => { const customSignals = { key: 'value', key1: 'value1' }; const updatedSignals = { key: 'value', key1: 'value2' }; @@ -165,7 +165,7 @@ describe('Storage', () => { expect(storedCustomSignals).to.deep.eq(updatedSignals); }); - it(`deletes custom signal when value supplied is null`, async () => { + it('deletes custom signal when value supplied is null', async () => { const customSignals = { key: 'value', key1: 'value1' }; const updatedSignals = { key: 'value' }; @@ -178,7 +178,7 @@ describe('Storage', () => { expect(storedCustomSignals).to.deep.eq(updatedSignals); }); - it(`throws an error when supplied with excess custom signals`, async () => { + it('throws an error when supplied with excess custom signals', async () => { const customSignals: { [key: string]: string } = {}; for (let i = 0; i < 101; i++) { customSignals[`key${i}`] = `value${i}`; From 872be7f51c251fd41fc6bfe472a2653f413cb7bb Mon Sep 17 00:00:00 2001 From: kjelko Date: Thu, 9 Jan 2025 11:35:39 -0500 Subject: [PATCH 09/19] run formatter --- packages/remote-config/src/api.ts | 13 +++- packages/remote-config/src/public_types.ts | 7 +- packages/remote-config/src/register.ts | 8 +- packages/remote-config/src/storage/storage.ts | 8 +- packages/remote-config/test/api.test.ts | 76 +++++++++++++------ .../test/storage/storage.test.ts | 15 ++-- 6 files changed, 82 insertions(+), 45 deletions(-) diff --git a/packages/remote-config/src/api.ts b/packages/remote-config/src/api.ts index 9cb27d8cb24..072a7c43e43 100644 --- a/packages/remote-config/src/api.ts +++ b/packages/remote-config/src/api.ts @@ -42,7 +42,10 @@ import { LogLevel as FirebaseLogLevel } from '@firebase/logger'; * * @public */ -export function getRemoteConfig(app: FirebaseApp = getApp(), options: RemoteConfigOptions = {}): RemoteConfig { +export function getRemoteConfig( + app: FirebaseApp = getApp(), + options: RemoteConfigOptions = {} +): RemoteConfig { app = getModularInstance(app); const rcProvider = _getProvider(app, RC_COMPONENT_NAME); if (rcProvider.isInitialized()) { @@ -63,7 +66,9 @@ export function getRemoteConfig(app: FirebaseApp = getApp(), options: RemoteConf rc._storage.setActiveConfigEtag(options.initialFetchResponse?.eTag || ''), rc._storageCache.setLastSuccessfulFetchTimestampMillis(Date.now()), rc._storageCache.setLastFetchStatus('success'), - rc._storageCache.setActiveConfig(options.initialFetchResponse?.config || {}) + rc._storageCache.setActiveConfig( + options.initialFetchResponse?.config || {} + ) ]).then(); // The storageCache methods above set their in-memory fields sycnhronously, so it's // safe to declare our initialization complete at this point. @@ -249,7 +254,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { if (!rc._isInitializationComplete) { rc._logger.debug( `A value was requested for key "${key}" before SDK initialization completed.` + - ' Await on ensureInitialized if the intent was to get a previously activated value.' + ' Await on ensureInitialized if the intent was to get a previously activated value.' ); } const activeConfig = rc._storageCache.getActiveConfig(); @@ -260,7 +265,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { } rc._logger.debug( `Returning static value for key "${key}".` + - ' Define a default or remote value if this is unintentional.' + ' Define a default or remote value if this is unintentional.' ); return new ValueImpl('static'); } diff --git a/packages/remote-config/src/public_types.ts b/packages/remote-config/src/public_types.ts index 514971ed05e..cd6d9a0d1d8 100644 --- a/packages/remote-config/src/public_types.ts +++ b/packages/remote-config/src/public_types.ts @@ -18,11 +18,14 @@ import { FirebaseApp } from '@firebase/app'; import { FetchResponse } from './client/remote_config_fetch_client'; -export { FetchResponse, FirebaseRemoteConfigObject } from './client/remote_config_fetch_client'; +export { + FetchResponse, + FirebaseRemoteConfigObject +} from './client/remote_config_fetch_client'; /** * Options for Remote Config initialization. - * + * * @public */ export interface RemoteConfigOptions { diff --git a/packages/remote-config/src/register.ts b/packages/remote-config/src/register.ts index 14426621f12..dda6cc544de 100644 --- a/packages/remote-config/src/register.ts +++ b/packages/remote-config/src/register.ts @@ -23,7 +23,7 @@ import { isIndexedDBAvailable } from '@firebase/util'; import { Component, ComponentType, - ComponentContainer, + ComponentContainer } from '@firebase/component'; import { Logger, LogLevel as FirebaseLogLevel } from '@firebase/logger'; import { RemoteConfig, RemoteConfigOptions } from './public_types'; @@ -79,9 +79,9 @@ export function registerRemoteConfig(): void { } const namespace = options?.templateId || 'firebase'; - const storage = isIndexedDBAvailable() ? - new IndexedDbStorage(appId, app.name, namespace) : - new InMemoryStorage(); + const storage = isIndexedDBAvailable() + ? new IndexedDbStorage(appId, app.name, namespace) + : new InMemoryStorage(); const storageCache = new StorageCache(storage); const logger = new Logger(packageName); diff --git a/packages/remote-config/src/storage/storage.ts b/packages/remote-config/src/storage/storage.ts index f0aa8ad8913..1c9c0d1db95 100644 --- a/packages/remote-config/src/storage/storage.ts +++ b/packages/remote-config/src/storage/storage.ts @@ -175,7 +175,9 @@ export abstract class Storage { return this.get('custom_signals'); } - abstract setCustomSignals(customSignals: CustomSignals): Promise; + abstract setCustomSignals( + customSignals: CustomSignals + ): Promise; abstract get(key: ProjectNamespaceKeyFieldValue): Promise; abstract set(key: ProjectNamespaceKeyFieldValue, value: T): Promise; abstract delete(key: ProjectNamespaceKeyFieldValue): Promise; @@ -373,8 +375,8 @@ export class InMemoryStorage extends Storage { async setCustomSignals(customSignals: CustomSignals): Promise { const combinedSignals = { - ...this.storage['custom_signals'] as CustomSignals, - ...customSignals, + ...(this.storage['custom_signals'] as CustomSignals), + ...customSignals }; const updatedSignals = Object.fromEntries( diff --git a/packages/remote-config/test/api.test.ts b/packages/remote-config/test/api.test.ts index 0a30a947a1b..ae28517d524 100644 --- a/packages/remote-config/test/api.test.ts +++ b/packages/remote-config/test/api.test.ts @@ -1,18 +1,39 @@ +/** + * @license + * Copyright 2025 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + import { expect } from 'chai'; -import { ensureInitialized, fetchAndActivate, getRemoteConfig, getString } from '../src/index'; +import { + ensureInitialized, + fetchAndActivate, + getRemoteConfig, + getString +} from '../src/index'; import '../test/setup'; -import { deleteApp, FirebaseApp, initializeApp, _addOrOverwriteComponent } from '@firebase/app'; +import { + deleteApp, + FirebaseApp, + initializeApp, + _addOrOverwriteComponent +} from '@firebase/app'; import * as sinon from 'sinon'; import { FetchResponse } from '../src/client/remote_config_fetch_client'; -import { - Component, - ComponentType -} from '@firebase/component'; +import { Component, ComponentType } from '@firebase/component'; import { FirebaseInstallations } from '@firebase/installations-types'; -import { - openDatabase, - APP_NAMESPACE_STORE, -} from '../src/storage/storage'; +import { openDatabase, APP_NAMESPACE_STORE } from '../src/storage/storage'; const fakeFirebaseConfig = { apiKey: 'api-key', @@ -36,7 +57,7 @@ describe('Remote Config API', () => { const STUB_FETCH_RESPONSE: FetchResponse = { status: 200, eTag: 'asdf', - config: { 'foobar': 'hello world' }, + config: { 'foobar': 'hello world' } }; let fetchStub: sinon.SinonStub; @@ -50,11 +71,11 @@ describe('Remote Config API', () => { () => { return { getId: () => Promise.resolve('fis-id'), - getToken: () => Promise.resolve('fis-token'), + getToken: () => Promise.resolve('fis-token') } as any as FirebaseInstallations; }, ComponentType.PUBLIC - ) as any, + ) as any ); }); @@ -65,16 +86,18 @@ describe('Remote Config API', () => { }); function setFetchResponse(response: FetchResponse = { status: 200 }): void { - fetchStub.returns(Promise.resolve({ - ok: response.status === 200, - status: response.status, - headers: new Headers({ ETag: response.eTag || '' }), - json: () => - Promise.resolve({ - entries: response.config, - state: 'OK' - }) - } as Response)); + fetchStub.returns( + Promise.resolve({ + ok: response.status === 200, + status: response.status, + headers: new Headers({ ETag: response.eTag || '' }), + json: () => + Promise.resolve({ + entries: response.config, + state: 'OK' + }) + } as Response) + ); } it('allows multiple initializations if options are same', () => { @@ -115,11 +138,14 @@ describe('Remote Config API', () => { await fetchAndActivate(rc); expect(fetchStub).to.be.calledOnceWith( 'https://firebaseremoteconfig.googleapis.com/v1/projects/project-id/namespaces/altTemplate:fetch?key=api-key', - sinon.match.object); + sinon.match.object + ); }); it('hydrates with initialFetchResponse', async () => { - const rc = getRemoteConfig(app, { initialFetchResponse: STUB_FETCH_RESPONSE }); + const rc = getRemoteConfig(app, { + initialFetchResponse: STUB_FETCH_RESPONSE + }); await ensureInitialized(rc); expect(getString(rc, 'foobar')).to.equal('hello world'); }); diff --git a/packages/remote-config/test/storage/storage.test.ts b/packages/remote-config/test/storage/storage.test.ts index cdf09e897b3..96be895366e 100644 --- a/packages/remote-config/test/storage/storage.test.ts +++ b/packages/remote-config/test/storage/storage.test.ts @@ -36,15 +36,14 @@ async function clearDatabase(): Promise { } describe('Storage', () => { - const indexedDbTestCase = { getStorage: () => new IndexedDbStorage('appId', 'appName', 'namespace'), - name: 'IndexedDbStorage', + name: 'IndexedDbStorage' }; const inMemoryStorage = { getStorage: () => new InMemoryStorage(), - name: 'InMemoryStorage', + name: 'InMemoryStorage' }; beforeEach(async () => { @@ -53,9 +52,9 @@ describe('Storage', () => { it(`${indexedDbTestCase.name} constructs a composite key`, async () => { // This is defensive, but the cost of accidentally changing the key composition is high. - expect(indexedDbTestCase.getStorage().createCompositeKey('throttle_metadata')).to.eq( - 'appId,appName,namespace,throttle_metadata' - ); + expect( + indexedDbTestCase.getStorage().createCompositeKey('throttle_metadata') + ).to.eq('appId,appName,namespace,throttle_metadata'); }); for (const { name, getStorage } of [indexedDbTestCase, inMemoryStorage]) { @@ -92,7 +91,9 @@ describe('Storage', () => { it('sets and gets last successful fetch response', async () => { const lastSuccessfulFetchResponse = { status: 200 } as FetchResponse; - await storage.setLastSuccessfulFetchResponse(lastSuccessfulFetchResponse); + await storage.setLastSuccessfulFetchResponse( + lastSuccessfulFetchResponse + ); const actualConfig = await storage.getLastSuccessfulFetchResponse(); From efab458a17cc65a3a38c5bdc9892b880aa203882 Mon Sep 17 00:00:00 2001 From: kjelko Date: Thu, 9 Jan 2025 14:11:58 -0500 Subject: [PATCH 10/19] add changeset --- .changeset/flat-plums-hope.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/flat-plums-hope.md diff --git a/.changeset/flat-plums-hope.md b/.changeset/flat-plums-hope.md new file mode 100644 index 00000000000..57e7c32b9ca --- /dev/null +++ b/.changeset/flat-plums-hope.md @@ -0,0 +1,5 @@ +--- +'@firebase/remote-config': minor +--- + +Adds support for initial state hydration (from SSR contexts) From 7b767746f08d074f7603ee5a72f518907e7fac67 Mon Sep 17 00:00:00 2001 From: kjelko Date: Sun, 12 Jan 2025 08:42:47 -0500 Subject: [PATCH 11/19] bump minor version --- .changeset/flat-plums-hope.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/flat-plums-hope.md b/.changeset/flat-plums-hope.md index 57e7c32b9ca..ac71c7269aa 100644 --- a/.changeset/flat-plums-hope.md +++ b/.changeset/flat-plums-hope.md @@ -1,5 +1,6 @@ --- '@firebase/remote-config': minor +'firebase': minor --- Adds support for initial state hydration (from SSR contexts) From 689812353a5ebe5b278a77c0719d1d074d9eca91 Mon Sep 17 00:00:00 2001 From: kjelko Date: Thu, 16 Jan 2025 10:50:31 -0500 Subject: [PATCH 12/19] move FetchResponse types to public_types --- .../src/client/caching_client.ts | 10 +-- .../src/client/remote_config_fetch_client.ts | 45 +--------- .../remote-config/src/client/rest_client.ts | 6 +- .../src/client/retrying_client.ts | 4 +- packages/remote-config/src/public_types.ts | 84 ++++++++++++++----- packages/remote-config/src/storage/storage.ts | 5 +- .../src/storage/storage_cache.ts | 4 +- packages/remote-config/test/api.test.ts | 4 +- .../test/client/caching_client.test.ts | 2 +- .../test/client/retrying_client.test.ts | 2 +- .../remote-config/test/remote_config.test.ts | 2 +- .../test/storage/storage.test.ts | 2 +- 12 files changed, 80 insertions(+), 90 deletions(-) diff --git a/packages/remote-config/src/client/caching_client.ts b/packages/remote-config/src/client/caching_client.ts index c9de804d7e8..e27babffafb 100644 --- a/packages/remote-config/src/client/caching_client.ts +++ b/packages/remote-config/src/client/caching_client.ts @@ -16,8 +16,8 @@ */ import { StorageCache } from '../storage/storage_cache'; +import { FetchResponse } from '../public_types'; import { - FetchResponse, RemoteConfigFetchClient, FetchRequest } from './remote_config_fetch_client'; @@ -37,7 +37,7 @@ export class CachingClient implements RemoteConfigFetchClient { private readonly storage: Storage, private readonly storageCache: StorageCache, private readonly logger: Logger - ) {} + ) { } /** * Returns true if the age of the cached fetched configs is less than or equal to @@ -65,9 +65,9 @@ export class CachingClient implements RemoteConfigFetchClient { this.logger.debug( 'Config fetch cache check.' + - ` Cache age millis: ${cacheAgeMillis}.` + - ` Cache max age millis (minimumFetchIntervalMillis setting): ${cacheMaxAgeMillis}.` + - ` Is cache hit: ${isCachedDataFresh}.` + ` Cache age millis: ${cacheAgeMillis}.` + + ` Cache max age millis (minimumFetchIntervalMillis setting): ${cacheMaxAgeMillis}.` + + ` Is cache hit: ${isCachedDataFresh}.` ); return isCachedDataFresh; diff --git a/packages/remote-config/src/client/remote_config_fetch_client.ts b/packages/remote-config/src/client/remote_config_fetch_client.ts index 71ea66d5e50..cede1211d12 100644 --- a/packages/remote-config/src/client/remote_config_fetch_client.ts +++ b/packages/remote-config/src/client/remote_config_fetch_client.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { CustomSignals } from '../public_types'; +import { CustomSignals, FetchResponse } from '../public_types'; /** * Defines a client, as in https://en.wikipedia.org/wiki/Client%E2%80%93server_model, for the @@ -36,13 +36,6 @@ export interface RemoteConfigFetchClient { fetch(request: FetchRequest): Promise; } -/** - * Defines a self-descriptive reference for config key-value pairs. - */ -export interface FirebaseRemoteConfigObject { - [key: string]: string; -} - /** * Shims a minimal AbortSignal. * @@ -109,39 +102,3 @@ export interface FetchRequest { customSignals?: CustomSignals; } -/** - * Defines a successful response (200 or 304). - * - *

Modeled after the native {@link Response} interface, but simplified for Remote Config's - * use case. - */ -export interface FetchResponse { - /** - * The HTTP status, which is useful for differentiating success responses with data from - * those without. - * - *

{@link RemoteConfigClient} is modeled after the native {@link GlobalFetch} interface, so - * HTTP status is first-class. - * - *

Disambiguation: the fetch response returns a legacy "state" value that is redundant with the - * HTTP status code. The former is normalized into the latter. - */ - status: number; - - /** - * Defines the ETag response header value. - * - *

Only defined for 200 and 304 responses. - */ - eTag?: string; - - /** - * Defines the map of parameters returned as "entries" in the fetch response body. - * - *

Only defined for 200 responses. - */ - config?: FirebaseRemoteConfigObject; - - // Note: we're not extracting experiment metadata until - // ABT and Analytics have Web SDKs. -} diff --git a/packages/remote-config/src/client/rest_client.ts b/packages/remote-config/src/client/rest_client.ts index 9d87ffbb1ac..e38d7be2f23 100644 --- a/packages/remote-config/src/client/rest_client.ts +++ b/packages/remote-config/src/client/rest_client.ts @@ -15,11 +15,9 @@ * limitations under the License. */ -import { CustomSignals } from '../public_types'; +import { CustomSignals, FetchResponse, FirebaseRemoteConfigObject } from '../public_types'; import { - FetchResponse, RemoteConfigFetchClient, - FirebaseRemoteConfigObject, FetchRequest } from './remote_config_fetch_client'; import { ERROR_FACTORY, ErrorCode } from '../errors'; @@ -57,7 +55,7 @@ export class RestClient implements RemoteConfigFetchClient { private readonly projectId: string, private readonly apiKey: string, private readonly appId: string - ) {} + ) { } /** * Fetches from the Remote Config REST API. diff --git a/packages/remote-config/src/client/retrying_client.ts b/packages/remote-config/src/client/retrying_client.ts index 874a37bd8f4..37cc9747a81 100644 --- a/packages/remote-config/src/client/retrying_client.ts +++ b/packages/remote-config/src/client/retrying_client.ts @@ -15,10 +15,10 @@ * limitations under the License. */ +import { FetchResponse } from '../public_types'; import { RemoteConfigAbortSignal, RemoteConfigFetchClient, - FetchResponse, FetchRequest } from './remote_config_fetch_client'; import { ThrottleMetadata, Storage } from '../storage/storage'; @@ -91,7 +91,7 @@ export class RetryingClient implements RemoteConfigFetchClient { constructor( private readonly client: RemoteConfigFetchClient, private readonly storage: Storage - ) {} + ) { } async fetch(request: FetchRequest): Promise { const throttleMetadata = (await this.storage.getThrottleMetadata()) || { diff --git a/packages/remote-config/src/public_types.ts b/packages/remote-config/src/public_types.ts index cd6d9a0d1d8..7f80f4ac6f0 100644 --- a/packages/remote-config/src/public_types.ts +++ b/packages/remote-config/src/public_types.ts @@ -16,29 +16,6 @@ */ import { FirebaseApp } from '@firebase/app'; -import { FetchResponse } from './client/remote_config_fetch_client'; - -export { - FetchResponse, - FirebaseRemoteConfigObject -} from './client/remote_config_fetch_client'; - -/** - * Options for Remote Config initialization. - * - * @public - */ -export interface RemoteConfigOptions { - /** - * The ID of the template to use. If not provided, defaults to "firebase". - */ - templateId?: string; - - /** - * Hydrates the state with an initial fetch response. - */ - initialFetchResponse?: FetchResponse; -} /** * The Firebase Remote Config service interface. @@ -73,6 +50,67 @@ export interface RemoteConfig { lastFetchStatus: FetchStatus; } +/** + * Defines a self-descriptive reference for config key-value pairs. + */ +export interface FirebaseRemoteConfigObject { + [key: string]: string; +} + +/** + * Defines a successful response (200 or 304). + * + *

Modeled after the native {@link Response} interface, but simplified for Remote Config's + * use case. + */ +export interface FetchResponse { + /** + * The HTTP status, which is useful for differentiating success responses with data from + * those without. + * + *

{@link RemoteConfigClient} is modeled after the native {@link GlobalFetch} interface, so + * HTTP status is first-class. + * + *

Disambiguation: the fetch response returns a legacy "state" value that is redundant with the + * HTTP status code. The former is normalized into the latter. + */ + status: number; + + /** + * Defines the ETag response header value. + * + *

Only defined for 200 and 304 responses. + */ + eTag?: string; + + /** + * Defines the map of parameters returned as "entries" in the fetch response body. + * + *

Only defined for 200 responses. + */ + config?: FirebaseRemoteConfigObject; + + // Note: we're not extracting experiment metadata until + // ABT and Analytics have Web SDKs. +} + +/** + * Options for Remote Config initialization. + * + * @public + */ +export interface RemoteConfigOptions { + /** + * The ID of the template to use. If not provided, defaults to "firebase". + */ + templateId?: string; + + /** + * Hydrates the state with an initial fetch response. + */ + initialFetchResponse?: FetchResponse; +} + /** * Indicates the source of a value. * diff --git a/packages/remote-config/src/storage/storage.ts b/packages/remote-config/src/storage/storage.ts index 1c9c0d1db95..03fb5f2b50e 100644 --- a/packages/remote-config/src/storage/storage.ts +++ b/packages/remote-config/src/storage/storage.ts @@ -16,10 +16,7 @@ */ import { FetchStatus, CustomSignals } from '@firebase/remote-config-types'; -import { - FetchResponse, - FirebaseRemoteConfigObject -} from '../client/remote_config_fetch_client'; +import { FetchResponse, FirebaseRemoteConfigObject } from '../public_types'; import { ERROR_FACTORY, ErrorCode } from '../errors'; import { RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS } from '../constants'; import { FirebaseError } from '@firebase/util'; diff --git a/packages/remote-config/src/storage/storage_cache.ts b/packages/remote-config/src/storage/storage_cache.ts index fc419b0068e..6df9892f838 100644 --- a/packages/remote-config/src/storage/storage_cache.ts +++ b/packages/remote-config/src/storage/storage_cache.ts @@ -16,14 +16,14 @@ */ import { FetchStatus, CustomSignals } from '@firebase/remote-config-types'; -import { FirebaseRemoteConfigObject } from '../client/remote_config_fetch_client'; +import { FirebaseRemoteConfigObject } from '../public_types'; import { Storage } from './storage'; /** * A memory cache layer over storage to support the SDK's synchronous read requirements. */ export class StorageCache { - constructor(private readonly storage: Storage) {} + constructor(private readonly storage: Storage) { } /** * Memory caches. diff --git a/packages/remote-config/test/api.test.ts b/packages/remote-config/test/api.test.ts index ae28517d524..b1fe658ebae 100644 --- a/packages/remote-config/test/api.test.ts +++ b/packages/remote-config/test/api.test.ts @@ -19,9 +19,10 @@ import { expect } from 'chai'; import { ensureInitialized, fetchAndActivate, + FetchResponse, getRemoteConfig, getString -} from '../src/index'; +} from '../src'; import '../test/setup'; import { deleteApp, @@ -30,7 +31,6 @@ import { _addOrOverwriteComponent } from '@firebase/app'; import * as sinon from 'sinon'; -import { FetchResponse } from '../src/client/remote_config_fetch_client'; import { Component, ComponentType } from '@firebase/component'; import { FirebaseInstallations } from '@firebase/installations-types'; import { openDatabase, APP_NAMESPACE_STORE } from '../src/storage/storage'; diff --git a/packages/remote-config/test/client/caching_client.test.ts b/packages/remote-config/test/client/caching_client.test.ts index a808dffb605..7f8fa04ca9d 100644 --- a/packages/remote-config/test/client/caching_client.test.ts +++ b/packages/remote-config/test/client/caching_client.test.ts @@ -17,9 +17,9 @@ import '../setup'; import { expect } from 'chai'; +import { FetchResponse } from '../../src'; import { RemoteConfigFetchClient, - FetchResponse, FetchRequest, RemoteConfigAbortSignal } from '../../src/client/remote_config_fetch_client'; diff --git a/packages/remote-config/test/client/retrying_client.test.ts b/packages/remote-config/test/client/retrying_client.test.ts index 65641b438bd..9b06ea5a957 100644 --- a/packages/remote-config/test/client/retrying_client.test.ts +++ b/packages/remote-config/test/client/retrying_client.test.ts @@ -18,10 +18,10 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; import { Storage, ThrottleMetadata } from '../../src/storage/storage'; +import { FetchResponse } from '../../src'; import { RemoteConfigFetchClient, FetchRequest, - FetchResponse, RemoteConfigAbortSignal } from '../../src/client/remote_config_fetch_client'; import { diff --git a/packages/remote-config/test/remote_config.test.ts b/packages/remote-config/test/remote_config.test.ts index 51304bc3b2f..cea07fcaf3e 100644 --- a/packages/remote-config/test/remote_config.test.ts +++ b/packages/remote-config/test/remote_config.test.ts @@ -17,6 +17,7 @@ import { FirebaseApp } from '@firebase/app'; import { + FetchResponse, RemoteConfig as RemoteConfigType, LogLevel as RemoteConfigLogLevel } from '../src/public_types'; @@ -27,7 +28,6 @@ import { Storage } from '../src/storage/storage'; import { RemoteConfig } from '../src/remote_config'; import { RemoteConfigFetchClient, - FetchResponse } from '../src/client/remote_config_fetch_client'; import { Value } from '../src/value'; import './setup'; diff --git a/packages/remote-config/test/storage/storage.test.ts b/packages/remote-config/test/storage/storage.test.ts index 96be895366e..5b8418a1187 100644 --- a/packages/remote-config/test/storage/storage.test.ts +++ b/packages/remote-config/test/storage/storage.test.ts @@ -25,7 +25,7 @@ import { InMemoryStorage, Storage } from '../../src/storage/storage'; -import { FetchResponse } from '../../src/client/remote_config_fetch_client'; +import { FetchResponse } from '../../src'; // Clears global IndexedDB state. async function clearDatabase(): Promise { From 62e1e847dcc768d497c206991acd57b88278e7b1 Mon Sep 17 00:00:00 2001 From: kjelko Date: Thu, 16 Jan 2025 10:51:55 -0500 Subject: [PATCH 13/19] extract custom signal merging logic to common function --- packages/remote-config/src/storage/storage.ts | 87 +++++++------------ 1 file changed, 33 insertions(+), 54 deletions(-) diff --git a/packages/remote-config/src/storage/storage.ts b/packages/remote-config/src/storage/storage.ts index 03fb5f2b50e..30bdf7575c0 100644 --- a/packages/remote-config/src/storage/storage.ts +++ b/packages/remote-config/src/storage/storage.ts @@ -202,33 +202,7 @@ export class IndexedDbStorage extends Storage { 'custom_signals', transaction ); - const combinedSignals = { - ...storedSignals, - ...customSignals - }; - // Filter out key-value assignments with null values since they are signals being unset - const updatedSignals = Object.fromEntries( - Object.entries(combinedSignals) - .filter(([_, v]) => v !== null) - .map(([k, v]) => { - // Stringify numbers to store a map of string keys and values which can be sent - // as-is in a fetch call. - if (typeof v === 'number') { - return [k, v.toString()]; - } - return [k, v]; - }) - ); - - // Throw an error if the number of custom signals to be stored exceeds the limit - if ( - Object.keys(updatedSignals).length > RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS - ) { - throw ERROR_FACTORY.create(ErrorCode.CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS, { - maxSignals: RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS - }); - } - + const updatedSignals = mergeCustomSignals(customSignals, storedSignals || {}); await this.setWithTransaction( 'custom_signals', updatedSignals, @@ -371,34 +345,39 @@ export class InMemoryStorage extends Storage { } async setCustomSignals(customSignals: CustomSignals): Promise { - const combinedSignals = { - ...(this.storage['custom_signals'] as CustomSignals), - ...customSignals - }; - - const updatedSignals = Object.fromEntries( - Object.entries(combinedSignals) - .filter(([_, v]) => v !== null) - .map(([k, v]) => { - // Stringify numbers to store a map of string keys and values which can be sent - // as-is in a fetch call. - if (typeof v === 'number') { - return [k, v.toString()]; - } - return [k, v]; - }) - ); - - if ( - Object.keys(updatedSignals).length > RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS - ) { - throw ERROR_FACTORY.create(ErrorCode.CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS, { - maxSignals: RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS - }); - } + const storedSignals = (this.storage['custom_signals'] || {}) as CustomSignals; + this.storage['custom_signals'] = mergeCustomSignals(customSignals, storedSignals); + return Promise.resolve(this.storage['custom_signals'] as CustomSignals); + } +} - this.storage['custom_signals'] = updatedSignals; +function mergeCustomSignals(customSignals: CustomSignals, storedSignals: CustomSignals): CustomSignals { + const combinedSignals = { + ...storedSignals, + ...customSignals + }; + + // Filter out key-value assignments with null values since they are signals being unset + const updatedSignals = Object.fromEntries( + Object.entries(combinedSignals) + .filter(([_, v]) => v !== null) + .map(([k, v]) => { + // Stringify numbers to store a map of string keys and values which can be sent + // as-is in a fetch call. + if (typeof v === 'number') { + return [k, v.toString()]; + } + return [k, v]; + }) + ); - return Promise.resolve(this.storage['custom_signals'] as CustomSignals); + // Throw an error if the number of custom signals to be stored exceeds the limit + if ( + Object.keys(updatedSignals).length > RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS + ) { + throw ERROR_FACTORY.create(ErrorCode.CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS, { + maxSignals: RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS + }); } + return updatedSignals; } From 551020eecb3e01f5ac5b6f5a9c0c2f64d81088d7 Mon Sep 17 00:00:00 2001 From: kjelko Date: Thu, 16 Jan 2025 10:59:03 -0500 Subject: [PATCH 14/19] run formatted again --- .../remote-config/src/client/caching_client.ts | 8 ++++---- .../src/client/remote_config_fetch_client.ts | 1 - .../remote-config/src/client/rest_client.ts | 8 ++++++-- .../src/client/retrying_client.ts | 2 +- packages/remote-config/src/storage/storage.ts | 18 ++++++++++++++---- .../remote-config/src/storage/storage_cache.ts | 2 +- .../remote-config/test/remote_config.test.ts | 4 +--- 7 files changed, 27 insertions(+), 16 deletions(-) diff --git a/packages/remote-config/src/client/caching_client.ts b/packages/remote-config/src/client/caching_client.ts index e27babffafb..2c7009c54be 100644 --- a/packages/remote-config/src/client/caching_client.ts +++ b/packages/remote-config/src/client/caching_client.ts @@ -37,7 +37,7 @@ export class CachingClient implements RemoteConfigFetchClient { private readonly storage: Storage, private readonly storageCache: StorageCache, private readonly logger: Logger - ) { } + ) {} /** * Returns true if the age of the cached fetched configs is less than or equal to @@ -65,9 +65,9 @@ export class CachingClient implements RemoteConfigFetchClient { this.logger.debug( 'Config fetch cache check.' + - ` Cache age millis: ${cacheAgeMillis}.` + - ` Cache max age millis (minimumFetchIntervalMillis setting): ${cacheMaxAgeMillis}.` + - ` Is cache hit: ${isCachedDataFresh}.` + ` Cache age millis: ${cacheAgeMillis}.` + + ` Cache max age millis (minimumFetchIntervalMillis setting): ${cacheMaxAgeMillis}.` + + ` Is cache hit: ${isCachedDataFresh}.` ); return isCachedDataFresh; diff --git a/packages/remote-config/src/client/remote_config_fetch_client.ts b/packages/remote-config/src/client/remote_config_fetch_client.ts index cede1211d12..359bb7c0409 100644 --- a/packages/remote-config/src/client/remote_config_fetch_client.ts +++ b/packages/remote-config/src/client/remote_config_fetch_client.ts @@ -101,4 +101,3 @@ export interface FetchRequest { */ customSignals?: CustomSignals; } - diff --git a/packages/remote-config/src/client/rest_client.ts b/packages/remote-config/src/client/rest_client.ts index e38d7be2f23..57f55f53d88 100644 --- a/packages/remote-config/src/client/rest_client.ts +++ b/packages/remote-config/src/client/rest_client.ts @@ -15,7 +15,11 @@ * limitations under the License. */ -import { CustomSignals, FetchResponse, FirebaseRemoteConfigObject } from '../public_types'; +import { + CustomSignals, + FetchResponse, + FirebaseRemoteConfigObject +} from '../public_types'; import { RemoteConfigFetchClient, FetchRequest @@ -55,7 +59,7 @@ export class RestClient implements RemoteConfigFetchClient { private readonly projectId: string, private readonly apiKey: string, private readonly appId: string - ) { } + ) {} /** * Fetches from the Remote Config REST API. diff --git a/packages/remote-config/src/client/retrying_client.ts b/packages/remote-config/src/client/retrying_client.ts index 37cc9747a81..ea5bc6e2fe1 100644 --- a/packages/remote-config/src/client/retrying_client.ts +++ b/packages/remote-config/src/client/retrying_client.ts @@ -91,7 +91,7 @@ export class RetryingClient implements RemoteConfigFetchClient { constructor( private readonly client: RemoteConfigFetchClient, private readonly storage: Storage - ) { } + ) {} async fetch(request: FetchRequest): Promise { const throttleMetadata = (await this.storage.getThrottleMetadata()) || { diff --git a/packages/remote-config/src/storage/storage.ts b/packages/remote-config/src/storage/storage.ts index 30bdf7575c0..f03ff41377b 100644 --- a/packages/remote-config/src/storage/storage.ts +++ b/packages/remote-config/src/storage/storage.ts @@ -202,7 +202,10 @@ export class IndexedDbStorage extends Storage { 'custom_signals', transaction ); - const updatedSignals = mergeCustomSignals(customSignals, storedSignals || {}); + const updatedSignals = mergeCustomSignals( + customSignals, + storedSignals || {} + ); await this.setWithTransaction( 'custom_signals', updatedSignals, @@ -345,13 +348,20 @@ export class InMemoryStorage extends Storage { } async setCustomSignals(customSignals: CustomSignals): Promise { - const storedSignals = (this.storage['custom_signals'] || {}) as CustomSignals; - this.storage['custom_signals'] = mergeCustomSignals(customSignals, storedSignals); + const storedSignals = (this.storage['custom_signals'] || + {}) as CustomSignals; + this.storage['custom_signals'] = mergeCustomSignals( + customSignals, + storedSignals + ); return Promise.resolve(this.storage['custom_signals'] as CustomSignals); } } -function mergeCustomSignals(customSignals: CustomSignals, storedSignals: CustomSignals): CustomSignals { +function mergeCustomSignals( + customSignals: CustomSignals, + storedSignals: CustomSignals +): CustomSignals { const combinedSignals = { ...storedSignals, ...customSignals diff --git a/packages/remote-config/src/storage/storage_cache.ts b/packages/remote-config/src/storage/storage_cache.ts index 6df9892f838..21add07ccd3 100644 --- a/packages/remote-config/src/storage/storage_cache.ts +++ b/packages/remote-config/src/storage/storage_cache.ts @@ -23,7 +23,7 @@ import { Storage } from './storage'; * A memory cache layer over storage to support the SDK's synchronous read requirements. */ export class StorageCache { - constructor(private readonly storage: Storage) { } + constructor(private readonly storage: Storage) {} /** * Memory caches. diff --git a/packages/remote-config/test/remote_config.test.ts b/packages/remote-config/test/remote_config.test.ts index cea07fcaf3e..8010f54f26d 100644 --- a/packages/remote-config/test/remote_config.test.ts +++ b/packages/remote-config/test/remote_config.test.ts @@ -26,9 +26,7 @@ import * as sinon from 'sinon'; import { StorageCache } from '../src/storage/storage_cache'; import { Storage } from '../src/storage/storage'; import { RemoteConfig } from '../src/remote_config'; -import { - RemoteConfigFetchClient, -} from '../src/client/remote_config_fetch_client'; +import { RemoteConfigFetchClient } from '../src/client/remote_config_fetch_client'; import { Value } from '../src/value'; import './setup'; import { ERROR_FACTORY, ErrorCode } from '../src/errors'; From edb7abe6cafb5e775181ae2146454b886d479881 Mon Sep 17 00:00:00 2001 From: kjelko Date: Wed, 22 Jan 2025 15:08:50 -0500 Subject: [PATCH 15/19] Update comments after review --- packages/remote-config/src/api.ts | 8 ++++---- packages/remote-config/src/public_types.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/remote-config/src/api.ts b/packages/remote-config/src/api.ts index 072a7c43e43..e895588d046 100644 --- a/packages/remote-config/src/api.ts +++ b/packages/remote-config/src/api.ts @@ -60,7 +60,7 @@ export function getRemoteConfig( if (options.initialFetchResponse) { // We use these initial writes as the initialization promise since they will hydrate the same - // fields that storageCache.loadFromStorage would set. + // fields that `storageCache.loadFromStorage` would set. rc._initializePromise = Promise.all([ rc._storage.setLastSuccessfulFetchResponse(options.initialFetchResponse), rc._storage.setActiveConfigEtag(options.initialFetchResponse?.eTag || ''), @@ -70,7 +70,7 @@ export function getRemoteConfig( options.initialFetchResponse?.config || {} ) ]).then(); - // The storageCache methods above set their in-memory fields sycnhronously, so it's + // The `storageCache` methods above set their in-memory fields synchronously, so it's // safe to declare our initialization complete at this point. rc._isInitializationComplete = true; } @@ -254,7 +254,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { if (!rc._isInitializationComplete) { rc._logger.debug( `A value was requested for key "${key}" before SDK initialization completed.` + - ' Await on ensureInitialized if the intent was to get a previously activated value.' + ' Await on ensureInitialized if the intent was to get a previously activated value.' ); } const activeConfig = rc._storageCache.getActiveConfig(); @@ -265,7 +265,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { } rc._logger.debug( `Returning static value for key "${key}".` + - ' Define a default or remote value if this is unintentional.' + ' Define a default or remote value if this is unintentional.' ); return new ValueImpl('static'); } diff --git a/packages/remote-config/src/public_types.ts b/packages/remote-config/src/public_types.ts index 7f80f4ac6f0..927bc84ca10 100644 --- a/packages/remote-config/src/public_types.ts +++ b/packages/remote-config/src/public_types.ts @@ -60,7 +60,7 @@ export interface FirebaseRemoteConfigObject { /** * Defines a successful response (200 or 304). * - *

Modeled after the native {@link Response} interface, but simplified for Remote Config's + *

Modeled after the native `Response` interface, but simplified for Remote Config's * use case. */ export interface FetchResponse { @@ -68,7 +68,7 @@ export interface FetchResponse { * The HTTP status, which is useful for differentiating success responses with data from * those without. * - *

{@link RemoteConfigClient} is modeled after the native {@link GlobalFetch} interface, so + *

The Remote Config client is modeled after the native `Fetch` interface, so * HTTP status is first-class. * *

Disambiguation: the fetch response returns a legacy "state" value that is redundant with the From 221d659d75bb49ec52c85adf8300500b686ab0b1 Mon Sep 17 00:00:00 2001 From: kjelko Date: Wed, 22 Jan 2025 15:21:39 -0500 Subject: [PATCH 16/19] format and docgen --- docs-devsite/remote-config.fetchresponse.md | 6 +++--- docs-devsite/remote-config.md | 2 +- packages/remote-config/src/api.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs-devsite/remote-config.fetchresponse.md b/docs-devsite/remote-config.fetchresponse.md index 222cd9cb37c..414188e72bb 100644 --- a/docs-devsite/remote-config.fetchresponse.md +++ b/docs-devsite/remote-config.fetchresponse.md @@ -12,7 +12,7 @@ https://github.com/firebase/firebase-js-sdk # FetchResponse interface Defines a successful response (200 or 304). -

Modeled after the native interface, but simplified for Remote Config's use case. +

Modeled after the native `Response` interface, but simplified for Remote Config's use case. Signature: @@ -26,7 +26,7 @@ export interface FetchResponse | --- | --- | --- | | [config](./remote-config.fetchresponse.md#fetchresponseconfig) | [FirebaseRemoteConfigObject](./remote-config.firebaseremoteconfigobject.md#firebaseremoteconfigobject_interface) | Defines the map of parameters returned as "entries" in the fetch response body.

Only defined for 200 responses. | | [eTag](./remote-config.fetchresponse.md#fetchresponseetag) | string | Defines the ETag response header value.

Only defined for 200 and 304 responses. | -| [status](./remote-config.fetchresponse.md#fetchresponsestatus) | number | The HTTP status, which is useful for differentiating success responses with data from those without.

is modeled after the native interface, so HTTP status is first-class.

Disambiguation: the fetch response returns a legacy "state" value that is redundant with the HTTP status code. The former is normalized into the latter. | +| [status](./remote-config.fetchresponse.md#fetchresponsestatus) | number | The HTTP status, which is useful for differentiating success responses with data from those without.

The Remote Config client is modeled after the native Fetch interface, so HTTP status is first-class.

Disambiguation: the fetch response returns a legacy "state" value that is redundant with the HTTP status code. The former is normalized into the latter. | ## FetchResponse.config @@ -56,7 +56,7 @@ eTag?: string; The HTTP status, which is useful for differentiating success responses with data from those without. -

is modeled after the native interface, so HTTP status is first-class. +

The Remote Config client is modeled after the native `Fetch` interface, so HTTP status is first-class.

Disambiguation: the fetch response returns a legacy "state" value that is redundant with the HTTP status code. The former is normalized into the latter. diff --git a/docs-devsite/remote-config.md b/docs-devsite/remote-config.md index bcab3ce1e7f..38df48ac794 100644 --- a/docs-devsite/remote-config.md +++ b/docs-devsite/remote-config.md @@ -38,7 +38,7 @@ The Firebase Remote Config Web SDK. This SDK does not work in a Node.js environm | Interface | Description | | --- | --- | | [CustomSignals](./remote-config.customsignals.md#customsignals_interface) | Defines the type for representing custom signals and their values.

The values in CustomSignals must be one of the following types:

| -| [FetchResponse](./remote-config.fetchresponse.md#fetchresponse_interface) | Defines a successful response (200 or 304).

Modeled after the native interface, but simplified for Remote Config's use case. | +| [FetchResponse](./remote-config.fetchresponse.md#fetchresponse_interface) | Defines a successful response (200 or 304).

Modeled after the native Response interface, but simplified for Remote Config's use case. | | [FirebaseRemoteConfigObject](./remote-config.firebaseremoteconfigobject.md#firebaseremoteconfigobject_interface) | Defines a self-descriptive reference for config key-value pairs. | | [RemoteConfig](./remote-config.remoteconfig.md#remoteconfig_interface) | The Firebase Remote Config service interface. | | [RemoteConfigOptions](./remote-config.remoteconfigoptions.md#remoteconfigoptions_interface) | Options for Remote Config initialization. | diff --git a/packages/remote-config/src/api.ts b/packages/remote-config/src/api.ts index e895588d046..7708d182148 100644 --- a/packages/remote-config/src/api.ts +++ b/packages/remote-config/src/api.ts @@ -254,7 +254,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { if (!rc._isInitializationComplete) { rc._logger.debug( `A value was requested for key "${key}" before SDK initialization completed.` + - ' Await on ensureInitialized if the intent was to get a previously activated value.' + ' Await on ensureInitialized if the intent was to get a previously activated value.' ); } const activeConfig = rc._storageCache.getActiveConfig(); @@ -265,7 +265,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { } rc._logger.debug( `Returning static value for key "${key}".` + - ' Define a default or remote value if this is unintentional.' + ' Define a default or remote value if this is unintentional.' ); return new ValueImpl('static'); } From 676627d8c3a72c814641d248c2d946f74cb38ea7 Mon Sep 17 00:00:00 2001 From: kjelko Date: Mon, 27 Jan 2025 10:23:09 -0500 Subject: [PATCH 17/19] Add RemoteConfigOptions param doc comment --- docs-devsite/remote-config.md | 2 +- packages/remote-config/src/api.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs-devsite/remote-config.md b/docs-devsite/remote-config.md index 38df48ac794..e0464a70a73 100644 --- a/docs-devsite/remote-config.md +++ b/docs-devsite/remote-config.md @@ -68,7 +68,7 @@ export declare function getRemoteConfig(app?: FirebaseApp, options?: RemoteConfi | Parameter | Type | Description | | --- | --- | --- | | app | [FirebaseApp](./app.firebaseapp.md#firebaseapp_interface) | The [FirebaseApp](./app.firebaseapp.md#firebaseapp_interface) instance. | -| options | [RemoteConfigOptions](./remote-config.remoteconfigoptions.md#remoteconfigoptions_interface) | | +| options | [RemoteConfigOptions](./remote-config.remoteconfigoptions.md#remoteconfigoptions_interface) | The [RemoteConfigOptions](./remote-config.remoteconfigoptions.md#remoteconfigoptions_interface) with which to instantiate the Remote Config instance. | Returns: diff --git a/packages/remote-config/src/api.ts b/packages/remote-config/src/api.ts index 7708d182148..895d70c560e 100644 --- a/packages/remote-config/src/api.ts +++ b/packages/remote-config/src/api.ts @@ -38,6 +38,8 @@ import { LogLevel as FirebaseLogLevel } from '@firebase/logger'; /** * * @param app - The {@link @firebase/app#FirebaseApp} instance. + * @param options - The {@link RemoteConfigOptions} with which to instantiate the + * Remote Config instance. * @returns A {@link RemoteConfig} instance. * * @public From 6756dfae9ad044c113fe85bbe71f0713523401ef Mon Sep 17 00:00:00 2001 From: kjelko Date: Mon, 27 Jan 2025 11:13:54 -0500 Subject: [PATCH 18/19] Mark options as optional --- docs-devsite/remote-config.md | 2 +- packages/remote-config/src/api.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs-devsite/remote-config.md b/docs-devsite/remote-config.md index e0464a70a73..58d23cfd647 100644 --- a/docs-devsite/remote-config.md +++ b/docs-devsite/remote-config.md @@ -68,7 +68,7 @@ export declare function getRemoteConfig(app?: FirebaseApp, options?: RemoteConfi | Parameter | Type | Description | | --- | --- | --- | | app | [FirebaseApp](./app.firebaseapp.md#firebaseapp_interface) | The [FirebaseApp](./app.firebaseapp.md#firebaseapp_interface) instance. | -| options | [RemoteConfigOptions](./remote-config.remoteconfigoptions.md#remoteconfigoptions_interface) | The [RemoteConfigOptions](./remote-config.remoteconfigoptions.md#remoteconfigoptions_interface) with which to instantiate the Remote Config instance. | +| options | [RemoteConfigOptions](./remote-config.remoteconfigoptions.md#remoteconfigoptions_interface) | Optional. The [RemoteConfigOptions](./remote-config.remoteconfigoptions.md#remoteconfigoptions_interface) with which to instantiate the Remote Config instance. | Returns: diff --git a/packages/remote-config/src/api.ts b/packages/remote-config/src/api.ts index 895d70c560e..b14ac8cbf20 100644 --- a/packages/remote-config/src/api.ts +++ b/packages/remote-config/src/api.ts @@ -38,7 +38,7 @@ import { LogLevel as FirebaseLogLevel } from '@firebase/logger'; /** * * @param app - The {@link @firebase/app#FirebaseApp} instance. - * @param options - The {@link RemoteConfigOptions} with which to instantiate the + * @param options - Optional. The {@link RemoteConfigOptions} with which to instantiate the * Remote Config instance. * @returns A {@link RemoteConfig} instance. * @@ -256,7 +256,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { if (!rc._isInitializationComplete) { rc._logger.debug( `A value was requested for key "${key}" before SDK initialization completed.` + - ' Await on ensureInitialized if the intent was to get a previously activated value.' + ' Await on ensureInitialized if the intent was to get a previously activated value.' ); } const activeConfig = rc._storageCache.getActiveConfig(); @@ -267,7 +267,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { } rc._logger.debug( `Returning static value for key "${key}".` + - ' Define a default or remote value if this is unintentional.' + ' Define a default or remote value if this is unintentional.' ); return new ValueImpl('static'); } From bb990314544a3f7126e602832e2df1fde3f4fbde Mon Sep 17 00:00:00 2001 From: kjelko Date: Mon, 27 Jan 2025 11:16:58 -0500 Subject: [PATCH 19/19] format --- packages/remote-config/src/api.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/remote-config/src/api.ts b/packages/remote-config/src/api.ts index b14ac8cbf20..1431864edd5 100644 --- a/packages/remote-config/src/api.ts +++ b/packages/remote-config/src/api.ts @@ -256,7 +256,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { if (!rc._isInitializationComplete) { rc._logger.debug( `A value was requested for key "${key}" before SDK initialization completed.` + - ' Await on ensureInitialized if the intent was to get a previously activated value.' + ' Await on ensureInitialized if the intent was to get a previously activated value.' ); } const activeConfig = rc._storageCache.getActiveConfig(); @@ -267,7 +267,7 @@ export function getValue(remoteConfig: RemoteConfig, key: string): Value { } rc._logger.debug( `Returning static value for key "${key}".` + - ' Define a default or remote value if this is unintentional.' + ' Define a default or remote value if this is unintentional.' ); return new ValueImpl('static'); }