From 14332f07caec94c30c2ab56d85568e95f6611abe Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Thu, 15 Aug 2019 16:41:32 -0700 Subject: [PATCH 01/54] Component implementation --- packages/app-types/private.d.ts | 29 +- packages/app/index.node.ts | 6 +- packages/app/index.rn.ts | 6 +- packages/app/index.ts | 3 +- packages/app/karma.conf.js | 3 +- packages/app/src/firebaseApp.ts | 128 ++----- packages/app/src/firebaseNamespace.ts | 4 +- packages/app/src/firebaseNamespaceCore.ts | 141 +++----- packages/app/src/lite/firebaseAppLite.ts | 58 +--- .../app/src/lite/firebaseNamespaceLite.ts | 33 +- packages/app/test/setup.ts | 28 ++ packages/app/tsconfig.json | 3 +- packages/component/.eslintrc.json | 6 + packages/component/.npmignore | 1 + packages/component/README.md | 13 + packages/component/index.ts | 19 + packages/component/karma.conf.js | 37 ++ packages/component/package.json | 75 ++++ packages/component/rollup.config.js | 80 +++++ packages/component/src/component.ts | 62 ++++ packages/component/src/component_container.ts | 72 ++++ .../src/component_containers.test.ts | 94 +++++ packages/component/src/contants.ts | 1 + packages/component/src/provider.test.ts | 326 ++++++++++++++++++ packages/component/src/provider.ts | 147 ++++++++ packages/component/src/types.ts | 19 + packages/component/test/setup.ts | 28 ++ packages/component/test/util.ts | 35 ++ packages/component/tsconfig.json | 10 + 29 files changed, 1185 insertions(+), 282 deletions(-) create mode 100644 packages/app/test/setup.ts create mode 100644 packages/component/.eslintrc.json create mode 100644 packages/component/.npmignore create mode 100644 packages/component/README.md create mode 100644 packages/component/index.ts create mode 100644 packages/component/karma.conf.js create mode 100644 packages/component/package.json create mode 100644 packages/component/rollup.config.js create mode 100644 packages/component/src/component.ts create mode 100644 packages/component/src/component_container.ts create mode 100644 packages/component/src/component_containers.test.ts create mode 100644 packages/component/src/contants.ts create mode 100644 packages/component/src/provider.test.ts create mode 100644 packages/component/src/provider.ts create mode 100644 packages/component/src/types.ts create mode 100644 packages/component/test/setup.ts create mode 100644 packages/component/test/util.ts create mode 100644 packages/component/tsconfig.json diff --git a/packages/app-types/private.d.ts b/packages/app-types/private.d.ts index 045a8734456..b1e79bf8fbc 100644 --- a/packages/app-types/private.d.ts +++ b/packages/app-types/private.d.ts @@ -23,6 +23,17 @@ import { FirebaseApp, FirebaseNamespace } from '@firebase/app-types'; import { Observer, Subscribe } from '@firebase/util'; import { FirebaseError, ErrorFactory } from '@firebase/util'; +import { Deferred } from '../firestore/test/util/promise'; +import { Component } from '@firebase/component'; + +/** + * Factory responsible for creating a component of type T, given a ComponentContainer. + * ComponentContainer is the IOC container that provides PROVIDERS + * for dependencies. + * + * NOTE: The container only provides PROVIDERS instead of the actual instances of dependencies + * to account for lazily loaded or optional dependencies + */ export interface FirebaseServiceInternals { /** @@ -80,8 +91,8 @@ export interface FirebaseAppInternals { } export interface _FirebaseApp extends FirebaseApp { - INTERNAL: FirebaseAppInternals; - _removeServiceInstance: (name: string, instanceIdentifier?: string) => void; + _addComponent(component: Component): void; + _removeServiceInstance(name: string, instanceIdentifier?: string): void; } export interface _FirebaseNamespace extends FirebaseNamespace { INTERNAL: { @@ -99,13 +110,9 @@ export interface _FirebaseNamespace extends FirebaseNamespace { * @param allowMultipleInstances Whether the registered service supports * multiple instances per app. If not specified, the default is false. */ - registerService( - name: string, - createService: FirebaseServiceFactory, - serviceProperties?: { [prop: string]: any }, - appHook?: AppHook, - allowMultipleInstances?: boolean - ): FirebaseServiceNamespace; + registerComponent( + component: Component + ): FirebaseServiceNamespace | null; /** * Just used for testing to start from a fresh namespace. @@ -139,9 +146,9 @@ export interface _FirebaseNamespace extends FirebaseNamespace { removeApp(name: string): void; /** - * Service factories for each registered service. + * registered components. */ - factories: { [name: string]: FirebaseServiceFactory }; + components: Map; /* * Convert service name to factory name to use. diff --git a/packages/app/index.node.ts b/packages/app/index.node.ts index 12a396fa1f1..f7e81b690e5 100644 --- a/packages/app/index.node.ts +++ b/packages/app/index.node.ts @@ -17,16 +17,14 @@ import { FirebaseNamespace } from '@firebase/app-types'; import { _FirebaseNamespace } from '@firebase/app-types/private'; -import { createFirebaseNamespace } from './src/firebaseNamespace'; +import { firebase as _firebase } from './src/firebaseNamespace'; // Node specific packages. // @ts-ignore import Storage from 'dom-storage'; // @ts-ignore import { XMLHttpRequest } from 'xmlhttprequest'; -const _firebase = createFirebaseNamespace() as _FirebaseNamespace; - -_firebase.INTERNAL.extendNamespace({ +(_firebase as _FirebaseNamespace).INTERNAL.extendNamespace({ INTERNAL: { node: { localStorage: new Storage(null, { strict: true }), diff --git a/packages/app/index.rn.ts b/packages/app/index.rn.ts index 2669b4a7ac8..a3cad68f513 100644 --- a/packages/app/index.rn.ts +++ b/packages/app/index.rn.ts @@ -17,7 +17,7 @@ import { FirebaseNamespace } from '@firebase/app-types'; import { _FirebaseNamespace } from '@firebase/app-types/private'; -import { createFirebaseNamespace } from './src/firebaseNamespace'; +import { firebase as _firebase } from './src/firebaseNamespace'; /** * To avoid having to include the @types/react-native package, which breaks @@ -27,9 +27,7 @@ import { createFirebaseNamespace } from './src/firebaseNamespace'; // eslint-disable-next-line @typescript-eslint/no-require-imports const { AsyncStorage } = require('react-native'); -const _firebase = createFirebaseNamespace() as _FirebaseNamespace; - -_firebase.INTERNAL.extendNamespace({ +(_firebase as _FirebaseNamespace).INTERNAL.extendNamespace({ INTERNAL: { reactNative: { AsyncStorage diff --git a/packages/app/index.ts b/packages/app/index.ts index 350112c536c..2170cce61a0 100644 --- a/packages/app/index.ts +++ b/packages/app/index.ts @@ -16,7 +16,7 @@ */ import { FirebaseNamespace } from '@firebase/app-types'; -import { createFirebaseNamespace } from './src/firebaseNamespace'; +import { firebase as firebaseNamespace } from './src/firebaseNamespace'; import { isNode, isBrowser } from '@firebase/util'; import { logger } from './src/logger'; @@ -38,7 +38,6 @@ if (isBrowser() && (self as any).firebase !== undefined) { } } -const firebaseNamespace = createFirebaseNamespace(); const initializeApp = firebaseNamespace.initializeApp; // TODO: This disable can be removed and the 'ignoreRestArgs' option added to diff --git a/packages/app/karma.conf.js b/packages/app/karma.conf.js index c422d2666ef..16c3a0d9bc3 100644 --- a/packages/app/karma.conf.js +++ b/packages/app/karma.conf.js @@ -19,7 +19,7 @@ const karma = require('karma'); const path = require('path'); const karmaBase = require('../../config/karma.base'); -const files = [`test/**/*`]; +const files = ['test/**/*', 'src/**/*.test.ts']; module.exports = function(config) { const karmaConfig = Object.assign({}, karmaBase, { @@ -27,6 +27,7 @@ module.exports = function(config) { files: files, // frameworks to use // available frameworks: https://npmjs.org/browse/keyword/karma-adapter + preprocessors: { '**/*.ts': ['webpack', 'sourcemap'] }, frameworks: ['mocha'] }); diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index 3613315dcde..5a755c62b70 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -23,19 +23,13 @@ import { import { _FirebaseApp, _FirebaseNamespace, - FirebaseService, - FirebaseAppInternals + FirebaseService } from '@firebase/app-types/private'; -import { deepCopy, deepExtend } from '@firebase/util'; +import { deepCopy } from '@firebase/util'; +import { ComponentContainer, Component } from '@firebase/component'; import { AppError, ERROR_FACTORY } from './errors'; import { DEFAULT_ENTRY_NAME } from './constants'; -interface ServicesCache { - [name: string]: { - [serviceName: string]: FirebaseService; - }; -} - /** * Global context object for a collection of services using * a shared authentication state. @@ -44,15 +38,8 @@ export class FirebaseAppImpl implements FirebaseApp { private readonly options_: FirebaseOptions; private readonly name_: string; private isDeleted_ = false; - private services_: ServicesCache = {}; private automaticDataCollectionEnabled_: boolean; - // An array to capture listeners before the true auth functions exist - private tokenListeners_: Array<(token: string | null) => void> = []; - // An array to capture requests to send events before analytics component loads. Use type any to make using function.apply easier - // eslint-disable-next-line @typescript-eslint/no-explicit-any - private analyticsEventRequests_: any[] = []; - - INTERNAL: FirebaseAppInternals; + private container: ComponentContainer; constructor( options: FirebaseOptions, @@ -63,26 +50,14 @@ export class FirebaseAppImpl implements FirebaseApp { this.automaticDataCollectionEnabled_ = config.automaticDataCollectionEnabled || false; this.options_ = deepCopy(options); - const self = this; - this.INTERNAL = { - getUid: () => null, - getToken: () => Promise.resolve(null), - addAuthTokenListener: (callback: (token: string | null) => void) => { - this.tokenListeners_.push(callback); - // Make sure callback is called, asynchronously, in the absence of the auth module - setTimeout(() => callback(null), 0); - }, - removeAuthTokenListener: callback => { - this.tokenListeners_ = this.tokenListeners_.filter( - listener => listener !== callback - ); - }, - analytics: { - logEvent() { - self.analyticsEventRequests_.push(arguments); - } - } - }; + this.container = new ComponentContainer(config.name!); + + // add itself to container + this.container.addComponent(new Component('app', () => this)); + // populate ComponentContainer with existing components + for (const component of this.firebase_.INTERNAL.components.values()) { + this.container.addComponent(component); + } } get automaticDataCollectionEnabled(): boolean { @@ -112,23 +87,13 @@ export class FirebaseAppImpl implements FirebaseApp { }) .then(() => { this.firebase_.INTERNAL.removeApp(this.name_); - const services: FirebaseService[] = []; - - for (const serviceKey of Object.keys(this.services_)) { - for (const instanceKey of Object.keys(this.services_[serviceKey])) { - services.push(this.services_[serviceKey][instanceKey]); - } - } return Promise.all( - services - .filter(service => 'INTERNAL' in service) - .map(service => service.INTERNAL!.delete()) + this.container.getProviders().map(provider => provider.delete()) ); }) .then((): void => { this.isDeleted_ = true; - this.services_ = {}; }); } @@ -152,28 +117,10 @@ export class FirebaseAppImpl implements FirebaseApp { ): FirebaseService { this.checkDestroyed_(); - if (!this.services_[name]) { - this.services_[name] = {}; - } - - if (!this.services_[name][instanceIdentifier]) { - /** - * If a custom instance has been defined (i.e. not '[DEFAULT]') - * then we will pass that instance on, otherwise we pass `null` - */ - const instanceSpecifier = - instanceIdentifier !== DEFAULT_ENTRY_NAME - ? instanceIdentifier - : undefined; - const service = this.firebase_.INTERNAL.factories[name]( - this, - this.extendApp.bind(this), - instanceSpecifier - ); - this.services_[name][instanceIdentifier] = service; - } - - return this.services_[name][instanceIdentifier]; + // getImmediate will always succeed because _getService is only called for registered components. + return this.container + .getProvider(name) + .getImmediate(instanceIdentifier) as FirebaseService; } /** * Remove a service instance from the cache, so we will create a new instance for this service @@ -189,46 +136,11 @@ export class FirebaseAppImpl implements FirebaseApp { name: string, instanceIdentifier: string = DEFAULT_ENTRY_NAME ): void { - if (this.services_[name] && this.services_[name][instanceIdentifier]) { - delete this.services_[name][instanceIdentifier]; - } + this.container.getProvider(name).clearCache(instanceIdentifier); } - /** - * Callback function used to extend an App instance at the time - * of service instance creation. - */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - private extendApp(props: { [name: string]: any }): void { - // Copy the object onto the FirebaseAppImpl prototype - deepExtend(this, props); - - if (props.INTERNAL) { - /** - * If the app has overwritten the addAuthTokenListener stub, forward - * the active token listeners on to the true fxn. - * - * TODO: This function is required due to our current module - * structure. Once we are able to rely strictly upon a single module - * implementation, this code should be refactored and Auth should - * provide these stubs and the upgrade logic - */ - if (props.INTERNAL.addAuthTokenListener) { - for (const listener of this.tokenListeners_) { - this.INTERNAL.addAuthTokenListener(listener); - } - this.tokenListeners_ = []; - } - - if (props.INTERNAL.analytics) { - for (const request of this.analyticsEventRequests_) { - // logEvent is the actual implementation at this point. - // We forward the queued events to it. - this.INTERNAL.analytics.logEvent.apply(undefined, request); - } - this.analyticsEventRequests_ = []; - } - } + _addComponent(component: Component): void { + this.container.addComponent(component); } /** diff --git a/packages/app/src/firebaseNamespace.ts b/packages/app/src/firebaseNamespace.ts index 78ad157b1da..65c1056bfd5 100644 --- a/packages/app/src/firebaseNamespace.ts +++ b/packages/app/src/firebaseNamespace.ts @@ -28,7 +28,7 @@ import { createFirebaseNamespaceCore } from './firebaseNamespaceCore'; * assigned to the 'firebase' global. It may be called multiple times * in unit tests. */ -export function createFirebaseNamespace(): FirebaseNamespace { +function createFirebaseNamespace(): FirebaseNamespace { const namespace = createFirebaseNamespaceCore(FirebaseAppImpl); (namespace as _FirebaseNamespace).INTERNAL = { ...(namespace as _FirebaseNamespace).INTERNAL, @@ -50,3 +50,5 @@ export function createFirebaseNamespace(): FirebaseNamespace { return namespace; } + +export const firebase = createFirebaseNamespace(); diff --git a/packages/app/src/firebaseNamespaceCore.ts b/packages/app/src/firebaseNamespaceCore.ts index 924f76288b8..57a3ceb694d 100644 --- a/packages/app/src/firebaseNamespaceCore.ts +++ b/packages/app/src/firebaseNamespaceCore.ts @@ -25,9 +25,7 @@ import { _FirebaseApp, _FirebaseNamespace, FirebaseService, - FirebaseServiceFactory, - FirebaseServiceNamespace, - AppHook + FirebaseServiceNamespace } from '@firebase/app-types/private'; import { deepExtend, contains } from '@firebase/util'; import { FirebaseAppImpl } from './firebaseApp'; @@ -36,6 +34,7 @@ import { FirebaseAppLiteImpl } from './lite/firebaseAppLite'; import { DEFAULT_ENTRY_NAME } from './constants'; import { version } from '../../firebase/package.json'; import { logger } from './logger'; +import { Component, ComponentType } from '@firebase/component'; /** * Because auth can't share code with other components, we attach the utility functions @@ -48,8 +47,8 @@ export function createFirebaseNamespaceCore( firebaseAppImpl: typeof FirebaseAppImpl | typeof FirebaseAppLiteImpl ): FirebaseNamespace { const apps: { [name: string]: FirebaseApp } = {}; - const factories: { [service: string]: FirebaseServiceFactory } = {}; - const appHooks: { [service: string]: AppHook } = {}; + // const factories: { [service: string]: FirebaseServiceFactory } = {}; + const components = new Map(); // A namespace is a plain JavaScript Object. const namespace: FirebaseNamespace = { @@ -64,9 +63,9 @@ export function createFirebaseNamespaceCore( apps: null, SDK_VERSION: version, INTERNAL: { - registerService, + registerComponent, removeApp, - factories, + components, useAsService } }; @@ -94,8 +93,6 @@ export function createFirebaseNamespaceCore( * are deleted. */ function removeApp(name: string): void { - const app = apps[name]; - callAppHooks(app, 'delete'); delete apps[name]; } @@ -154,7 +151,6 @@ export function createFirebaseNamespaceCore( ); apps[name] = app; - callAppHooks(app, 'create'); return app; } @@ -167,93 +163,64 @@ export function createFirebaseNamespaceCore( return Object.keys(apps).map(name => apps[name]); } - /* - * Register a Firebase Service. - * - * firebase.INTERNAL.registerService() - * - * TODO: Implement serviceProperties. - */ - function registerService( - name: string, - createService: FirebaseServiceFactory, - serviceProperties?: { [prop: string]: unknown }, - appHook?: AppHook, - allowMultipleInstances = false - ): FirebaseServiceNamespace { - // If re-registering a service that already exists, return existing service - if (factories[name]) { - logger.debug(`There were multiple attempts to register service ${name}.`); + function registerComponent( + component: Component + ): FirebaseServiceNamespace | null { + const componentName = component.name; + if (components.has(componentName)) { + console.debug( + `There were multiple attempts to register component ${componentName}.` + ); // eslint-disable-next-line @typescript-eslint/no-explicit-any - return (namespace as any)[name] as FirebaseServiceNamespace< - FirebaseService - >; + return component.type === ComponentType.PUBLIC ? (namespace as any)[componentName] : null; } - // Capture the service factory for later service instantiation - factories[name] = createService; - - // Capture the appHook, if passed - if (appHook) { - appHooks[name] = appHook; - - // Run the **new** app hook on all existing apps - getApps().forEach(app => { - appHook('create', app); - }); - } - - // The Service namespace is an accessor function ... - function serviceNamespace(appArg: FirebaseApp = app()): FirebaseService { - // @ts-ignore - if (typeof appArg[name] !== 'function') { - // Invalid argument. - // This happens in the following case: firebase.storage('gs:/') - throw ERROR_FACTORY.create(AppError.INVALID_APP_ARGUMENT, { - appName: name - }); + components.set(componentName, component); + + // create service namespace for public components + if (component.type === ComponentType.PUBLIC) { + // The Service namespace is an accessor function ... + const serviceNamespace = (appArg: FirebaseApp = app()): FirebaseService => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + if (typeof (appArg as any)[componentName] !== 'function') { + // Invalid argument. + // This happens in the following case: firebase.storage('gs:/') + throw ERROR_FACTORY.create(AppError.INVALID_APP_ARGUMENT, { + appName: componentName + }); + } + + // Forward service instance lookup to the FirebaseApp. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (appArg as any)[componentName](); } - // Forward service instance lookup to the FirebaseApp. - // @ts-ignore - return appArg[name](); - } - - // ... and a container for service-level properties. - if (serviceProperties !== undefined) { - deepExtend(serviceNamespace, serviceProperties); - } - - // Monkey-patch the serviceNamespace onto the firebase namespace - // @ts-ignore - namespace[name] = serviceNamespace; + // ... and a container for service-level properties. + if (component.serviceProps !== undefined) { + deepExtend(serviceNamespace, component.serviceProps); + } - // Patch the FirebaseAppImpl prototype - // @ts-ignore - firebaseAppImpl.prototype[name] = - // TODO: The eslint disable can be removed and the 'ignoreRestArgs' - // option added to the no-explicit-any rule when ESlint releases it. // eslint-disable-next-line @typescript-eslint/no-explicit-any - function(...args: any) { - const serviceFxn = this._getService.bind(this, name); - return serviceFxn.apply(this, allowMultipleInstances ? args : []); - }; - - return serviceNamespace; - } + (namespace as any)[componentName] = serviceNamespace; - function callAppHooks(app: FirebaseApp, eventName: string): void { - for (const serviceName of Object.keys(factories)) { - // Ignore virtual services - const factoryName = useAsService(app, serviceName); - if (factoryName === null) { - return; - } + // Patch the FirebaseAppImpl prototype + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (firebaseAppImpl.prototype as any)[componentName] = + // TODO: The eslint disable can be removed and the 'ignoreRestArgs' + // option added to the no-explicit-any rule when ESlint releases it. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + function (...args: any) { + const serviceFxn = this._getService.bind(this, componentName); + return serviceFxn.apply(this, component.multipleInstances ? args : []); + }; + } - if (appHooks[factoryName]) { - appHooks[factoryName](eventName, app); - } + // add the component to existing app instances + for (const appName of Object.keys(apps)) { + (apps[appName] as _FirebaseApp)._addComponent(component); } + + return component.type === ComponentType.PUBLIC ? (namespace as any)[componentName] : null; } // Map the requested service to a registered service name diff --git a/packages/app/src/lite/firebaseAppLite.ts b/packages/app/src/lite/firebaseAppLite.ts index 8e9349cbcd3..ef98dd35481 100644 --- a/packages/app/src/lite/firebaseAppLite.ts +++ b/packages/app/src/lite/firebaseAppLite.ts @@ -28,6 +28,7 @@ import { import { deepCopy, deepExtend } from '@firebase/util'; import { ERROR_FACTORY, AppError } from '../errors'; import { DEFAULT_ENTRY_NAME } from '../constants'; +import { ComponentContainer, Component } from '@firebase/component'; interface ServicesCache { [name: string]: { @@ -43,8 +44,8 @@ export class FirebaseAppLiteImpl implements FirebaseApp { private readonly options_: FirebaseOptions; private readonly name_: string; private isDeleted_ = false; - private services_: ServicesCache = {}; private automaticDataCollectionEnabled_: boolean; + private container: ComponentContainer; // lite version has an empty INTERNAL namespace readonly INTERNAL = {}; @@ -58,6 +59,14 @@ export class FirebaseAppLiteImpl implements FirebaseApp { this.automaticDataCollectionEnabled_ = config.automaticDataCollectionEnabled || false; this.options_ = deepCopy(options); + this.container = new ComponentContainer(config.name!); + + // add itself to container + this.container.addComponent(new Component('app', () => this)); + // populate ComponentContainer with existing components + for (const component of this.firebase_.INTERNAL.components.values()) { + this.container.addComponent(component); + } } get automaticDataCollectionEnabled(): boolean { @@ -87,23 +96,13 @@ export class FirebaseAppLiteImpl implements FirebaseApp { }) .then(() => { this.firebase_.INTERNAL.removeApp(this.name_); - const services: FirebaseService[] = []; - - for (const serviceKey of Object.keys(this.services_)) { - for (const instanceKey of Object.keys(this.services_[serviceKey])) { - services.push(this.services_[serviceKey][instanceKey]); - } - } return Promise.all( - services - .filter(service => 'INTERNAL' in service) - .map(service => service.INTERNAL!.delete()) + this.container.getProviders().map(provider => provider.delete()) ); }) .then((): void => { this.isDeleted_ = true; - this.services_ = {}; }); } @@ -127,37 +126,10 @@ export class FirebaseAppLiteImpl implements FirebaseApp { ): FirebaseService { this.checkDestroyed_(); - if (!this.services_[name]) { - this.services_[name] = {}; - } - - if (!this.services_[name][instanceIdentifier]) { - /** - * If a custom instance has been defined (i.e. not '[DEFAULT]') - * then we will pass that instance on, otherwise we pass `null` - */ - const instanceSpecifier = - instanceIdentifier !== DEFAULT_ENTRY_NAME - ? instanceIdentifier - : undefined; - const service = this.firebase_.INTERNAL.factories[name]( - this, - this.extendApp.bind(this), - instanceSpecifier - ); - this.services_[name][instanceIdentifier] = service; - } - - return this.services_[name][instanceIdentifier]; - } - - /** - * Callback function used to extend an App instance at the time - * of service instance creation. - */ - private extendApp(props: { [name: string]: unknown }): void { - // Copy the object onto the FirebaseAppImpl prototype - deepExtend(this, props); + // getImmediate will always succeed because _getService is only called for registered components. + return this.container + .getProvider(name) + .getImmediate(instanceIdentifier) as FirebaseService; } /** diff --git a/packages/app/src/lite/firebaseNamespaceLite.ts b/packages/app/src/lite/firebaseNamespaceLite.ts index 1083db4c254..55e4c274bf0 100644 --- a/packages/app/src/lite/firebaseNamespaceLite.ts +++ b/packages/app/src/lite/firebaseNamespaceLite.ts @@ -19,46 +19,39 @@ import { FirebaseNamespace } from '@firebase/app-types'; import { _FirebaseApp, _FirebaseNamespace, - FirebaseServiceFactory, - AppHook, FirebaseServiceNamespace, FirebaseService } from '@firebase/app-types/private'; import { FirebaseAppLiteImpl } from './firebaseAppLite'; import { createFirebaseNamespaceCore } from '../firebaseNamespaceCore'; +import { Component, ComponentType } from '@firebase/component'; export function createFirebaseNamespaceLite(): FirebaseNamespace { const namespace = createFirebaseNamespaceCore(FirebaseAppLiteImpl); namespace.SDK_VERSION = `${namespace.SDK_VERSION}_LITE`; - const registerService = (namespace as _FirebaseNamespace).INTERNAL - .registerService; - (namespace as _FirebaseNamespace).INTERNAL.registerService = registerServiceForLite; + const registerComponent = (namespace as _FirebaseNamespace).INTERNAL + .registerComponent; + (namespace as _FirebaseNamespace).INTERNAL.registerComponent = registerComponentForLite; /** * This is a special implementation, so it only works with performance. * only allow performance SDK to register. */ - function registerServiceForLite( - name: string, - createService: FirebaseServiceFactory, - serviceProperties?: { [prop: string]: unknown }, - appHook?: AppHook, - allowMultipleInstances?: boolean - ): FirebaseServiceNamespace { + function registerComponentForLite( + component: Component + ): FirebaseServiceNamespace | null { // only allow performance to register with firebase lite - if (name !== 'performance' && name !== 'installations') { + if ( + component.type === ComponentType.PUBLIC + && component.name !== 'performance' + && component.name !== 'installations' + ) { throw Error(`${name} cannot register with the standalone perf instance`); } - return registerService( - name, - createService, - serviceProperties, - appHook, - allowMultipleInstances - ); + return registerComponent(component); } return namespace; diff --git a/packages/app/test/setup.ts b/packages/app/test/setup.ts new file mode 100644 index 00000000000..5f3858ba898 --- /dev/null +++ b/packages/app/test/setup.ts @@ -0,0 +1,28 @@ +/** + * @license + * Copyright 2019 Google Inc. + * + * 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 { use } from 'chai'; +import * as chaiAsPromised from 'chai-as-promised'; +import { restore } from 'sinon'; +import * as sinonChai from 'sinon-chai'; + +use(chaiAsPromised); +use(sinonChai); + +afterEach(async () => { + restore(); +}); diff --git a/packages/app/tsconfig.json b/packages/app/tsconfig.json index f2942111423..72e0736c2b7 100644 --- a/packages/app/tsconfig.json +++ b/packages/app/tsconfig.json @@ -2,7 +2,8 @@ "extends": "../../config/tsconfig.base.json", "compilerOptions": { "outDir": "dist", - "resolveJsonModule": true + "resolveJsonModule": true, + "downlevelIteration": true }, "exclude": ["dist/**/*"] } diff --git a/packages/component/.eslintrc.json b/packages/component/.eslintrc.json new file mode 100644 index 00000000000..2b39e9f6f85 --- /dev/null +++ b/packages/component/.eslintrc.json @@ -0,0 +1,6 @@ +{ + "extends": "../../config/.eslintrc.json", + "parserOptions": { + "project": "tsconfig.json" + } +} \ No newline at end of file diff --git a/packages/component/.npmignore b/packages/component/.npmignore new file mode 100644 index 00000000000..6de0b6d2896 --- /dev/null +++ b/packages/component/.npmignore @@ -0,0 +1 @@ +# This file is left intentionally blank \ No newline at end of file diff --git a/packages/component/README.md b/packages/component/README.md new file mode 100644 index 00000000000..ee3853f11b5 --- /dev/null +++ b/packages/component/README.md @@ -0,0 +1,13 @@ +# @firebase/template + +This package can be used as a template for anyone creating new packages in the +Firebase JS SDK. It will give you a couple things OOTB: + +- **Typescript Support:** Your code should be written in TS to be consistent + with the rest of the SDK. +- **Isomorphic Testing/Coverage:** Your tests will be run in both Node.js and + Browser environments and coverage from both, collected. +- **Links to all of the other packages:** Should your new package need to take + a dependency on any of the other packages in this monorepo (e.g. + `@firebase/app`, `@firebase/util`, etc), all those dependencies are already + set up, you can just remove the ones you don't need. diff --git a/packages/component/index.ts b/packages/component/index.ts new file mode 100644 index 00000000000..462a697e6a9 --- /dev/null +++ b/packages/component/index.ts @@ -0,0 +1,19 @@ +/** + * @license + * Copyright 2017 Google Inc. + * + * 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. + */ + +export { Component, ComponentType } from './src/component'; +export { ComponentContainer } from './src/component_container'; diff --git a/packages/component/karma.conf.js b/packages/component/karma.conf.js new file mode 100644 index 00000000000..d5f19a8ce2f --- /dev/null +++ b/packages/component/karma.conf.js @@ -0,0 +1,37 @@ +/** + * @license + * Copyright 2017 Google Inc. + * + * 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. + */ + +const karma = require('karma'); +const path = require('path'); +const karmaBase = require('../../config/karma.base'); + +const files = ['src/**/*.test.ts']; + +module.exports = function (config) { + config.set({ + ...karmaBase, + // files to load into karma + files: files, + preprocessors: { '**/*.ts': ['webpack', 'sourcemap'] }, + // frameworks to use + // available frameworks: https://npmjs.org/browse/keyword/karma-adapter + frameworks: ['mocha'] + }); + +}; + +module.exports.files = files; diff --git a/packages/component/package.json b/packages/component/package.json new file mode 100644 index 00000000000..a758b37a7e4 --- /dev/null +++ b/packages/component/package.json @@ -0,0 +1,75 @@ +{ + "name": "@firebase/component", + "version": "0.1.0", + "private": true, + "description": "Firebase Component Platform", + "author": "Firebase (https://firebase.google.com/)", + "main": "dist/index.cjs.js", + "browser": "dist/index.cjs.js", + "module": "dist/index.esm.js", + "esm2017": "dist/index.esm2017.js", + "files": [ + "dist" + ], + "scripts": { + "lint": "eslint -c .eslintrc.json '**/*.ts' --ignore-path '../../.gitignore'", + "lint:fix": "eslint --fix -c .eslintrc.json '**/*.ts' --ignore-path '../../.gitignore'", + "build": "rollup -c", + "dev": "rollup -c -w", + "test": "run-p lint test:browser test:node", + "test:browser": "karma start --single-run", + "test:node": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha src/**/*.test.ts --opts ../../config/mocha.node.opts", + "prepare": "yarn build" + }, + "dependencies": { + "@firebase/util": "0.2.25", + "tslib": "1.10.0" + }, + "license": "Apache-2.0", + "devDependencies": { + "@types/chai": "4.2.0", + "@types/mocha": "5.2.7", + "@types/sinon": "7.0.13", + "chai": "4.2.0", + "chai-as-promised": "7.1.1", + "karma": "4.2.0", + "sinon": "7.4.1", + "sinon-chai": "3.3.0", + "karma-chrome-launcher": "3.1.0", + "karma-cli": "2.0.0", + "karma-firefox-launcher": "1.2.0", + "karma-mocha": "1.3.0", + "karma-sauce-launcher": "1.2.0", + "karma-spec-reporter": "0.0.32", + "karma-webpack": "4.0.2", + "mocha": "6.2.0", + "npm-run-all": "4.1.5", + "nyc": "14.1.1", + "rollup": "1.19.4", + "rollup-plugin-typescript2": "0.22.1", + "ts-loader": "6.0.4", + "ts-node": "8.3.0", + "typescript": "3.5.3", + "webpack": "4.39.2", + "eslint": "5.16.0", + "@typescript-eslint/parser": "1.13.0", + "@typescript-eslint/eslint-plugin": "1.13.0", + "@typescript-eslint/eslint-plugin-tslint": "1.13.0", + "eslint-plugin-import": "2.18.2" + }, + "repository": { + "directory": "packages/component", + "type": "git", + "url": "https://github.com/firebase/firebase-js-sdk.git" + }, + "bugs": { + "url": "https://github.com/firebase/firebase-js-sdk/issues" + }, + "typings": "dist/index.d.ts", + "nyc": { + "extension": [ + ".ts" + ], + "reportDir": "./coverage/node" + } +} diff --git a/packages/component/rollup.config.js b/packages/component/rollup.config.js new file mode 100644 index 00000000000..8cfe95d6751 --- /dev/null +++ b/packages/component/rollup.config.js @@ -0,0 +1,80 @@ +/** + * @license + * Copyright 2018 Google Inc. + * + * 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 typescriptPlugin from 'rollup-plugin-typescript2'; +import typescript from 'typescript'; +import pkg from './package.json'; + +const deps = Object.keys( + Object.assign({}, pkg.peerDependencies, pkg.dependencies) +); + +/** + * ES5 Builds + */ +const es5BuildPlugins = [ + typescriptPlugin({ + typescript + }) +]; + +const es5Builds = [ + /** + * Browser Builds + */ + { + input: 'index.ts', + output: [ + { file: pkg.browser, format: 'cjs', sourcemap: true }, + { file: pkg.module, format: 'es', sourcemap: true } + ], + plugins: es5BuildPlugins, + external: id => deps.some(dep => id === dep || id.startsWith(`${dep}/`)) + } +]; + +/** + * ES2017 Builds + */ +const es2017BuildPlugins = [ + typescriptPlugin({ + typescript, + tsconfigOverride: { + compilerOptions: { + target: 'es2017' + } + } + }) +]; + +const es2017Builds = [ + /** + * Browser Builds + */ + { + input: 'index.ts', + output: { + file: pkg.esm2017, + format: 'es', + sourcemap: true + }, + plugins: es2017BuildPlugins, + external: id => deps.some(dep => id === dep || id.startsWith(`${dep}/`)) + } +]; + +export default [...es5Builds, ...es2017Builds]; diff --git a/packages/component/src/component.ts b/packages/component/src/component.ts new file mode 100644 index 00000000000..9ec10ca82c7 --- /dev/null +++ b/packages/component/src/component.ts @@ -0,0 +1,62 @@ +/** + * @license + * Copyright 2019 Google Inc. + * + * 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 { InstantiationMode, ServiceFactory } from './types'; + + +export class Component { + + multipleInstances: boolean = false; + /** + * Properties to be added to the service namespace + */ + serviceProps: { [key: string]: unknown } = {}; + + instantiationMode: InstantiationMode = InstantiationMode.LAZY; + + /** + * + * @param name The public service name, e.g. app, auth, firestore, database + * @param serviceFactory Service factory responsible for creating the public interface + * @param type whehter the service provided by the component is public or private + */ + constructor( + public readonly name: string, + public readonly serviceFactory: ServiceFactory, + public readonly type = ComponentType.PRIVATE + ) {} + + setInstantiationMode(mode: InstantiationMode, ): this { + this.instantiationMode = mode; + return this; + } + + setMultipleInstance(multipleInstances: boolean): this { + this.multipleInstances = multipleInstances; + return this; + } + + setServiceProps(props: { [key: string]: unknown }): this { + this.serviceProps = props; + return this; + } + +} + +export enum ComponentType { + PUBLIC, + PRIVATE +} diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts new file mode 100644 index 00000000000..7afa4c2d91d --- /dev/null +++ b/packages/component/src/component_container.ts @@ -0,0 +1,72 @@ +/** + * @license + * Copyright 2019 Google Inc. + * + * 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 { Provider } from './provider'; +import { Component } from './component'; +import { InstantiationMode } from './types'; + +export class ComponentContainer { + private providers = new Map(); + private components = new Map(); + + constructor(private name: string) {} + + addComponent(component: Component): void { + // noop if a component with the same name has been registered. + if (this.components.has(component.name)) { + console.warn( + `Component ${component.name} has already been registered with ${this.name}` + ); + return; + } + + this.components.set(component.name, component); + + const serviceProvider = this.getProvider( + component.name + ); + + // NOTE: currently InstantiationMode is only for the public service, all private services are LAZY. + // It is because currently all private services delegate tasks to the public service. There is no point to instantiate private services eagerly. + const isEager = this.isComponentEager(component); + serviceProvider.provideFactory( + component.serviceFactory, + component.multipleInstances, + isEager + ); + } + + getProvider(name: string): Provider { + if (this.providers.has(name)) { + return this.providers.get(name) as Provider; + } + + // create a Provider for a service that hasn't registered with Firebase + const provider = new Provider(name, this); + this.providers.set(name, provider); + + return provider as Provider; + } + + getProviders(): Provider[] { + return Array.from(this.providers, entry => entry[1]); + } + + private isComponentEager(component: Component): boolean { + return component.instantiationMode === InstantiationMode.EAGER; + } +} diff --git a/packages/component/src/component_containers.test.ts b/packages/component/src/component_containers.test.ts new file mode 100644 index 00000000000..bdbb312e26c --- /dev/null +++ b/packages/component/src/component_containers.test.ts @@ -0,0 +1,94 @@ +/** + * @license + * Copyright 2019 Google Inc. + * + * 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 { stub } from 'sinon'; +import { + ComponentContainer +} from './component_container'; +import '../test/setup'; +import { Component } from './component'; +import { Provider } from './provider'; +import { InstantiationMode } from './types'; +import { DEFAULT_ENTRY_NAME } from './contants'; + +function getFakeComponent( + name: string, + instantiationMode: InstantiationMode, + multipleInstances: boolean = false +): Component { + return new Component(name, () => ({ fire: true })) + .setInstantiationMode(instantiationMode) + .setMultipleInstance(multipleInstances); +} + +describe('Component Container', () => { + let container: ComponentContainer; + beforeEach(() => { + container = new ComponentContainer(DEFAULT_ENTRY_NAME); + }); + + it('returns a service provider given a name', () => { + expect(container.getProvider('rocket')).to.be.an.instanceof(Provider); + }); + + it('returns the same provider instance for the same name', () => { + const provider1 = container.getProvider('ship'); + const provider2 = container.getProvider('ship'); + + expect(provider1).to.eq(provider2); + }); + + it('calls provideFactory with eager flag set to true when registering an EAGER component', () => { + const provider = container.getProvider('fireball'); + const provideFactoryStub = stub(provider, 'provideFactory').callThrough(); + const component = getFakeComponent('fireball', InstantiationMode.EAGER); + container.addComponent(component); + + expect(provideFactoryStub).has.been.calledWith( + component.serviceFactory, + false, + true + ); + }); + + it('calls provideFactory with eager flag set to false when registering a LAZY component', () => { + const provider = container.getProvider('fireball'); + const provideFactoryStub = stub(provider, 'provideFactory').callThrough(); + const component = getFakeComponent('fireball', InstantiationMode.LAZY); + container.addComponent(component); + + expect(provideFactoryStub).has.been.calledWith( + component.serviceFactory, + false, + false + ); + }); + + it('injects service factory into the Provider with the correct multipleInstances flag', () => { + const provider = container.getProvider('multiple'); + const provideFactoryStub = stub(provider, 'provideFactory').callThrough(); + const component = getFakeComponent('multiple', InstantiationMode.LAZY, true); + container.addComponent(component); + + expect(provideFactoryStub).has.been.calledWith( + component.serviceFactory, + true, + false + ); + }); +}); diff --git a/packages/component/src/contants.ts b/packages/component/src/contants.ts new file mode 100644 index 00000000000..77bde2d690e --- /dev/null +++ b/packages/component/src/contants.ts @@ -0,0 +1 @@ +export const DEFAULT_ENTRY_NAME = '[DEFAULT]'; diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts new file mode 100644 index 00000000000..0e19743f8a5 --- /dev/null +++ b/packages/component/src/provider.test.ts @@ -0,0 +1,326 @@ +/** + * @license + * Copyright 2019 Google Inc. + * + * 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 { fake, SinonSpy } from 'sinon'; +import { ComponentContainer } from './component_container'; +import { FirebaseService } from '@firebase/app-types/private'; +import { Provider } from './provider'; +import { getFakeApp } from '../test/util'; +import '../test/setup'; + + +describe('Provider', () => { + let provider: Provider; + + beforeEach(() => { + provider = new Provider( + 'spider-queen', + new ComponentContainer('test') + ); + }); + + describe('Provider (multipleInstances = false)', () => { + describe('getImmediate()', () => { + it('throws if the service is not available', () => { + expect(provider.getImmediate.bind(provider)).to.throw( + 'Service spider-queen is not available' + ); + }); + + it('returns null if the service is not available with optional flag', () => { + expect(provider.getImmediate(undefined, { optional: true })).to.equal( + null + ); + }); + + it('returns the service instance synchronously', () => { + provider.provideFactory(() => ({ test: true })); + expect(provider.getImmediate()).to.deep.equal({ test: true }); + }); + + it('returns cached service instance', () => { + provider.provideFactory(() => ({ test: true })); + const service1 = provider.getImmediate(); + const service2 = provider.getImmediate(); + expect(service1).to.equal(service2); + }); + + it('ignores parameter identifier and return the default service', () => { + provider.provideFactory(() => ({ test: true })); + const defaultService = provider.getImmediate(); + expect(provider.getImmediate('spider1')).to.equal(defaultService); + expect(provider.getImmediate('spider2')).to.equal(defaultService); + }); + }); + + describe('get()', () => { + it('get the service instance asynchronouly', async () => { + provider.provideFactory(() => ({ test: true })); + await expect(provider.get()).to.eventually.deep.equal({ test: true }); + }); + + it('ignore parameter identifier and return the default service instance asyn', async () => { + provider.provideFactory(() => ({ test: true })); + const defaultService = provider.getImmediate(); + await expect(provider.get('spider1')).to.eventually.equal( + defaultService + ); + await expect(provider.get('spider2')).to.eventually.equal( + defaultService + ); + }); + }); + + describe('provideFactory()', () => { + it('instantiates the service if there is a pending promise and the service is eager', () => { + // create a pending promise + // tslint:disable-next-line:no-floating-promises + provider.get(); + + provider.provideFactory(() => ({}), false, true); + expect((provider as any).serviceInstances.size).to.equal(1); + }); + + it('instantiates the service if there is a pending promise and the service is NOT eager', () => { + // create a pending promise + // tslint:disable-next-line:no-floating-promises + provider.get(); + + provider.provideFactory(() => ({})); + expect((provider as any).serviceInstances.size).to.equal(1); + }); + + it('instantiates the service if there is no pending promise and the service is eager', () => { + provider.provideFactory(() => ({}), false, true); + expect((provider as any).serviceInstances.size).to.equal(1); + }); + + it('does NOT instantiate the service if there is no pending promise and the service is not eager', () => { + provider.provideFactory(() => ({})); + expect((provider as any).serviceInstances.size).to.equal(0); + }); + + it('instantiates only the default service even if there are pending promises with identifiers', async () => { + // create a pending promise with identifiers. + const promise1 = provider.get('name1'); + const promise2 = provider.get('name2'); + + provider.provideFactory(() => ({})); + expect((provider as any).serviceInstances.size).to.equal(1); + + const defaultService = provider.getImmediate(); + + await expect(promise1).to.eventually.equal(defaultService); + await expect(promise2).to.eventually.equal(defaultService); + }); + }); + + describe('delete()', () => { + it('calls delete() on the service instance that implements FirebaseService', () => { + const deleteFake = fake(); + const myService: FirebaseService = { + app: getFakeApp(), + INTERNAL: { + delete: deleteFake + } + }; + + // provide factory and create a service instance + provider.provideFactory(() => myService, false, true); + + // tslint:disable-next-line:no-floating-promises + provider.delete(); + + expect(deleteFake).to.have.been.called; + }); + }); + + describe('clearCache()', () => { + it('removes the service instance from cache', () => { + provider.provideFactory(() => ({})); + // create serviec instance + const instance = provider.getImmediate(); + expect((provider as any).serviceInstances.size).to.equal(1); + + provider.clearCache(); + expect((provider as any).serviceInstances.size).to.equal(0); + + // get a new instance after cache has been cleared + const newInstance = provider.getImmediate(); + expect(newInstance).to.not.eq(instance); + }); + }); + }); + + describe('Provider (multipleInstances = true)', () => { + describe('getImmediate(identifier)', () => { + it('throws if the service is not available', () => { + expect(provider.getImmediate.bind(provider, 'guardian')).to.throw(); + }); + + it('returns null if the service is not available with optional flag', () => { + expect(provider.getImmediate('guardian', { optional: true })).to.equal( + null + ); + }); + + it('returns different service instances for different identifiers synchronously', () => { + provider.provideFactory(() => ({ test: true }), true); + const defaultService = provider.getImmediate(); + const service1 = provider.getImmediate('guardian'); + const service2 = provider.getImmediate('servant'); + + expect(defaultService).to.deep.equal({ test: true }); + expect(service1).to.deep.equal({ test: true }); + expect(service2).to.deep.equal({ test: true }); + expect(defaultService).to.not.equal(service1); + expect(defaultService).to.not.equal(service2); + expect(service1).to.not.equal(service2); + }); + }); + + describe('get(identifier)', () => { + it('returns different service instances for different identifiers asynchronouly', async () => { + provider.provideFactory(() => ({ test: true }), true); + + const defaultService = await provider.get(); + const service1 = await provider.get('name1'); + const service2 = await provider.get('name2'); + + expect(defaultService).to.deep.equal({ test: true }); + expect(service1).to.deep.equal({ test: true }); + expect(service2).to.deep.equal({ test: true }); + expect(defaultService).to.not.equal(service1); + expect(defaultService).to.not.equal(service2); + expect(service1).to.not.equal(service2); + }); + }); + + describe('provideFactory()', () => { + it('instantiates services for the pending promises for all instance identifiers', async () => { + /* tslint:disable:no-floating-promises */ + // create 3 promises for 3 different identifiers + provider.get(); + provider.get('name1'); + provider.get('name2'); + /* tslint:enable::no-floating-promises */ + + provider.provideFactory(() => ({ test: true }), true); + + expect((provider as any).serviceInstances.size).to.equal(3); + }); + + it('instantiates the default service if there is no pending promise and the service is eager', () => { + provider.provideFactory(() => ({ test: true }), true, true); + expect((provider as any).serviceInstances.size).to.equal(1); + }); + + it(`instantiates the default serviec if there are pending promises for other identifiers + but not for the default identifer and the service is eager`, () => { + provider.get('name1'); + provider.provideFactory(() => ({ test: true }), true, true); + + expect((provider as any).serviceInstances.size).to.equal(2); + }); + }); + + describe('delete()', () => { + it('calls delete() on all service instances that implement FirebaseService', () => { + const deleteFakes: SinonSpy[] = []; + + function getService(): FirebaseService { + const deleteFake = fake(); + deleteFakes.push(deleteFake); + return { + app: getFakeApp(), + INTERNAL: { + delete: deleteFake + } + }; + } + + // provide factory that produces mulitpleInstances + provider.provideFactory(getService, true); + + // create 2 service instances with different names + provider.getImmediate('instance1'); + provider.getImmediate('instance2'); + + provider.delete(); + + expect(deleteFakes.length).to.eq(2); + for (const f of deleteFakes) { + expect(f).to.have.been.called; + } + }); + }); + describe('clearCache()', () => { + it('returns new service instances sync after cache is cleared', () => { + provider.provideFactory(() => ({}), true); + // create serviec instances with different identifiers + const defaultInstance = provider.getImmediate(); + const instance1 = provider.getImmediate('instance1'); + + expect((provider as any).serviceInstances.size).to.equal(2); + + // remove the default instance from cache and create a new default instance + provider.clearCache(); + expect((provider as any).serviceInstances.size).to.equal(1); + const newDefaultInstance = provider.getImmediate(); + expect(newDefaultInstance).to.not.eq(defaultInstance); + expect((provider as any).serviceInstances.size).to.equal(2); + + // remove the named instance from cache and create a new instance with the same identifier + provider.clearCache('instance1'); + expect((provider as any).serviceInstances.size).to.equal(1); + const newInstance1 = provider.getImmediate('instance1'); + expect(newInstance1).to.not.eq(instance1); + expect((provider as any).serviceInstances.size).to.equal(2); + }); + + it('returns new services asynchronously after cache is cleared', async () => { + provider.provideFactory(() => ({}), true); + // create serviec instances with different identifiers + const defaultInstance = await provider.get(); + const instance1 = await provider.get('instance1'); + + expect((provider as any).serviceInstances.size).to.equal(2); + expect((provider as any).serviceInstancesDeferred.size).to.equal(2); + + // remove the default instance from cache and create a new default instance + provider.clearCache(); + expect((provider as any).serviceInstances.size).to.equal(1); + expect((provider as any).serviceInstancesDeferred.size).to.equal(1); + + const newDefaultInstance = await provider.get(); + expect(newDefaultInstance).to.not.eq(defaultInstance); + expect((provider as any).serviceInstances.size).to.equal(2); + expect((provider as any).serviceInstancesDeferred.size).to.equal(2); + + // remove the named instance from cache and create a new instance with the same identifier + provider.clearCache('instance1'); + expect((provider as any).serviceInstances.size).to.equal(1); + expect((provider as any).serviceInstancesDeferred.size).to.equal(1); + const newInstance1 = await provider.get('instance1'); + expect(newInstance1).to.not.eq(instance1); + expect((provider as any).serviceInstances.size).to.equal(2); + expect((provider as any).serviceInstancesDeferred.size).to.equal(2); + }); + }); + }); +}); diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts new file mode 100644 index 00000000000..d4adb8ba02e --- /dev/null +++ b/packages/component/src/provider.ts @@ -0,0 +1,147 @@ +/** + * @license + * Copyright 2019 Google Inc. + * + * 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 { Deferred } from '@firebase/util'; +import { ComponentContainer } from './component_container'; +import { DEFAULT_ENTRY_NAME } from './contants'; +import { ServiceFactory } from './types'; + +/** + * Provider for interface of type T, e.g. Auth, RC + */ +export class Provider { + private factory: ServiceFactory | null = null; + private multipleInstances: boolean = true; // we assume multiple instances are supported by default + private serviceInstances: Map = new Map(); + private serviceInstancesDeferred: Map> = new Map(); + + constructor( + private name: string, + private container: ComponentContainer + ) {} + + get(identifier: string = DEFAULT_ENTRY_NAME): Promise { + // if multipleInstances is not supported, use the default name + const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); + + if (!this.serviceInstancesDeferred.has(normalizedIdentifier)) { + const deferred = new Deferred(); + this.serviceInstancesDeferred.set(normalizedIdentifier, deferred); + // If the service instance is available, resolve the promise with it immediately + const instance = this.getOrInitializeService(normalizedIdentifier); + if (instance) { + deferred.resolve(instance); + } + } + + return this.serviceInstancesDeferred.get(normalizedIdentifier)!.promise; + } + + getImmediate( + identifier: string = DEFAULT_ENTRY_NAME, + options?: { optional: boolean } + ): T | null { + // if multipleInstances is not supported, use the default name + const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); + + const instance = this.getOrInitializeService(normalizedIdentifier); + + if (!instance) { + if (options && options.optional) { + return null; + } + throw Error(`Service ${this.name} is not available`); + } + + return instance; + } + + provideFactory( + factory: ServiceFactory, + multipleInstances: boolean = false, + eager: boolean = false + ): void { + if (this.factory) { + throw Error(`Service factory for ${this.name} has already been provided`); + } + + this.factory = factory; + this.multipleInstances = multipleInstances; + + // if the service is eager, initialize the default instance + if (eager) { + this.getOrInitializeService(DEFAULT_ENTRY_NAME); + } + + // Create service instances for the pending promises and resolve them + // NOTE: if this.multipleInstances is false, only the default instance will be created + // and all promises with resolve with it regardless of the identifier. + for (const [ + instanceIdentifier, + instanceDeferred + ] of this.serviceInstancesDeferred.entries()) { + const normalizedIdentifier = this.normalizeInstanceIdentifier( + instanceIdentifier + ); + const instance = this.getOrInitializeService(normalizedIdentifier); + + if (instance) { + instanceDeferred.resolve(instance); + } + } + } + + clearCache(identifier: string = DEFAULT_ENTRY_NAME): void { + this.serviceInstancesDeferred.delete(identifier); + this.serviceInstances.delete(identifier); + } + + // app.delete() will call this method on every provider to delete the services + // TODO: should we mark the provider as deleted? + delete(): Promise { + const services = Array.from(this.serviceInstances.values()); + + return Promise.all( + services + .filter(service => 'INTERNAL' in service) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .map(service => (service as any).INTERNAL!.delete()) + ); + } + + private getOrInitializeService(identifier: string): T | null { + let instance = this.serviceInstances.get(identifier); + if (!instance && this.factory) { + instance = this.factory( + this.container, + normalizeIdentifierForFactory(identifier) + ); + this.serviceInstances.set(identifier, instance); + } + + return instance || null; + } + + private normalizeInstanceIdentifier(identifier: string): string { + return this.multipleInstances ? identifier : DEFAULT_ENTRY_NAME; + } +} + +// undefined should be passed to the service factory for the default instance +function normalizeIdentifierForFactory(identifier: string): string | undefined { + return identifier === DEFAULT_ENTRY_NAME ? undefined : identifier; +} diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts new file mode 100644 index 00000000000..238209f491a --- /dev/null +++ b/packages/component/src/types.ts @@ -0,0 +1,19 @@ +import { ComponentContainer } from './component_container'; + +export const enum InstantiationMode { + LAZY, // Currently all components are LAZY in JS SDK + EAGER +} + +/** + * Factory to create a component of type T, given a ComponentContainer. + * ComponentContainer is the IOC container that provides PROVIDERS + * for dependencies. + * + * NOTE: The container only provides PROVIDERS rather than the actual instances of dependencies. + * It is useful for lazily loaded dependencies and optional dependencies. + */ +export type ServiceFactory = ( + container: ComponentContainer, + instanceIdentifier?: string +) => T; diff --git a/packages/component/test/setup.ts b/packages/component/test/setup.ts new file mode 100644 index 00000000000..5f3858ba898 --- /dev/null +++ b/packages/component/test/setup.ts @@ -0,0 +1,28 @@ +/** + * @license + * Copyright 2019 Google Inc. + * + * 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 { use } from 'chai'; +import * as chaiAsPromised from 'chai-as-promised'; +import { restore } from 'sinon'; +import * as sinonChai from 'sinon-chai'; + +use(chaiAsPromised); +use(sinonChai); + +afterEach(async () => { + restore(); +}); diff --git a/packages/component/test/util.ts b/packages/component/test/util.ts new file mode 100644 index 00000000000..e60e1bb358d --- /dev/null +++ b/packages/component/test/util.ts @@ -0,0 +1,35 @@ +/** + * @license + * Copyright 2019 Google Inc. + * + * 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 { DEFAULT_ENTRY_NAME } from '../src/contants'; +import { FirebaseApp } from '@firebase/app-types'; + +export function getFakeApp(appName: string = DEFAULT_ENTRY_NAME): FirebaseApp { + return { + name: appName, + options: { + apiKey: 'apiKey', + projectId: 'projectId', + authDomain: 'authDomain', + messagingSenderId: 'messagingSenderId', + databaseURL: 'databaseUrl', + storageBucket: 'storageBucket', + appId: '1:777777777777:web:d93b5ca1475efe57' + }, + automaticDataCollectionEnabled: true, + delete: async () => { } + }; +} diff --git a/packages/component/tsconfig.json b/packages/component/tsconfig.json new file mode 100644 index 00000000000..735ea623511 --- /dev/null +++ b/packages/component/tsconfig.json @@ -0,0 +1,10 @@ +{ + "extends": "../../config/tsconfig.base.json", + "compilerOptions": { + "outDir": "dist", + "downlevelIteration": true + }, + "exclude": [ + "dist/**/*" + ] +} \ No newline at end of file From 85487f8deebcbe4fef8037260c38a21e3177b913 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 12:10:46 -0700 Subject: [PATCH 02/54] update dep version --- packages/component/package.json | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/packages/component/package.json b/packages/component/package.json index a758b37a7e4..c085e9a654e 100644 --- a/packages/component/package.json +++ b/packages/component/package.json @@ -22,39 +22,40 @@ "prepare": "yarn build" }, "dependencies": { - "@firebase/util": "0.2.25", + "@firebase/util": "0.2.29", "tslib": "1.10.0" }, "license": "Apache-2.0", "devDependencies": { - "@types/chai": "4.2.0", + "@types/chai": "4.2.3", "@types/mocha": "5.2.7", - "@types/sinon": "7.0.13", + "@types/sinon": "7.5.0", "chai": "4.2.0", "chai-as-promised": "7.1.1", - "karma": "4.2.0", - "sinon": "7.4.1", + "karma": "4.3.0", + "sinon": "7.5.0", "sinon-chai": "3.3.0", "karma-chrome-launcher": "3.1.0", "karma-cli": "2.0.0", + "karma-coverage-istanbul-reporter": "2.1.0", "karma-firefox-launcher": "1.2.0", "karma-mocha": "1.3.0", "karma-sauce-launcher": "1.2.0", "karma-spec-reporter": "0.0.32", "karma-webpack": "4.0.2", - "mocha": "6.2.0", + "mocha": "6.2.1", "npm-run-all": "4.1.5", "nyc": "14.1.1", - "rollup": "1.19.4", - "rollup-plugin-typescript2": "0.22.1", - "ts-loader": "6.0.4", - "ts-node": "8.3.0", - "typescript": "3.5.3", - "webpack": "4.39.2", - "eslint": "5.16.0", - "@typescript-eslint/parser": "1.13.0", - "@typescript-eslint/eslint-plugin": "1.13.0", - "@typescript-eslint/eslint-plugin-tslint": "1.13.0", + "rollup": "1.23.1", + "rollup-plugin-typescript2": "0.24.3", + "ts-loader": "6.2.0", + "ts-node": "8.4.1", + "typescript": "3.6.4", + "webpack": "4.41.1", + "eslint": "6.5.1", + "@typescript-eslint/parser": "2.4.0", + "@typescript-eslint/eslint-plugin": "2.4.0", + "@typescript-eslint/eslint-plugin-tslint": "2.4.0", "eslint-plugin-import": "2.18.2" }, "repository": { From 832b19c67274d716399f7e12ba68bf9e4a091abc Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 12:10:54 -0700 Subject: [PATCH 03/54] [AUTOMATED]: Prettier Code Styling --- packages/app/src/firebaseNamespaceCore.ts | 21 +++++++++++++------ .../app/src/lite/firebaseNamespaceLite.ts | 6 +++--- packages/component/karma.conf.js | 3 +-- packages/component/src/component.ts | 7 ++----- packages/component/src/component_container.ts | 4 +--- .../src/component_containers.test.ts | 10 +++++---- packages/component/src/provider.test.ts | 6 +----- packages/component/src/provider.ts | 5 +---- packages/component/src/types.ts | 8 +++---- packages/component/test/util.ts | 2 +- 10 files changed, 35 insertions(+), 37 deletions(-) diff --git a/packages/app/src/firebaseNamespaceCore.ts b/packages/app/src/firebaseNamespaceCore.ts index 57a3ceb694d..b3b45191c97 100644 --- a/packages/app/src/firebaseNamespaceCore.ts +++ b/packages/app/src/firebaseNamespaceCore.ts @@ -172,7 +172,9 @@ export function createFirebaseNamespaceCore( `There were multiple attempts to register component ${componentName}.` ); // eslint-disable-next-line @typescript-eslint/no-explicit-any - return component.type === ComponentType.PUBLIC ? (namespace as any)[componentName] : null; + return component.type === ComponentType.PUBLIC + ? (namespace as any)[componentName] + : null; } components.set(componentName, component); @@ -180,7 +182,9 @@ export function createFirebaseNamespaceCore( // create service namespace for public components if (component.type === ComponentType.PUBLIC) { // The Service namespace is an accessor function ... - const serviceNamespace = (appArg: FirebaseApp = app()): FirebaseService => { + const serviceNamespace = ( + appArg: FirebaseApp = app() + ): FirebaseService => { // eslint-disable-next-line @typescript-eslint/no-explicit-any if (typeof (appArg as any)[componentName] !== 'function') { // Invalid argument. @@ -193,7 +197,7 @@ export function createFirebaseNamespaceCore( // Forward service instance lookup to the FirebaseApp. // eslint-disable-next-line @typescript-eslint/no-explicit-any return (appArg as any)[componentName](); - } + }; // ... and a container for service-level properties. if (component.serviceProps !== undefined) { @@ -209,9 +213,12 @@ export function createFirebaseNamespaceCore( // TODO: The eslint disable can be removed and the 'ignoreRestArgs' // option added to the no-explicit-any rule when ESlint releases it. // eslint-disable-next-line @typescript-eslint/no-explicit-any - function (...args: any) { + function(...args: any) { const serviceFxn = this._getService.bind(this, componentName); - return serviceFxn.apply(this, component.multipleInstances ? args : []); + return serviceFxn.apply( + this, + component.multipleInstances ? args : [] + ); }; } @@ -220,7 +227,9 @@ export function createFirebaseNamespaceCore( (apps[appName] as _FirebaseApp)._addComponent(component); } - return component.type === ComponentType.PUBLIC ? (namespace as any)[componentName] : null; + return component.type === ComponentType.PUBLIC + ? (namespace as any)[componentName] + : null; } // Map the requested service to a registered service name diff --git a/packages/app/src/lite/firebaseNamespaceLite.ts b/packages/app/src/lite/firebaseNamespaceLite.ts index 55e4c274bf0..e88fba664c9 100644 --- a/packages/app/src/lite/firebaseNamespaceLite.ts +++ b/packages/app/src/lite/firebaseNamespaceLite.ts @@ -44,9 +44,9 @@ export function createFirebaseNamespaceLite(): FirebaseNamespace { ): FirebaseServiceNamespace | null { // only allow performance to register with firebase lite if ( - component.type === ComponentType.PUBLIC - && component.name !== 'performance' - && component.name !== 'installations' + component.type === ComponentType.PUBLIC && + component.name !== 'performance' && + component.name !== 'installations' ) { throw Error(`${name} cannot register with the standalone perf instance`); } diff --git a/packages/component/karma.conf.js b/packages/component/karma.conf.js index d5f19a8ce2f..9da71b4327f 100644 --- a/packages/component/karma.conf.js +++ b/packages/component/karma.conf.js @@ -21,7 +21,7 @@ const karmaBase = require('../../config/karma.base'); const files = ['src/**/*.test.ts']; -module.exports = function (config) { +module.exports = function(config) { config.set({ ...karmaBase, // files to load into karma @@ -31,7 +31,6 @@ module.exports = function (config) { // available frameworks: https://npmjs.org/browse/keyword/karma-adapter frameworks: ['mocha'] }); - }; module.exports.files = files; diff --git a/packages/component/src/component.ts b/packages/component/src/component.ts index 9ec10ca82c7..cb22fbcd575 100644 --- a/packages/component/src/component.ts +++ b/packages/component/src/component.ts @@ -16,9 +16,7 @@ */ import { InstantiationMode, ServiceFactory } from './types'; - export class Component { - multipleInstances: boolean = false; /** * Properties to be added to the service namespace @@ -28,7 +26,7 @@ export class Component { instantiationMode: InstantiationMode = InstantiationMode.LAZY; /** - * + * * @param name The public service name, e.g. app, auth, firestore, database * @param serviceFactory Service factory responsible for creating the public interface * @param type whehter the service provided by the component is public or private @@ -39,7 +37,7 @@ export class Component { public readonly type = ComponentType.PRIVATE ) {} - setInstantiationMode(mode: InstantiationMode, ): this { + setInstantiationMode(mode: InstantiationMode): this { this.instantiationMode = mode; return this; } @@ -53,7 +51,6 @@ export class Component { this.serviceProps = props; return this; } - } export enum ComponentType { diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 7afa4c2d91d..3e92db08746 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -36,9 +36,7 @@ export class ComponentContainer { this.components.set(component.name, component); - const serviceProvider = this.getProvider( - component.name - ); + const serviceProvider = this.getProvider(component.name); // NOTE: currently InstantiationMode is only for the public service, all private services are LAZY. // It is because currently all private services delegate tasks to the public service. There is no point to instantiate private services eagerly. diff --git a/packages/component/src/component_containers.test.ts b/packages/component/src/component_containers.test.ts index bdbb312e26c..81529f1f371 100644 --- a/packages/component/src/component_containers.test.ts +++ b/packages/component/src/component_containers.test.ts @@ -17,9 +17,7 @@ import { expect } from 'chai'; import { stub } from 'sinon'; -import { - ComponentContainer -} from './component_container'; +import { ComponentContainer } from './component_container'; import '../test/setup'; import { Component } from './component'; import { Provider } from './provider'; @@ -82,7 +80,11 @@ describe('Component Container', () => { it('injects service factory into the Provider with the correct multipleInstances flag', () => { const provider = container.getProvider('multiple'); const provideFactoryStub = stub(provider, 'provideFactory').callThrough(); - const component = getFakeComponent('multiple', InstantiationMode.LAZY, true); + const component = getFakeComponent( + 'multiple', + InstantiationMode.LAZY, + true + ); container.addComponent(component); expect(provideFactoryStub).has.been.calledWith( diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 0e19743f8a5..6428535bedb 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -23,15 +23,11 @@ import { Provider } from './provider'; import { getFakeApp } from '../test/util'; import '../test/setup'; - describe('Provider', () => { let provider: Provider; beforeEach(() => { - provider = new Provider( - 'spider-queen', - new ComponentContainer('test') - ); + provider = new Provider('spider-queen', new ComponentContainer('test')); }); describe('Provider (multipleInstances = false)', () => { diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index d4adb8ba02e..437a044f527 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -29,10 +29,7 @@ export class Provider { private serviceInstances: Map = new Map(); private serviceInstancesDeferred: Map> = new Map(); - constructor( - private name: string, - private container: ComponentContainer - ) {} + constructor(private name: string, private container: ComponentContainer) {} get(identifier: string = DEFAULT_ENTRY_NAME): Promise { // if multipleInstances is not supported, use the default name diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 238209f491a..5134fea86d2 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -1,8 +1,8 @@ import { ComponentContainer } from './component_container'; export const enum InstantiationMode { - LAZY, // Currently all components are LAZY in JS SDK - EAGER + LAZY, // Currently all components are LAZY in JS SDK + EAGER } /** @@ -14,6 +14,6 @@ export const enum InstantiationMode { * It is useful for lazily loaded dependencies and optional dependencies. */ export type ServiceFactory = ( - container: ComponentContainer, - instanceIdentifier?: string + container: ComponentContainer, + instanceIdentifier?: string ) => T; diff --git a/packages/component/test/util.ts b/packages/component/test/util.ts index e60e1bb358d..4a217a764a7 100644 --- a/packages/component/test/util.ts +++ b/packages/component/test/util.ts @@ -30,6 +30,6 @@ export function getFakeApp(appName: string = DEFAULT_ENTRY_NAME): FirebaseApp { appId: '1:777777777777:web:d93b5ca1475efe57' }, automaticDataCollectionEnabled: true, - delete: async () => { } + delete: async () => {} }; } From ab51efb79cc12edca387f481be0aa1ed1321c5ea Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 12:10:55 -0700 Subject: [PATCH 04/54] [AUTOMATED]: License Headers --- packages/component/src/contants.ts | 17 +++++++++++++++++ packages/component/src/types.ts | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/packages/component/src/contants.ts b/packages/component/src/contants.ts index 77bde2d690e..2c6101879ea 100644 --- a/packages/component/src/contants.ts +++ b/packages/component/src/contants.ts @@ -1 +1,18 @@ +/** + * @license + * Copyright 2019 Google Inc. + * + * 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. + */ + export const DEFAULT_ENTRY_NAME = '[DEFAULT]'; diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 5134fea86d2..7083ed59c8b 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -1,3 +1,20 @@ +/** + * @license + * Copyright 2019 Google Inc. + * + * 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 { ComponentContainer } from './component_container'; export const enum InstantiationMode { From 369cffebe3fa753ff3e2b503944410ac67b0174a Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 13:37:12 -0700 Subject: [PATCH 05/54] rename variables --- packages/app/src/firebaseNamespaceCore.ts | 2 +- packages/component/src/component.ts | 12 ++-- packages/component/src/component_container.ts | 8 +-- .../src/component_containers.test.ts | 12 ++-- packages/component/src/provider.test.ts | 62 ++++++++++--------- packages/component/src/provider.ts | 28 ++++----- packages/component/src/types.ts | 2 +- 7 files changed, 63 insertions(+), 63 deletions(-) diff --git a/packages/app/src/firebaseNamespaceCore.ts b/packages/app/src/firebaseNamespaceCore.ts index b3b45191c97..d1e5ba10462 100644 --- a/packages/app/src/firebaseNamespaceCore.ts +++ b/packages/app/src/firebaseNamespaceCore.ts @@ -168,7 +168,7 @@ export function createFirebaseNamespaceCore( ): FirebaseServiceNamespace | null { const componentName = component.name; if (components.has(componentName)) { - console.debug( + logger.debug( `There were multiple attempts to register component ${componentName}.` ); // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/packages/component/src/component.ts b/packages/component/src/component.ts index cb22fbcd575..12f39db1ebc 100644 --- a/packages/component/src/component.ts +++ b/packages/component/src/component.ts @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { InstantiationMode, ServiceFactory } from './types'; +import { InstantiationMode, InstanceFactory } from './types'; export class Component { multipleInstances: boolean = false; @@ -28,21 +28,21 @@ export class Component { /** * * @param name The public service name, e.g. app, auth, firestore, database - * @param serviceFactory Service factory responsible for creating the public interface + * @param instanceFactory Service factory responsible for creating the public interface * @param type whehter the service provided by the component is public or private */ constructor( public readonly name: string, - public readonly serviceFactory: ServiceFactory, - public readonly type = ComponentType.PRIVATE - ) {} + public readonly instanceFactory: InstanceFactory, + public readonly type: ComponentType + ) { } setInstantiationMode(mode: InstantiationMode): this { this.instantiationMode = mode; return this; } - setMultipleInstance(multipleInstances: boolean): this { + setMultipleInstances(multipleInstances: boolean): this { this.multipleInstances = multipleInstances; return this; } diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 3e92db08746..7ede37ed26f 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -36,13 +36,11 @@ export class ComponentContainer { this.components.set(component.name, component); - const serviceProvider = this.getProvider(component.name); + const provider = this.getProvider(component.name); - // NOTE: currently InstantiationMode is only for the public service, all private services are LAZY. - // It is because currently all private services delegate tasks to the public service. There is no point to instantiate private services eagerly. const isEager = this.isComponentEager(component); - serviceProvider.provideFactory( - component.serviceFactory, + provider.provideFactory( + component.instanceFactory, component.multipleInstances, isEager ); diff --git a/packages/component/src/component_containers.test.ts b/packages/component/src/component_containers.test.ts index 81529f1f371..92ca1327daf 100644 --- a/packages/component/src/component_containers.test.ts +++ b/packages/component/src/component_containers.test.ts @@ -19,7 +19,7 @@ import { expect } from 'chai'; import { stub } from 'sinon'; import { ComponentContainer } from './component_container'; import '../test/setup'; -import { Component } from './component'; +import { Component, ComponentType } from './component'; import { Provider } from './provider'; import { InstantiationMode } from './types'; import { DEFAULT_ENTRY_NAME } from './contants'; @@ -29,9 +29,9 @@ function getFakeComponent( instantiationMode: InstantiationMode, multipleInstances: boolean = false ): Component { - return new Component(name, () => ({ fire: true })) + return new Component(name, () => ({ fire: true }), ComponentType.PRIVATE) .setInstantiationMode(instantiationMode) - .setMultipleInstance(multipleInstances); + .setMultipleInstances(multipleInstances); } describe('Component Container', () => { @@ -58,7 +58,7 @@ describe('Component Container', () => { container.addComponent(component); expect(provideFactoryStub).has.been.calledWith( - component.serviceFactory, + component.instanceFactory, false, true ); @@ -71,7 +71,7 @@ describe('Component Container', () => { container.addComponent(component); expect(provideFactoryStub).has.been.calledWith( - component.serviceFactory, + component.instanceFactory, false, false ); @@ -88,7 +88,7 @@ describe('Component Container', () => { container.addComponent(component); expect(provideFactoryStub).has.been.calledWith( - component.serviceFactory, + component.instanceFactory, true, false ); diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 6428535bedb..1aac74917c6 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -85,30 +85,30 @@ describe('Provider', () => { describe('provideFactory()', () => { it('instantiates the service if there is a pending promise and the service is eager', () => { // create a pending promise - // tslint:disable-next-line:no-floating-promises + // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.get(); provider.provideFactory(() => ({}), false, true); - expect((provider as any).serviceInstances.size).to.equal(1); + expect((provider as any).instances.size).to.equal(1); }); it('instantiates the service if there is a pending promise and the service is NOT eager', () => { // create a pending promise - // tslint:disable-next-line:no-floating-promises + // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.get(); provider.provideFactory(() => ({})); - expect((provider as any).serviceInstances.size).to.equal(1); + expect((provider as any).instances.size).to.equal(1); }); it('instantiates the service if there is no pending promise and the service is eager', () => { provider.provideFactory(() => ({}), false, true); - expect((provider as any).serviceInstances.size).to.equal(1); + expect((provider as any).instances.size).to.equal(1); }); it('does NOT instantiate the service if there is no pending promise and the service is not eager', () => { provider.provideFactory(() => ({})); - expect((provider as any).serviceInstances.size).to.equal(0); + expect((provider as any).instances.size).to.equal(0); }); it('instantiates only the default service even if there are pending promises with identifiers', async () => { @@ -117,7 +117,7 @@ describe('Provider', () => { const promise2 = provider.get('name2'); provider.provideFactory(() => ({})); - expect((provider as any).serviceInstances.size).to.equal(1); + expect((provider as any).instances.size).to.equal(1); const defaultService = provider.getImmediate(); @@ -139,7 +139,7 @@ describe('Provider', () => { // provide factory and create a service instance provider.provideFactory(() => myService, false, true); - // tslint:disable-next-line:no-floating-promises + // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.delete(); expect(deleteFake).to.have.been.called; @@ -151,10 +151,10 @@ describe('Provider', () => { provider.provideFactory(() => ({})); // create serviec instance const instance = provider.getImmediate(); - expect((provider as any).serviceInstances.size).to.equal(1); + expect((provider as any).instances.size).to.equal(1); provider.clearCache(); - expect((provider as any).serviceInstances.size).to.equal(0); + expect((provider as any).instances.size).to.equal(0); // get a new instance after cache has been cleared const newInstance = provider.getImmediate(); @@ -209,29 +209,30 @@ describe('Provider', () => { describe('provideFactory()', () => { it('instantiates services for the pending promises for all instance identifiers', async () => { - /* tslint:disable:no-floating-promises */ + /* eslint-disable @typescript-eslint/no-floating-promises */ // create 3 promises for 3 different identifiers provider.get(); provider.get('name1'); provider.get('name2'); - /* tslint:enable::no-floating-promises */ + /* eslint-enable @typescript-eslint/no-floating-promises */ provider.provideFactory(() => ({ test: true }), true); - expect((provider as any).serviceInstances.size).to.equal(3); + expect((provider as any).instances.size).to.equal(3); }); it('instantiates the default service if there is no pending promise and the service is eager', () => { provider.provideFactory(() => ({ test: true }), true, true); - expect((provider as any).serviceInstances.size).to.equal(1); + expect((provider as any).instances.size).to.equal(1); }); it(`instantiates the default serviec if there are pending promises for other identifiers but not for the default identifer and the service is eager`, () => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.get('name1'); provider.provideFactory(() => ({ test: true }), true, true); - expect((provider as any).serviceInstances.size).to.equal(2); + expect((provider as any).instances.size).to.equal(2); }); }); @@ -257,6 +258,7 @@ describe('Provider', () => { provider.getImmediate('instance1'); provider.getImmediate('instance2'); + // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.delete(); expect(deleteFakes.length).to.eq(2); @@ -272,21 +274,21 @@ describe('Provider', () => { const defaultInstance = provider.getImmediate(); const instance1 = provider.getImmediate('instance1'); - expect((provider as any).serviceInstances.size).to.equal(2); + expect((provider as any).instances.size).to.equal(2); // remove the default instance from cache and create a new default instance provider.clearCache(); - expect((provider as any).serviceInstances.size).to.equal(1); + expect((provider as any).instances.size).to.equal(1); const newDefaultInstance = provider.getImmediate(); expect(newDefaultInstance).to.not.eq(defaultInstance); - expect((provider as any).serviceInstances.size).to.equal(2); + expect((provider as any).instances.size).to.equal(2); // remove the named instance from cache and create a new instance with the same identifier provider.clearCache('instance1'); - expect((provider as any).serviceInstances.size).to.equal(1); + expect((provider as any).instances.size).to.equal(1); const newInstance1 = provider.getImmediate('instance1'); expect(newInstance1).to.not.eq(instance1); - expect((provider as any).serviceInstances.size).to.equal(2); + expect((provider as any).instances.size).to.equal(2); }); it('returns new services asynchronously after cache is cleared', async () => { @@ -295,27 +297,27 @@ describe('Provider', () => { const defaultInstance = await provider.get(); const instance1 = await provider.get('instance1'); - expect((provider as any).serviceInstances.size).to.equal(2); - expect((provider as any).serviceInstancesDeferred.size).to.equal(2); + expect((provider as any).instances.size).to.equal(2); + expect((provider as any).instancesDeferred.size).to.equal(2); // remove the default instance from cache and create a new default instance provider.clearCache(); - expect((provider as any).serviceInstances.size).to.equal(1); - expect((provider as any).serviceInstancesDeferred.size).to.equal(1); + expect((provider as any).instances.size).to.equal(1); + expect((provider as any).instancesDeferred.size).to.equal(1); const newDefaultInstance = await provider.get(); expect(newDefaultInstance).to.not.eq(defaultInstance); - expect((provider as any).serviceInstances.size).to.equal(2); - expect((provider as any).serviceInstancesDeferred.size).to.equal(2); + expect((provider as any).instances.size).to.equal(2); + expect((provider as any).instancesDeferred.size).to.equal(2); // remove the named instance from cache and create a new instance with the same identifier provider.clearCache('instance1'); - expect((provider as any).serviceInstances.size).to.equal(1); - expect((provider as any).serviceInstancesDeferred.size).to.equal(1); + expect((provider as any).instances.size).to.equal(1); + expect((provider as any).instancesDeferred.size).to.equal(1); const newInstance1 = await provider.get('instance1'); expect(newInstance1).to.not.eq(instance1); - expect((provider as any).serviceInstances.size).to.equal(2); - expect((provider as any).serviceInstancesDeferred.size).to.equal(2); + expect((provider as any).instances.size).to.equal(2); + expect((provider as any).instancesDeferred.size).to.equal(2); }); }); }); diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 437a044f527..4545acc7f4f 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -18,16 +18,16 @@ import { Deferred } from '@firebase/util'; import { ComponentContainer } from './component_container'; import { DEFAULT_ENTRY_NAME } from './contants'; -import { ServiceFactory } from './types'; +import { InstanceFactory } from './types'; /** * Provider for interface of type T, e.g. Auth, RC */ export class Provider { - private factory: ServiceFactory | null = null; + private factory: InstanceFactory | null = null; private multipleInstances: boolean = true; // we assume multiple instances are supported by default - private serviceInstances: Map = new Map(); - private serviceInstancesDeferred: Map> = new Map(); + private instances: Map = new Map(); + private instancesDeferred: Map> = new Map(); constructor(private name: string, private container: ComponentContainer) {} @@ -35,9 +35,9 @@ export class Provider { // if multipleInstances is not supported, use the default name const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); - if (!this.serviceInstancesDeferred.has(normalizedIdentifier)) { + if (!this.instancesDeferred.has(normalizedIdentifier)) { const deferred = new Deferred(); - this.serviceInstancesDeferred.set(normalizedIdentifier, deferred); + this.instancesDeferred.set(normalizedIdentifier, deferred); // If the service instance is available, resolve the promise with it immediately const instance = this.getOrInitializeService(normalizedIdentifier); if (instance) { @@ -45,7 +45,7 @@ export class Provider { } } - return this.serviceInstancesDeferred.get(normalizedIdentifier)!.promise; + return this.instancesDeferred.get(normalizedIdentifier)!.promise; } getImmediate( @@ -68,7 +68,7 @@ export class Provider { } provideFactory( - factory: ServiceFactory, + factory: InstanceFactory, multipleInstances: boolean = false, eager: boolean = false ): void { @@ -90,7 +90,7 @@ export class Provider { for (const [ instanceIdentifier, instanceDeferred - ] of this.serviceInstancesDeferred.entries()) { + ] of this.instancesDeferred.entries()) { const normalizedIdentifier = this.normalizeInstanceIdentifier( instanceIdentifier ); @@ -103,14 +103,14 @@ export class Provider { } clearCache(identifier: string = DEFAULT_ENTRY_NAME): void { - this.serviceInstancesDeferred.delete(identifier); - this.serviceInstances.delete(identifier); + this.instancesDeferred.delete(identifier); + this.instances.delete(identifier); } // app.delete() will call this method on every provider to delete the services // TODO: should we mark the provider as deleted? delete(): Promise { - const services = Array.from(this.serviceInstances.values()); + const services = Array.from(this.instances.values()); return Promise.all( services @@ -121,13 +121,13 @@ export class Provider { } private getOrInitializeService(identifier: string): T | null { - let instance = this.serviceInstances.get(identifier); + let instance = this.instances.get(identifier); if (!instance && this.factory) { instance = this.factory( this.container, normalizeIdentifierForFactory(identifier) ); - this.serviceInstances.set(identifier, instance); + this.instances.set(identifier, instance); } return instance || null; diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 7083ed59c8b..b77c5f21792 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -30,7 +30,7 @@ export const enum InstantiationMode { * NOTE: The container only provides PROVIDERS rather than the actual instances of dependencies. * It is useful for lazily loaded dependencies and optional dependencies. */ -export type ServiceFactory = ( +export type InstanceFactory = ( container: ComponentContainer, instanceIdentifier?: string ) => T; From bed9fb4ae83e2640e514282c47373ca4c1d9c88e Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 14:19:14 -0700 Subject: [PATCH 06/54] address comments --- packages/app/src/firebaseApp.ts | 13 +++++++++---- packages/app/src/firebaseNamespaceCore.ts | 1 - packages/component/src/component_container.ts | 6 +----- packages/component/src/types.ts | 6 +++--- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index 5a755c62b70..28ee7ebee90 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -26,9 +26,10 @@ import { FirebaseService } from '@firebase/app-types/private'; import { deepCopy } from '@firebase/util'; -import { ComponentContainer, Component } from '@firebase/component'; +import { ComponentContainer, Component, ComponentType } from '@firebase/component'; import { AppError, ERROR_FACTORY } from './errors'; import { DEFAULT_ENTRY_NAME } from './constants'; +import { logger } from './logger'; /** * Global context object for a collection of services using @@ -53,10 +54,10 @@ export class FirebaseAppImpl implements FirebaseApp { this.container = new ComponentContainer(config.name!); // add itself to container - this.container.addComponent(new Component('app', () => this)); + this._addComponent(new Component('app', () => this, ComponentType.PUBLIC)); // populate ComponentContainer with existing components for (const component of this.firebase_.INTERNAL.components.values()) { - this.container.addComponent(component); + this._addComponent(component); } } @@ -140,7 +141,11 @@ export class FirebaseAppImpl implements FirebaseApp { } _addComponent(component: Component): void { - this.container.addComponent(component); + try { + this.container.addComponent(component); + } catch (e) { + logger.debug(`Component ${component.name} failed to register with FirebaseApp ${this.name}`, e); + } } /** diff --git a/packages/app/src/firebaseNamespaceCore.ts b/packages/app/src/firebaseNamespaceCore.ts index d1e5ba10462..385e0a09d5c 100644 --- a/packages/app/src/firebaseNamespaceCore.ts +++ b/packages/app/src/firebaseNamespaceCore.ts @@ -47,7 +47,6 @@ export function createFirebaseNamespaceCore( firebaseAppImpl: typeof FirebaseAppImpl | typeof FirebaseAppLiteImpl ): FirebaseNamespace { const apps: { [name: string]: FirebaseApp } = {}; - // const factories: { [service: string]: FirebaseServiceFactory } = {}; const components = new Map(); // A namespace is a plain JavaScript Object. diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 7ede37ed26f..3d013ab29b7 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -26,12 +26,8 @@ export class ComponentContainer { constructor(private name: string) {} addComponent(component: Component): void { - // noop if a component with the same name has been registered. if (this.components.has(component.name)) { - console.warn( - `Component ${component.name} has already been registered with ${this.name}` - ); - return; + throw new Error(`Component ${component.name} has already been registered with ${this.name}`); } this.components.set(component.name, component); diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index b77c5f21792..794579891a8 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -23,11 +23,11 @@ export const enum InstantiationMode { } /** - * Factory to create a component of type T, given a ComponentContainer. - * ComponentContainer is the IOC container that provides PROVIDERS + * Factory to create an instance of type T, given a ComponentContainer. + * ComponentContainer is the IOC container that provides {@link Provider} * for dependencies. * - * NOTE: The container only provides PROVIDERS rather than the actual instances of dependencies. + * NOTE: The container only provides {@link Provider} rather than the actual instances of dependencies. * It is useful for lazily loaded dependencies and optional dependencies. */ export type InstanceFactory = ( From 2f13e667641b35f077831432e4c27b58ff1c92b3 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 14:19:21 -0700 Subject: [PATCH 07/54] [AUTOMATED]: Prettier Code Styling --- packages/app/src/firebaseApp.ts | 11 +++++++++-- packages/component/src/component.ts | 2 +- packages/component/src/component_container.ts | 4 +++- packages/component/src/provider.test.ts | 2 +- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index 28ee7ebee90..0f8dee48b86 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -26,7 +26,11 @@ import { FirebaseService } from '@firebase/app-types/private'; import { deepCopy } from '@firebase/util'; -import { ComponentContainer, Component, ComponentType } from '@firebase/component'; +import { + ComponentContainer, + Component, + ComponentType +} from '@firebase/component'; import { AppError, ERROR_FACTORY } from './errors'; import { DEFAULT_ENTRY_NAME } from './constants'; import { logger } from './logger'; @@ -144,7 +148,10 @@ export class FirebaseAppImpl implements FirebaseApp { try { this.container.addComponent(component); } catch (e) { - logger.debug(`Component ${component.name} failed to register with FirebaseApp ${this.name}`, e); + logger.debug( + `Component ${component.name} failed to register with FirebaseApp ${this.name}`, + e + ); } } diff --git a/packages/component/src/component.ts b/packages/component/src/component.ts index 12f39db1ebc..2f7d33f8795 100644 --- a/packages/component/src/component.ts +++ b/packages/component/src/component.ts @@ -35,7 +35,7 @@ export class Component { public readonly name: string, public readonly instanceFactory: InstanceFactory, public readonly type: ComponentType - ) { } + ) {} setInstantiationMode(mode: InstantiationMode): this { this.instantiationMode = mode; diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 3d013ab29b7..3fc08479529 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -27,7 +27,9 @@ export class ComponentContainer { addComponent(component: Component): void { if (this.components.has(component.name)) { - throw new Error(`Component ${component.name} has already been registered with ${this.name}`); + throw new Error( + `Component ${component.name} has already been registered with ${this.name}` + ); } this.components.set(component.name, component); diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 1aac74917c6..7ade5a9e3fc 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -228,7 +228,7 @@ describe('Provider', () => { it(`instantiates the default serviec if there are pending promises for other identifiers but not for the default identifer and the service is eager`, () => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises + // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.get('name1'); provider.provideFactory(() => ({ test: true }), true, true); From de6cc28d73251cb22c1393041844a2d257ff2413 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 14:20:37 -0700 Subject: [PATCH 08/54] remove unused comment --- packages/app-types/private.d.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/app-types/private.d.ts b/packages/app-types/private.d.ts index b1e79bf8fbc..40b232910fa 100644 --- a/packages/app-types/private.d.ts +++ b/packages/app-types/private.d.ts @@ -26,15 +26,6 @@ import { FirebaseError, ErrorFactory } from '@firebase/util'; import { Deferred } from '../firestore/test/util/promise'; import { Component } from '@firebase/component'; -/** - * Factory responsible for creating a component of type T, given a ComponentContainer. - * ComponentContainer is the IOC container that provides PROVIDERS - * for dependencies. - * - * NOTE: The container only provides PROVIDERS instead of the actual instances of dependencies - * to account for lazily loaded or optional dependencies - */ - export interface FirebaseServiceInternals { /** * Delete the service and free it's resources - called from From baab7940c2bdc5af2e471eb3d90d803776b49d61 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 14:22:44 -0700 Subject: [PATCH 09/54] update code --- packages/app/src/lite/firebaseAppLite.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/app/src/lite/firebaseAppLite.ts b/packages/app/src/lite/firebaseAppLite.ts index ef98dd35481..154374bc546 100644 --- a/packages/app/src/lite/firebaseAppLite.ts +++ b/packages/app/src/lite/firebaseAppLite.ts @@ -25,10 +25,10 @@ import { _FirebaseNamespace, FirebaseService } from '@firebase/app-types/private'; -import { deepCopy, deepExtend } from '@firebase/util'; +import { deepCopy } from '@firebase/util'; import { ERROR_FACTORY, AppError } from '../errors'; import { DEFAULT_ENTRY_NAME } from '../constants'; -import { ComponentContainer, Component } from '@firebase/component'; +import { ComponentContainer, Component, ComponentType } from '@firebase/component'; interface ServicesCache { [name: string]: { @@ -62,7 +62,7 @@ export class FirebaseAppLiteImpl implements FirebaseApp { this.container = new ComponentContainer(config.name!); // add itself to container - this.container.addComponent(new Component('app', () => this)); + this.container.addComponent(new Component('app', () => this, ComponentType.PUBLIC)); // populate ComponentContainer with existing components for (const component of this.firebase_.INTERNAL.components.values()) { this.container.addComponent(component); From f68319a7f526e0d700fbd6ddb27362a172852b57 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 14:22:50 -0700 Subject: [PATCH 10/54] [AUTOMATED]: Prettier Code Styling --- packages/app/src/lite/firebaseAppLite.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/app/src/lite/firebaseAppLite.ts b/packages/app/src/lite/firebaseAppLite.ts index 154374bc546..b8a1b20ba87 100644 --- a/packages/app/src/lite/firebaseAppLite.ts +++ b/packages/app/src/lite/firebaseAppLite.ts @@ -28,7 +28,11 @@ import { import { deepCopy } from '@firebase/util'; import { ERROR_FACTORY, AppError } from '../errors'; import { DEFAULT_ENTRY_NAME } from '../constants'; -import { ComponentContainer, Component, ComponentType } from '@firebase/component'; +import { + ComponentContainer, + Component, + ComponentType +} from '@firebase/component'; interface ServicesCache { [name: string]: { @@ -62,7 +66,9 @@ export class FirebaseAppLiteImpl implements FirebaseApp { this.container = new ComponentContainer(config.name!); // add itself to container - this.container.addComponent(new Component('app', () => this, ComponentType.PUBLIC)); + this.container.addComponent( + new Component('app', () => this, ComponentType.PUBLIC) + ); // populate ComponentContainer with existing components for (const component of this.firebase_.INTERNAL.components.values()) { this.container.addComponent(component); From 368b2a268324ea3e340e085f1e91cb1cefe29982 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 16:34:13 -0700 Subject: [PATCH 11/54] rename to clearInstance to have naming consistency --- packages/component/src/provider.test.ts | 12 ++++++------ packages/component/src/provider.ts | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 7ade5a9e3fc..ac9cc6d564f 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -49,7 +49,7 @@ describe('Provider', () => { expect(provider.getImmediate()).to.deep.equal({ test: true }); }); - it('returns cached service instance', () => { + it('returns the cached service instance', () => { provider.provideFactory(() => ({ test: true })); const service1 = provider.getImmediate(); const service2 = provider.getImmediate(); @@ -153,7 +153,7 @@ describe('Provider', () => { const instance = provider.getImmediate(); expect((provider as any).instances.size).to.equal(1); - provider.clearCache(); + provider.clearInstance(); expect((provider as any).instances.size).to.equal(0); // get a new instance after cache has been cleared @@ -277,14 +277,14 @@ describe('Provider', () => { expect((provider as any).instances.size).to.equal(2); // remove the default instance from cache and create a new default instance - provider.clearCache(); + provider.clearInstance(); expect((provider as any).instances.size).to.equal(1); const newDefaultInstance = provider.getImmediate(); expect(newDefaultInstance).to.not.eq(defaultInstance); expect((provider as any).instances.size).to.equal(2); // remove the named instance from cache and create a new instance with the same identifier - provider.clearCache('instance1'); + provider.clearInstance('instance1'); expect((provider as any).instances.size).to.equal(1); const newInstance1 = provider.getImmediate('instance1'); expect(newInstance1).to.not.eq(instance1); @@ -301,7 +301,7 @@ describe('Provider', () => { expect((provider as any).instancesDeferred.size).to.equal(2); // remove the default instance from cache and create a new default instance - provider.clearCache(); + provider.clearInstance(); expect((provider as any).instances.size).to.equal(1); expect((provider as any).instancesDeferred.size).to.equal(1); @@ -311,7 +311,7 @@ describe('Provider', () => { expect((provider as any).instancesDeferred.size).to.equal(2); // remove the named instance from cache and create a new instance with the same identifier - provider.clearCache('instance1'); + provider.clearInstance('instance1'); expect((provider as any).instances.size).to.equal(1); expect((provider as any).instancesDeferred.size).to.equal(1); const newInstance1 = await provider.get('instance1'); diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 4545acc7f4f..51da130cfa9 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -102,7 +102,7 @@ export class Provider { } } - clearCache(identifier: string = DEFAULT_ENTRY_NAME): void { + clearInstance(identifier: string = DEFAULT_ENTRY_NAME): void { this.instancesDeferred.delete(identifier); this.instances.delete(identifier); } From 7dce8de7423984a869511e6cdfb250166eb1a3ed Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 17:35:40 -0700 Subject: [PATCH 12/54] make FirebaseApp tests work again --- packages/app-types/index.d.ts | 4 +- packages/app/index.node.ts | 9 +- packages/app/index.rn.ts | 10 +- packages/app/index.ts | 11 +- packages/app/src/firebaseNamespace.ts | 2 +- packages/app/test/firebaseApp.test.ts | 454 ++++++-------------------- packages/component/index.ts | 1 + 7 files changed, 130 insertions(+), 361 deletions(-) diff --git a/packages/app-types/index.d.ts b/packages/app-types/index.d.ts index 10b89b83bb9..1dd18812d88 100644 --- a/packages/app-types/index.d.ts +++ b/packages/app-types/index.d.ts @@ -1,3 +1,5 @@ +import { Provider } from "@firebase/component"; + /** * @license * Copyright 2017 Google Inc. @@ -100,4 +102,4 @@ export interface FirebaseNamespace { // The current SDK version. SDK_VERSION: string; -} +} \ No newline at end of file diff --git a/packages/app/index.node.ts b/packages/app/index.node.ts index f7e81b690e5..e7a4c3583ef 100644 --- a/packages/app/index.node.ts +++ b/packages/app/index.node.ts @@ -15,9 +15,10 @@ * limitations under the License. */ -import { FirebaseNamespace } from '@firebase/app-types'; +import { FirebaseNamespace, FirebaseApp } from '@firebase/app-types'; import { _FirebaseNamespace } from '@firebase/app-types/private'; import { firebase as _firebase } from './src/firebaseNamespace'; +import { Provider } from '@firebase/component'; // Node specific packages. // @ts-ignore import Storage from 'dom-storage'; @@ -38,3 +39,9 @@ export const firebase = _firebase as FirebaseNamespace; // eslint-disable-next-line import/no-default-export export default firebase; + +declare module '@firebase/component' { + interface ComponentContainer { + getProvider(name: 'app'): Provider; + } +} \ No newline at end of file diff --git a/packages/app/index.rn.ts b/packages/app/index.rn.ts index a3cad68f513..af417f5f05b 100644 --- a/packages/app/index.rn.ts +++ b/packages/app/index.rn.ts @@ -15,10 +15,10 @@ * limitations under the License. */ -import { FirebaseNamespace } from '@firebase/app-types'; +import { FirebaseNamespace, FirebaseApp } from '@firebase/app-types'; import { _FirebaseNamespace } from '@firebase/app-types/private'; import { firebase as _firebase } from './src/firebaseNamespace'; - +import { Provider } from '@firebase/component'; /** * To avoid having to include the @types/react-native package, which breaks * some of our tests because of duplicate symbols, we are using require syntax @@ -39,3 +39,9 @@ export const firebase = _firebase as FirebaseNamespace; // eslint-disable-next-line import/no-default-export export default firebase; + +declare module '@firebase/component' { + interface ComponentContainer { + getProvider(name: 'app'): Provider; + } +} \ No newline at end of file diff --git a/packages/app/index.ts b/packages/app/index.ts index 2170cce61a0..56a4af83f9e 100644 --- a/packages/app/index.ts +++ b/packages/app/index.ts @@ -15,9 +15,10 @@ * limitations under the License. */ -import { FirebaseNamespace } from '@firebase/app-types'; +import { FirebaseNamespace, FirebaseApp } from '@firebase/app-types'; import { firebase as firebaseNamespace } from './src/firebaseNamespace'; import { isNode, isBrowser } from '@firebase/util'; +import { Provider } from '@firebase/component'; import { logger } from './src/logger'; // Firebase Lite detection @@ -43,7 +44,7 @@ const initializeApp = firebaseNamespace.initializeApp; // TODO: This disable can be removed and the 'ignoreRestArgs' option added to // the no-explicit-any rule when ESlint releases it. // eslint-disable-next-line @typescript-eslint/no-explicit-any -firebaseNamespace.initializeApp = function(...args: any) { +firebaseNamespace.initializeApp = function (...args: any) { // Environment check before initializing app // Do the check in initializeApp, so people have a chance to disable it by setting logLevel // in @firebase/logger @@ -69,3 +70,9 @@ export const firebase = firebaseNamespace; // eslint-disable-next-line import/no-default-export export default firebase; + +declare module '@firebase/component' { + interface ComponentContainer { + getProvider(name: 'app'): Provider; + } +} diff --git a/packages/app/src/firebaseNamespace.ts b/packages/app/src/firebaseNamespace.ts index 65c1056bfd5..ef2528dbe6b 100644 --- a/packages/app/src/firebaseNamespace.ts +++ b/packages/app/src/firebaseNamespace.ts @@ -28,7 +28,7 @@ import { createFirebaseNamespaceCore } from './firebaseNamespaceCore'; * assigned to the 'firebase' global. It may be called multiple times * in unit tests. */ -function createFirebaseNamespace(): FirebaseNamespace { +export function createFirebaseNamespace(): FirebaseNamespace { const namespace = createFirebaseNamespaceCore(FirebaseAppImpl); (namespace as _FirebaseNamespace).INTERNAL = { ...(namespace as _FirebaseNamespace).INTERNAL, diff --git a/packages/app/test/firebaseApp.test.ts b/packages/app/test/firebaseApp.test.ts index 5cd4f80e020..bf55cd2b290 100644 --- a/packages/app/test/firebaseApp.test.ts +++ b/packages/app/test/firebaseApp.test.ts @@ -27,9 +27,10 @@ import { } from '@firebase/app-types/private'; import { createFirebaseNamespace } from '../src/firebaseNamespace'; import { createFirebaseNamespaceLite } from '../src/lite/firebaseNamespaceLite'; -import { assert } from 'chai'; +import { expect } from 'chai'; import { stub } from 'sinon'; - +import { Component, ComponentType } from '@firebase/component'; +import "./setup"; executeFirebaseTests(); executeFirebaseLiteTests(); @@ -42,298 +43,52 @@ function executeFirebaseTests(): void { beforeEach(() => { firebase = createFirebaseNamespace(); }); - it('Register App Hook', done => { - const events = ['create', 'delete']; - let hookEvents = 0; - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'test', - (app: FirebaseApp) => { - return new TestService(app); - }, - undefined, - (event: string, _app: FirebaseApp) => { - assert.equal(event, events[hookEvents]); - hookEvents += 1; - if (hookEvents === events.length) { - done(); - } - } - ); - const app = firebase.initializeApp({}); - // Ensure the hook is called synchronously - assert.equal(hookEvents, 1); - // eslint-disable-next-line @typescript-eslint/no-floating-promises - app.delete(); - }); - it('Only calls createService on first use (per app).', () => { - let registrations = 0; - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'test', - (app: FirebaseApp) => { - registrations += 1; - return new TestService(app); - } - ); - let app = firebase.initializeApp({}); - assert.equal(registrations, 0); - (firebase as any).test(); - assert.equal(registrations, 1); - (firebase as any).test(); - assert.equal(registrations, 1); - (firebase as any).test(app); - assert.equal(registrations, 1); - (app as any).test(); - assert.equal(registrations, 1); - - app = firebase.initializeApp({}, 'second'); - assert.equal(registrations, 1); - (app as any).test(); - assert.equal(registrations, 2); - }); - - it('Will do nothing if registerService is called again with the same name', () => { + it('will do nothing if registerComponent is called again with the same name', () => { const registerStub = stub( (firebase as _FirebaseNamespace).INTERNAL, - 'registerService' + 'registerComponent' ).callThrough(); - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'test', - (app: FirebaseApp) => new TestService(app) - ); - firebase.initializeApp({}); - const serviceNamespace = (firebase as any).test; - - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'test', - (app: FirebaseApp) => new TestService(app) - ); - - const serviceNamespace2 = (firebase as any).test; - assert.strictEqual(serviceNamespace, serviceNamespace2); - assert.doesNotThrow(registerStub); - }); - - it('Can lazy load a service', () => { - let registrations = 0; - - const app1 = firebase.initializeApp({}); - assert.isUndefined((app1 as any).lazyService); - - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'lazyService', - (app: FirebaseApp) => { - registrations += 1; - return new TestService(app); - } - ); - - assert.isDefined((app1 as any).lazyService); - - // Initial service registration happens on first invocation - assert.equal(registrations, 0); - // Verify service has been registered - (firebase as any).lazyService(); - assert.equal(registrations, 1); + const testComponent = createTestComponent('test'); - // Service should only be created once - (firebase as any).lazyService(); - assert.equal(registrations, 1); - - // Service should only be created once... regardless of how you invoke the function - (firebase as any).lazyService(app1); - assert.equal(registrations, 1); - - // Service should already be defined for the second app - const app2 = firebase.initializeApp({}, 'second'); - assert.isDefined((app1 as any).lazyService); - - // Service still should not have registered for the second app - assert.equal(registrations, 1); - - // Service should initialize once called - (app2 as any).lazyService(); - assert.equal(registrations, 2); - }); - - it('Can lazy register App Hook', done => { - const events = ['create', 'delete']; - let hookEvents = 0; - const app = firebase.initializeApp({}); - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'lazyServiceWithHook', - (app: FirebaseApp) => { - return new TestService(app); - }, - undefined, - (event: string, _app: FirebaseApp) => { - assert.equal(event, events[hookEvents]); - hookEvents += 1; - if (hookEvents === events.length) { - done(); - } - } - ); - // Ensure the hook is called synchronously - assert.equal(hookEvents, 1); - // eslint-disable-next-line @typescript-eslint/no-floating-promises - app.delete(); - }); - - it('Can register multiple instances of some services', () => { - // Register Multi Instance Service - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'multiInstance', - (...args) => { - const [app, , instanceIdentifier] = args; - return new TestService(app, instanceIdentifier); - }, - undefined, - undefined, - true - ); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent(testComponent); firebase.initializeApp({}); + const serviceNamespace = (firebase as any).test; - // Capture a given service ref - const service = (firebase.app() as any).multiInstance(); - assert.strictEqual(service, (firebase.app() as any).multiInstance()); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent(testComponent); - // Capture a custom instance service ref - const serviceIdentifier = 'custom instance identifier'; - const service2 = (firebase.app() as any).multiInstance(serviceIdentifier); - assert.strictEqual( - service2, - (firebase.app() as any).multiInstance(serviceIdentifier) - ); + const serviceNamespace2 = (firebase as any).test; - // Ensure that the two services **are not equal** - assert.notStrictEqual( - service.instanceIdentifier, - service2.instanceIdentifier, - '`instanceIdentifier` is not being set correctly' - ); - assert.notStrictEqual(service, service2); - assert.notStrictEqual( - (firebase.app() as any).multiInstance(), - (firebase.app() as any).multiInstance(serviceIdentifier) - ); + expect(serviceNamespace).to.eq(serviceNamespace2); + expect(registerStub).to.have.not.thrown(); }); - it(`Should return the same instance of a service if a service doesn't support multi instance`, () => { - // Register Multi Instance Service - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'singleInstance', - (...args) => { - const [app, , instanceIdentifier] = args; - return new TestService(app, instanceIdentifier); - }, - undefined, - undefined, - false // <-- multi instance flag - ); + it('returns cached service instances', () => { firebase.initializeApp({}); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('test')); - // Capture a given service ref - const serviceIdentifier = 'custom instance identifier'; - const service = (firebase.app() as any).singleInstance(); - const service2 = (firebase.app() as any).singleInstance( - serviceIdentifier - ); - - // Ensure that the two services **are equal** - assert.strictEqual( - service.instanceIdentifier, - service2.instanceIdentifier, - '`instanceIdentifier` is not being set correctly' - ); - assert.strictEqual(service, service2); - }); - - it(`Should pass null to the factory method if using default instance`, () => { - // Register Multi Instance Service - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'testService', - (...args) => { - const [app, , instanceIdentifier] = args; - assert.isUndefined( - instanceIdentifier, - '`instanceIdentifier` is not `undefined`' - ); - return new TestService(app, instanceIdentifier); - } - ); - firebase.initializeApp({}); - }); + const service = (firebase as any).test(); - it(`Should extend INTERNAL per app instance`, () => { - let counter: number = 0; - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'test', - (app: FirebaseApp, extendApp: any) => { - const service = new TestService(app); - (service as any).token = 'tokenFor' + counter++; - extendApp({ - INTERNAL: { - getToken: () => { - return Promise.resolve({ - accessToken: (service as any).token - }); - } - } - }); - return service; - } - ); - // Initialize 2 apps and their corresponding services. - const app = firebase.initializeApp({}); - (app as any).test(); - const app2 = firebase.initializeApp({}, 'app2'); - (app2 as any).test(); - // Confirm extended INTERNAL getToken resolve with the corresponding - // service's value. - return (app as _FirebaseApp).INTERNAL.getToken() - .then(token => { - assert.isNotNull(token); - assert.equal('tokenFor0', token!.accessToken); - return (app2 as _FirebaseApp).INTERNAL.getToken(); - }) - .then(token => { - assert.isNotNull(token); - assert.equal('tokenFor1', token!.accessToken); - }); + expect(service).to.eq((firebase as any).test()); }); - it(`Should create a new instance of a service after removing the existing instance`, () => { + it(`creates a new instance of a service after removing the existing instance`, () => { const app = firebase.initializeApp({}); - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'test', - (app: FirebaseApp) => { - return new TestService(app); - } - ); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('test')); const service = (firebase as any).test(); - assert.equal(service, (firebase as any).test()); + expect(service).to.eq((firebase as any).test()); (app as _FirebaseApp)._removeServiceInstance('test'); - assert.notEqual(service, (firebase as any).test()); + expect(service, (firebase as any).test()); }); - it(`Should create a new instance of a service after removing the existing instance - for service that supports multiple instances`, () => { + it(`creates a new instance of a service after removing the existing instance - for service that supports multiple instances`, () => { const app = firebase.initializeApp({}); - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'multiInstance', - (...args) => { - const [app, , instanceIdentifier] = args; - return new TestService(app, instanceIdentifier); - }, - undefined, - undefined, - true - ); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('multiInstance', true)); // default instance const instance1 = (firebase.app() as any).multiInstance(); @@ -348,12 +103,9 @@ function executeFirebaseTests(): void { ); // default instance should not be changed - assert.equal(instance1, (firebase.app() as any).multiInstance()); + expect(instance1).to.eq((firebase.app() as any).multiInstance()); - assert.notEqual( - instance2, - (firebase.app() as any).multiInstance(serviceIdentifier) - ); + expect(instance2).to.not.eq((firebase.app() as any).multiInstance(serviceIdentifier)); }); }); } @@ -368,27 +120,28 @@ function executeFirebaseLiteTests(): void { firebase = createFirebaseNamespaceLite(); }); - it('should allow Performance service to register', () => { - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'performance', - (app: FirebaseApp) => { - return new TestService(app); - } - ); + it('allows Performance service to register', () => { + (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('performance')); const app = firebase.initializeApp({}); const perf = (app as any).performance(); - assert.isTrue(perf instanceof TestService); + expect(perf).to.be.instanceof(TestService); }); - it('should NOT allow services other than Performance to register', () => { - assert.throws(() => { - (firebase as _FirebaseNamespace).INTERNAL.registerService( - 'test', - (app: FirebaseApp) => { - return new TestService(app); - } - ); - }); + it('allows Installations service to register', () => { + (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('installations')); + const app = firebase.initializeApp({}); + const perf = (app as any).installations(); + expect(perf).to.be.instanceof(TestService); + }); + + it('does NOT allow services other than Performance and installations to register', () => { + expect(() => (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('auth'))).to.throw(); + }); + + it('allows any private component to register', () => { + expect(() => (firebase as _FirebaseNamespace).INTERNAL.registerComponent( + createTestComponent('auth-internal', false, ComponentType.PRIVATE) + )).to.not.throw(); }); }); } @@ -404,68 +157,65 @@ function firebaseAppTests( firebase = firebaseNamespaceFactory(); }); - it('No initial apps.', () => { - assert.equal(firebase.apps.length, 0); + it(' has no initial apps.', () => { + expect(firebase.apps.length).to.eq(0); + }); + + it('Can get app via firebase namespace.', () => { + const app = firebase.initializeApp({}); + expect(app).to.be.not.null; }); - it('Can initialize DEFAULT App.', () => { + it('can initialize DEFAULT App.', () => { const app = firebase.initializeApp({}); - assert.equal(firebase.apps.length, 1); - assert.strictEqual(app, firebase.apps[0]); - assert.equal(app.name, '[DEFAULT]'); - assert.strictEqual(firebase.app(), app); - assert.strictEqual(firebase.app('[DEFAULT]'), app); + expect(firebase.apps.length).to.eq(1); + expect(app).to.eq(firebase.apps[0]); + expect(app.name).to.eq('[DEFAULT]'); + expect(firebase.app()).to.eq(app); + expect(firebase.app('[DEFAULT]')).to.eq(app); }); - it('Can get options of App.', () => { + it('can get options of App.', () => { const options: FirebaseOptions = { projectId: 'projectId' }; const app = firebase.initializeApp(options); - assert.deepEqual(app.options, options); + expect(app.options).to.deep.eq(options); }); - it('Can delete App.', () => { + it('can delete App.', async () => { const app = firebase.initializeApp({}); - assert.equal(firebase.apps.length, 1); - return app.delete().then(() => { - assert.equal(firebase.apps.length, 0); - }); + expect(firebase.apps.length).to.eq(1); + await app.delete(); + expect(firebase.apps.length).to.eq(0); }); - it('Can create named App.', () => { + it('can create named App.', () => { const app = firebase.initializeApp({}, 'my-app'); - assert.equal(firebase.apps.length, 1); - assert.equal(app.name, 'my-app'); - assert.strictEqual(firebase.app('my-app'), app); + expect(firebase.apps.length).to.eq(1); + expect(app.name).to.eq('my-app'); + expect(firebase.app('my-app')).to.eq(app); }); - it('Can create named App and DEFAULT app.', () => { + it('can create named App and DEFAULT app.', () => { firebase.initializeApp({}, 'my-app'); - assert.equal(firebase.apps.length, 1); - firebase.initializeApp({}); - assert.equal(firebase.apps.length, 2); - }); - - it('Can get app via firebase namespace.', () => { + expect(firebase.apps.length).to.eq(1); firebase.initializeApp({}); + expect(firebase.apps.length).to.eq(2); }); - it('Duplicate DEFAULT initialize is an error.', () => { + it('duplicate DEFAULT initialize is an error.', () => { firebase.initializeApp({}); - assert.throws(() => { - firebase.initializeApp({}); - }, /\[DEFAULT\].*exists/i); + expect(() => firebase.initializeApp({})).throws(/\[DEFAULT\].*exists/i); }); - it('Duplicate named App initialize is an error.', () => { + it('duplicate named App initialize is an error.', () => { firebase.initializeApp({}, 'abc'); - assert.throws(() => { - firebase.initializeApp({}, 'abc'); - }, /'abc'.*exists/i); + + expect(() => firebase.initializeApp({}, 'abc')).throws(/'abc'.*exists/i); }); it('automaticDataCollectionEnabled is `false` by default', () => { const app = firebase.initializeApp({}, 'my-app'); - assert.equal(app.automaticDataCollectionEnabled, false); + expect(app.automaticDataCollectionEnabled).to.eq(false); }); it('automaticDataCollectionEnabled can be set via the config object', () => { @@ -473,7 +223,7 @@ function firebaseAppTests( {}, { automaticDataCollectionEnabled: true } ); - assert.equal(app.automaticDataCollectionEnabled, true); + expect(app.automaticDataCollectionEnabled).to.eq(true); }); it('Modifying options object does not change options.', () => { @@ -484,63 +234,53 @@ function firebaseAppTests( firebase.initializeApp(options); options.appId = 'changed'; delete options.measurementId; - assert.deepEqual(firebase.app().options, { + expect(firebase.app().options).to.deep.eq({ appId: 'original', measurementId: 'someId' }); }); - it('Error to use app after it is deleted.', () => { + it('Error to use app after it is deleted.', async () => { const app = firebase.initializeApp({}); - return app.delete().then(() => { - assert.throws(() => { - console.log(app.name); - }, /already.*deleted/); - }); + await app.delete(); + expect(() => console.log(app.name)).throws(/already.*deleted/); }); - it('OK to create same-name app after it is deleted.', () => { + it('OK to create same-name app after it is deleted.', async () => { const app = firebase.initializeApp({}, 'app-name'); - return app.delete().then(() => { - const app2 = firebase.initializeApp({}, 'app-name'); - assert.ok(app !== app2, 'Expect new instance.'); - // But original app id still orphaned. - assert.throws(() => { - console.log(app.name); - }, /already.*deleted/); - }); + await app.delete(); + + const app2 = firebase.initializeApp({}, 'app-name'); + expect(app).to.not.eq(app2, 'Expect new instance.'); + // But original app id still orphaned. + expect(() => console.log(app.name)).throws(/already.*deleted/); }); it('OK to use Object.prototype member names as app name.', () => { const app = firebase.initializeApp({}, 'toString'); - assert.equal(firebase.apps.length, 1); - assert.equal(app.name, 'toString'); - assert.strictEqual(firebase.app('toString'), app); + expect(firebase.apps.length).to.eq(1); + expect(app.name).to.eq('toString'); + expect(firebase.app('toString')).to.eq(app); }); it('Error to get uninitialized app using Object.prototype member name.', () => { - assert.throws(() => { - firebase.app('toString'); - }, /'toString'.*created/i); + expect(() => firebase.app('toString')).throws(/'toString'.*created/i); }); describe('Check for bad app names', () => { const tests = ['', 123, false, null]; for (const data of tests) { it("where name == '" + data + "'", () => { - assert.throws(() => { - firebase.initializeApp({}, data as string); - }, /Illegal app name/i); + expect(() => firebase.initializeApp({}, data as string)).throws(/Illegal app name/i); }); } }); + describe('Check for bad app names, passed as an object', () => { const tests = ['', 123, false, null]; for (const name of tests) { it("where name == '" + name + "'", () => { - assert.throws(() => { - firebase.initializeApp({}, { name: name as string }); - }, /Illegal app name/i); + expect(() => firebase.initializeApp({}, { name: name as string })).throws(/Illegal app name/i); }); } }); @@ -548,7 +288,7 @@ function firebaseAppTests( } class TestService implements FirebaseService { - constructor(private app_: FirebaseApp, public instanceIdentifier?: string) {} + constructor(private app_: FirebaseApp, public instanceIdentifier?: string) { } // TODO(koss): Shouldn't this just be an added method on // the service instance? @@ -562,3 +302,9 @@ class TestService implements FirebaseService { }); } } + +function createTestComponent(name: string, multiInstances = false, type = ComponentType.PUBLIC) { + const component = new Component(name, container => new TestService(container.getProvider('app').getImmediate()!), type); + component.setMultipleInstances(multiInstances); + return component; +} diff --git a/packages/component/index.ts b/packages/component/index.ts index 462a697e6a9..9135e5f5858 100644 --- a/packages/component/index.ts +++ b/packages/component/index.ts @@ -17,3 +17,4 @@ export { Component, ComponentType } from './src/component'; export { ComponentContainer } from './src/component_container'; +export { Provider } from './src/provider'; From 89c31e1e39a0b75a58fd36aec1ab43ae7c80d091 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Thu, 17 Oct 2019 11:21:06 -0700 Subject: [PATCH 13/54] fix node tests --- packages/app/package.json | 4 +++- packages/app/src/firebaseNamespaceCore.ts | 6 ++++-- packages/app/test/firebaseApp.test.ts | 5 +++-- packages/app/test/setup.ts | 2 -- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/app/package.json b/packages/app/package.json index d27f58e01d7..b8c5d32cf97 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -21,7 +21,7 @@ "test": "run-p lint test:browser test:node", "test:browser": "karma start --single-run", "test:browser:debug": "karma start --browsers Chrome --auto-watch", - "test:node": "TS_NODE_CACHE=NO TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha test/**/*.test.* --opts ../../config/mocha.node.opts", + "test:node": "TS_NODE_FILES=true TS_NODE_CACHE=NO TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha test/**/*.test.* --opts ../../config/mocha.node.opts", "prepare": "yarn build" }, "license": "Apache-2.0", @@ -29,6 +29,7 @@ "@firebase/app-types": "0.4.6", "@firebase/util": "0.2.31", "@firebase/logger": "0.1.28", + "@firebase/component": "0.1.0", "tslib": "1.10.0", "dom-storage": "2.1.0", "xmlhttprequest": "1.8.0" @@ -56,6 +57,7 @@ "rollup-plugin-typescript2": "0.24.3", "rollup-plugin-json": "4.0.0", "sinon": "7.5.0", + "sinon-chai": "3.3.0", "source-map-loader": "0.2.4", "ts-loader": "6.2.1", "ts-node": "8.4.1", diff --git a/packages/app/src/firebaseNamespaceCore.ts b/packages/app/src/firebaseNamespaceCore.ts index 385e0a09d5c..ea8252c3095 100644 --- a/packages/app/src/firebaseNamespaceCore.ts +++ b/packages/app/src/firebaseNamespaceCore.ts @@ -170,8 +170,9 @@ export function createFirebaseNamespaceCore( logger.debug( `There were multiple attempts to register component ${componentName}.` ); - // eslint-disable-next-line @typescript-eslint/no-explicit-any + return component.type === ComponentType.PUBLIC + // eslint-disable-next-line @typescript-eslint/no-explicit-any ? (namespace as any)[componentName] : null; } @@ -212,7 +213,7 @@ export function createFirebaseNamespaceCore( // TODO: The eslint disable can be removed and the 'ignoreRestArgs' // option added to the no-explicit-any rule when ESlint releases it. // eslint-disable-next-line @typescript-eslint/no-explicit-any - function(...args: any) { + function (...args: any) { const serviceFxn = this._getService.bind(this, componentName); return serviceFxn.apply( this, @@ -227,6 +228,7 @@ export function createFirebaseNamespaceCore( } return component.type === ComponentType.PUBLIC + // eslint-disable-next-line @typescript-eslint/no-explicit-any ? (namespace as any)[componentName] : null; } diff --git a/packages/app/test/firebaseApp.test.ts b/packages/app/test/firebaseApp.test.ts index bf55cd2b290..4e3035dfe75 100644 --- a/packages/app/test/firebaseApp.test.ts +++ b/packages/app/test/firebaseApp.test.ts @@ -31,6 +31,7 @@ import { expect } from 'chai'; import { stub } from 'sinon'; import { Component, ComponentType } from '@firebase/component'; import "./setup"; + executeFirebaseTests(); executeFirebaseLiteTests(); @@ -303,8 +304,8 @@ class TestService implements FirebaseService { } } -function createTestComponent(name: string, multiInstances = false, type = ComponentType.PUBLIC) { +function createTestComponent(name: string, multiInstances = false, type = ComponentType.PUBLIC): Component { const component = new Component(name, container => new TestService(container.getProvider('app').getImmediate()!), type); component.setMultipleInstances(multiInstances); return component; -} +} \ No newline at end of file diff --git a/packages/app/test/setup.ts b/packages/app/test/setup.ts index 5f3858ba898..e1d5d7c35b7 100644 --- a/packages/app/test/setup.ts +++ b/packages/app/test/setup.ts @@ -16,11 +16,9 @@ */ import { use } from 'chai'; -import * as chaiAsPromised from 'chai-as-promised'; import { restore } from 'sinon'; import * as sinonChai from 'sinon-chai'; -use(chaiAsPromised); use(sinonChai); afterEach(async () => { From 062975da721bee2b8a82f5f4001a4e1b3d51f65c Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Thu, 17 Oct 2019 11:21:18 -0700 Subject: [PATCH 14/54] [AUTOMATED]: Prettier Code Styling --- packages/app-types/index.d.ts | 4 +- packages/app/index.node.ts | 2 +- packages/app/index.rn.ts | 2 +- packages/app/index.ts | 2 +- packages/app/src/firebaseNamespaceCore.ts | 10 ++-- packages/app/test/firebaseApp.test.ts | 72 +++++++++++++++++------ 6 files changed, 63 insertions(+), 29 deletions(-) diff --git a/packages/app-types/index.d.ts b/packages/app-types/index.d.ts index 1dd18812d88..eab147e16c8 100644 --- a/packages/app-types/index.d.ts +++ b/packages/app-types/index.d.ts @@ -1,4 +1,4 @@ -import { Provider } from "@firebase/component"; +import { Provider } from '@firebase/component'; /** * @license @@ -102,4 +102,4 @@ export interface FirebaseNamespace { // The current SDK version. SDK_VERSION: string; -} \ No newline at end of file +} diff --git a/packages/app/index.node.ts b/packages/app/index.node.ts index e7a4c3583ef..28ec06d1ca3 100644 --- a/packages/app/index.node.ts +++ b/packages/app/index.node.ts @@ -44,4 +44,4 @@ declare module '@firebase/component' { interface ComponentContainer { getProvider(name: 'app'): Provider; } -} \ No newline at end of file +} diff --git a/packages/app/index.rn.ts b/packages/app/index.rn.ts index af417f5f05b..698f59dfa9b 100644 --- a/packages/app/index.rn.ts +++ b/packages/app/index.rn.ts @@ -44,4 +44,4 @@ declare module '@firebase/component' { interface ComponentContainer { getProvider(name: 'app'): Provider; } -} \ No newline at end of file +} diff --git a/packages/app/index.ts b/packages/app/index.ts index 56a4af83f9e..62e13fd0bf0 100644 --- a/packages/app/index.ts +++ b/packages/app/index.ts @@ -44,7 +44,7 @@ const initializeApp = firebaseNamespace.initializeApp; // TODO: This disable can be removed and the 'ignoreRestArgs' option added to // the no-explicit-any rule when ESlint releases it. // eslint-disable-next-line @typescript-eslint/no-explicit-any -firebaseNamespace.initializeApp = function (...args: any) { +firebaseNamespace.initializeApp = function(...args: any) { // Environment check before initializing app // Do the check in initializeApp, so people have a chance to disable it by setting logLevel // in @firebase/logger diff --git a/packages/app/src/firebaseNamespaceCore.ts b/packages/app/src/firebaseNamespaceCore.ts index ea8252c3095..607b7497edd 100644 --- a/packages/app/src/firebaseNamespaceCore.ts +++ b/packages/app/src/firebaseNamespaceCore.ts @@ -172,8 +172,8 @@ export function createFirebaseNamespaceCore( ); return component.type === ComponentType.PUBLIC - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ? (namespace as any)[componentName] + ? // eslint-disable-next-line @typescript-eslint/no-explicit-any + (namespace as any)[componentName] : null; } @@ -213,7 +213,7 @@ export function createFirebaseNamespaceCore( // TODO: The eslint disable can be removed and the 'ignoreRestArgs' // option added to the no-explicit-any rule when ESlint releases it. // eslint-disable-next-line @typescript-eslint/no-explicit-any - function (...args: any) { + function(...args: any) { const serviceFxn = this._getService.bind(this, componentName); return serviceFxn.apply( this, @@ -228,8 +228,8 @@ export function createFirebaseNamespaceCore( } return component.type === ComponentType.PUBLIC - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ? (namespace as any)[componentName] + ? // eslint-disable-next-line @typescript-eslint/no-explicit-any + (namespace as any)[componentName] : null; } diff --git a/packages/app/test/firebaseApp.test.ts b/packages/app/test/firebaseApp.test.ts index 4e3035dfe75..671a050ddb7 100644 --- a/packages/app/test/firebaseApp.test.ts +++ b/packages/app/test/firebaseApp.test.ts @@ -30,7 +30,7 @@ import { createFirebaseNamespaceLite } from '../src/lite/firebaseNamespaceLite'; import { expect } from 'chai'; import { stub } from 'sinon'; import { Component, ComponentType } from '@firebase/component'; -import "./setup"; +import './setup'; executeFirebaseTests(); executeFirebaseLiteTests(); @@ -53,11 +53,15 @@ function executeFirebaseTests(): void { const testComponent = createTestComponent('test'); - (firebase as _FirebaseNamespace).INTERNAL.registerComponent(testComponent); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent( + testComponent + ); firebase.initializeApp({}); const serviceNamespace = (firebase as any).test; - (firebase as _FirebaseNamespace).INTERNAL.registerComponent(testComponent); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent( + testComponent + ); const serviceNamespace2 = (firebase as any).test; @@ -67,7 +71,9 @@ function executeFirebaseTests(): void { it('returns cached service instances', () => { firebase.initializeApp({}); - (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('test')); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent( + createTestComponent('test') + ); const service = (firebase as any).test(); @@ -76,7 +82,9 @@ function executeFirebaseTests(): void { it(`creates a new instance of a service after removing the existing instance`, () => { const app = firebase.initializeApp({}); - (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('test')); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent( + createTestComponent('test') + ); const service = (firebase as any).test(); @@ -89,7 +97,9 @@ function executeFirebaseTests(): void { it(`creates a new instance of a service after removing the existing instance - for service that supports multiple instances`, () => { const app = firebase.initializeApp({}); - (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('multiInstance', true)); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent( + createTestComponent('multiInstance', true) + ); // default instance const instance1 = (firebase.app() as any).multiInstance(); @@ -106,7 +116,9 @@ function executeFirebaseTests(): void { // default instance should not be changed expect(instance1).to.eq((firebase.app() as any).multiInstance()); - expect(instance2).to.not.eq((firebase.app() as any).multiInstance(serviceIdentifier)); + expect(instance2).to.not.eq( + (firebase.app() as any).multiInstance(serviceIdentifier) + ); }); }); } @@ -122,27 +134,37 @@ function executeFirebaseLiteTests(): void { }); it('allows Performance service to register', () => { - (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('performance')); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent( + createTestComponent('performance') + ); const app = firebase.initializeApp({}); const perf = (app as any).performance(); expect(perf).to.be.instanceof(TestService); }); it('allows Installations service to register', () => { - (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('installations')); + (firebase as _FirebaseNamespace).INTERNAL.registerComponent( + createTestComponent('installations') + ); const app = firebase.initializeApp({}); const perf = (app as any).installations(); expect(perf).to.be.instanceof(TestService); }); it('does NOT allow services other than Performance and installations to register', () => { - expect(() => (firebase as _FirebaseNamespace).INTERNAL.registerComponent(createTestComponent('auth'))).to.throw(); + expect(() => + (firebase as _FirebaseNamespace).INTERNAL.registerComponent( + createTestComponent('auth') + ) + ).to.throw(); }); it('allows any private component to register', () => { - expect(() => (firebase as _FirebaseNamespace).INTERNAL.registerComponent( - createTestComponent('auth-internal', false, ComponentType.PRIVATE) - )).to.not.throw(); + expect(() => + (firebase as _FirebaseNamespace).INTERNAL.registerComponent( + createTestComponent('auth-internal', false, ComponentType.PRIVATE) + ) + ).to.not.throw(); }); }); } @@ -272,7 +294,9 @@ function firebaseAppTests( const tests = ['', 123, false, null]; for (const data of tests) { it("where name == '" + data + "'", () => { - expect(() => firebase.initializeApp({}, data as string)).throws(/Illegal app name/i); + expect(() => firebase.initializeApp({}, data as string)).throws( + /Illegal app name/i + ); }); } }); @@ -281,7 +305,9 @@ function firebaseAppTests( const tests = ['', 123, false, null]; for (const name of tests) { it("where name == '" + name + "'", () => { - expect(() => firebase.initializeApp({}, { name: name as string })).throws(/Illegal app name/i); + expect(() => + firebase.initializeApp({}, { name: name as string }) + ).throws(/Illegal app name/i); }); } }); @@ -289,7 +315,7 @@ function firebaseAppTests( } class TestService implements FirebaseService { - constructor(private app_: FirebaseApp, public instanceIdentifier?: string) { } + constructor(private app_: FirebaseApp, public instanceIdentifier?: string) {} // TODO(koss): Shouldn't this just be an added method on // the service instance? @@ -304,8 +330,16 @@ class TestService implements FirebaseService { } } -function createTestComponent(name: string, multiInstances = false, type = ComponentType.PUBLIC): Component { - const component = new Component(name, container => new TestService(container.getProvider('app').getImmediate()!), type); +function createTestComponent( + name: string, + multiInstances = false, + type = ComponentType.PUBLIC +): Component { + const component = new Component( + name, + container => new TestService(container.getProvider('app').getImmediate()!), + type + ); component.setMultipleInstances(multiInstances); return component; -} \ No newline at end of file +} From fb6ea298fd21ef0e1ff9a8218e73db3426adbcbf Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 21 Oct 2019 13:57:54 -0700 Subject: [PATCH 15/54] add comments for ComponentType --- packages/component/src/component.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/component/src/component.ts b/packages/component/src/component.ts index 2f7d33f8795..fa84f968f96 100644 --- a/packages/component/src/component.ts +++ b/packages/component/src/component.ts @@ -53,6 +53,14 @@ export class Component { } } +/** + * PUBLIC: A public component provides a set of public APIs to customers. A service namespace will be patched + * onto `firebase` namespace. Assume the component name is `test`, customers will be able + * to get the service by calling `firebase.test()` or `app.test()` where `app` is a `FirebaseApp` instance. + * + * PRIVATE: A private component provides a set of private APIs that are used internally by other + * Firebase SDKs. No serivce namespace is created in `firebase` namespace and customers have no way to get them. + */ export enum ComponentType { PUBLIC, PRIVATE From 766fb5885d5d77c61a921c31e25a0877332e8e79 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 21 Oct 2019 13:58:03 -0700 Subject: [PATCH 16/54] [AUTOMATED]: Prettier Code Styling --- packages/component/src/component.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/component/src/component.ts b/packages/component/src/component.ts index fa84f968f96..ad180f0aab4 100644 --- a/packages/component/src/component.ts +++ b/packages/component/src/component.ts @@ -54,10 +54,10 @@ export class Component { } /** - * PUBLIC: A public component provides a set of public APIs to customers. A service namespace will be patched + * PUBLIC: A public component provides a set of public APIs to customers. A service namespace will be patched * onto `firebase` namespace. Assume the component name is `test`, customers will be able * to get the service by calling `firebase.test()` or `app.test()` where `app` is a `FirebaseApp` instance. - * + * * PRIVATE: A private component provides a set of private APIs that are used internally by other * Firebase SDKs. No serivce namespace is created in `firebase` namespace and customers have no way to get them. */ From b692476708f96bb94d4797bc7697a23cf74c7c53 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 21 Oct 2019 15:53:28 -0700 Subject: [PATCH 17/54] pass Component directly into Providers --- packages/component/src/component_container.ts | 19 +------ .../src/component_containers.test.ts | 54 ++++-------------- packages/component/src/provider.test.ts | 52 ++++++++++-------- packages/component/src/provider.ts | 55 ++++++++++++------- packages/component/test/util.ts | 15 ++++- 5 files changed, 90 insertions(+), 105 deletions(-) diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 3fc08479529..3942e02fd30 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -21,27 +21,18 @@ import { InstantiationMode } from './types'; export class ComponentContainer { private providers = new Map(); - private components = new Map(); constructor(private name: string) {} addComponent(component: Component): void { - if (this.components.has(component.name)) { + const provider = this.getProvider(component.name); + if (provider.isComponentSet()) { throw new Error( `Component ${component.name} has already been registered with ${this.name}` ); } - this.components.set(component.name, component); - - const provider = this.getProvider(component.name); - - const isEager = this.isComponentEager(component); - provider.provideFactory( - component.instanceFactory, - component.multipleInstances, - isEager - ); + provider.setComponent(component); } getProvider(name: string): Provider { @@ -59,8 +50,4 @@ export class ComponentContainer { getProviders(): Provider[] { return Array.from(this.providers, entry => entry[1]); } - - private isComponentEager(component: Component): boolean { - return component.instantiationMode === InstantiationMode.EAGER; - } } diff --git a/packages/component/src/component_containers.test.ts b/packages/component/src/component_containers.test.ts index 92ca1327daf..6953931a65c 100644 --- a/packages/component/src/component_containers.test.ts +++ b/packages/component/src/component_containers.test.ts @@ -19,20 +19,11 @@ import { expect } from 'chai'; import { stub } from 'sinon'; import { ComponentContainer } from './component_container'; import '../test/setup'; -import { Component, ComponentType } from './component'; import { Provider } from './provider'; import { InstantiationMode } from './types'; import { DEFAULT_ENTRY_NAME } from './contants'; +import { getFakeComponent } from '../test/util'; -function getFakeComponent( - name: string, - instantiationMode: InstantiationMode, - multipleInstances: boolean = false -): Component { - return new Component(name, () => ({ fire: true }), ComponentType.PRIVATE) - .setInstantiationMode(instantiationMode) - .setMultipleInstances(multipleInstances); -} describe('Component Container', () => { let container: ComponentContainer; @@ -51,46 +42,21 @@ describe('Component Container', () => { expect(provider1).to.eq(provider2); }); - it('calls provideFactory with eager flag set to true when registering an EAGER component', () => { + it('calls setComponent() on provider with the same name when registering a component', () => { const provider = container.getProvider('fireball'); - const provideFactoryStub = stub(provider, 'provideFactory').callThrough(); - const component = getFakeComponent('fireball', InstantiationMode.EAGER); + const setComponentStub = stub(provider, 'setComponent').callThrough(); + const component = getFakeComponent('fireball', ()=>({}), true, InstantiationMode.EAGER); container.addComponent(component); - expect(provideFactoryStub).has.been.calledWith( - component.instanceFactory, - false, - true - ); + expect(setComponentStub).has.been.calledWith(component); }); - it('calls provideFactory with eager flag set to false when registering a LAZY component', () => { - const provider = container.getProvider('fireball'); - const provideFactoryStub = stub(provider, 'provideFactory').callThrough(); - const component = getFakeComponent('fireball', InstantiationMode.LAZY); - container.addComponent(component); + it('throws when registering multiple components with the same name', () => { + const component1 = getFakeComponent('fireball', ()=>({}), true, InstantiationMode.EAGER); + const component2 = getFakeComponent('fireball', ()=>({ test: true }), false, InstantiationMode.LAZY); - expect(provideFactoryStub).has.been.calledWith( - component.instanceFactory, - false, - false - ); - }); - - it('injects service factory into the Provider with the correct multipleInstances flag', () => { - const provider = container.getProvider('multiple'); - const provideFactoryStub = stub(provider, 'provideFactory').callThrough(); - const component = getFakeComponent( - 'multiple', - InstantiationMode.LAZY, - true - ); - container.addComponent(component); + expect(() => container.addComponent(component1)).to.not.throw(); + expect(() => container.addComponent(component2)).to.throw(/Component fireball has already been registered with/); - expect(provideFactoryStub).has.been.calledWith( - component.instanceFactory, - true, - false - ); }); }); diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index ac9cc6d564f..b6ccdc17b42 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -20,21 +20,27 @@ import { fake, SinonSpy } from 'sinon'; import { ComponentContainer } from './component_container'; import { FirebaseService } from '@firebase/app-types/private'; import { Provider } from './provider'; -import { getFakeApp } from '../test/util'; +import { getFakeApp, getFakeComponent } from '../test/util'; import '../test/setup'; +import { InstantiationMode } from './types'; describe('Provider', () => { let provider: Provider; beforeEach(() => { - provider = new Provider('spider-queen', new ComponentContainer('test')); + provider = new Provider('test', new ComponentContainer('test-container')); + }); + + it('throws if setComponent() is called with a component with a different name than the provider name', () => { + expect(() => provider.setComponent(getFakeComponent('not a test', () => ({})))).to.throw(/^Mismatching Component/); + expect(() => provider.setComponent(getFakeComponent('test', () =>({})))).to.not.throw(); }); describe('Provider (multipleInstances = false)', () => { describe('getImmediate()', () => { it('throws if the service is not available', () => { expect(provider.getImmediate.bind(provider)).to.throw( - 'Service spider-queen is not available' + 'Service test is not available' ); }); @@ -45,19 +51,19 @@ describe('Provider', () => { }); it('returns the service instance synchronously', () => { - provider.provideFactory(() => ({ test: true })); + provider.setComponent(getFakeComponent('test', () => ({ test: true }))); expect(provider.getImmediate()).to.deep.equal({ test: true }); }); it('returns the cached service instance', () => { - provider.provideFactory(() => ({ test: true })); + provider.setComponent(getFakeComponent('test', () => ({ test: true }))); const service1 = provider.getImmediate(); const service2 = provider.getImmediate(); expect(service1).to.equal(service2); }); it('ignores parameter identifier and return the default service', () => { - provider.provideFactory(() => ({ test: true })); + provider.setComponent(getFakeComponent('test', () => ({ test: true }))); const defaultService = provider.getImmediate(); expect(provider.getImmediate('spider1')).to.equal(defaultService); expect(provider.getImmediate('spider2')).to.equal(defaultService); @@ -66,12 +72,12 @@ describe('Provider', () => { describe('get()', () => { it('get the service instance asynchronouly', async () => { - provider.provideFactory(() => ({ test: true })); + provider.setComponent(getFakeComponent('test', () => ({ test: true }))); await expect(provider.get()).to.eventually.deep.equal({ test: true }); }); it('ignore parameter identifier and return the default service instance asyn', async () => { - provider.provideFactory(() => ({ test: true })); + provider.setComponent(getFakeComponent('test', () => ({ test: true }))); const defaultService = provider.getImmediate(); await expect(provider.get('spider1')).to.eventually.equal( defaultService @@ -88,7 +94,7 @@ describe('Provider', () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.get(); - provider.provideFactory(() => ({}), false, true); + provider.setComponent(getFakeComponent('test', () => ({}), false, InstantiationMode.EAGER)); expect((provider as any).instances.size).to.equal(1); }); @@ -97,17 +103,17 @@ describe('Provider', () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.get(); - provider.provideFactory(() => ({})); + provider.setComponent(getFakeComponent('test', () => ({}))); expect((provider as any).instances.size).to.equal(1); }); it('instantiates the service if there is no pending promise and the service is eager', () => { - provider.provideFactory(() => ({}), false, true); + provider.setComponent(getFakeComponent('test', () => ({}), false, InstantiationMode.EAGER)); expect((provider as any).instances.size).to.equal(1); }); it('does NOT instantiate the service if there is no pending promise and the service is not eager', () => { - provider.provideFactory(() => ({})); + provider.setComponent(getFakeComponent('test', () => ({}))); expect((provider as any).instances.size).to.equal(0); }); @@ -116,7 +122,7 @@ describe('Provider', () => { const promise1 = provider.get('name1'); const promise2 = provider.get('name2'); - provider.provideFactory(() => ({})); + provider.setComponent(getFakeComponent('test', () => ({}))); expect((provider as any).instances.size).to.equal(1); const defaultService = provider.getImmediate(); @@ -137,7 +143,7 @@ describe('Provider', () => { }; // provide factory and create a service instance - provider.provideFactory(() => myService, false, true); + provider.setComponent(getFakeComponent('test', () => myService, false, InstantiationMode.EAGER)); // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.delete(); @@ -148,7 +154,7 @@ describe('Provider', () => { describe('clearCache()', () => { it('removes the service instance from cache', () => { - provider.provideFactory(() => ({})); + provider.setComponent(getFakeComponent('test', () => ({}))); // create serviec instance const instance = provider.getImmediate(); expect((provider as any).instances.size).to.equal(1); @@ -176,7 +182,7 @@ describe('Provider', () => { }); it('returns different service instances for different identifiers synchronously', () => { - provider.provideFactory(() => ({ test: true }), true); + provider.setComponent(getFakeComponent('test', () => ({ test: true }), true)); const defaultService = provider.getImmediate(); const service1 = provider.getImmediate('guardian'); const service2 = provider.getImmediate('servant'); @@ -192,7 +198,7 @@ describe('Provider', () => { describe('get(identifier)', () => { it('returns different service instances for different identifiers asynchronouly', async () => { - provider.provideFactory(() => ({ test: true }), true); + provider.setComponent(getFakeComponent('test', () =>({ test: true }), true)); const defaultService = await provider.get(); const service1 = await provider.get('name1'); @@ -216,13 +222,13 @@ describe('Provider', () => { provider.get('name2'); /* eslint-enable @typescript-eslint/no-floating-promises */ - provider.provideFactory(() => ({ test: true }), true); + provider.setComponent(getFakeComponent('test', () =>({ test: true }), true)); expect((provider as any).instances.size).to.equal(3); }); it('instantiates the default service if there is no pending promise and the service is eager', () => { - provider.provideFactory(() => ({ test: true }), true, true); + provider.setComponent(getFakeComponent('test', () =>({ test: true }), true, InstantiationMode.EAGER)); expect((provider as any).instances.size).to.equal(1); }); @@ -230,7 +236,7 @@ describe('Provider', () => { but not for the default identifer and the service is eager`, () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.get('name1'); - provider.provideFactory(() => ({ test: true }), true, true); + provider.setComponent(getFakeComponent('test', () =>({ test: true }), true, InstantiationMode.EAGER)); expect((provider as any).instances.size).to.equal(2); }); @@ -252,7 +258,7 @@ describe('Provider', () => { } // provide factory that produces mulitpleInstances - provider.provideFactory(getService, true); + provider.setComponent(getFakeComponent('test', getService, true)); // create 2 service instances with different names provider.getImmediate('instance1'); @@ -269,7 +275,7 @@ describe('Provider', () => { }); describe('clearCache()', () => { it('returns new service instances sync after cache is cleared', () => { - provider.provideFactory(() => ({}), true); + provider.setComponent(getFakeComponent('test', () => ({}), true)); // create serviec instances with different identifiers const defaultInstance = provider.getImmediate(); const instance1 = provider.getImmediate('instance1'); @@ -292,7 +298,7 @@ describe('Provider', () => { }); it('returns new services asynchronously after cache is cleared', async () => { - provider.provideFactory(() => ({}), true); + provider.setComponent(getFakeComponent('test', () => ({}), true)); // create serviec instances with different identifiers const defaultInstance = await provider.get(); const instance1 = await provider.get('instance1'); diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 51da130cfa9..569506e2174 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -18,18 +18,18 @@ import { Deferred } from '@firebase/util'; import { ComponentContainer } from './component_container'; import { DEFAULT_ENTRY_NAME } from './contants'; -import { InstanceFactory } from './types'; +import { InstantiationMode } from './types'; +import { Component } from './component'; /** - * Provider for interface of type T, e.g. Auth, RC + * Provider for instance of type T, e.g. Auth, RC */ export class Provider { - private factory: InstanceFactory | null = null; - private multipleInstances: boolean = true; // we assume multiple instances are supported by default + private component: Component | null = null; private instances: Map = new Map(); private instancesDeferred: Map> = new Map(); - constructor(private name: string, private container: ComponentContainer) {} + constructor(private name: string, private container: ComponentContainer) { } get(identifier: string = DEFAULT_ENTRY_NAME): Promise { // if multipleInstances is not supported, use the default name @@ -67,20 +67,21 @@ export class Provider { return instance; } - provideFactory( - factory: InstanceFactory, - multipleInstances: boolean = false, - eager: boolean = false + setComponent( + component: Component ): void { - if (this.factory) { - throw Error(`Service factory for ${this.name} has already been provided`); + + if (component.name !== this.name) { + throw Error(`Mismatching Component ${component.name} for Provider ${this.name}.`); } - this.factory = factory; - this.multipleInstances = multipleInstances; + if (this.component) { + throw Error(`Component for ${this.name} has already been provided`); + } + this.component = component; // if the service is eager, initialize the default instance - if (eager) { + if (isComponentEager(component)) { this.getOrInitializeService(DEFAULT_ENTRY_NAME); } @@ -94,11 +95,11 @@ export class Provider { const normalizedIdentifier = this.normalizeInstanceIdentifier( instanceIdentifier ); - const instance = this.getOrInitializeService(normalizedIdentifier); - if (instance) { - instanceDeferred.resolve(instance); - } + // `getOrInitializeService()` should always return a valid instance since a component is guaranteed. use ! to make typescript happy. + const instance = this.getOrInitializeService(normalizedIdentifier)!; + + instanceDeferred.resolve(instance); } } @@ -120,10 +121,14 @@ export class Provider { ); } + isComponentSet(): boolean { + return this.component != null; + } + private getOrInitializeService(identifier: string): T | null { let instance = this.instances.get(identifier); - if (!instance && this.factory) { - instance = this.factory( + if (!instance && this.component) { + instance = this.component.instanceFactory( this.container, normalizeIdentifierForFactory(identifier) ); @@ -134,7 +139,11 @@ export class Provider { } private normalizeInstanceIdentifier(identifier: string): string { - return this.multipleInstances ? identifier : DEFAULT_ENTRY_NAME; + if (this.component) { + return this.component.multipleInstances ? identifier : DEFAULT_ENTRY_NAME; + } else { + return identifier; // assume multiple instances are supported before the component is provided. + } } } @@ -142,3 +151,7 @@ export class Provider { function normalizeIdentifierForFactory(identifier: string): string | undefined { return identifier === DEFAULT_ENTRY_NAME ? undefined : identifier; } + +function isComponentEager(component: Component): boolean { + return component.instantiationMode === InstantiationMode.EAGER; +} diff --git a/packages/component/test/util.ts b/packages/component/test/util.ts index 4a217a764a7..f399d40daed 100644 --- a/packages/component/test/util.ts +++ b/packages/component/test/util.ts @@ -16,6 +16,8 @@ */ import { DEFAULT_ENTRY_NAME } from '../src/contants'; import { FirebaseApp } from '@firebase/app-types'; +import { InstanceFactory, InstantiationMode } from '../src/types'; +import { Component, ComponentType } from '../src/component'; export function getFakeApp(appName: string = DEFAULT_ENTRY_NAME): FirebaseApp { return { @@ -30,6 +32,17 @@ export function getFakeApp(appName: string = DEFAULT_ENTRY_NAME): FirebaseApp { appId: '1:777777777777:web:d93b5ca1475efe57' }, automaticDataCollectionEnabled: true, - delete: async () => {} + delete: async () => { } }; } + +export function getFakeComponent( + name: string, + factory: InstanceFactory, + multipleInstance: boolean = false, + instantiationMode = InstantiationMode.LAZY +) { + return new Component(name, factory, ComponentType.PUBLIC) + .setMultipleInstances(multipleInstance) + .setInstantiationMode(instantiationMode); +} \ No newline at end of file From 2b7878876bd3d24fb5ea2e8f511285b1de07fb5f Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 21 Oct 2019 15:53:36 -0700 Subject: [PATCH 18/54] [AUTOMATED]: Prettier Code Styling --- .../src/component_containers.test.ts | 27 +++++++-- packages/component/src/provider.test.ts | 55 +++++++++++++++---- packages/component/src/provider.ts | 13 ++--- packages/component/test/util.ts | 4 +- 4 files changed, 74 insertions(+), 25 deletions(-) diff --git a/packages/component/src/component_containers.test.ts b/packages/component/src/component_containers.test.ts index 6953931a65c..08c6a807536 100644 --- a/packages/component/src/component_containers.test.ts +++ b/packages/component/src/component_containers.test.ts @@ -24,7 +24,6 @@ import { InstantiationMode } from './types'; import { DEFAULT_ENTRY_NAME } from './contants'; import { getFakeComponent } from '../test/util'; - describe('Component Container', () => { let container: ComponentContainer; beforeEach(() => { @@ -45,18 +44,34 @@ describe('Component Container', () => { it('calls setComponent() on provider with the same name when registering a component', () => { const provider = container.getProvider('fireball'); const setComponentStub = stub(provider, 'setComponent').callThrough(); - const component = getFakeComponent('fireball', ()=>({}), true, InstantiationMode.EAGER); + const component = getFakeComponent( + 'fireball', + () => ({}), + true, + InstantiationMode.EAGER + ); container.addComponent(component); expect(setComponentStub).has.been.calledWith(component); }); it('throws when registering multiple components with the same name', () => { - const component1 = getFakeComponent('fireball', ()=>({}), true, InstantiationMode.EAGER); - const component2 = getFakeComponent('fireball', ()=>({ test: true }), false, InstantiationMode.LAZY); + const component1 = getFakeComponent( + 'fireball', + () => ({}), + true, + InstantiationMode.EAGER + ); + const component2 = getFakeComponent( + 'fireball', + () => ({ test: true }), + false, + InstantiationMode.LAZY + ); expect(() => container.addComponent(component1)).to.not.throw(); - expect(() => container.addComponent(component2)).to.throw(/Component fireball has already been registered with/); - + expect(() => container.addComponent(component2)).to.throw( + /Component fireball has already been registered with/ + ); }); }); diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index b6ccdc17b42..b16678e71da 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -32,8 +32,12 @@ describe('Provider', () => { }); it('throws if setComponent() is called with a component with a different name than the provider name', () => { - expect(() => provider.setComponent(getFakeComponent('not a test', () => ({})))).to.throw(/^Mismatching Component/); - expect(() => provider.setComponent(getFakeComponent('test', () =>({})))).to.not.throw(); + expect(() => + provider.setComponent(getFakeComponent('not a test', () => ({}))) + ).to.throw(/^Mismatching Component/); + expect(() => + provider.setComponent(getFakeComponent('test', () => ({}))) + ).to.not.throw(); }); describe('Provider (multipleInstances = false)', () => { @@ -94,7 +98,9 @@ describe('Provider', () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.get(); - provider.setComponent(getFakeComponent('test', () => ({}), false, InstantiationMode.EAGER)); + provider.setComponent( + getFakeComponent('test', () => ({}), false, InstantiationMode.EAGER) + ); expect((provider as any).instances.size).to.equal(1); }); @@ -108,7 +114,9 @@ describe('Provider', () => { }); it('instantiates the service if there is no pending promise and the service is eager', () => { - provider.setComponent(getFakeComponent('test', () => ({}), false, InstantiationMode.EAGER)); + provider.setComponent( + getFakeComponent('test', () => ({}), false, InstantiationMode.EAGER) + ); expect((provider as any).instances.size).to.equal(1); }); @@ -143,7 +151,14 @@ describe('Provider', () => { }; // provide factory and create a service instance - provider.setComponent(getFakeComponent('test', () => myService, false, InstantiationMode.EAGER)); + provider.setComponent( + getFakeComponent( + 'test', + () => myService, + false, + InstantiationMode.EAGER + ) + ); // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.delete(); @@ -182,7 +197,9 @@ describe('Provider', () => { }); it('returns different service instances for different identifiers synchronously', () => { - provider.setComponent(getFakeComponent('test', () => ({ test: true }), true)); + provider.setComponent( + getFakeComponent('test', () => ({ test: true }), true) + ); const defaultService = provider.getImmediate(); const service1 = provider.getImmediate('guardian'); const service2 = provider.getImmediate('servant'); @@ -198,7 +215,9 @@ describe('Provider', () => { describe('get(identifier)', () => { it('returns different service instances for different identifiers asynchronouly', async () => { - provider.setComponent(getFakeComponent('test', () =>({ test: true }), true)); + provider.setComponent( + getFakeComponent('test', () => ({ test: true }), true) + ); const defaultService = await provider.get(); const service1 = await provider.get('name1'); @@ -222,13 +241,22 @@ describe('Provider', () => { provider.get('name2'); /* eslint-enable @typescript-eslint/no-floating-promises */ - provider.setComponent(getFakeComponent('test', () =>({ test: true }), true)); + provider.setComponent( + getFakeComponent('test', () => ({ test: true }), true) + ); expect((provider as any).instances.size).to.equal(3); }); it('instantiates the default service if there is no pending promise and the service is eager', () => { - provider.setComponent(getFakeComponent('test', () =>({ test: true }), true, InstantiationMode.EAGER)); + provider.setComponent( + getFakeComponent( + 'test', + () => ({ test: true }), + true, + InstantiationMode.EAGER + ) + ); expect((provider as any).instances.size).to.equal(1); }); @@ -236,7 +264,14 @@ describe('Provider', () => { but not for the default identifer and the service is eager`, () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.get('name1'); - provider.setComponent(getFakeComponent('test', () =>({ test: true }), true, InstantiationMode.EAGER)); + provider.setComponent( + getFakeComponent( + 'test', + () => ({ test: true }), + true, + InstantiationMode.EAGER + ) + ); expect((provider as any).instances.size).to.equal(2); }); diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 569506e2174..cfa6cbd10fd 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -29,7 +29,7 @@ export class Provider { private instances: Map = new Map(); private instancesDeferred: Map> = new Map(); - constructor(private name: string, private container: ComponentContainer) { } + constructor(private name: string, private container: ComponentContainer) {} get(identifier: string = DEFAULT_ENTRY_NAME): Promise { // if multipleInstances is not supported, use the default name @@ -67,12 +67,11 @@ export class Provider { return instance; } - setComponent( - component: Component - ): void { - + setComponent(component: Component): void { if (component.name !== this.name) { - throw Error(`Mismatching Component ${component.name} for Provider ${this.name}.`); + throw Error( + `Mismatching Component ${component.name} for Provider ${this.name}.` + ); } if (this.component) { @@ -96,7 +95,7 @@ export class Provider { instanceIdentifier ); - // `getOrInitializeService()` should always return a valid instance since a component is guaranteed. use ! to make typescript happy. + // `getOrInitializeService()` should always return a valid instance since a component is guaranteed. use ! to make typescript happy. const instance = this.getOrInitializeService(normalizedIdentifier)!; instanceDeferred.resolve(instance); diff --git a/packages/component/test/util.ts b/packages/component/test/util.ts index f399d40daed..6805db39151 100644 --- a/packages/component/test/util.ts +++ b/packages/component/test/util.ts @@ -32,7 +32,7 @@ export function getFakeApp(appName: string = DEFAULT_ENTRY_NAME): FirebaseApp { appId: '1:777777777777:web:d93b5ca1475efe57' }, automaticDataCollectionEnabled: true, - delete: async () => { } + delete: async () => {} }; } @@ -45,4 +45,4 @@ export function getFakeComponent( return new Component(name, factory, ComponentType.PUBLIC) .setMultipleInstances(multipleInstance) .setInstantiationMode(instantiationMode); -} \ No newline at end of file +} From 6f69e1c6166c479746dd3c988ef172e9934526d1 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 21 Oct 2019 16:03:06 -0700 Subject: [PATCH 19/54] correct spellings --- ...component_containers.test.ts => component_container.test.ts} | 2 +- packages/component/src/{contants.ts => constants.ts} | 0 packages/component/src/provider.ts | 2 +- packages/component/test/util.ts | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename packages/component/src/{component_containers.test.ts => component_container.test.ts} (97%) rename packages/component/src/{contants.ts => constants.ts} (100%) diff --git a/packages/component/src/component_containers.test.ts b/packages/component/src/component_container.test.ts similarity index 97% rename from packages/component/src/component_containers.test.ts rename to packages/component/src/component_container.test.ts index 08c6a807536..07ada959bc4 100644 --- a/packages/component/src/component_containers.test.ts +++ b/packages/component/src/component_container.test.ts @@ -21,7 +21,7 @@ import { ComponentContainer } from './component_container'; import '../test/setup'; import { Provider } from './provider'; import { InstantiationMode } from './types'; -import { DEFAULT_ENTRY_NAME } from './contants'; +import { DEFAULT_ENTRY_NAME } from './constants'; import { getFakeComponent } from '../test/util'; describe('Component Container', () => { diff --git a/packages/component/src/contants.ts b/packages/component/src/constants.ts similarity index 100% rename from packages/component/src/contants.ts rename to packages/component/src/constants.ts diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index cfa6cbd10fd..83d60427913 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -17,7 +17,7 @@ import { Deferred } from '@firebase/util'; import { ComponentContainer } from './component_container'; -import { DEFAULT_ENTRY_NAME } from './contants'; +import { DEFAULT_ENTRY_NAME } from './constants'; import { InstantiationMode } from './types'; import { Component } from './component'; diff --git a/packages/component/test/util.ts b/packages/component/test/util.ts index 6805db39151..0854bab88c3 100644 --- a/packages/component/test/util.ts +++ b/packages/component/test/util.ts @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { DEFAULT_ENTRY_NAME } from '../src/contants'; +import { DEFAULT_ENTRY_NAME } from '../src/constants'; import { FirebaseApp } from '@firebase/app-types'; import { InstanceFactory, InstantiationMode } from '../src/types'; import { Component, ComponentType } from '../src/component'; From f60cdd24836620320ba9768b2e392a7c6d4398cd Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 21 Oct 2019 16:05:50 -0700 Subject: [PATCH 20/54] update readme --- packages/component/.npmignore | 1 - packages/component/README.md | 33 ++++++++++++++++++++------------- 2 files changed, 20 insertions(+), 14 deletions(-) delete mode 100644 packages/component/.npmignore diff --git a/packages/component/.npmignore b/packages/component/.npmignore deleted file mode 100644 index 6de0b6d2896..00000000000 --- a/packages/component/.npmignore +++ /dev/null @@ -1 +0,0 @@ -# This file is left intentionally blank \ No newline at end of file diff --git a/packages/component/README.md b/packages/component/README.md index ee3853f11b5..cade170b435 100644 --- a/packages/component/README.md +++ b/packages/component/README.md @@ -1,13 +1,20 @@ -# @firebase/template - -This package can be used as a template for anyone creating new packages in the -Firebase JS SDK. It will give you a couple things OOTB: - -- **Typescript Support:** Your code should be written in TS to be consistent - with the rest of the SDK. -- **Isomorphic Testing/Coverage:** Your tests will be run in both Node.js and - Browser environments and coverage from both, collected. -- **Links to all of the other packages:** Should your new package need to take - a dependency on any of the other packages in this monorepo (e.g. - `@firebase/app`, `@firebase/util`, etc), all those dependencies are already - set up, you can just remove the ones you don't need. +# @firebase/component + +_NOTE: This is specifically tailored for Firebase JS SDK usage, if you are not a +member of the Firebase team, please avoid using this package_ + +## Installation + +You can install this wrapper by running the following in your project: + +```bash +$ npm install @firebase/component +``` + +## Usage + +**ES Modules** + +```javascript +import { Component } from '@firebase/component'; +``` From f32b11073b60d6e4b0c1922e88ab7497fc59fa07 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 21 Oct 2019 16:13:25 -0700 Subject: [PATCH 21/54] fix lint issue --- packages/component/src/component_container.ts | 1 - packages/component/test/util.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 3942e02fd30..8e9576fe7d4 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -17,7 +17,6 @@ import { Provider } from './provider'; import { Component } from './component'; -import { InstantiationMode } from './types'; export class ComponentContainer { private providers = new Map(); diff --git a/packages/component/test/util.ts b/packages/component/test/util.ts index 0854bab88c3..c366d9a0727 100644 --- a/packages/component/test/util.ts +++ b/packages/component/test/util.ts @@ -41,7 +41,7 @@ export function getFakeComponent( factory: InstanceFactory, multipleInstance: boolean = false, instantiationMode = InstantiationMode.LAZY -) { +): Component { return new Component(name, factory, ComponentType.PUBLIC) .setMultipleInstances(multipleInstance) .setInstantiationMode(instantiationMode); From 837c1a56c9f30245411aba1dda3fb6ebfea4d96c Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 21 Oct 2019 16:40:37 -0700 Subject: [PATCH 22/54] remove unused import --- packages/app-types/index.d.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/app-types/index.d.ts b/packages/app-types/index.d.ts index eab147e16c8..10b89b83bb9 100644 --- a/packages/app-types/index.d.ts +++ b/packages/app-types/index.d.ts @@ -1,5 +1,3 @@ -import { Provider } from '@firebase/component'; - /** * @license * Copyright 2017 Google Inc. From c4f26f8d50916fb716ad1fc74497162e3e93df9a Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 21 Oct 2019 16:57:38 -0700 Subject: [PATCH 23/54] fix API change --- packages/app/src/firebaseApp.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index 0f8dee48b86..0bf9c51b9b8 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -141,7 +141,7 @@ export class FirebaseAppImpl implements FirebaseApp { name: string, instanceIdentifier: string = DEFAULT_ENTRY_NAME ): void { - this.container.getProvider(name).clearCache(instanceIdentifier); + this.container.getProvider(name).clearInstance(instanceIdentifier); } _addComponent(component: Component): void { From 2f454ac5f777a9287f09f4fcc8fa7ba41cc90348 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Tue, 22 Oct 2019 11:41:09 -0700 Subject: [PATCH 24/54] move types around --- packages/component/index.ts | 3 ++- packages/component/src/component.ts | 15 +-------------- packages/component/src/types.ts | 17 +++++++++++++++-- packages/component/test/util.ts | 4 ++-- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/packages/component/index.ts b/packages/component/index.ts index 9135e5f5858..e6fb3e9370d 100644 --- a/packages/component/index.ts +++ b/packages/component/index.ts @@ -15,6 +15,7 @@ * limitations under the License. */ -export { Component, ComponentType } from './src/component'; +export { Component } from './src/component'; export { ComponentContainer } from './src/component_container'; export { Provider } from './src/provider'; +export { ComponentType, InstanceFactory} from './src/types' diff --git a/packages/component/src/component.ts b/packages/component/src/component.ts index ad180f0aab4..60dc2062139 100644 --- a/packages/component/src/component.ts +++ b/packages/component/src/component.ts @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { InstantiationMode, InstanceFactory } from './types'; +import { InstantiationMode, InstanceFactory, ComponentType } from './types'; export class Component { multipleInstances: boolean = false; @@ -52,16 +52,3 @@ export class Component { return this; } } - -/** - * PUBLIC: A public component provides a set of public APIs to customers. A service namespace will be patched - * onto `firebase` namespace. Assume the component name is `test`, customers will be able - * to get the service by calling `firebase.test()` or `app.test()` where `app` is a `FirebaseApp` instance. - * - * PRIVATE: A private component provides a set of private APIs that are used internally by other - * Firebase SDKs. No serivce namespace is created in `firebase` namespace and customers have no way to get them. - */ -export enum ComponentType { - PUBLIC, - PRIVATE -} diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 794579891a8..f42efc4d8af 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -18,8 +18,21 @@ import { ComponentContainer } from './component_container'; export const enum InstantiationMode { - LAZY, // Currently all components are LAZY in JS SDK - EAGER + LAZY = "LAZY", // Currently all components are LAZY in JS SDK + EAGER = "EAGER" +} + +/** + * PUBLIC: A public component provides a set of public APIs to customers. A service namespace will be patched + * onto `firebase` namespace. Assume the component name is `test`, customers will be able + * to get the service by calling `firebase.test()` or `app.test()` where `app` is a `FirebaseApp` instance. + * + * PRIVATE: A private component provides a set of private APIs that are used internally by other + * Firebase SDKs. No serivce namespace is created in `firebase` namespace and customers have no way to get them. + */ +export enum ComponentType { + PUBLIC, + PRIVATE } /** diff --git a/packages/component/test/util.ts b/packages/component/test/util.ts index c366d9a0727..84e030899cb 100644 --- a/packages/component/test/util.ts +++ b/packages/component/test/util.ts @@ -16,8 +16,8 @@ */ import { DEFAULT_ENTRY_NAME } from '../src/constants'; import { FirebaseApp } from '@firebase/app-types'; -import { InstanceFactory, InstantiationMode } from '../src/types'; -import { Component, ComponentType } from '../src/component'; +import { InstanceFactory, InstantiationMode, ComponentType } from '../src/types'; +import { Component } from '../src/component'; export function getFakeApp(appName: string = DEFAULT_ENTRY_NAME): FirebaseApp { return { From ab2908e495e92ef77c2293aca00e4353eedb9350 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Tue, 22 Oct 2019 11:41:19 -0700 Subject: [PATCH 25/54] [AUTOMATED]: Prettier Code Styling --- packages/component/index.ts | 2 +- packages/component/src/types.ts | 4 ++-- packages/component/test/util.ts | 6 +++++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/component/index.ts b/packages/component/index.ts index e6fb3e9370d..d0a0f59538d 100644 --- a/packages/component/index.ts +++ b/packages/component/index.ts @@ -18,4 +18,4 @@ export { Component } from './src/component'; export { ComponentContainer } from './src/component_container'; export { Provider } from './src/provider'; -export { ComponentType, InstanceFactory} from './src/types' +export { ComponentType, InstanceFactory } from './src/types'; diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index f42efc4d8af..8a3a1a580b0 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -18,8 +18,8 @@ import { ComponentContainer } from './component_container'; export const enum InstantiationMode { - LAZY = "LAZY", // Currently all components are LAZY in JS SDK - EAGER = "EAGER" + LAZY = 'LAZY', // Currently all components are LAZY in JS SDK + EAGER = 'EAGER' } /** diff --git a/packages/component/test/util.ts b/packages/component/test/util.ts index 84e030899cb..558615f0e7b 100644 --- a/packages/component/test/util.ts +++ b/packages/component/test/util.ts @@ -16,7 +16,11 @@ */ import { DEFAULT_ENTRY_NAME } from '../src/constants'; import { FirebaseApp } from '@firebase/app-types'; -import { InstanceFactory, InstantiationMode, ComponentType } from '../src/types'; +import { + InstanceFactory, + InstantiationMode, + ComponentType +} from '../src/types'; import { Component } from '../src/component'; export function getFakeApp(appName: string = DEFAULT_ENTRY_NAME): FirebaseApp { From cb71e3c98a1b7d815ebb6d9c9b7eefe533a83089 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Tue, 22 Oct 2019 12:21:06 -0700 Subject: [PATCH 26/54] improve provider typing --- packages/component/src/provider.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 83d60427913..f946bda3264 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -48,6 +48,8 @@ export class Provider { return this.instancesDeferred.get(normalizedIdentifier)!.promise; } + getImmediate(identifier: string | undefined, options: { optional: true }): T | null + getImmediate(identifier?: string, options?: {optional: false}): T getImmediate( identifier: string = DEFAULT_ENTRY_NAME, options?: { optional: boolean } From d2d63fa32bbcdd7b8b1920b1101122050cd851fc Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Tue, 22 Oct 2019 12:21:13 -0700 Subject: [PATCH 27/54] [AUTOMATED]: Prettier Code Styling --- packages/component/src/provider.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index f946bda3264..11bc8343bc0 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -48,8 +48,11 @@ export class Provider { return this.instancesDeferred.get(normalizedIdentifier)!.promise; } - getImmediate(identifier: string | undefined, options: { optional: true }): T | null - getImmediate(identifier?: string, options?: {optional: false}): T + getImmediate( + identifier: string | undefined, + options: { optional: true } + ): T | null; + getImmediate(identifier?: string, options?: { optional: false }): T; getImmediate( identifier: string = DEFAULT_ENTRY_NAME, options?: { optional: boolean } From a71cbb645c0052cdc33aff52ca57f2ad55f6d91d Mon Sep 17 00:00:00 2001 From: Feiyang Date: Wed, 23 Oct 2019 15:32:44 -0700 Subject: [PATCH 28/54] Migrate analytics to component platform (#220) * migrate analytics * minor analytics refactoring * interface merge * [AUTOMATED]: Prettier Code Styling * [AUTOMATED]: License Headers --- packages/analytics-interop-types/README.md | 3 + packages/analytics-interop-types/index.d.ts | 50 ++++++++++++++++ packages/analytics-interop-types/package.json | 24 ++++++++ .../analytics-interop-types/tsconfig.json | 9 +++ packages/analytics/index.test.ts | 10 ++-- packages/analytics/index.ts | 57 ++++++++++++++----- packages/analytics/package.json | 1 + packages/analytics/src/errors.ts | 8 ++- packages/analytics/src/factory.ts | 16 +----- 9 files changed, 144 insertions(+), 34 deletions(-) create mode 100644 packages/analytics-interop-types/README.md create mode 100644 packages/analytics-interop-types/index.d.ts create mode 100644 packages/analytics-interop-types/package.json create mode 100644 packages/analytics-interop-types/tsconfig.json diff --git a/packages/analytics-interop-types/README.md b/packages/analytics-interop-types/README.md new file mode 100644 index 00000000000..dc295ac7122 --- /dev/null +++ b/packages/analytics-interop-types/README.md @@ -0,0 +1,3 @@ +# @firebase/analytics-interop-types + +**This package is not intended for direct usage, and should only be used via the officially supported [firebase](https://www.npmjs.com/package/firebase) package.** diff --git a/packages/analytics-interop-types/index.d.ts b/packages/analytics-interop-types/index.d.ts new file mode 100644 index 00000000000..77538fe7883 --- /dev/null +++ b/packages/analytics-interop-types/index.d.ts @@ -0,0 +1,50 @@ +/** + * @license + * Copyright 2019 Google Inc. + * + * 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. + */ + +export interface FirebaseAnalyticsInternal { + /** + * Sends analytics event with given `eventParams`. This method + * automatically associates this logged event with this Firebase web + * app instance on this device. + * List of official event parameters can be found in + * {@link https://developers.google.com/gtagjs/reference/event + * the gtag.js reference documentation}. + */ + logEvent( + eventName: string, + eventParams?: { [key: string]: unknown }, + options?: AnalyticsCallOptions + ): void; +} + +export interface AnalyticsCallOptions { + /** + * If true, this config or event call applies globally to all + * analytics properties on the page. + */ + global: boolean; +} + +declare module '@firebase/component' { + interface ComponentContainer { + getProvider( + name: 'analytics-internal' + ): Provider; + } + + interface Provider {} +} diff --git a/packages/analytics-interop-types/package.json b/packages/analytics-interop-types/package.json new file mode 100644 index 00000000000..9fc46357fef --- /dev/null +++ b/packages/analytics-interop-types/package.json @@ -0,0 +1,24 @@ +{ + "name": "@firebase/analytics-interop-types", + "version": "0.1.0", + "description": "@firebase/analytics Types", + "author": "Firebase (https://firebase.google.com/)", + "license": "Apache-2.0", + "scripts": { + "test": "tsc" + }, + "files": [ + "index.d.ts" + ], + "repository": { + "directory": "packages/analytics-interop-types", + "type": "git", + "url": "https://github.com/firebase/firebase-js-sdk.git" + }, + "bugs": { + "url": "https://github.com/firebase/firebase-js-sdk/issues" + }, + "devDependencies": { + "typescript": "3.6.4" + } +} diff --git a/packages/analytics-interop-types/tsconfig.json b/packages/analytics-interop-types/tsconfig.json new file mode 100644 index 00000000000..9a785433d90 --- /dev/null +++ b/packages/analytics-interop-types/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "../../config/tsconfig.base.json", + "compilerOptions": { + "noEmit": true + }, + "exclude": [ + "dist/**/*" + ] +} diff --git a/packages/analytics/index.test.ts b/packages/analytics/index.test.ts index 58964597a28..c54c46a6c7f 100644 --- a/packages/analytics/index.test.ts +++ b/packages/analytics/index.test.ts @@ -41,12 +41,12 @@ const customDataLayerName = 'customDataLayer'; describe('FirebaseAnalytics instance tests', () => { it('Throws if no analyticsId in config', () => { const app = getFakeApp(); - expect(() => analyticsFactory(app, () => {})).to.throw('field is empty'); + expect(() => analyticsFactory(app)).to.throw('field is empty'); }); it('Throws if creating an instance with already-used analytics ID', () => { const app = getFakeApp(analyticsId); resetGlobalVars(false, { [analyticsId]: Promise.resolve() }); - expect(() => analyticsFactory(app, () => {})).to.throw('already exists'); + expect(() => analyticsFactory(app)).to.throw('already exists'); }); describe('Standard app, page already has user gtag script', () => { let app: FirebaseApp = {} as FirebaseApp; @@ -55,7 +55,7 @@ describe('FirebaseAnalytics instance tests', () => { app = getFakeApp(analyticsId); window['gtag'] = gtagStub; window['dataLayer'] = []; - analyticsInstance = analyticsFactory(app, () => {}); + analyticsInstance = analyticsFactory(app); }); after(() => { delete window['gtag']; @@ -121,7 +121,7 @@ describe('FirebaseAnalytics instance tests', () => { dataLayerName: customDataLayerName, gtagName: customGtagName }); - analyticsInstance = analyticsFactory(app, () => {}); + analyticsInstance = analyticsFactory(app); }); after(() => { delete window[customGtagName]; @@ -164,7 +164,7 @@ describe('FirebaseAnalytics instance tests', () => { before(() => { resetGlobalVars(); const app = getFakeApp(analyticsId); - analyticsInstance = analyticsFactory(app, () => {}); + analyticsInstance = analyticsFactory(app); }); after(() => { delete window['gtag']; diff --git a/packages/analytics/index.ts b/packages/analytics/index.ts index 9d4e4b248c7..1fac3e52091 100644 --- a/packages/analytics/index.ts +++ b/packages/analytics/index.ts @@ -16,12 +16,11 @@ */ import firebase from '@firebase/app'; import { FirebaseAnalytics } from '@firebase/analytics-types'; -import { - FirebaseServiceFactory, - _FirebaseNamespace -} from '@firebase/app-types/private'; +import { _FirebaseNamespace } from '@firebase/app-types/private'; import { factory, settings, resetGlobalVars } from './src/factory'; import { EventName } from './src/constants'; +import { Component, ComponentType } from '@firebase/component'; +import { ERROR_FACTORY, AnalyticsError } from './src/errors'; declare global { interface Window { @@ -35,17 +34,41 @@ declare global { const ANALYTICS_TYPE = 'analytics'; export function registerAnalytics(instance: _FirebaseNamespace): void { - instance.INTERNAL.registerService( - ANALYTICS_TYPE, - factory as FirebaseServiceFactory, - { + instance.INTERNAL.registerComponent( + new Component( + ANALYTICS_TYPE, + container => { + // getImmediate for FirebaseApp will always succeed + const app = container.getProvider('app').getImmediate()!; + return factory(app); + }, + ComponentType.PUBLIC + ).setServiceProps({ settings, EventName - }, - // We don't need to wait on any AppHooks. - undefined, - // Allow multiple analytics instances per app. - false + }) + ); + + instance.INTERNAL.registerComponent( + new Component( + 'analytics-internal', + container => { + try { + const analytics = container + .getProvider(ANALYTICS_TYPE) + .getImmediate(); + return { + logEvent: analytics.logEvent + }; + } catch (e) { + throw ERROR_FACTORY.create( + AnalyticsError.INTEROP_COMPONENT_REG_FAILED, + { reason: e } + ); + } + }, + ComponentType.PRIVATE + ) ); } @@ -64,3 +87,11 @@ declare module '@firebase/app-types' { analytics(): FirebaseAnalytics; } } + +declare module '@firebase/component' { + interface ComponentContainer { + getProvider(name: typeof ANALYTICS_TYPE): Provider; + } + + interface Provider {} +} diff --git a/packages/analytics/package.json b/packages/analytics/package.json index 05e2eee576f..3dae058f30a 100644 --- a/packages/analytics/package.json +++ b/packages/analytics/package.json @@ -26,6 +26,7 @@ "@firebase/analytics-types": "0.2.2", "@firebase/installations": "0.3.2", "@firebase/util": "0.2.31", + "@firebase/component": "0.1.0", "tslib": "1.10.0" }, "license": "Apache-2.0", diff --git a/packages/analytics/src/errors.ts b/packages/analytics/src/errors.ts index 8487e0dc992..dc03b32b8c4 100644 --- a/packages/analytics/src/errors.ts +++ b/packages/analytics/src/errors.ts @@ -21,7 +21,8 @@ import { ANALYTICS_ID_FIELD } from './constants'; export const enum AnalyticsError { NO_GA_ID = 'no-ga-id', ALREADY_EXISTS = 'already-exists', - ALREADY_INITIALIZED = 'already-initialized' + ALREADY_INITIALIZED = 'already-initialized', + INTEROP_COMPONENT_REG_FAILED = 'interop-component-reg-failed' } const ERRORS: ErrorMap = { @@ -36,11 +37,14 @@ const ERRORS: ErrorMap = { [AnalyticsError.ALREADY_INITIALIZED]: 'Firebase Analytics has already been initialized.' + 'settings() must be called before initializing any Analytics instance' + - 'or it will have no effect.' + 'or it will have no effect.', + [AnalyticsError.INTEROP_COMPONENT_REG_FAILED]: + 'Firebase Analytics Interop Component failed to instantiate' }; interface ErrorParams { [AnalyticsError.ALREADY_EXISTS]: { id: string }; + [AnalyticsError.INTEROP_COMPONENT_REG_FAILED]: { reason: Error }; } export const ERROR_FACTORY = new ErrorFactory( diff --git a/packages/analytics/src/factory.ts b/packages/analytics/src/factory.ts index d8e6e2717c9..e094f43c674 100644 --- a/packages/analytics/src/factory.ts +++ b/packages/analytics/src/factory.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -import { FirebaseApp } from '@firebase/app-types'; import { FirebaseAnalytics, Gtag, @@ -38,6 +37,7 @@ import { } from './helpers'; import { ANALYTICS_ID_FIELD } from './constants'; import { AnalyticsError, ERROR_FACTORY } from './errors'; +import { FirebaseApp } from '@firebase/app-types'; /** * Maps gaId to FID fetch promises. @@ -102,11 +102,7 @@ export function settings(options: SettingsOptions): void { } } -export function factory( - app: FirebaseApp, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - extendApp: (props: { [prop: string]: any }) => void -): FirebaseAnalytics { +export function factory(app: FirebaseApp): FirebaseAnalytics { const analyticsId = app.options[ANALYTICS_ID_FIELD]; if (!analyticsId) { throw ERROR_FACTORY.create(AnalyticsError.NO_GA_ID); @@ -162,13 +158,5 @@ export function factory( setAnalyticsCollectionEnabled(analyticsId, enabled) }; - extendApp({ - INTERNAL: { - analytics: { - logEvent: analyticsInstance.logEvent - } - } - }); - return analyticsInstance; } From 188aa5d0edc10ccb992607fd106a4ae3589d318e Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 28 Oct 2019 13:48:05 -0700 Subject: [PATCH 29/54] allow overwriting a registered component --- packages/app/src/firebaseApp.ts | 11 ++++- .../component/src/component_container.test.ts | 45 ++++++++++++++++++- packages/component/src/component_container.ts | 26 ++++++++--- 3 files changed, 73 insertions(+), 9 deletions(-) diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index 0bf9c51b9b8..d2f4ab2cab0 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -144,9 +144,16 @@ export class FirebaseAppImpl implements FirebaseApp { this.container.getProvider(name).clearInstance(instanceIdentifier); } - _addComponent(component: Component): void { + /** + * + * @param component the component being added to this app's container + * @param overwrite When a component with the same name has already been registered, + * if overwrite is true: overwrite the existing component + * if overwrite is false: throw an expection + */ + _addComponent(component: Component, overwrite = false): void { try { - this.container.addComponent(component); + this.container.addComponent(component, overwrite); } catch (e) { logger.debug( `Component ${component.name} failed to register with FirebaseApp ${this.name}`, diff --git a/packages/component/src/component_container.test.ts b/packages/component/src/component_container.test.ts index 07ada959bc4..1119f7b3167 100644 --- a/packages/component/src/component_container.test.ts +++ b/packages/component/src/component_container.test.ts @@ -55,7 +55,7 @@ describe('Component Container', () => { expect(setComponentStub).has.been.calledWith(component); }); - it('throws when registering multiple components with the same name', () => { + it('throws when registering multiple components with the same name, when overwrite is false', () => { const component1 = getFakeComponent( 'fireball', () => ({}), @@ -74,4 +74,47 @@ describe('Component Container', () => { /Component fireball has already been registered with/ ); }); + + it('does not throw when registering multiple components with the same name, when overwrite is true', () => { + const component1 = getFakeComponent( + 'fireball', + () => ({}), + true, + InstantiationMode.EAGER + ); + const component2 = getFakeComponent( + 'fireball', + () => ({ test: true }), + false, + InstantiationMode.LAZY + ); + + expect(() => container.addComponent(component1)).to.not.throw(); + expect(() => container.addComponent(component2, true)).to.not.throw(); + }); + + it('registers a component with a name that is already registered and return the provider for the new component', () => { + const component1 = getFakeComponent( + 'fireball', + () => ({ test: false }), + true, + InstantiationMode.EAGER + ); + const component2 = getFakeComponent( + 'fireball', + () => ({ test: true }), + false, + InstantiationMode.LAZY + ); + + container.addComponent(component1); + const oldProvider = container.getProvider('fireball'); + expect(oldProvider.getImmediate()).to.deep.eq({ test: false }); + + container.addComponent(component2, true); + const newProvider = container.getProvider('fireball'); + expect(oldProvider).to.not.eq(newProvider); + expect(newProvider.getImmediate()).to.deep.eq({ test: true }); + + }); }); diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 8e9576fe7d4..4677ea83038 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -21,14 +21,28 @@ import { Component } from './component'; export class ComponentContainer { private providers = new Map(); - constructor(private name: string) {} + constructor(private name: string) { } - addComponent(component: Component): void { - const provider = this.getProvider(component.name); + /** + * + * @param component Component being added + * @param overwrite When a component with the same name has already been registered, + * if overwrite is true: overwrite the existing component with the new component and create a new provider with the new component + * if overwrite is false: throw an expection + */ + addComponent(component: Component, overwrite = false): void { + let provider = this.getProvider(component.name); if (provider.isComponentSet()) { - throw new Error( - `Component ${component.name} has already been registered with ${this.name}` - ); + if (!overwrite) { + throw new Error( + `Component ${component.name} has already been registered with ${this.name}` + ); + } else { // use the new component to replace the existing component + // delete the existing provider from the container + this.providers.delete(component.name); + // create a new provider + provider = this.getProvider(component.name); + } } provider.setComponent(component); From 7e975822646ecd6fb722c255c38f0a2516532a20 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 28 Oct 2019 13:48:12 -0700 Subject: [PATCH 30/54] [AUTOMATED]: Prettier Code Styling --- packages/app/src/firebaseApp.ts | 2 +- packages/component/src/component_container.test.ts | 1 - packages/component/src/component_container.ts | 9 +++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index d2f4ab2cab0..abd26ab790b 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -145,7 +145,7 @@ export class FirebaseAppImpl implements FirebaseApp { } /** - * + * * @param component the component being added to this app's container * @param overwrite When a component with the same name has already been registered, * if overwrite is true: overwrite the existing component diff --git a/packages/component/src/component_container.test.ts b/packages/component/src/component_container.test.ts index 1119f7b3167..7adcb420927 100644 --- a/packages/component/src/component_container.test.ts +++ b/packages/component/src/component_container.test.ts @@ -115,6 +115,5 @@ describe('Component Container', () => { const newProvider = container.getProvider('fireball'); expect(oldProvider).to.not.eq(newProvider); expect(newProvider.getImmediate()).to.deep.eq({ test: true }); - }); }); diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 4677ea83038..7b0e8636760 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -21,10 +21,10 @@ import { Component } from './component'; export class ComponentContainer { private providers = new Map(); - constructor(private name: string) { } + constructor(private name: string) {} /** - * + * * @param component Component being added * @param overwrite When a component with the same name has already been registered, * if overwrite is true: overwrite the existing component with the new component and create a new provider with the new component @@ -37,11 +37,12 @@ export class ComponentContainer { throw new Error( `Component ${component.name} has already been registered with ${this.name}` ); - } else { // use the new component to replace the existing component + } else { + // use the new component to replace the existing component // delete the existing provider from the container this.providers.delete(component.name); // create a new provider - provider = this.getProvider(component.name); + provider = this.getProvider(component.name); } } From af4fdc4eddfe29e3329fa62d517b0b15d26e50c0 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Tue, 29 Oct 2019 15:46:47 -0700 Subject: [PATCH 31/54] change ComponentType to string enum --- packages/component/src/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 8a3a1a580b0..eda50046c55 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -31,8 +31,8 @@ export const enum InstantiationMode { * Firebase SDKs. No serivce namespace is created in `firebase` namespace and customers have no way to get them. */ export enum ComponentType { - PUBLIC, - PRIVATE + PUBLIC = 'PUBLIC', + PRIVATE = 'PRIVARE' } /** From 56bbdee26dd491a93f0a0a50d957c6cf50928a0a Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 4 Nov 2019 13:35:16 -0800 Subject: [PATCH 32/54] address comments --- packages/component/src/component.ts | 16 ++++++++-------- .../component/src/component_container.test.ts | 2 +- packages/component/src/component_container.ts | 12 +++++++----- packages/component/src/provider.test.ts | 2 +- packages/component/src/provider.ts | 10 +++++----- packages/component/src/types.ts | 6 +++++- 6 files changed, 27 insertions(+), 21 deletions(-) diff --git a/packages/component/src/component.ts b/packages/component/src/component.ts index 60dc2062139..9100b596e11 100644 --- a/packages/component/src/component.ts +++ b/packages/component/src/component.ts @@ -14,16 +14,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { InstantiationMode, InstanceFactory, ComponentType } from './types'; +import { InstantiationMode, InstanceFactory, ComponentType, Dictionary } from './types'; export class Component { - multipleInstances: boolean = false; + multipleInstances = false; /** * Properties to be added to the service namespace */ - serviceProps: { [key: string]: unknown } = {}; + serviceProps: Dictionary = {}; - instantiationMode: InstantiationMode = InstantiationMode.LAZY; + instantiationMode = InstantiationMode.LAZY; /** * @@ -32,9 +32,9 @@ export class Component { * @param type whehter the service provided by the component is public or private */ constructor( - public readonly name: string, - public readonly instanceFactory: InstanceFactory, - public readonly type: ComponentType + readonly name: string, + readonly instanceFactory: InstanceFactory, + readonly type: ComponentType ) {} setInstantiationMode(mode: InstantiationMode): this { @@ -47,7 +47,7 @@ export class Component { return this; } - setServiceProps(props: { [key: string]: unknown }): this { + setServiceProps(props: Dictionary): this { this.serviceProps = props; return this; } diff --git a/packages/component/src/component_container.test.ts b/packages/component/src/component_container.test.ts index 7adcb420927..a34338f6a97 100644 --- a/packages/component/src/component_container.test.ts +++ b/packages/component/src/component_container.test.ts @@ -38,7 +38,7 @@ describe('Component Container', () => { const provider1 = container.getProvider('ship'); const provider2 = container.getProvider('ship'); - expect(provider1).to.eq(provider2); + expect(provider1).to.equal(provider2); }); it('calls setComponent() on provider with the same name when registering a component', () => { diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 7b0e8636760..69d53e67570 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -19,16 +19,18 @@ import { Provider } from './provider'; import { Component } from './component'; export class ComponentContainer { - private providers = new Map(); + private readonly providers = new Map(); - constructor(private name: string) {} + constructor(private readonly name: string) {} /** * * @param component Component being added * @param overwrite When a component with the same name has already been registered, - * if overwrite is true: overwrite the existing component with the new component and create a new provider with the new component - * if overwrite is false: throw an expection + * if overwrite is true: overwrite the existing component with the new component and create a new + * provider with the new component. It can be useful in tests where you want to use different mocks + * for different tests. + * if overwrite is false: throw an exception */ addComponent(component: Component, overwrite = false): void { let provider = this.getProvider(component.name); @@ -62,6 +64,6 @@ export class ComponentContainer { } getProviders(): Provider[] { - return Array.from(this.providers, entry => entry[1]); + return Array.from(this.providers.values()); } } diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index b16678e71da..39ddddecc98 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -302,7 +302,7 @@ describe('Provider', () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.delete(); - expect(deleteFakes.length).to.eq(2); + expect(deleteFakes.length).to.equal(2); for (const f of deleteFakes) { expect(f).to.have.been.called; } diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 11bc8343bc0..95404243f70 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -26,10 +26,10 @@ import { Component } from './component'; */ export class Provider { private component: Component | null = null; - private instances: Map = new Map(); - private instancesDeferred: Map> = new Map(); + private readonly instances: Map = new Map(); + private readonly instancesDeferred: Map> = new Map(); - constructor(private name: string, private container: ComponentContainer) {} + constructor(private readonly name: string, private readonly container: ComponentContainer) {} get(identifier: string = DEFAULT_ENTRY_NAME): Promise { // if multipleInstances is not supported, use the default name @@ -114,10 +114,10 @@ export class Provider { // app.delete() will call this method on every provider to delete the services // TODO: should we mark the provider as deleted? - delete(): Promise { + async delete(): Promise { const services = Array.from(this.instances.values()); - return Promise.all( + await Promise.all( services .filter(service => 'INTERNAL' in service) // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index eda50046c55..2dcf3144ffc 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -30,7 +30,7 @@ export const enum InstantiationMode { * PRIVATE: A private component provides a set of private APIs that are used internally by other * Firebase SDKs. No serivce namespace is created in `firebase` namespace and customers have no way to get them. */ -export enum ComponentType { +export const enum ComponentType { PUBLIC = 'PUBLIC', PRIVATE = 'PRIVARE' } @@ -47,3 +47,7 @@ export type InstanceFactory = ( container: ComponentContainer, instanceIdentifier?: string ) => T; + +export type Dictionary = { + [key: string]: unknown +}; \ No newline at end of file From 3007773a462877297645a608cb9f69a5fd8ac615 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 4 Nov 2019 13:35:25 -0800 Subject: [PATCH 33/54] [AUTOMATED]: Prettier Code Styling --- packages/component/src/component.ts | 7 ++++++- packages/component/src/component_container.ts | 2 +- packages/component/src/provider.ts | 5 ++++- packages/component/src/types.ts | 4 ++-- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/component/src/component.ts b/packages/component/src/component.ts index 9100b596e11..d2c62d2eee9 100644 --- a/packages/component/src/component.ts +++ b/packages/component/src/component.ts @@ -14,7 +14,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { InstantiationMode, InstanceFactory, ComponentType, Dictionary } from './types'; +import { + InstantiationMode, + InstanceFactory, + ComponentType, + Dictionary +} from './types'; export class Component { multipleInstances = false; diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 69d53e67570..10aac3e347a 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -27,7 +27,7 @@ export class ComponentContainer { * * @param component Component being added * @param overwrite When a component with the same name has already been registered, - * if overwrite is true: overwrite the existing component with the new component and create a new + * if overwrite is true: overwrite the existing component with the new component and create a new * provider with the new component. It can be useful in tests where you want to use different mocks * for different tests. * if overwrite is false: throw an exception diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 95404243f70..81f6515ba85 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -29,7 +29,10 @@ export class Provider { private readonly instances: Map = new Map(); private readonly instancesDeferred: Map> = new Map(); - constructor(private readonly name: string, private readonly container: ComponentContainer) {} + constructor( + private readonly name: string, + private readonly container: ComponentContainer + ) {} get(identifier: string = DEFAULT_ENTRY_NAME): Promise { // if multipleInstances is not supported, use the default name diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 2dcf3144ffc..721e955d4b8 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -49,5 +49,5 @@ export type InstanceFactory = ( ) => T; export type Dictionary = { - [key: string]: unknown -}; \ No newline at end of file + [key: string]: unknown; +}; From 7b8439d8a0b5f09c7681e885a65b958244177f95 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 4 Nov 2019 13:56:15 -0800 Subject: [PATCH 34/54] remove return only generics --- packages/component/src/component_container.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 10aac3e347a..53832b72cc0 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -51,16 +51,16 @@ export class ComponentContainer { provider.setComponent(component); } - getProvider(name: string): Provider { + getProvider(name: string): Provider { if (this.providers.has(name)) { - return this.providers.get(name) as Provider; + return this.providers.get(name) as Provider; } // create a Provider for a service that hasn't registered with Firebase const provider = new Provider(name, this); this.providers.set(name, provider); - return provider as Provider; + return provider as Provider; } getProviders(): Provider[] { From 8c172b8c931cdc5ea246f1dcdcde840557824df5 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 4 Nov 2019 15:47:13 -0800 Subject: [PATCH 35/54] Move identifier to options object for getImmediate() --- packages/component/index.ts | 2 +- packages/component/package.json | 20 +++++++------- packages/component/src/provider.test.ts | 22 +++++++-------- packages/component/src/provider.ts | 36 ++++++++++++++++++++----- packages/component/src/types.ts | 2 +- 5 files changed, 52 insertions(+), 30 deletions(-) diff --git a/packages/component/index.ts b/packages/component/index.ts index d0a0f59538d..04199ac50ac 100644 --- a/packages/component/index.ts +++ b/packages/component/index.ts @@ -18,4 +18,4 @@ export { Component } from './src/component'; export { ComponentContainer } from './src/component_container'; export { Provider } from './src/provider'; -export { ComponentType, InstanceFactory } from './src/types'; +export { ComponentType, InstanceFactory, InstantiationMode } from './src/types'; diff --git a/packages/component/package.json b/packages/component/package.json index c085e9a654e..37190271fa8 100644 --- a/packages/component/package.json +++ b/packages/component/package.json @@ -22,17 +22,17 @@ "prepare": "yarn build" }, "dependencies": { - "@firebase/util": "0.2.29", + "@firebase/util": "0.2.31", "tslib": "1.10.0" }, "license": "Apache-2.0", "devDependencies": { - "@types/chai": "4.2.3", + "@types/chai": "4.2.4", "@types/mocha": "5.2.7", "@types/sinon": "7.5.0", "chai": "4.2.0", "chai-as-promised": "7.1.1", - "karma": "4.3.0", + "karma": "4.4.1", "sinon": "7.5.0", "sinon-chai": "3.3.0", "karma-chrome-launcher": "3.1.0", @@ -43,19 +43,19 @@ "karma-sauce-launcher": "1.2.0", "karma-spec-reporter": "0.0.32", "karma-webpack": "4.0.2", - "mocha": "6.2.1", + "mocha": "6.2.2", "npm-run-all": "4.1.5", "nyc": "14.1.1", - "rollup": "1.23.1", + "rollup": "1.25.2", "rollup-plugin-typescript2": "0.24.3", - "ts-loader": "6.2.0", + "ts-loader": "6.2.1", "ts-node": "8.4.1", "typescript": "3.6.4", - "webpack": "4.41.1", + "webpack": "4.41.2", "eslint": "6.5.1", - "@typescript-eslint/parser": "2.4.0", - "@typescript-eslint/eslint-plugin": "2.4.0", - "@typescript-eslint/eslint-plugin-tslint": "2.4.0", + "@typescript-eslint/parser": "2.5.0", + "@typescript-eslint/eslint-plugin": "2.5.0", + "@typescript-eslint/eslint-plugin-tslint": "2.5.0", "eslint-plugin-import": "2.18.2" }, "repository": { diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 39ddddecc98..aad5eb137aa 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -49,7 +49,7 @@ describe('Provider', () => { }); it('returns null if the service is not available with optional flag', () => { - expect(provider.getImmediate(undefined, { optional: true })).to.equal( + expect(provider.getImmediate({ optional: true })).to.equal( null ); }); @@ -69,8 +69,8 @@ describe('Provider', () => { it('ignores parameter identifier and return the default service', () => { provider.setComponent(getFakeComponent('test', () => ({ test: true }))); const defaultService = provider.getImmediate(); - expect(provider.getImmediate('spider1')).to.equal(defaultService); - expect(provider.getImmediate('spider2')).to.equal(defaultService); + expect(provider.getImmediate({identifier: 'spider1'})).to.equal(defaultService); + expect(provider.getImmediate({identifier: 'spider2'})).to.equal(defaultService); }); }); @@ -187,11 +187,11 @@ describe('Provider', () => { describe('Provider (multipleInstances = true)', () => { describe('getImmediate(identifier)', () => { it('throws if the service is not available', () => { - expect(provider.getImmediate.bind(provider, 'guardian')).to.throw(); + expect(provider.getImmediate.bind(provider, {identifier: 'guardian'})).to.throw(); }); it('returns null if the service is not available with optional flag', () => { - expect(provider.getImmediate('guardian', { optional: true })).to.equal( + expect(provider.getImmediate({ identifier: 'guardian', optional: true })).to.equal( null ); }); @@ -201,8 +201,8 @@ describe('Provider', () => { getFakeComponent('test', () => ({ test: true }), true) ); const defaultService = provider.getImmediate(); - const service1 = provider.getImmediate('guardian'); - const service2 = provider.getImmediate('servant'); + const service1 = provider.getImmediate({identifier: 'guardian'}); + const service2 = provider.getImmediate({identifier: 'servant'}); expect(defaultService).to.deep.equal({ test: true }); expect(service1).to.deep.equal({ test: true }); @@ -296,8 +296,8 @@ describe('Provider', () => { provider.setComponent(getFakeComponent('test', getService, true)); // create 2 service instances with different names - provider.getImmediate('instance1'); - provider.getImmediate('instance2'); + provider.getImmediate({identifier: 'instance1'}); + provider.getImmediate({identifier: 'instance2'}); // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.delete(); @@ -313,7 +313,7 @@ describe('Provider', () => { provider.setComponent(getFakeComponent('test', () => ({}), true)); // create serviec instances with different identifiers const defaultInstance = provider.getImmediate(); - const instance1 = provider.getImmediate('instance1'); + const instance1 = provider.getImmediate({identifier: 'instance1'}); expect((provider as any).instances.size).to.equal(2); @@ -327,7 +327,7 @@ describe('Provider', () => { // remove the named instance from cache and create a new instance with the same identifier provider.clearInstance('instance1'); expect((provider as any).instances.size).to.equal(1); - const newInstance1 = provider.getImmediate('instance1'); + const newInstance1 = provider.getImmediate({identifier: 'instance1'}); expect(newInstance1).to.not.eq(instance1); expect((provider as any).instances.size).to.equal(2); }); diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 81f6515ba85..c95c7f7204a 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -32,8 +32,12 @@ export class Provider { constructor( private readonly name: string, private readonly container: ComponentContainer - ) {} + ) { } + /** + * @param identifier A provider can provide mulitple instances of a service + * if this.component.multipleInstances is true. + */ get(identifier: string = DEFAULT_ENTRY_NAME): Promise { // if multipleInstances is not supported, use the default name const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); @@ -51,22 +55,40 @@ export class Provider { return this.instancesDeferred.get(normalizedIdentifier)!.promise; } +/** + * + * @param options.identifier A provider can provide mulitple instances of a service + * if this.component.multipleInstances is true. + * @param options.optional If optional is false or not provided, the method throws an error when + * the service is not immediately available. + * If optional is true, the method returns null if the service is not immediately available. + */ getImmediate( - identifier: string | undefined, - options: { optional: true } + options: { + identifier?: string + optional: true + } ): T | null; - getImmediate(identifier?: string, options?: { optional: false }): T; getImmediate( - identifier: string = DEFAULT_ENTRY_NAME, - options?: { optional: boolean } + options?: { + identifier?: string, + optional?: false + }): T; + getImmediate( + options?: { + identifier?: string, + optional?: boolean + } ): T | null { + + const { identifier, optional } = { identifier: DEFAULT_ENTRY_NAME, optional: false, ...options }; // if multipleInstances is not supported, use the default name const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); const instance = this.getOrInitializeService(normalizedIdentifier); if (!instance) { - if (options && options.optional) { + if (optional) { return null; } throw Error(`Service ${this.name} is not available`); diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 721e955d4b8..cab13895b3e 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -48,6 +48,6 @@ export type InstanceFactory = ( instanceIdentifier?: string ) => T; -export type Dictionary = { +export interface Dictionary { [key: string]: unknown; }; From 086df99c063e23980e17f1abda5564943864f6a1 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Mon, 4 Nov 2019 15:47:21 -0800 Subject: [PATCH 36/54] [AUTOMATED]: Prettier Code Styling --- packages/component/src/provider.test.ts | 34 ++++++++++-------- packages/component/src/provider.ts | 48 +++++++++++-------------- packages/component/src/types.ts | 2 +- 3 files changed, 40 insertions(+), 44 deletions(-) diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index aad5eb137aa..7b307f6bc90 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -49,9 +49,7 @@ describe('Provider', () => { }); it('returns null if the service is not available with optional flag', () => { - expect(provider.getImmediate({ optional: true })).to.equal( - null - ); + expect(provider.getImmediate({ optional: true })).to.equal(null); }); it('returns the service instance synchronously', () => { @@ -69,8 +67,12 @@ describe('Provider', () => { it('ignores parameter identifier and return the default service', () => { provider.setComponent(getFakeComponent('test', () => ({ test: true }))); const defaultService = provider.getImmediate(); - expect(provider.getImmediate({identifier: 'spider1'})).to.equal(defaultService); - expect(provider.getImmediate({identifier: 'spider2'})).to.equal(defaultService); + expect(provider.getImmediate({ identifier: 'spider1' })).to.equal( + defaultService + ); + expect(provider.getImmediate({ identifier: 'spider2' })).to.equal( + defaultService + ); }); }); @@ -187,13 +189,15 @@ describe('Provider', () => { describe('Provider (multipleInstances = true)', () => { describe('getImmediate(identifier)', () => { it('throws if the service is not available', () => { - expect(provider.getImmediate.bind(provider, {identifier: 'guardian'})).to.throw(); + expect( + provider.getImmediate.bind(provider, { identifier: 'guardian' }) + ).to.throw(); }); it('returns null if the service is not available with optional flag', () => { - expect(provider.getImmediate({ identifier: 'guardian', optional: true })).to.equal( - null - ); + expect( + provider.getImmediate({ identifier: 'guardian', optional: true }) + ).to.equal(null); }); it('returns different service instances for different identifiers synchronously', () => { @@ -201,8 +205,8 @@ describe('Provider', () => { getFakeComponent('test', () => ({ test: true }), true) ); const defaultService = provider.getImmediate(); - const service1 = provider.getImmediate({identifier: 'guardian'}); - const service2 = provider.getImmediate({identifier: 'servant'}); + const service1 = provider.getImmediate({ identifier: 'guardian' }); + const service2 = provider.getImmediate({ identifier: 'servant' }); expect(defaultService).to.deep.equal({ test: true }); expect(service1).to.deep.equal({ test: true }); @@ -296,8 +300,8 @@ describe('Provider', () => { provider.setComponent(getFakeComponent('test', getService, true)); // create 2 service instances with different names - provider.getImmediate({identifier: 'instance1'}); - provider.getImmediate({identifier: 'instance2'}); + provider.getImmediate({ identifier: 'instance1' }); + provider.getImmediate({ identifier: 'instance2' }); // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.delete(); @@ -313,7 +317,7 @@ describe('Provider', () => { provider.setComponent(getFakeComponent('test', () => ({}), true)); // create serviec instances with different identifiers const defaultInstance = provider.getImmediate(); - const instance1 = provider.getImmediate({identifier: 'instance1'}); + const instance1 = provider.getImmediate({ identifier: 'instance1' }); expect((provider as any).instances.size).to.equal(2); @@ -327,7 +331,7 @@ describe('Provider', () => { // remove the named instance from cache and create a new instance with the same identifier provider.clearInstance('instance1'); expect((provider as any).instances.size).to.equal(1); - const newInstance1 = provider.getImmediate({identifier: 'instance1'}); + const newInstance1 = provider.getImmediate({ identifier: 'instance1' }); expect(newInstance1).to.not.eq(instance1); expect((provider as any).instances.size).to.equal(2); }); diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index c95c7f7204a..5b24e11ffa7 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -32,10 +32,10 @@ export class Provider { constructor( private readonly name: string, private readonly container: ComponentContainer - ) { } + ) {} /** - * @param identifier A provider can provide mulitple instances of a service + * @param identifier A provider can provide mulitple instances of a service * if this.component.multipleInstances is true. */ get(identifier: string = DEFAULT_ENTRY_NAME): Promise { @@ -55,33 +55,25 @@ export class Provider { return this.instancesDeferred.get(normalizedIdentifier)!.promise; } -/** - * - * @param options.identifier A provider can provide mulitple instances of a service + /** + * + * @param options.identifier A provider can provide mulitple instances of a service * if this.component.multipleInstances is true. - * @param options.optional If optional is false or not provided, the method throws an error when - * the service is not immediately available. - * If optional is true, the method returns null if the service is not immediately available. - */ - getImmediate( - options: { - identifier?: string - optional: true - } - ): T | null; - getImmediate( - options?: { - identifier?: string, - optional?: false - }): T; - getImmediate( - options?: { - identifier?: string, - optional?: boolean - } - ): T | null { - - const { identifier, optional } = { identifier: DEFAULT_ENTRY_NAME, optional: false, ...options }; + * @param options.optional If optional is false or not provided, the method throws an error when + * the service is not immediately available. + * If optional is true, the method returns null if the service is not immediately available. + */ + getImmediate(options: { identifier?: string; optional: true }): T | null; + getImmediate(options?: { identifier?: string; optional?: false }): T; + getImmediate(options?: { + identifier?: string; + optional?: boolean; + }): T | null { + const { identifier, optional } = { + identifier: DEFAULT_ENTRY_NAME, + optional: false, + ...options + }; // if multipleInstances is not supported, use the default name const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index cab13895b3e..444460782d1 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -50,4 +50,4 @@ export type InstanceFactory = ( export interface Dictionary { [key: string]: unknown; -}; +} From 9971b33ba459e65abd7c8c8435edbc1d79d8e951 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Tue, 5 Nov 2019 14:16:04 -0800 Subject: [PATCH 37/54] Make getProvider() type safe --- packages/analytics-interop-types/index.d.ts | 8 ++---- packages/component/index.ts | 2 +- .../component/src/component_container.test.ts | 11 ++++++++ packages/component/src/component_container.ts | 28 +++++++++++++------ packages/component/src/types.ts | 9 ++++++ 5 files changed, 43 insertions(+), 15 deletions(-) diff --git a/packages/analytics-interop-types/index.d.ts b/packages/analytics-interop-types/index.d.ts index 77538fe7883..1fcf74da4da 100644 --- a/packages/analytics-interop-types/index.d.ts +++ b/packages/analytics-interop-types/index.d.ts @@ -40,11 +40,7 @@ export interface AnalyticsCallOptions { } declare module '@firebase/component' { - interface ComponentContainer { - getProvider( - name: 'analytics-internal' - ): Provider; + interface NameServiceMapping { + 'analytics-internal': FirebaseAnalyticsInternal } - - interface Provider {} } diff --git a/packages/component/index.ts b/packages/component/index.ts index 04199ac50ac..52e1b03e2b9 100644 --- a/packages/component/index.ts +++ b/packages/component/index.ts @@ -18,4 +18,4 @@ export { Component } from './src/component'; export { ComponentContainer } from './src/component_container'; export { Provider } from './src/provider'; -export { ComponentType, InstanceFactory, InstantiationMode } from './src/types'; +export { ComponentType, InstanceFactory, InstantiationMode, NameServiceMapping } from './src/types'; diff --git a/packages/component/src/component_container.test.ts b/packages/component/src/component_container.test.ts index a34338f6a97..ad317cff990 100644 --- a/packages/component/src/component_container.test.ts +++ b/packages/component/src/component_container.test.ts @@ -24,6 +24,17 @@ import { InstantiationMode } from './types'; import { DEFAULT_ENTRY_NAME } from './constants'; import { getFakeComponent } from '../test/util'; +// extend NameServiceMapping with the service names we are going to use in the tests +// It is because ComponentContainer.getProvider is strongly typed, and it can only be called +// with a field name present in NameServiceMapping interface. +declare module './types' { + interface NameServiceMapping { + rocket: {}, + ship: {}, + fireball: {} + } +} + describe('Component Container', () => { let container: ComponentContainer; beforeEach(() => { diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 53832b72cc0..f1130b309a6 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -17,11 +17,12 @@ import { Provider } from './provider'; import { Component } from './component'; +import { Name, NameServiceMapping } from './types'; export class ComponentContainer { private readonly providers = new Map(); - constructor(private readonly name: string) {} + constructor(private readonly name: string) { } /** * @@ -33,7 +34,7 @@ export class ComponentContainer { * if overwrite is false: throw an exception */ addComponent(component: Component, overwrite = false): void { - let provider = this.getProvider(component.name); + let provider = this.getProviderInternal(component.name); if (provider.isComponentSet()) { if (!overwrite) { throw new Error( @@ -44,14 +45,29 @@ export class ComponentContainer { // delete the existing provider from the container this.providers.delete(component.name); // create a new provider - provider = this.getProvider(component.name); + provider = this.getProviderInternal(component.name); } } provider.setComponent(component); } - getProvider(name: string): Provider { + /** + * getProvider provides a type safe interface where it can only be called with a field name + * present in NameServiceMapping interface. + * + * Firebase SDKs providing services should extend NameServiceMapping interface to register + * themselves. + */ + getProvider(name: T): Provider { + return this.getProviderInternal(name) as Provider; + } + + getProviders(): Provider[] { + return Array.from(this.providers.values()); + } + + private getProviderInternal(name: string): Provider { if (this.providers.has(name)) { return this.providers.get(name) as Provider; } @@ -62,8 +78,4 @@ export class ComponentContainer { return provider as Provider; } - - getProviders(): Provider[] { - return Array.from(this.providers.values()); - } } diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 444460782d1..a7ec641ff2d 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -51,3 +51,12 @@ export type InstanceFactory = ( export interface Dictionary { [key: string]: unknown; } + +/** + * This interface will be extended by Firebase SDKs to provide service name and service type mapping. + * It is used by ComponentContainer.getProvider() method to provide a type safe interface. + */ +export interface NameServiceMapping { +} + +export type Name = keyof NameServiceMapping; \ No newline at end of file From e38d251aff7ca84f0bfb9ec318ee1dfb3dc16910 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Tue, 5 Nov 2019 14:16:12 -0800 Subject: [PATCH 38/54] [AUTOMATED]: Prettier Code Styling --- packages/analytics-interop-types/index.d.ts | 2 +- packages/component/index.ts | 7 ++++++- packages/component/src/component_container.test.ts | 6 +++--- packages/component/src/component_container.ts | 4 ++-- packages/component/src/types.ts | 5 ++--- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/analytics-interop-types/index.d.ts b/packages/analytics-interop-types/index.d.ts index 1fcf74da4da..d246734599c 100644 --- a/packages/analytics-interop-types/index.d.ts +++ b/packages/analytics-interop-types/index.d.ts @@ -41,6 +41,6 @@ export interface AnalyticsCallOptions { declare module '@firebase/component' { interface NameServiceMapping { - 'analytics-internal': FirebaseAnalyticsInternal + 'analytics-internal': FirebaseAnalyticsInternal; } } diff --git a/packages/component/index.ts b/packages/component/index.ts index 52e1b03e2b9..b070304ce1b 100644 --- a/packages/component/index.ts +++ b/packages/component/index.ts @@ -18,4 +18,9 @@ export { Component } from './src/component'; export { ComponentContainer } from './src/component_container'; export { Provider } from './src/provider'; -export { ComponentType, InstanceFactory, InstantiationMode, NameServiceMapping } from './src/types'; +export { + ComponentType, + InstanceFactory, + InstantiationMode, + NameServiceMapping +} from './src/types'; diff --git a/packages/component/src/component_container.test.ts b/packages/component/src/component_container.test.ts index ad317cff990..d54af847ddd 100644 --- a/packages/component/src/component_container.test.ts +++ b/packages/component/src/component_container.test.ts @@ -29,9 +29,9 @@ import { getFakeComponent } from '../test/util'; // with a field name present in NameServiceMapping interface. declare module './types' { interface NameServiceMapping { - rocket: {}, - ship: {}, - fireball: {} + rocket: {}; + ship: {}; + fireball: {}; } } diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index f1130b309a6..39af5d9f0f1 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -22,7 +22,7 @@ import { Name, NameServiceMapping } from './types'; export class ComponentContainer { private readonly providers = new Map(); - constructor(private readonly name: string) { } + constructor(private readonly name: string) {} /** * @@ -55,7 +55,7 @@ export class ComponentContainer { /** * getProvider provides a type safe interface where it can only be called with a field name * present in NameServiceMapping interface. - * + * * Firebase SDKs providing services should extend NameServiceMapping interface to register * themselves. */ diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index a7ec641ff2d..3ed08b44ffc 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -56,7 +56,6 @@ export interface Dictionary { * This interface will be extended by Firebase SDKs to provide service name and service type mapping. * It is used by ComponentContainer.getProvider() method to provide a type safe interface. */ -export interface NameServiceMapping { -} +export interface NameServiceMapping {} -export type Name = keyof NameServiceMapping; \ No newline at end of file +export type Name = keyof NameServiceMapping; From 15ec97e330158ba2831ba4a5f61881687cfc1699 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Tue, 5 Nov 2019 14:48:51 -0800 Subject: [PATCH 39/54] define a new method to replace overwrite flag --- packages/component/src/component_container.ts | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 39af5d9f0f1..114c260a759 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -22,7 +22,7 @@ import { Name, NameServiceMapping } from './types'; export class ComponentContainer { private readonly providers = new Map(); - constructor(private readonly name: string) {} + constructor(private readonly name: string) { } /** * @@ -33,25 +33,27 @@ export class ComponentContainer { * for different tests. * if overwrite is false: throw an exception */ - addComponent(component: Component, overwrite = false): void { + addComponent(component: Component): void { let provider = this.getProviderInternal(component.name); if (provider.isComponentSet()) { - if (!overwrite) { - throw new Error( - `Component ${component.name} has already been registered with ${this.name}` - ); - } else { - // use the new component to replace the existing component - // delete the existing provider from the container - this.providers.delete(component.name); - // create a new provider - provider = this.getProviderInternal(component.name); - } + throw new Error( + `Component ${component.name} has already been registered with ${this.name}` + ); } provider.setComponent(component); } + addOrOverwriteComponent(component: Component): void { + let provider = this.getProviderInternal(component.name); + if (provider.isComponentSet()) { + // delete the existing provider from the container, so we can register the new component + this.providers.delete(component.name); + } + + this.addComponent(component); + } + /** * getProvider provides a type safe interface where it can only be called with a field name * present in NameServiceMapping interface. From f425f8c59bc8ee9e241c6a54b18e270ee4b09789 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Tue, 5 Nov 2019 14:48:58 -0800 Subject: [PATCH 40/54] [AUTOMATED]: Prettier Code Styling --- packages/component/src/component_container.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 114c260a759..e9d45d37ace 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -22,7 +22,7 @@ import { Name, NameServiceMapping } from './types'; export class ComponentContainer { private readonly providers = new Map(); - constructor(private readonly name: string) { } + constructor(private readonly name: string) {} /** * From 6888f35e0d4cc0da0b2b77ebf3c5fc7ce19fdbc1 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 6 Nov 2019 11:29:03 -0800 Subject: [PATCH 41/54] Make component type safe --- packages/component/src/component.ts | 10 +++-- .../component/src/component_container.test.ts | 6 +-- packages/component/src/component_container.ts | 39 +++++++++---------- packages/component/src/provider.test.ts | 15 +++++-- packages/component/src/provider.ts | 31 ++++++++------- packages/component/src/types.ts | 10 +++-- packages/component/test/util.ts | 11 +++--- 7 files changed, 69 insertions(+), 53 deletions(-) diff --git a/packages/component/src/component.ts b/packages/component/src/component.ts index d2c62d2eee9..65c1df976c4 100644 --- a/packages/component/src/component.ts +++ b/packages/component/src/component.ts @@ -18,10 +18,14 @@ import { InstantiationMode, InstanceFactory, ComponentType, - Dictionary + Dictionary, + Name } from './types'; -export class Component { +/** + * Component for service name T, e.g. `auth`, `auth-internal` + */ +export class Component { multipleInstances = false; /** * Properties to be added to the service namespace @@ -37,7 +41,7 @@ export class Component { * @param type whehter the service provided by the component is public or private */ constructor( - readonly name: string, + readonly name: T, readonly instanceFactory: InstanceFactory, readonly type: ComponentType ) {} diff --git a/packages/component/src/component_container.test.ts b/packages/component/src/component_container.test.ts index d54af847ddd..b98e46b8cf9 100644 --- a/packages/component/src/component_container.test.ts +++ b/packages/component/src/component_container.test.ts @@ -57,7 +57,7 @@ describe('Component Container', () => { const setComponentStub = stub(provider, 'setComponent').callThrough(); const component = getFakeComponent( 'fireball', - () => ({}), + () => ({test: 1}), true, InstantiationMode.EAGER ); @@ -101,7 +101,7 @@ describe('Component Container', () => { ); expect(() => container.addComponent(component1)).to.not.throw(); - expect(() => container.addComponent(component2, true)).to.not.throw(); + expect(() => container.addOrOverwriteComponent(component2)).to.not.throw(); }); it('registers a component with a name that is already registered and return the provider for the new component', () => { @@ -122,7 +122,7 @@ describe('Component Container', () => { const oldProvider = container.getProvider('fireball'); expect(oldProvider.getImmediate()).to.deep.eq({ test: false }); - container.addComponent(component2, true); + container.addOrOverwriteComponent(component2); const newProvider = container.getProvider('fireball'); expect(oldProvider).to.not.eq(newProvider); expect(newProvider.getImmediate()).to.deep.eq({ test: true }); diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index e9d45d37ace..3322f86596c 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -17,12 +17,15 @@ import { Provider } from './provider'; import { Component } from './component'; -import { Name, NameServiceMapping } from './types'; +import { Name } from './types'; -export class ComponentContainer { - private readonly providers = new Map(); +/** + * ComponentContainer that provides Providers for service name T, e.g. `auth`, `auth-internal` + */ +export class ComponentContainer { + private readonly providers = new Map>(); - constructor(private readonly name: string) {} + constructor(private readonly name: string) { } /** * @@ -33,8 +36,8 @@ export class ComponentContainer { * for different tests. * if overwrite is false: throw an exception */ - addComponent(component: Component): void { - let provider = this.getProviderInternal(component.name); + addComponent(component: Component): void { + const provider = this.getProvider(component.name); if (provider.isComponentSet()) { throw new Error( `Component ${component.name} has already been registered with ${this.name}` @@ -44,8 +47,8 @@ export class ComponentContainer { provider.setComponent(component); } - addOrOverwriteComponent(component: Component): void { - let provider = this.getProviderInternal(component.name); + addOrOverwriteComponent(component: Component): void { + const provider = this.getProvider(component.name); if (provider.isComponentSet()) { // delete the existing provider from the container, so we can register the new component this.providers.delete(component.name); @@ -61,23 +64,19 @@ export class ComponentContainer { * Firebase SDKs providing services should extend NameServiceMapping interface to register * themselves. */ - getProvider(name: T): Provider { - return this.getProviderInternal(name) as Provider; - } - - getProviders(): Provider[] { - return Array.from(this.providers.values()); - } - - private getProviderInternal(name: string): Provider { + getProvider(name: T): Provider { if (this.providers.has(name)) { - return this.providers.get(name) as Provider; + return this.providers.get(name) as Provider; } // create a Provider for a service that hasn't registered with Firebase - const provider = new Provider(name, this); + const provider = new Provider(name, this); this.providers.set(name, provider); - return provider as Provider; + return provider as Provider; + } + + getProviders(): Array> { + return Array.from(this.providers.values()); } } diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 7b307f6bc90..09a3ddd0b78 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -22,10 +22,18 @@ import { FirebaseService } from '@firebase/app-types/private'; import { Provider } from './provider'; import { getFakeApp, getFakeComponent } from '../test/util'; import '../test/setup'; -import { InstantiationMode } from './types'; +import { InstantiationMode, } from './types'; + +// define the types for the fake services we use in the tests +declare module './types' { + interface NameServiceMapping { + test: {}; + badtest: {} + } +} describe('Provider', () => { - let provider: Provider; + let provider: Provider<'test'>; beforeEach(() => { provider = new Provider('test', new ComponentContainer('test-container')); @@ -33,7 +41,8 @@ describe('Provider', () => { it('throws if setComponent() is called with a component with a different name than the provider name', () => { expect(() => - provider.setComponent(getFakeComponent('not a test', () => ({}))) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + provider.setComponent(getFakeComponent('badtest', () => ({})) as any) ).to.throw(/^Mismatching Component/); expect(() => provider.setComponent(getFakeComponent('test', () => ({}))) diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 5b24e11ffa7..680840261d4 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -18,32 +18,33 @@ import { Deferred } from '@firebase/util'; import { ComponentContainer } from './component_container'; import { DEFAULT_ENTRY_NAME } from './constants'; -import { InstantiationMode } from './types'; +import { InstantiationMode, Name, NameServiceMapping } from './types'; import { Component } from './component'; /** - * Provider for instance of type T, e.g. Auth, RC + * Provider for instance for service name T, e.g. 'auth', 'auth-internal' + * U is an alias for the type of the instance */ -export class Provider { +export class Provider { private component: Component | null = null; - private readonly instances: Map = new Map(); - private readonly instancesDeferred: Map> = new Map(); + private readonly instances: Map = new Map(); + private readonly instancesDeferred: Map> = new Map(); constructor( - private readonly name: string, + private readonly name: T, private readonly container: ComponentContainer - ) {} + ) { } /** * @param identifier A provider can provide mulitple instances of a service * if this.component.multipleInstances is true. */ - get(identifier: string = DEFAULT_ENTRY_NAME): Promise { + get(identifier: string = DEFAULT_ENTRY_NAME): Promise { // if multipleInstances is not supported, use the default name const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); if (!this.instancesDeferred.has(normalizedIdentifier)) { - const deferred = new Deferred(); + const deferred = new Deferred(); this.instancesDeferred.set(normalizedIdentifier, deferred); // If the service instance is available, resolve the promise with it immediately const instance = this.getOrInitializeService(normalizedIdentifier); @@ -63,12 +64,12 @@ export class Provider { * the service is not immediately available. * If optional is true, the method returns null if the service is not immediately available. */ - getImmediate(options: { identifier?: string; optional: true }): T | null; - getImmediate(options?: { identifier?: string; optional?: false }): T; + getImmediate(options: { identifier?: string; optional: true }): NameServiceMapping[T] | null; + getImmediate(options?: { identifier?: string; optional?: false }): NameServiceMapping[T]; getImmediate(options?: { identifier?: string; optional?: boolean; - }): T | null { + }): NameServiceMapping[T] | null { const { identifier, optional } = { identifier: DEFAULT_ENTRY_NAME, optional: false, @@ -146,13 +147,13 @@ export class Provider { return this.component != null; } - private getOrInitializeService(identifier: string): T | null { + private getOrInitializeService(identifier: string): U | null { let instance = this.instances.get(identifier); if (!instance && this.component) { instance = this.component.instanceFactory( this.container, normalizeIdentifierForFactory(identifier) - ); + ) as U; this.instances.set(identifier, instance); } @@ -173,6 +174,6 @@ function normalizeIdentifierForFactory(identifier: string): string | undefined { return identifier === DEFAULT_ENTRY_NAME ? undefined : identifier; } -function isComponentEager(component: Component): boolean { +function isComponentEager(component: Component): boolean { return component.instantiationMode === InstantiationMode.EAGER; } diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 3ed08b44ffc..f8620ef9517 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -43,10 +43,10 @@ export const enum ComponentType { * NOTE: The container only provides {@link Provider} rather than the actual instances of dependencies. * It is useful for lazily loaded dependencies and optional dependencies. */ -export type InstanceFactory = ( +export type InstanceFactory = ( container: ComponentContainer, instanceIdentifier?: string -) => T; +) => NameServiceMapping[T]; export interface Dictionary { [key: string]: unknown; @@ -54,8 +54,10 @@ export interface Dictionary { /** * This interface will be extended by Firebase SDKs to provide service name and service type mapping. - * It is used by ComponentContainer.getProvider() method to provide a type safe interface. + * It is used as a generic constraint to ensure type safety. */ -export interface NameServiceMapping {} +export interface NameServiceMapping { +} export type Name = keyof NameServiceMapping; +export type Service = NameServiceMapping[Name]; diff --git a/packages/component/test/util.ts b/packages/component/test/util.ts index 558615f0e7b..2045c1780b9 100644 --- a/packages/component/test/util.ts +++ b/packages/component/test/util.ts @@ -19,7 +19,8 @@ import { FirebaseApp } from '@firebase/app-types'; import { InstanceFactory, InstantiationMode, - ComponentType + ComponentType, + Name } from '../src/types'; import { Component } from '../src/component'; @@ -40,12 +41,12 @@ export function getFakeApp(appName: string = DEFAULT_ENTRY_NAME): FirebaseApp { }; } -export function getFakeComponent( - name: string, - factory: InstanceFactory, +export function getFakeComponent( + name: T, + factory: InstanceFactory, multipleInstance: boolean = false, instantiationMode = InstantiationMode.LAZY -): Component { +): Component { return new Component(name, factory, ComponentType.PUBLIC) .setMultipleInstances(multipleInstance) .setInstantiationMode(instantiationMode); From e84bad8c90e02cba366803ffa283c4854848ab63 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 6 Nov 2019 11:29:10 -0800 Subject: [PATCH 42/54] [AUTOMATED]: Prettier Code Styling --- packages/component/src/component_container.test.ts | 2 +- packages/component/src/component_container.ts | 2 +- packages/component/src/provider.test.ts | 4 ++-- packages/component/src/provider.ts | 12 +++++++++--- packages/component/src/types.ts | 3 +-- 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/component/src/component_container.test.ts b/packages/component/src/component_container.test.ts index b98e46b8cf9..2df34f631cb 100644 --- a/packages/component/src/component_container.test.ts +++ b/packages/component/src/component_container.test.ts @@ -57,7 +57,7 @@ describe('Component Container', () => { const setComponentStub = stub(provider, 'setComponent').callThrough(); const component = getFakeComponent( 'fireball', - () => ({test: 1}), + () => ({ test: 1 }), true, InstantiationMode.EAGER ); diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 3322f86596c..4e63a21978f 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -25,7 +25,7 @@ import { Name } from './types'; export class ComponentContainer { private readonly providers = new Map>(); - constructor(private readonly name: string) { } + constructor(private readonly name: string) {} /** * diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 09a3ddd0b78..e83fe04c51c 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -22,13 +22,13 @@ import { FirebaseService } from '@firebase/app-types/private'; import { Provider } from './provider'; import { getFakeApp, getFakeComponent } from '../test/util'; import '../test/setup'; -import { InstantiationMode, } from './types'; +import { InstantiationMode } from './types'; // define the types for the fake services we use in the tests declare module './types' { interface NameServiceMapping { test: {}; - badtest: {} + badtest: {}; } } diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 680840261d4..912bcd57fb3 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -33,7 +33,7 @@ export class Provider { constructor( private readonly name: T, private readonly container: ComponentContainer - ) { } + ) {} /** * @param identifier A provider can provide mulitple instances of a service @@ -64,8 +64,14 @@ export class Provider { * the service is not immediately available. * If optional is true, the method returns null if the service is not immediately available. */ - getImmediate(options: { identifier?: string; optional: true }): NameServiceMapping[T] | null; - getImmediate(options?: { identifier?: string; optional?: false }): NameServiceMapping[T]; + getImmediate(options: { + identifier?: string; + optional: true; + }): NameServiceMapping[T] | null; + getImmediate(options?: { + identifier?: string; + optional?: false; + }): NameServiceMapping[T]; getImmediate(options?: { identifier?: string; optional?: boolean; diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index f8620ef9517..249214dcde4 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -56,8 +56,7 @@ export interface Dictionary { * This interface will be extended by Firebase SDKs to provide service name and service type mapping. * It is used as a generic constraint to ensure type safety. */ -export interface NameServiceMapping { -} +export interface NameServiceMapping {} export type Name = keyof NameServiceMapping; export type Service = NameServiceMapping[Name]; From 9538682f7311dc7257f2d7abcf18fab3ec2fb29b Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 6 Nov 2019 12:21:07 -0800 Subject: [PATCH 43/54] remove the generic type from component container --- packages/component/src/component.ts | 2 +- packages/component/src/component_container.ts | 12 ++++++------ packages/component/src/provider.ts | 9 +++++---- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/component/src/component.ts b/packages/component/src/component.ts index 65c1df976c4..896aa6f3ec1 100644 --- a/packages/component/src/component.ts +++ b/packages/component/src/component.ts @@ -25,7 +25,7 @@ import { /** * Component for service name T, e.g. `auth`, `auth-internal` */ -export class Component { +export class Component { multipleInstances = false; /** * Properties to be added to the service namespace diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 4e63a21978f..49f4e2dde86 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -22,8 +22,8 @@ import { Name } from './types'; /** * ComponentContainer that provides Providers for service name T, e.g. `auth`, `auth-internal` */ -export class ComponentContainer { - private readonly providers = new Map>(); +export class ComponentContainer { + private readonly providers = new Map(); constructor(private readonly name: string) {} @@ -36,7 +36,7 @@ export class ComponentContainer { * for different tests. * if overwrite is false: throw an exception */ - addComponent(component: Component): void { + addComponent(component: Component): void { const provider = this.getProvider(component.name); if (provider.isComponentSet()) { throw new Error( @@ -47,7 +47,7 @@ export class ComponentContainer { provider.setComponent(component); } - addOrOverwriteComponent(component: Component): void { + addOrOverwriteComponent(component: Component): void { const provider = this.getProvider(component.name); if (provider.isComponentSet()) { // delete the existing provider from the container, so we can register the new component @@ -64,7 +64,7 @@ export class ComponentContainer { * Firebase SDKs providing services should extend NameServiceMapping interface to register * themselves. */ - getProvider(name: T): Provider { + getProvider(name: T): Provider { if (this.providers.has(name)) { return this.providers.get(name) as Provider; } @@ -76,7 +76,7 @@ export class ComponentContainer { return provider as Provider; } - getProviders(): Array> { + getProviders(): Provider[] { return Array.from(this.providers.values()); } } diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 912bcd57fb3..b40c4e6344f 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -25,7 +25,8 @@ import { Component } from './component'; * Provider for instance for service name T, e.g. 'auth', 'auth-internal' * U is an alias for the type of the instance */ -export class Provider { +export class Provider { + private component: Component | null = null; private readonly instances: Map = new Map(); private readonly instancesDeferred: Map> = new Map(); @@ -67,15 +68,15 @@ export class Provider { getImmediate(options: { identifier?: string; optional: true; - }): NameServiceMapping[T] | null; + }): U | null; getImmediate(options?: { identifier?: string; optional?: false; - }): NameServiceMapping[T]; + }): U; getImmediate(options?: { identifier?: string; optional?: boolean; - }): NameServiceMapping[T] | null { + }): U | null { const { identifier, optional } = { identifier: DEFAULT_ENTRY_NAME, optional: false, From 3dd9ec386479e2e507618cb9862aeabc04781041 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 6 Nov 2019 12:36:48 -0800 Subject: [PATCH 44/54] Update FirebaseApp and Analytics --- packages/analytics/index.ts | 11 +++++----- packages/app/index.node.ts | 5 ++--- packages/app/index.rn.ts | 5 ++--- packages/app/index.ts | 7 +++---- packages/app/src/firebaseApp.ts | 20 ++++++++++--------- packages/app/src/firebaseNamespaceCore.ts | 3 ++- packages/app/src/lite/firebaseAppLite.ts | 5 +++-- .../app/src/lite/firebaseNamespaceLite.ts | 3 ++- packages/app/test/firebaseApp.test.ts | 7 ++++--- packages/component/src/types.ts | 4 +++- 10 files changed, 37 insertions(+), 33 deletions(-) diff --git a/packages/analytics/index.ts b/packages/analytics/index.ts index 1fac3e52091..84a1329c54e 100644 --- a/packages/analytics/index.ts +++ b/packages/analytics/index.ts @@ -16,6 +16,7 @@ */ import firebase from '@firebase/app'; import { FirebaseAnalytics } from '@firebase/analytics-types'; +import { FirebaseAnalyticsInternal } from '@firebase/analytics-interop-types'; import { _FirebaseNamespace } from '@firebase/app-types/private'; import { factory, settings, resetGlobalVars } from './src/factory'; import { EventName } from './src/constants'; @@ -39,7 +40,7 @@ export function registerAnalytics(instance: _FirebaseNamespace): void { ANALYTICS_TYPE, container => { // getImmediate for FirebaseApp will always succeed - const app = container.getProvider('app').getImmediate()!; + const app = container.getProvider('app').getImmediate(); return factory(app); }, ComponentType.PUBLIC @@ -59,7 +60,7 @@ export function registerAnalytics(instance: _FirebaseNamespace): void { .getImmediate(); return { logEvent: analytics.logEvent - }; + } as FirebaseAnalyticsInternal; } catch (e) { throw ERROR_FACTORY.create( AnalyticsError.INTEROP_COMPONENT_REG_FAILED, @@ -89,9 +90,7 @@ declare module '@firebase/app-types' { } declare module '@firebase/component' { - interface ComponentContainer { - getProvider(name: typeof ANALYTICS_TYPE): Provider; + interface NameServiceMapping { + 'analytics': FirebaseAnalytics } - - interface Provider {} } diff --git a/packages/app/index.node.ts b/packages/app/index.node.ts index 28ec06d1ca3..086c3946e4e 100644 --- a/packages/app/index.node.ts +++ b/packages/app/index.node.ts @@ -18,7 +18,6 @@ import { FirebaseNamespace, FirebaseApp } from '@firebase/app-types'; import { _FirebaseNamespace } from '@firebase/app-types/private'; import { firebase as _firebase } from './src/firebaseNamespace'; -import { Provider } from '@firebase/component'; // Node specific packages. // @ts-ignore import Storage from 'dom-storage'; @@ -41,7 +40,7 @@ export const firebase = _firebase as FirebaseNamespace; export default firebase; declare module '@firebase/component' { - interface ComponentContainer { - getProvider(name: 'app'): Provider; + interface NameServiceMapping { + 'app': FirebaseApp } } diff --git a/packages/app/index.rn.ts b/packages/app/index.rn.ts index 698f59dfa9b..d1199f74995 100644 --- a/packages/app/index.rn.ts +++ b/packages/app/index.rn.ts @@ -18,7 +18,6 @@ import { FirebaseNamespace, FirebaseApp } from '@firebase/app-types'; import { _FirebaseNamespace } from '@firebase/app-types/private'; import { firebase as _firebase } from './src/firebaseNamespace'; -import { Provider } from '@firebase/component'; /** * To avoid having to include the @types/react-native package, which breaks * some of our tests because of duplicate symbols, we are using require syntax @@ -41,7 +40,7 @@ export const firebase = _firebase as FirebaseNamespace; export default firebase; declare module '@firebase/component' { - interface ComponentContainer { - getProvider(name: 'app'): Provider; + interface NameServiceMapping { + 'app': FirebaseApp } } diff --git a/packages/app/index.ts b/packages/app/index.ts index 62e13fd0bf0..ad2e2ef35d9 100644 --- a/packages/app/index.ts +++ b/packages/app/index.ts @@ -18,7 +18,6 @@ import { FirebaseNamespace, FirebaseApp } from '@firebase/app-types'; import { firebase as firebaseNamespace } from './src/firebaseNamespace'; import { isNode, isBrowser } from '@firebase/util'; -import { Provider } from '@firebase/component'; import { logger } from './src/logger'; // Firebase Lite detection @@ -44,7 +43,7 @@ const initializeApp = firebaseNamespace.initializeApp; // TODO: This disable can be removed and the 'ignoreRestArgs' option added to // the no-explicit-any rule when ESlint releases it. // eslint-disable-next-line @typescript-eslint/no-explicit-any -firebaseNamespace.initializeApp = function(...args: any) { +firebaseNamespace.initializeApp = function (...args: any) { // Environment check before initializing app // Do the check in initializeApp, so people have a chance to disable it by setting logLevel // in @firebase/logger @@ -72,7 +71,7 @@ export const firebase = firebaseNamespace; export default firebase; declare module '@firebase/component' { - interface ComponentContainer { - getProvider(name: 'app'): Provider; + interface NameServiceMapping { + 'app': FirebaseApp } } diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index abd26ab790b..0ea4759b11b 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -124,8 +124,9 @@ export class FirebaseAppImpl implements FirebaseApp { // getImmediate will always succeed because _getService is only called for registered components. return this.container - .getProvider(name) - .getImmediate(instanceIdentifier) as FirebaseService; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .getProvider(name as any) + .getImmediate({ identifier: instanceIdentifier }) as unknown as FirebaseService; } /** * Remove a service instance from the cache, so we will create a new instance for this service @@ -141,19 +142,16 @@ export class FirebaseAppImpl implements FirebaseApp { name: string, instanceIdentifier: string = DEFAULT_ENTRY_NAME ): void { - this.container.getProvider(name).clearInstance(instanceIdentifier); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + this.container.getProvider(name as any).clearInstance(instanceIdentifier); } /** - * * @param component the component being added to this app's container - * @param overwrite When a component with the same name has already been registered, - * if overwrite is true: overwrite the existing component - * if overwrite is false: throw an expection */ - _addComponent(component: Component, overwrite = false): void { + _addComponent(component: Component): void { try { - this.container.addComponent(component, overwrite); + this.container.addComponent(component); } catch (e) { logger.debug( `Component ${component.name} failed to register with FirebaseApp ${this.name}`, @@ -162,6 +160,10 @@ export class FirebaseAppImpl implements FirebaseApp { } } + _addOrOverwriteComponent(component: Component): void { + this.container.addOrOverwriteComponent(component); + } + /** * This function will throw an Error if the App has already been deleted - * use before performing API actions on the App. diff --git a/packages/app/src/firebaseNamespaceCore.ts b/packages/app/src/firebaseNamespaceCore.ts index 607b7497edd..3af8795ad1d 100644 --- a/packages/app/src/firebaseNamespaceCore.ts +++ b/packages/app/src/firebaseNamespaceCore.ts @@ -47,7 +47,8 @@ export function createFirebaseNamespaceCore( firebaseAppImpl: typeof FirebaseAppImpl | typeof FirebaseAppLiteImpl ): FirebaseNamespace { const apps: { [name: string]: FirebaseApp } = {}; - const components = new Map(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const components = new Map>(); // A namespace is a plain JavaScript Object. const namespace: FirebaseNamespace = { diff --git a/packages/app/src/lite/firebaseAppLite.ts b/packages/app/src/lite/firebaseAppLite.ts index b8a1b20ba87..a7f54763377 100644 --- a/packages/app/src/lite/firebaseAppLite.ts +++ b/packages/app/src/lite/firebaseAppLite.ts @@ -134,8 +134,9 @@ export class FirebaseAppLiteImpl implements FirebaseApp { // getImmediate will always succeed because _getService is only called for registered components. return this.container - .getProvider(name) - .getImmediate(instanceIdentifier) as FirebaseService; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .getProvider(name as any) + .getImmediate({identifier: instanceIdentifier}) as unknown as FirebaseService; } /** diff --git a/packages/app/src/lite/firebaseNamespaceLite.ts b/packages/app/src/lite/firebaseNamespaceLite.ts index e88fba664c9..3b14081faaf 100644 --- a/packages/app/src/lite/firebaseNamespaceLite.ts +++ b/packages/app/src/lite/firebaseNamespaceLite.ts @@ -40,7 +40,8 @@ export function createFirebaseNamespaceLite(): FirebaseNamespace { * only allow performance SDK to register. */ function registerComponentForLite( - component: Component + // eslint-disable-next-line @typescript-eslint/no-explicit-any + component: Component ): FirebaseServiceNamespace | null { // only allow performance to register with firebase lite if ( diff --git a/packages/app/test/firebaseApp.test.ts b/packages/app/test/firebaseApp.test.ts index 671a050ddb7..03cdccda033 100644 --- a/packages/app/test/firebaseApp.test.ts +++ b/packages/app/test/firebaseApp.test.ts @@ -315,7 +315,7 @@ function firebaseAppTests( } class TestService implements FirebaseService { - constructor(private app_: FirebaseApp, public instanceIdentifier?: string) {} + constructor(private app_: FirebaseApp, public instanceIdentifier?: string) { } // TODO(koss): Shouldn't this just be an added method on // the service instance? @@ -336,8 +336,9 @@ function createTestComponent( type = ComponentType.PUBLIC ): Component { const component = new Component( - name, - container => new TestService(container.getProvider('app').getImmediate()!), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + name as any, + container => new TestService(container.getProvider('app').getImmediate() as FirebaseApp), type ); component.setMultipleInstances(multiInstances); diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 249214dcde4..6159cd2767a 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -56,7 +56,9 @@ export interface Dictionary { * This interface will be extended by Firebase SDKs to provide service name and service type mapping. * It is used as a generic constraint to ensure type safety. */ -export interface NameServiceMapping {} +export interface NameServiceMapping { + 'app': string +} export type Name = keyof NameServiceMapping; export type Service = NameServiceMapping[Name]; From e7d7e5be9c30458fd08e83729e60c95bf81efd91 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 6 Nov 2019 12:36:55 -0800 Subject: [PATCH 45/54] [AUTOMATED]: Prettier Code Styling --- packages/analytics/index.ts | 2 +- packages/app/index.node.ts | 2 +- packages/app/index.rn.ts | 2 +- packages/app/index.ts | 4 ++-- packages/app/src/firebaseApp.ts | 12 ++++++++---- packages/app/src/lite/firebaseAppLite.ts | 12 ++++++++---- packages/app/test/firebaseApp.test.ts | 7 +++++-- packages/component/src/provider.ts | 11 ++--------- packages/component/src/types.ts | 2 +- 9 files changed, 29 insertions(+), 25 deletions(-) diff --git a/packages/analytics/index.ts b/packages/analytics/index.ts index 84a1329c54e..bfca6a16a9a 100644 --- a/packages/analytics/index.ts +++ b/packages/analytics/index.ts @@ -91,6 +91,6 @@ declare module '@firebase/app-types' { declare module '@firebase/component' { interface NameServiceMapping { - 'analytics': FirebaseAnalytics + 'analytics': FirebaseAnalytics; } } diff --git a/packages/app/index.node.ts b/packages/app/index.node.ts index 086c3946e4e..82ee6561798 100644 --- a/packages/app/index.node.ts +++ b/packages/app/index.node.ts @@ -41,6 +41,6 @@ export default firebase; declare module '@firebase/component' { interface NameServiceMapping { - 'app': FirebaseApp + 'app': FirebaseApp; } } diff --git a/packages/app/index.rn.ts b/packages/app/index.rn.ts index d1199f74995..d2b48a7dad5 100644 --- a/packages/app/index.rn.ts +++ b/packages/app/index.rn.ts @@ -41,6 +41,6 @@ export default firebase; declare module '@firebase/component' { interface NameServiceMapping { - 'app': FirebaseApp + 'app': FirebaseApp; } } diff --git a/packages/app/index.ts b/packages/app/index.ts index ad2e2ef35d9..e5911df11f6 100644 --- a/packages/app/index.ts +++ b/packages/app/index.ts @@ -43,7 +43,7 @@ const initializeApp = firebaseNamespace.initializeApp; // TODO: This disable can be removed and the 'ignoreRestArgs' option added to // the no-explicit-any rule when ESlint releases it. // eslint-disable-next-line @typescript-eslint/no-explicit-any -firebaseNamespace.initializeApp = function (...args: any) { +firebaseNamespace.initializeApp = function(...args: any) { // Environment check before initializing app // Do the check in initializeApp, so people have a chance to disable it by setting logLevel // in @firebase/logger @@ -72,6 +72,6 @@ export default firebase; declare module '@firebase/component' { interface NameServiceMapping { - 'app': FirebaseApp + 'app': FirebaseApp; } } diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index 0ea4759b11b..8fff57b0b7b 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -123,10 +123,14 @@ export class FirebaseAppImpl implements FirebaseApp { this.checkDestroyed_(); // getImmediate will always succeed because _getService is only called for registered components. - return this.container - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .getProvider(name as any) - .getImmediate({ identifier: instanceIdentifier }) as unknown as FirebaseService; + return ( + (this.container + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .getProvider(name as any) + .getImmediate({ + identifier: instanceIdentifier + }) as unknown) as FirebaseService + ); } /** * Remove a service instance from the cache, so we will create a new instance for this service diff --git a/packages/app/src/lite/firebaseAppLite.ts b/packages/app/src/lite/firebaseAppLite.ts index a7f54763377..8ca434ca7c1 100644 --- a/packages/app/src/lite/firebaseAppLite.ts +++ b/packages/app/src/lite/firebaseAppLite.ts @@ -133,10 +133,14 @@ export class FirebaseAppLiteImpl implements FirebaseApp { this.checkDestroyed_(); // getImmediate will always succeed because _getService is only called for registered components. - return this.container - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .getProvider(name as any) - .getImmediate({identifier: instanceIdentifier}) as unknown as FirebaseService; + return ( + (this.container + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .getProvider(name as any) + .getImmediate({ + identifier: instanceIdentifier + }) as unknown) as FirebaseService + ); } /** diff --git a/packages/app/test/firebaseApp.test.ts b/packages/app/test/firebaseApp.test.ts index 03cdccda033..47b9b29838f 100644 --- a/packages/app/test/firebaseApp.test.ts +++ b/packages/app/test/firebaseApp.test.ts @@ -315,7 +315,7 @@ function firebaseAppTests( } class TestService implements FirebaseService { - constructor(private app_: FirebaseApp, public instanceIdentifier?: string) { } + constructor(private app_: FirebaseApp, public instanceIdentifier?: string) {} // TODO(koss): Shouldn't this just be an added method on // the service instance? @@ -338,7 +338,10 @@ function createTestComponent( const component = new Component( // eslint-disable-next-line @typescript-eslint/no-explicit-any name as any, - container => new TestService(container.getProvider('app').getImmediate() as FirebaseApp), + container => + new TestService(container + .getProvider('app') + .getImmediate() as FirebaseApp), type ); component.setMultipleInstances(multiInstances); diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index b40c4e6344f..8240c7f0d9e 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -26,7 +26,6 @@ import { Component } from './component'; * U is an alias for the type of the instance */ export class Provider { - private component: Component | null = null; private readonly instances: Map = new Map(); private readonly instancesDeferred: Map> = new Map(); @@ -65,14 +64,8 @@ export class Provider { * the service is not immediately available. * If optional is true, the method returns null if the service is not immediately available. */ - getImmediate(options: { - identifier?: string; - optional: true; - }): U | null; - getImmediate(options?: { - identifier?: string; - optional?: false; - }): U; + getImmediate(options: { identifier?: string; optional: true }): U | null; + getImmediate(options?: { identifier?: string; optional?: false }): U; getImmediate(options?: { identifier?: string; optional?: boolean; diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 6159cd2767a..372004410ee 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -57,7 +57,7 @@ export interface Dictionary { * It is used as a generic constraint to ensure type safety. */ export interface NameServiceMapping { - 'app': string + 'app': string; } export type Name = keyof NameServiceMapping; From 2b1e4d434d8df5e9ccd037a453b159b102c6c0d0 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 6 Nov 2019 12:40:12 -0800 Subject: [PATCH 46/54] remove unneccessary casting --- packages/app/test/firebaseApp.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/test/firebaseApp.test.ts b/packages/app/test/firebaseApp.test.ts index 47b9b29838f..a96ac0cfe1f 100644 --- a/packages/app/test/firebaseApp.test.ts +++ b/packages/app/test/firebaseApp.test.ts @@ -341,7 +341,7 @@ function createTestComponent( container => new TestService(container .getProvider('app') - .getImmediate() as FirebaseApp), + .getImmediate()), type ); component.setMultipleInstances(multiInstances); From c30b9c7d9b7b6c164a2cbcc4005981991f578bd5 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 6 Nov 2019 12:40:21 -0800 Subject: [PATCH 47/54] [AUTOMATED]: Prettier Code Styling --- packages/app/test/firebaseApp.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/app/test/firebaseApp.test.ts b/packages/app/test/firebaseApp.test.ts index a96ac0cfe1f..80ccfb7b5aa 100644 --- a/packages/app/test/firebaseApp.test.ts +++ b/packages/app/test/firebaseApp.test.ts @@ -338,10 +338,7 @@ function createTestComponent( const component = new Component( // eslint-disable-next-line @typescript-eslint/no-explicit-any name as any, - container => - new TestService(container - .getProvider('app') - .getImmediate()), + container => new TestService(container.getProvider('app').getImmediate()), type ); component.setMultipleInstances(multiInstances); From dab24867c7a0cbbb8fd898cd7a361af0fe14ac85 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Sun, 10 Nov 2019 20:32:25 -0800 Subject: [PATCH 48/54] fix typo --- packages/component/src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index 372004410ee..eadeff79bf5 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -32,7 +32,7 @@ export const enum InstantiationMode { */ export const enum ComponentType { PUBLIC = 'PUBLIC', - PRIVATE = 'PRIVARE' + PRIVATE = 'PRIVATE' } /** From 2187664a20942a75112207fa52b772215704516f Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Sun, 10 Nov 2019 22:30:52 -0800 Subject: [PATCH 49/54] address comments --- packages/analytics-types/index.d.ts | 7 ++++++ packages/analytics/index.ts | 8 +------ packages/app-types/index.d.ts | 6 +++++ packages/app/index.node.ts | 8 +------ packages/app/index.rn.ts | 8 +------ packages/app/index.ts | 8 +------ packages/app/src/firebaseApp.ts | 6 ++--- packages/component/index.ts | 3 ++- packages/component/src/component_container.ts | 4 ++-- packages/component/src/provider.ts | 22 +++++++++---------- packages/component/src/types.ts | 4 +--- 11 files changed, 36 insertions(+), 48 deletions(-) diff --git a/packages/analytics-types/index.d.ts b/packages/analytics-types/index.d.ts index ef4042fe393..3833c8048c6 100644 --- a/packages/analytics-types/index.d.ts +++ b/packages/analytics-types/index.d.ts @@ -201,3 +201,10 @@ export interface Promotion { id?: string; name?: string; } + + +declare module '@firebase/component' { + interface NameServiceMapping { + 'analytics': FirebaseAnalytics; + } +} \ No newline at end of file diff --git a/packages/analytics/index.ts b/packages/analytics/index.ts index bfca6a16a9a..50e88069f2d 100644 --- a/packages/analytics/index.ts +++ b/packages/analytics/index.ts @@ -60,7 +60,7 @@ export function registerAnalytics(instance: _FirebaseNamespace): void { .getImmediate(); return { logEvent: analytics.logEvent - } as FirebaseAnalyticsInternal; + }; } catch (e) { throw ERROR_FACTORY.create( AnalyticsError.INTEROP_COMPONENT_REG_FAILED, @@ -88,9 +88,3 @@ declare module '@firebase/app-types' { analytics(): FirebaseAnalytics; } } - -declare module '@firebase/component' { - interface NameServiceMapping { - 'analytics': FirebaseAnalytics; - } -} diff --git a/packages/app-types/index.d.ts b/packages/app-types/index.d.ts index 10b89b83bb9..9b2c33e96df 100644 --- a/packages/app-types/index.d.ts +++ b/packages/app-types/index.d.ts @@ -101,3 +101,9 @@ export interface FirebaseNamespace { // The current SDK version. SDK_VERSION: string; } + +declare module '@firebase/component' { + interface NameServiceMapping { + 'app': FirebaseApp; + } +} diff --git a/packages/app/index.node.ts b/packages/app/index.node.ts index 82ee6561798..f7e81b690e5 100644 --- a/packages/app/index.node.ts +++ b/packages/app/index.node.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { FirebaseNamespace, FirebaseApp } from '@firebase/app-types'; +import { FirebaseNamespace } from '@firebase/app-types'; import { _FirebaseNamespace } from '@firebase/app-types/private'; import { firebase as _firebase } from './src/firebaseNamespace'; // Node specific packages. @@ -38,9 +38,3 @@ export const firebase = _firebase as FirebaseNamespace; // eslint-disable-next-line import/no-default-export export default firebase; - -declare module '@firebase/component' { - interface NameServiceMapping { - 'app': FirebaseApp; - } -} diff --git a/packages/app/index.rn.ts b/packages/app/index.rn.ts index d2b48a7dad5..e0177ac2662 100644 --- a/packages/app/index.rn.ts +++ b/packages/app/index.rn.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { FirebaseNamespace, FirebaseApp } from '@firebase/app-types'; +import { FirebaseNamespace } from '@firebase/app-types'; import { _FirebaseNamespace } from '@firebase/app-types/private'; import { firebase as _firebase } from './src/firebaseNamespace'; /** @@ -38,9 +38,3 @@ export const firebase = _firebase as FirebaseNamespace; // eslint-disable-next-line import/no-default-export export default firebase; - -declare module '@firebase/component' { - interface NameServiceMapping { - 'app': FirebaseApp; - } -} diff --git a/packages/app/index.ts b/packages/app/index.ts index e5911df11f6..2170cce61a0 100644 --- a/packages/app/index.ts +++ b/packages/app/index.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { FirebaseNamespace, FirebaseApp } from '@firebase/app-types'; +import { FirebaseNamespace } from '@firebase/app-types'; import { firebase as firebaseNamespace } from './src/firebaseNamespace'; import { isNode, isBrowser } from '@firebase/util'; import { logger } from './src/logger'; @@ -69,9 +69,3 @@ export const firebase = firebaseNamespace; // eslint-disable-next-line import/no-default-export export default firebase; - -declare module '@firebase/component' { - interface NameServiceMapping { - 'app': FirebaseApp; - } -} diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index 8fff57b0b7b..1dd50b182e4 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -29,7 +29,8 @@ import { deepCopy } from '@firebase/util'; import { ComponentContainer, Component, - ComponentType + ComponentType, + Name } from '@firebase/component'; import { AppError, ERROR_FACTORY } from './errors'; import { DEFAULT_ENTRY_NAME } from './constants'; @@ -125,8 +126,7 @@ export class FirebaseAppImpl implements FirebaseApp { // getImmediate will always succeed because _getService is only called for registered components. return ( (this.container - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .getProvider(name as any) + .getProvider(name as Name) .getImmediate({ identifier: instanceIdentifier }) as unknown) as FirebaseService diff --git a/packages/component/index.ts b/packages/component/index.ts index b070304ce1b..3fb81c71e25 100644 --- a/packages/component/index.ts +++ b/packages/component/index.ts @@ -22,5 +22,6 @@ export { ComponentType, InstanceFactory, InstantiationMode, - NameServiceMapping + NameServiceMapping, + Name } from './src/types'; diff --git a/packages/component/src/component_container.ts b/packages/component/src/component_container.ts index 49f4e2dde86..f20cd415567 100644 --- a/packages/component/src/component_container.ts +++ b/packages/component/src/component_container.ts @@ -23,7 +23,7 @@ import { Name } from './types'; * ComponentContainer that provides Providers for service name T, e.g. `auth`, `auth-internal` */ export class ComponentContainer { - private readonly providers = new Map(); + private readonly providers = new Map>(); constructor(private readonly name: string) {} @@ -76,7 +76,7 @@ export class ComponentContainer { return provider as Provider; } - getProviders(): Provider[] { + getProviders(): Array> { return Array.from(this.providers.values()); } } diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 8240c7f0d9e..342dc851bea 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -23,12 +23,12 @@ import { Component } from './component'; /** * Provider for instance for service name T, e.g. 'auth', 'auth-internal' - * U is an alias for the type of the instance + * NameServiceMapping[T] is an alias for the type of the instance */ -export class Provider { +export class Provider { private component: Component | null = null; - private readonly instances: Map = new Map(); - private readonly instancesDeferred: Map> = new Map(); + private readonly instances: Map = new Map(); + private readonly instancesDeferred: Map> = new Map(); constructor( private readonly name: T, @@ -39,12 +39,12 @@ export class Provider { * @param identifier A provider can provide mulitple instances of a service * if this.component.multipleInstances is true. */ - get(identifier: string = DEFAULT_ENTRY_NAME): Promise { + get(identifier: string = DEFAULT_ENTRY_NAME): Promise { // if multipleInstances is not supported, use the default name const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); if (!this.instancesDeferred.has(normalizedIdentifier)) { - const deferred = new Deferred(); + const deferred = new Deferred(); this.instancesDeferred.set(normalizedIdentifier, deferred); // If the service instance is available, resolve the promise with it immediately const instance = this.getOrInitializeService(normalizedIdentifier); @@ -64,12 +64,12 @@ export class Provider { * the service is not immediately available. * If optional is true, the method returns null if the service is not immediately available. */ - getImmediate(options: { identifier?: string; optional: true }): U | null; - getImmediate(options?: { identifier?: string; optional?: false }): U; + getImmediate(options: { identifier?: string; optional: true }): NameServiceMapping[T] | null; + getImmediate(options?: { identifier?: string; optional?: false }): NameServiceMapping[T]; getImmediate(options?: { identifier?: string; optional?: boolean; - }): U | null { + }): NameServiceMapping[T] | null { const { identifier, optional } = { identifier: DEFAULT_ENTRY_NAME, optional: false, @@ -147,13 +147,13 @@ export class Provider { return this.component != null; } - private getOrInitializeService(identifier: string): U | null { + private getOrInitializeService(identifier: string): NameServiceMapping[T] | null { let instance = this.instances.get(identifier); if (!instance && this.component) { instance = this.component.instanceFactory( this.container, normalizeIdentifierForFactory(identifier) - ) as U; + ) as NameServiceMapping[T]; this.instances.set(identifier, instance); } diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index eadeff79bf5..a3527a0e76a 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -56,9 +56,7 @@ export interface Dictionary { * This interface will be extended by Firebase SDKs to provide service name and service type mapping. * It is used as a generic constraint to ensure type safety. */ -export interface NameServiceMapping { - 'app': string; -} +export interface NameServiceMapping { } export type Name = keyof NameServiceMapping; export type Service = NameServiceMapping[Name]; From c7255b98ac6e30415336db40f9e869d282c79c35 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Sun, 10 Nov 2019 22:31:01 -0800 Subject: [PATCH 50/54] [AUTOMATED]: Prettier Code Styling --- packages/analytics-types/index.d.ts | 3 +-- packages/app/src/firebaseApp.ts | 10 +++------- packages/component/src/provider.ts | 19 +++++++++++++++---- packages/component/src/types.ts | 2 +- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/analytics-types/index.d.ts b/packages/analytics-types/index.d.ts index 3833c8048c6..328458b6925 100644 --- a/packages/analytics-types/index.d.ts +++ b/packages/analytics-types/index.d.ts @@ -202,9 +202,8 @@ export interface Promotion { name?: string; } - declare module '@firebase/component' { interface NameServiceMapping { 'analytics': FirebaseAnalytics; } -} \ No newline at end of file +} diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index 1dd50b182e4..d406c20cdc0 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -124,13 +124,9 @@ export class FirebaseAppImpl implements FirebaseApp { this.checkDestroyed_(); // getImmediate will always succeed because _getService is only called for registered components. - return ( - (this.container - .getProvider(name as Name) - .getImmediate({ - identifier: instanceIdentifier - }) as unknown) as FirebaseService - ); + return (this.container.getProvider(name as Name).getImmediate({ + identifier: instanceIdentifier + }) as unknown) as FirebaseService; } /** * Remove a service instance from the cache, so we will create a new instance for this service diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 342dc851bea..ab4ab28d54f 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -28,7 +28,10 @@ import { Component } from './component'; export class Provider { private component: Component | null = null; private readonly instances: Map = new Map(); - private readonly instancesDeferred: Map> = new Map(); + private readonly instancesDeferred: Map< + string, + Deferred + > = new Map(); constructor( private readonly name: T, @@ -64,8 +67,14 @@ export class Provider { * the service is not immediately available. * If optional is true, the method returns null if the service is not immediately available. */ - getImmediate(options: { identifier?: string; optional: true }): NameServiceMapping[T] | null; - getImmediate(options?: { identifier?: string; optional?: false }): NameServiceMapping[T]; + getImmediate(options: { + identifier?: string; + optional: true; + }): NameServiceMapping[T] | null; + getImmediate(options?: { + identifier?: string; + optional?: false; + }): NameServiceMapping[T]; getImmediate(options?: { identifier?: string; optional?: boolean; @@ -147,7 +156,9 @@ export class Provider { return this.component != null; } - private getOrInitializeService(identifier: string): NameServiceMapping[T] | null { + private getOrInitializeService( + identifier: string + ): NameServiceMapping[T] | null { let instance = this.instances.get(identifier); if (!instance && this.component) { instance = this.component.instanceFactory( diff --git a/packages/component/src/types.ts b/packages/component/src/types.ts index a3527a0e76a..7d2d63a9ec1 100644 --- a/packages/component/src/types.ts +++ b/packages/component/src/types.ts @@ -56,7 +56,7 @@ export interface Dictionary { * This interface will be extended by Firebase SDKs to provide service name and service type mapping. * It is used as a generic constraint to ensure type safety. */ -export interface NameServiceMapping { } +export interface NameServiceMapping {} export type Name = keyof NameServiceMapping; export type Service = NameServiceMapping[Name]; From 777026d3196b5adf3a72ebb25442ee3e4bed5dda Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Sun, 10 Nov 2019 22:46:40 -0800 Subject: [PATCH 51/54] update some types --- packages/app/src/firebaseApp.ts | 4 ++-- packages/app/src/lite/firebaseAppLite.ts | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index d406c20cdc0..187d1cdb823 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -124,9 +124,9 @@ export class FirebaseAppImpl implements FirebaseApp { this.checkDestroyed_(); // getImmediate will always succeed because _getService is only called for registered components. - return (this.container.getProvider(name as Name).getImmediate({ + return this.container.getProvider(name as Name).getImmediate({ identifier: instanceIdentifier - }) as unknown) as FirebaseService; + }) as unknown as FirebaseService; } /** * Remove a service instance from the cache, so we will create a new instance for this service diff --git a/packages/app/src/lite/firebaseAppLite.ts b/packages/app/src/lite/firebaseAppLite.ts index 8ca434ca7c1..e8b428cf676 100644 --- a/packages/app/src/lite/firebaseAppLite.ts +++ b/packages/app/src/lite/firebaseAppLite.ts @@ -31,7 +31,8 @@ import { DEFAULT_ENTRY_NAME } from '../constants'; import { ComponentContainer, Component, - ComponentType + ComponentType, + Name } from '@firebase/component'; interface ServicesCache { @@ -134,12 +135,11 @@ export class FirebaseAppLiteImpl implements FirebaseApp { // getImmediate will always succeed because _getService is only called for registered components. return ( - (this.container - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .getProvider(name as any) + this.container + .getProvider(name as Name) .getImmediate({ identifier: instanceIdentifier - }) as unknown) as FirebaseService + }) as unknown as FirebaseService ); } From 059fb5da21d6885890d200ea7eae8e1cb10113a9 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Sun, 10 Nov 2019 22:46:47 -0800 Subject: [PATCH 52/54] [AUTOMATED]: Prettier Code Styling --- packages/app/src/firebaseApp.ts | 4 ++-- packages/app/src/lite/firebaseAppLite.ts | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/app/src/firebaseApp.ts b/packages/app/src/firebaseApp.ts index 187d1cdb823..d406c20cdc0 100644 --- a/packages/app/src/firebaseApp.ts +++ b/packages/app/src/firebaseApp.ts @@ -124,9 +124,9 @@ export class FirebaseAppImpl implements FirebaseApp { this.checkDestroyed_(); // getImmediate will always succeed because _getService is only called for registered components. - return this.container.getProvider(name as Name).getImmediate({ + return (this.container.getProvider(name as Name).getImmediate({ identifier: instanceIdentifier - }) as unknown as FirebaseService; + }) as unknown) as FirebaseService; } /** * Remove a service instance from the cache, so we will create a new instance for this service diff --git a/packages/app/src/lite/firebaseAppLite.ts b/packages/app/src/lite/firebaseAppLite.ts index e8b428cf676..e19cd940252 100644 --- a/packages/app/src/lite/firebaseAppLite.ts +++ b/packages/app/src/lite/firebaseAppLite.ts @@ -134,13 +134,9 @@ export class FirebaseAppLiteImpl implements FirebaseApp { this.checkDestroyed_(); // getImmediate will always succeed because _getService is only called for registered components. - return ( - this.container - .getProvider(name as Name) - .getImmediate({ - identifier: instanceIdentifier - }) as unknown as FirebaseService - ); + return (this.container.getProvider(name as Name).getImmediate({ + identifier: instanceIdentifier + }) as unknown) as FirebaseService; } /** From 917322c05e42c1933004bf604b424763c949c237 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Tue, 12 Nov 2019 23:36:08 -0800 Subject: [PATCH 53/54] handle errors from instance factory --- packages/component/src/provider.test.ts | 34 +++++++++++++++++ packages/component/src/provider.ts | 51 ++++++++++++++++++------- 2 files changed, 71 insertions(+), 14 deletions(-) diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index e83fe04c51c..9118cba92e3 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -49,6 +49,40 @@ describe('Provider', () => { ).to.not.throw(); }); + it('does not throw if instance factory throws when calling getImmediate() with optional flag', () => { + provider.setComponent(getFakeComponent('test', () => { throw Error('something went wrong!'); })); + expect(() => provider.getImmediate({ optional: true })).to.not.throw(); + }); + + it('throws if instance factory throws when calling getImmediate() without optional flag', () => { + provider.setComponent(getFakeComponent('test', () => { throw Error('something went wrong!'); })); + expect(() => provider.getImmediate()).to.throw(); + }); + + it('does not throw if instance factory throws when calling get()', () => { + provider.setComponent(getFakeComponent('test', () => { throw Error('something went wrong!'); })); + expect(() => provider.get()).to.not.throw(); + }); + + it('does not throw if instance factory throws when registering an eager component', () => { + const eagerComponent = getFakeComponent( + 'test', + () => { throw Error('something went wrong!'); }, + false, + InstantiationMode.EAGER + ); + + expect(() => provider.setComponent(eagerComponent)).to.not.throw(); + }); + + it('does not throw if instance factory throws when registering a component with a pending promise', () => { + // create a pending promise + // eslint-disable-next-line @typescript-eslint/no-floating-promises + provider.get(); + const component = getFakeComponent('test', () => { throw Error('something went wrong!'); }); + expect(() => provider.setComponent(component)).to.not.throw(); + }); + describe('Provider (multipleInstances = false)', () => { describe('getImmediate()', () => { it('throws if the service is not available', () => { diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index ab4ab28d54f..a067edc25bc 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -36,7 +36,7 @@ export class Provider { constructor( private readonly name: T, private readonly container: ComponentContainer - ) {} + ) { } /** * @param identifier A provider can provide mulitple instances of a service @@ -50,9 +50,14 @@ export class Provider { const deferred = new Deferred(); this.instancesDeferred.set(normalizedIdentifier, deferred); // If the service instance is available, resolve the promise with it immediately - const instance = this.getOrInitializeService(normalizedIdentifier); - if (instance) { - deferred.resolve(instance); + try { + const instance = this.getOrInitializeService(normalizedIdentifier); + if (instance) { + deferred.resolve(instance); + } + } catch (e) { + // when the instance factory throws an exception during get(), it should not cause + // an fatal error. We just return the unresolved promise in this case. } } @@ -86,17 +91,24 @@ 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(normalizedIdentifier); + if (!instance) { + if (optional) { + return null; + } + throw Error(`Service ${this.name} is not available`); + } - if (!instance) { + return instance; + } catch (e) { if (optional) { return null; + } else { + throw e; } - throw Error(`Service ${this.name} is not available`); } - - return instance; } setComponent(component: Component): void { @@ -113,7 +125,14 @@ export class Provider { this.component = component; // if the service is eager, initialize the default instance if (isComponentEager(component)) { - this.getOrInitializeService(DEFAULT_ENTRY_NAME); + try { + this.getOrInitializeService(DEFAULT_ENTRY_NAME); + } catch (e) { + // when the instance factory for an eager Component throws an exception during the eager + // initialization, it should not cause an fatal error. + // TODO: Investigate if we need to make it configurable, because some component may want to cause + // a fatal error in this case? + } } // Create service instances for the pending promises and resolve them @@ -127,10 +146,14 @@ export class Provider { instanceIdentifier ); - // `getOrInitializeService()` should always return a valid instance since a component is guaranteed. use ! to make typescript happy. - const instance = this.getOrInitializeService(normalizedIdentifier)!; - - instanceDeferred.resolve(instance); + try { + // `getOrInitializeService()` should always return a valid instance since a component is guaranteed. use ! to make typescript happy. + const instance = this.getOrInitializeService(normalizedIdentifier)!; + instanceDeferred.resolve(instance); + } catch (e) { + // when the instance factory throws an exception, it should not cause + // an fatal error. We just leave the promise unresolved. + } } } From 4e661d109f1eceb75152db7a13fbe413fe915862 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Tue, 12 Nov 2019 23:36:16 -0800 Subject: [PATCH 54/54] [AUTOMATED]: Prettier Code Styling --- packages/component/src/provider.test.ts | 26 ++++++++++++++++++++----- packages/component/src/provider.ts | 4 ++-- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 9118cba92e3..80b5168df27 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -50,24 +50,38 @@ describe('Provider', () => { }); it('does not throw if instance factory throws when calling getImmediate() with optional flag', () => { - provider.setComponent(getFakeComponent('test', () => { throw Error('something went wrong!'); })); + provider.setComponent( + getFakeComponent('test', () => { + throw Error('something went wrong!'); + }) + ); expect(() => provider.getImmediate({ optional: true })).to.not.throw(); }); it('throws if instance factory throws when calling getImmediate() without optional flag', () => { - provider.setComponent(getFakeComponent('test', () => { throw Error('something went wrong!'); })); + provider.setComponent( + getFakeComponent('test', () => { + throw Error('something went wrong!'); + }) + ); expect(() => provider.getImmediate()).to.throw(); }); it('does not throw if instance factory throws when calling get()', () => { - provider.setComponent(getFakeComponent('test', () => { throw Error('something went wrong!'); })); + provider.setComponent( + getFakeComponent('test', () => { + throw Error('something went wrong!'); + }) + ); expect(() => provider.get()).to.not.throw(); }); it('does not throw if instance factory throws when registering an eager component', () => { const eagerComponent = getFakeComponent( 'test', - () => { throw Error('something went wrong!'); }, + () => { + throw Error('something went wrong!'); + }, false, InstantiationMode.EAGER ); @@ -79,7 +93,9 @@ describe('Provider', () => { // create a pending promise // eslint-disable-next-line @typescript-eslint/no-floating-promises provider.get(); - const component = getFakeComponent('test', () => { throw Error('something went wrong!'); }); + const component = getFakeComponent('test', () => { + throw Error('something went wrong!'); + }); expect(() => provider.setComponent(component)).to.not.throw(); }); diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index a067edc25bc..063d4951805 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -36,7 +36,7 @@ export class Provider { constructor( private readonly name: T, private readonly container: ComponentContainer - ) { } + ) {} /** * @param identifier A provider can provide mulitple instances of a service @@ -128,7 +128,7 @@ export class Provider { try { this.getOrInitializeService(DEFAULT_ENTRY_NAME); } catch (e) { - // when the instance factory for an eager Component throws an exception during the eager + // when the instance factory for an eager Component throws an exception during the eager // initialization, it should not cause an fatal error. // TODO: Investigate if we need to make it configurable, because some component may want to cause // a fatal error in this case?