From 5c1a83ed70bae979322bd8751c0885d683ce4bf3 Mon Sep 17 00:00:00 2001 From: Feiyang Date: Fri, 12 Mar 2021 13:36:37 -0800 Subject: [PATCH] Add `Provider.initialize()` (#4595) * let factory accept options * add initialize() to Provider * use provider.initialize in perf * use provider.initialize in Auth * use provider.initialize in firestore * fix errors * Create clever-apricots-look.md --- .changeset/clever-apricots-look.md | 10 +++ packages-exp/analytics-compat/src/index.ts | 7 +- .../auth-exp/src/core/auth/initialize.ts | 3 +- .../auth-exp/src/core/auth/register.ts | 10 ++- packages-exp/functions-exp/src/config.ts | 2 +- packages-exp/performance-exp/src/index.ts | 13 ++-- .../remote-config-compat/src/index.ts | 5 +- .../remote-config-exp/src/register.ts | 5 +- packages/component/index.ts | 3 +- packages/component/src/provider.test.ts | 32 +++++++++- packages/component/src/provider.ts | 64 +++++++++++++++---- packages/component/src/types.ts | 9 ++- packages/database/exp/index.ts | 2 +- packages/database/index.node.ts | 2 +- packages/database/index.ts | 2 +- packages/firestore/exp/register.ts | 9 ++- packages/firestore/lite/register.ts | 9 ++- packages/firestore/src/exp/database.ts | 4 +- packages/firestore/src/lite/database.ts | 4 +- packages/functions/src/config.ts | 5 +- packages/remote-config/index.ts | 5 +- packages/storage/compat/index.ts | 5 +- packages/storage/exp/index.ts | 8 ++- 23 files changed, 161 insertions(+), 57 deletions(-) create mode 100644 .changeset/clever-apricots-look.md diff --git a/.changeset/clever-apricots-look.md b/.changeset/clever-apricots-look.md new file mode 100644 index 00000000000..0303e0bd6e0 --- /dev/null +++ b/.changeset/clever-apricots-look.md @@ -0,0 +1,10 @@ +--- +"@firebase/component": minor +"@firebase/database": patch +"@firebase/firestore": patch +"@firebase/functions": patch +"@firebase/remote-config": patch +"@firebase/storage": patch +--- + +Component facotry now takes an options object. And added `Provider.initialize()` that can be used to pass an options object to the component factory. diff --git a/packages-exp/analytics-compat/src/index.ts b/packages-exp/analytics-compat/src/index.ts index ee2d926bae2..567566240b5 100644 --- a/packages-exp/analytics-compat/src/index.ts +++ b/packages-exp/analytics-compat/src/index.ts @@ -41,16 +41,13 @@ declare module '@firebase/component' { } const factory: InstanceFactory<'analytics-compat'> = ( - container: ComponentContainer, - regionOrCustomDomain?: string + container: ComponentContainer ) => { // Dependencies const app = container.getProvider('app-compat').getImmediate(); const analyticsServiceExp = container .getProvider('analytics-exp') - .getImmediate({ - identifier: regionOrCustomDomain - }); + .getImmediate(); return new AnalyticsService(app as FirebaseApp, analyticsServiceExp); }; diff --git a/packages-exp/auth-exp/src/core/auth/initialize.ts b/packages-exp/auth-exp/src/core/auth/initialize.ts index cfb3d0fe655..56cdb81bf66 100644 --- a/packages-exp/auth-exp/src/core/auth/initialize.ts +++ b/packages-exp/auth-exp/src/core/auth/initialize.ts @@ -34,8 +34,7 @@ export function initializeAuth(app: FirebaseApp, deps?: Dependencies): Auth { _fail(auth, AuthErrorCode.ALREADY_INITIALIZED); } - const auth = provider.getImmediate() as AuthImpl; - _initializeAuthInstance(auth, deps); + const auth = provider.initialize({ options: deps }) as AuthImpl; return auth; } diff --git a/packages-exp/auth-exp/src/core/auth/register.ts b/packages-exp/auth-exp/src/core/auth/register.ts index 7baa42428b1..583681a1e77 100644 --- a/packages-exp/auth-exp/src/core/auth/register.ts +++ b/packages-exp/auth-exp/src/core/auth/register.ts @@ -25,6 +25,8 @@ import { _assert } from '../util/assert'; import { _getClientVersion, ClientPlatform } from '../util/version'; import { _castAuth, AuthImpl, DefaultConfig } from './auth_impl'; import { AuthInterop } from './firebase_internal'; +import { Dependencies } from '../../model/auth'; +import { _initializeAuthInstance } from './initialize'; export const enum _ComponentName { AUTH = 'auth-exp', @@ -53,7 +55,7 @@ export function registerAuth(clientPlatform: ClientPlatform): void { _registerComponent( new Component( _ComponentName.AUTH, - container => { + (container, { options: deps }: { options?: Dependencies }) => { const app = container.getProvider('app-exp').getImmediate()!; const { apiKey, authDomain } = app.options; return (app => { @@ -66,7 +68,11 @@ export function registerAuth(clientPlatform: ClientPlatform): void { apiScheme: DefaultConfig.API_SCHEME, sdkClientVersion: _getClientVersion(clientPlatform) }; - return new AuthImpl(app, config); + + const authInstance = new AuthImpl(app, config); + _initializeAuthInstance(authInstance, deps); + + return authInstance; })(app); }, ComponentType.PUBLIC diff --git a/packages-exp/functions-exp/src/config.ts b/packages-exp/functions-exp/src/config.ts index 9279835b2c7..a4d123871ab 100644 --- a/packages-exp/functions-exp/src/config.ts +++ b/packages-exp/functions-exp/src/config.ts @@ -30,7 +30,7 @@ export const DEFAULT_REGION = 'us-central1'; export function registerFunctions(fetchImpl: typeof fetch): void { const factory: InstanceFactory<'functions'> = ( container: ComponentContainer, - regionOrCustomDomain?: string + { instanceIdentifier: regionOrCustomDomain } ) => { // Dependencies const app = container.getProvider('app-exp').getImmediate(); diff --git a/packages-exp/performance-exp/src/index.ts b/packages-exp/performance-exp/src/index.ts index 6abe7e7634c..5083328f246 100644 --- a/packages-exp/performance-exp/src/index.ts +++ b/packages-exp/performance-exp/src/index.ts @@ -70,8 +70,9 @@ export function initializePerformance( throw ERROR_FACTORY.create(ErrorCode.ALREADY_INITIALIZED); } - const perfInstance = provider.getImmediate() as PerformanceController; - perfInstance._init(settings); + const perfInstance = provider.initialize({ + options: settings + }) as PerformanceController; return perfInstance; } @@ -89,7 +90,8 @@ export function trace( } const factory: InstanceFactory<'performance-exp'> = ( - container: ComponentContainer + container: ComponentContainer, + { options: settings }: { options?: PerformanceSettings } ) => { // Dependencies const app = container.getProvider('app-exp').getImmediate(); @@ -104,7 +106,10 @@ const factory: InstanceFactory<'performance-exp'> = ( throw ERROR_FACTORY.create(ErrorCode.NO_WINDOW); } setupApi(window); - return new PerformanceController(app, installations); + const perfInstance = new PerformanceController(app, installations); + perfInstance._init(settings); + + return perfInstance; }; function registerPerformance(): void { diff --git a/packages-exp/remote-config-compat/src/index.ts b/packages-exp/remote-config-compat/src/index.ts index 04b968a5625..1db9dd82727 100644 --- a/packages-exp/remote-config-compat/src/index.ts +++ b/packages-exp/remote-config-compat/src/index.ts @@ -19,7 +19,8 @@ import firebase, { _FirebaseNamespace } from '@firebase/app-compat'; import { Component, ComponentContainer, - ComponentType + ComponentType, + InstanceFactoryOptions } from '@firebase/component'; import { RemoteConfigCompatImpl } from './remoteConfig'; import { name as packageName, version } from '../package.json'; @@ -48,7 +49,7 @@ function registerRemoteConfigCompat( function remoteConfigFactory( container: ComponentContainer, - namespace?: string + { instanceIdentifier: namespace }: InstanceFactoryOptions ): RemoteConfigCompatImpl { const app = container.getProvider('app-compat').getImmediate(); // The following call will always succeed because rc `import {...} from '@firebase/remote-config-exp'` diff --git a/packages-exp/remote-config-exp/src/register.ts b/packages-exp/remote-config-exp/src/register.ts index ab3c7aab6cd..ea71928ffcc 100644 --- a/packages-exp/remote-config-exp/src/register.ts +++ b/packages-exp/remote-config-exp/src/register.ts @@ -22,7 +22,8 @@ import { import { Component, ComponentType, - ComponentContainer + ComponentContainer, + InstanceFactoryOptions } from '@firebase/component'; import { Logger, LogLevel as FirebaseLogLevel } from '@firebase/logger'; import { RemoteConfig } from './public_types'; @@ -53,7 +54,7 @@ export function registerRemoteConfig(): void { function remoteConfigFactory( container: ComponentContainer, - namespace?: string + { instanceIdentifier: namespace }: InstanceFactoryOptions ): RemoteConfig { /* Dependencies */ // getImmediate for FirebaseApp will always succeed diff --git a/packages/component/index.ts b/packages/component/index.ts index a304b6827d8..c7392c452f9 100644 --- a/packages/component/index.ts +++ b/packages/component/index.ts @@ -23,5 +23,6 @@ export { InstanceFactory, InstantiationMode, NameServiceMapping, - Name + Name, + InstanceFactoryOptions } from './src/types'; diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 039f640b629..c9bc3b1976c 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -101,6 +101,34 @@ describe('Provider', () => { expect(() => provider.setComponent(component)).to.not.throw(); }); + describe('initialize()', () => { + it('throws if the provider is already initialized', () => { + provider.setComponent(getFakeComponent('test', () => ({}))); + provider.initialize(); + + expect(() => provider.initialize()).to.throw(); + }); + + it('throws if the component has not been registered', () => { + expect(() => provider.initialize()).to.throw(); + }); + + it('accepts an options parameter and passes it to the instance factory', () => { + const options = { + configurable: true, + test: true + }; + provider.setComponent( + getFakeComponent('test', (_container, opts) => ({ + options: opts.options + })) + ); + const instance = provider.initialize({ options }); + + expect((instance as any).options).to.deep.equal(options); + }); + }); + describe('Provider (multipleInstances = false)', () => { describe('getImmediate()', () => { it('throws if the service is not available', () => { @@ -155,7 +183,7 @@ describe('Provider', () => { }); }); - describe('provideFactory()', () => { + describe('setComponent()', () => { it('instantiates the service if there is a pending promise and the service is eager', () => { // create a pending promise // eslint-disable-next-line @typescript-eslint/no-floating-promises @@ -320,7 +348,7 @@ describe('Provider', () => { }); }); - describe('provideFactory()', () => { + describe('setComponent()', () => { it('instantiates services for the pending promises for all instance identifiers', async () => { /* eslint-disable @typescript-eslint/no-floating-promises */ // create 3 promises for 3 different identifiers diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index b5069dadd07..43a32746233 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -18,7 +18,12 @@ import { Deferred } from '@firebase/util'; import { ComponentContainer } from './component_container'; import { DEFAULT_ENTRY_NAME } from './constants'; -import { InstantiationMode, Name, NameServiceMapping } from './types'; +import { + InitializeOptions, + InstantiationMode, + Name, + NameServiceMapping +} from './types'; import { Component } from './component'; /** @@ -51,7 +56,9 @@ export class Provider { this.instancesDeferred.set(normalizedIdentifier, deferred); // If the service instance is available, resolve the promise with it immediately try { - const instance = this.getOrInitializeService(normalizedIdentifier); + const instance = this.getOrInitializeService({ + instanceIdentifier: normalizedIdentifier + }); if (instance) { deferred.resolve(instance); } @@ -92,7 +99,9 @@ export class Provider { // if multipleInstances is not supported, use the default name const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); try { - const instance = this.getOrInitializeService(normalizedIdentifier); + const instance = this.getOrInitializeService({ + instanceIdentifier: normalizedIdentifier + }); if (!instance) { if (optional) { @@ -129,7 +138,7 @@ export class Provider { // if the service is eager, initialize the default instance if (isComponentEager(component)) { try { - this.getOrInitializeService(DEFAULT_ENTRY_NAME); + this.getOrInitializeService({ instanceIdentifier: DEFAULT_ENTRY_NAME }); } catch (e) { // when the instance factory for an eager Component throws an exception during the eager // initialization, it should not cause a fatal error. @@ -151,7 +160,9 @@ export class Provider { try { // `getOrInitializeService()` should always return a valid instance since a component is guaranteed. use ! to make typescript happy. - const instance = this.getOrInitializeService(normalizedIdentifier)!; + const instance = this.getOrInitializeService({ + instanceIdentifier: normalizedIdentifier + })!; instanceDeferred.resolve(instance); } catch (e) { // when the instance factory throws an exception, it should not cause @@ -190,16 +201,41 @@ export class Provider { return this.instances.has(identifier); } - private getOrInitializeService( - identifier: string - ): NameServiceMapping[T] | null { - let instance = this.instances.get(identifier); + initialize(opts: InitializeOptions = {}): NameServiceMapping[T] { + const { instanceIdentifier = DEFAULT_ENTRY_NAME, options = {} } = opts; + const normalizedIdentifier = this.normalizeInstanceIdentifier( + instanceIdentifier + ); + if (this.isInitialized(normalizedIdentifier)) { + throw Error( + `${this.name}(${normalizedIdentifier}) has already been initialized` + ); + } + + if (!this.isComponentSet()) { + throw Error(`Component ${this.name} has not been registered yet`); + } + + return this.getOrInitializeService({ + instanceIdentifier: normalizedIdentifier, + options + })!; + } + + private getOrInitializeService({ + instanceIdentifier, + options = {} + }: { + instanceIdentifier: string; + options?: Record; + }): NameServiceMapping[T] | null { + let instance = this.instances.get(instanceIdentifier); if (!instance && this.component) { - instance = this.component.instanceFactory( - this.container, - normalizeIdentifierForFactory(identifier) - ) as NameServiceMapping[T]; - this.instances.set(identifier, instance); + instance = this.component.instanceFactory(this.container, { + instanceIdentifier: normalizeIdentifierForFactory(instanceIdentifier), + options + }); + this.instances.set(instanceIdentifier, instance); } return instance || null; diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 5bea4341ccc..d0a94349d30 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -36,6 +36,13 @@ export const enum ComponentType { VERSION = 'VERSION' } +export interface InstanceFactoryOptions { + instanceIdentifier?: string; + options?: {}; +} + +export type InitializeOptions = InstanceFactoryOptions; + /** * Factory to create an instance of type T, given a ComponentContainer. * ComponentContainer is the IOC container that provides {@link Provider} @@ -46,7 +53,7 @@ export const enum ComponentType { */ export type InstanceFactory = ( container: ComponentContainer, - instanceIdentifier?: string + options: InstanceFactoryOptions ) => NameServiceMapping[T]; export interface Dictionary { diff --git a/packages/database/exp/index.ts b/packages/database/exp/index.ts index 49084283906..7feae5b767a 100644 --- a/packages/database/exp/index.ts +++ b/packages/database/exp/index.ts @@ -35,7 +35,7 @@ function registerDatabase(): void { _registerComponent( new Component( 'database-exp', - (container, url) => { + (container, { instanceIdentifier: url }) => { const app = container.getProvider('app-exp').getImmediate()!; const authProvider = container.getProvider('auth-internal'); return new FirebaseDatabase(app, authProvider, url); diff --git a/packages/database/index.node.ts b/packages/database/index.node.ts index ae981426e37..3f65d93e82f 100644 --- a/packages/database/index.node.ts +++ b/packages/database/index.node.ts @@ -84,7 +84,7 @@ export function registerDatabase(instance: FirebaseNamespace) { const namespace = (instance as _FirebaseNamespace).INTERNAL.registerComponent( new Component( 'database', - (container, url) => { + (container, { instanceIdentifier: url }) => { /* Dependencies */ // getImmediate for FirebaseApp will always succeed const app = container.getProvider('app').getImmediate(); diff --git a/packages/database/index.ts b/packages/database/index.ts index 4911de67194..7a2a63c565f 100644 --- a/packages/database/index.ts +++ b/packages/database/index.ts @@ -44,7 +44,7 @@ export function registerDatabase(instance: FirebaseNamespace) { const namespace = (instance as _FirebaseNamespace).INTERNAL.registerComponent( new Component( 'database', - (container, url) => { + (container, { instanceIdentifier: url }) => { /* Dependencies */ // getImmediate for FirebaseApp will always succeed const app = container.getProvider('app').getImmediate(); diff --git a/packages/firestore/exp/register.ts b/packages/firestore/exp/register.ts index 005b9c1b745..22e7093b267 100644 --- a/packages/firestore/exp/register.ts +++ b/packages/firestore/exp/register.ts @@ -20,6 +20,7 @@ import { Component, ComponentType } from '@firebase/component'; import { version } from '../package.json'; import { FirebaseFirestore } from '../src/exp/database'; +import { Settings } from '../src/exp/settings'; declare module '@firebase/component' { interface NameServiceMapping { @@ -31,12 +32,16 @@ export function registerFirestore(): void { _registerComponent( new Component( 'firestore-exp', - container => { + (container, { options: settings }: { options?: Settings }) => { const app = container.getProvider('app-exp').getImmediate()!; - return ((app, auth) => new FirebaseFirestore(app, auth))( + const firestoreInstance = new FirebaseFirestore( app, container.getProvider('auth-internal') ); + if (settings) { + firestoreInstance._setSettings(settings); + } + return firestoreInstance; }, ComponentType.PUBLIC ) diff --git a/packages/firestore/lite/register.ts b/packages/firestore/lite/register.ts index 2fa6f9c6a64..cb17b6a25fe 100644 --- a/packages/firestore/lite/register.ts +++ b/packages/firestore/lite/register.ts @@ -20,6 +20,7 @@ import { Component, ComponentType } from '@firebase/component'; import { version } from '../package.json'; import { FirebaseFirestore } from '../src/lite/database'; +import { Settings } from '../src/lite/settings'; declare module '@firebase/component' { interface NameServiceMapping { @@ -31,12 +32,16 @@ export function registerFirestore(): void { _registerComponent( new Component( 'firestore/lite', - container => { + (container, { options: settings }: { options?: Settings }) => { const app = container.getProvider('app-exp').getImmediate()!; - return ((app, auth) => new FirebaseFirestore(app, auth))( + const firestoreInstance = new FirebaseFirestore( app, container.getProvider('auth-internal') ); + if (settings) { + firestoreInstance._setSettings(settings); + } + return firestoreInstance; }, ComponentType.PUBLIC ) diff --git a/packages/firestore/src/exp/database.ts b/packages/firestore/src/exp/database.ts index 00b4c54865a..a100c529e39 100644 --- a/packages/firestore/src/exp/database.ts +++ b/packages/firestore/src/exp/database.ts @@ -128,7 +128,6 @@ export function initializeFirestore( ); } - const firestore = provider.getImmediate() as FirebaseFirestore; if ( settings.cacheSizeBytes !== undefined && settings.cacheSizeBytes !== CACHE_SIZE_UNLIMITED && @@ -140,8 +139,7 @@ export function initializeFirestore( ); } - firestore._setSettings(settings); - return firestore; + return provider.initialize({ options: settings }); } /** diff --git a/packages/firestore/src/lite/database.ts b/packages/firestore/src/lite/database.ts index f926ae89b35..0ee13cd3ea4 100644 --- a/packages/firestore/src/lite/database.ts +++ b/packages/firestore/src/lite/database.ts @@ -193,9 +193,7 @@ export function initializeFirestore( ); } - const firestore = provider.getImmediate() as FirebaseFirestore; - firestore._setSettings(settings); - return firestore; + return provider.initialize({ options: settings }); } /** diff --git a/packages/functions/src/config.ts b/packages/functions/src/config.ts index df833a072ba..943630f8640 100644 --- a/packages/functions/src/config.ts +++ b/packages/functions/src/config.ts @@ -19,7 +19,8 @@ import { Service } from './api/service'; import { Component, ComponentType, - ComponentContainer + ComponentContainer, + InstanceFactoryOptions } from '@firebase/component'; import { _FirebaseNamespace } from '@firebase/app-types/private'; @@ -39,7 +40,7 @@ export function registerFunctions( function factory( container: ComponentContainer, - regionOrCustomDomain?: string + { instanceIdentifier: regionOrCustomDomain }: InstanceFactoryOptions ): Service { // Dependencies const app = container.getProvider('app').getImmediate(); diff --git a/packages/remote-config/index.ts b/packages/remote-config/index.ts index 54f38f5325d..ba4ae9eb034 100644 --- a/packages/remote-config/index.ts +++ b/packages/remote-config/index.ts @@ -31,7 +31,8 @@ import { name as packageName, version } from './package.json'; import { Component, ComponentType, - ComponentContainer + ComponentContainer, + InstanceFactoryOptions } from '@firebase/component'; // Facilitates debugging by enabling settings changes without rebuilding asset. @@ -59,7 +60,7 @@ export function registerRemoteConfig( function remoteConfigFactory( container: ComponentContainer, - namespace?: string + { instanceIdentifier: namespace }: InstanceFactoryOptions ): RemoteConfig { /* Dependencies */ // getImmediate for FirebaseApp will always succeed diff --git a/packages/storage/compat/index.ts b/packages/storage/compat/index.ts index 8712162ffee..2b177bf7f92 100644 --- a/packages/storage/compat/index.ts +++ b/packages/storage/compat/index.ts @@ -28,7 +28,8 @@ import * as types from '@firebase/storage-types'; import { Component, ComponentType, - ComponentContainer + ComponentContainer, + InstanceFactoryOptions } from '@firebase/component'; import { name, version } from '../package.json'; @@ -40,7 +41,7 @@ const STORAGE_TYPE = 'storage'; function factory( container: ComponentContainer, - url?: string + { instanceIdentifier: url }: InstanceFactoryOptions ): types.FirebaseStorage { // Dependencies // TODO: This should eventually be 'app-compat' diff --git a/packages/storage/exp/index.ts b/packages/storage/exp/index.ts index d1dbc11407d..a78b4b969c6 100644 --- a/packages/storage/exp/index.ts +++ b/packages/storage/exp/index.ts @@ -33,7 +33,8 @@ import { Component, ComponentType, ComponentContainer, - Provider + Provider, + InstanceFactoryOptions } from '@firebase/component'; import { name, version } from '../package.json'; @@ -295,7 +296,10 @@ export function getStorage( return storageInstance; } -function factory(container: ComponentContainer, url?: string): StorageService { +function factory( + container: ComponentContainer, + { instanceIdentifier: url }: InstanceFactoryOptions +): StorageService { const app = container.getProvider('app-exp').getImmediate(); const authProvider = container.getProvider('auth-internal');