From 1cdbdf6d1dd1ff35280e6230b8d18358c21f9dbf Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Wed, 16 Oct 2019 17:35:40 -0700 Subject: [PATCH 1/3] Migrate Performance to component framework --- packages/app-types/index.d.ts | 4 ++- packages/app/index.ts | 2 +- packages/app/src/firebaseNamespaceCore.ts | 2 +- packages/app/test/firebaseApp.test.ts | 4 +-- packages/performance/index.ts | 35 +++++++++++++------ packages/performance/package.json | 1 + .../src/services/iid_service.test.ts | 15 ++++---- .../performance/src/services/iid_service.ts | 10 ++---- .../src/services/settings_service.ts | 3 ++ 9 files changed, 46 insertions(+), 30 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.ts b/packages/app/index.ts index 62e13fd0bf0..56a4af83f9e 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 607b7497edd..4ff023d59dd 100644 --- a/packages/app/src/firebaseNamespaceCore.ts +++ b/packages/app/src/firebaseNamespaceCore.ts @@ -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, diff --git a/packages/app/test/firebaseApp.test.ts b/packages/app/test/firebaseApp.test.ts index 671a050ddb7..e8999f5a2e4 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? @@ -342,4 +342,4 @@ function createTestComponent( ); component.setMultipleInstances(multiInstances); return component; -} +} \ No newline at end of file diff --git a/packages/performance/index.ts b/packages/performance/index.ts index c009313e1d9..b6bfecf9abc 100644 --- a/packages/performance/index.ts +++ b/packages/performance/index.ts @@ -16,22 +16,23 @@ */ import firebase from '@firebase/app'; +import '@firebase/installations'; import { FirebaseApp, FirebaseNamespace } from '@firebase/app-types'; -import { - _FirebaseNamespace, - FirebaseServiceFactory -} from '@firebase/app-types/private'; +import { _FirebaseNamespace } from '@firebase/app-types/private'; import { PerformanceController } from './src/controllers/perf'; import { setupApi } from './src/services/api_service'; import { SettingsService } from './src/services/settings_service'; import { ERROR_FACTORY, ErrorCode } from './src/utils/errors'; import { FirebasePerformance } from '@firebase/performance-types'; +import { Component, ComponentType } from '@firebase/component'; +import { FirebaseInstallations } from '@firebase/installations-types'; const DEFAULT_ENTRY_NAME = '[DEFAULT]'; export function registerPerformance(instance: FirebaseNamespace): void { - const factoryMethod: FirebaseServiceFactory = ( - app: FirebaseApp + const factoryMethod = ( + app: FirebaseApp, + installations: FirebaseInstallations ): PerformanceController => { if (app.name !== DEFAULT_ENTRY_NAME) { throw ERROR_FACTORY.create(ErrorCode.FB_NOT_DEFAULT); @@ -41,15 +42,27 @@ export function registerPerformance(instance: FirebaseNamespace): void { } setupApi(window); SettingsService.getInstance().firebaseAppInstance = app; + SettingsService.getInstance().installationsService = installations; return new PerformanceController(app); }; // Register performance with firebase-app. - const namespaceExports = {}; - (instance as _FirebaseNamespace).INTERNAL.registerService( - 'performance', - factoryMethod, - namespaceExports + (instance as _FirebaseNamespace).INTERNAL.registerComponent( + new Component( + 'performance', + container => { + /* Dependencies */ + // getImmediate for FirebaseApp will always succeed + const app = container.getProvider('app').getImmediate(); + // The following call will always succeed because perf has `import '@firebase/installations'` + const installations = container + .getProvider('installations') + .getImmediate(); + + return factoryMethod(app, installations); + }, + ComponentType.PUBLIC + ) ); } diff --git a/packages/performance/package.json b/packages/performance/package.json index 9236023673b..5e3aebf4a48 100644 --- a/packages/performance/package.json +++ b/packages/performance/package.json @@ -30,6 +30,7 @@ "@firebase/installations": "0.3.2", "@firebase/util": "0.2.31", "@firebase/performance-types": "0.0.5", + "@firebase/component": "0.1.0", "tslib": "1.10.0" }, "license": "Apache-2.0", diff --git a/packages/performance/src/services/iid_service.test.ts b/packages/performance/src/services/iid_service.test.ts index 36bc6e2afbf..cd1444e1b09 100644 --- a/packages/performance/src/services/iid_service.test.ts +++ b/packages/performance/src/services/iid_service.test.ts @@ -24,18 +24,21 @@ import { getAuthenticationToken, getAuthTokenPromise } from './iid_service'; -import { FirebaseApp } from '@firebase/app-types'; import '../../test/setup'; +import { FirebaseInstallations } from '@firebase/installations-types'; describe('Firebase Perofmrance > iid_service', () => { const IID = 'fid'; const AUTH_TOKEN = 'authToken'; - const getId = stub().resolves(IID); - const getToken = stub().resolves(AUTH_TOKEN); - SettingsService.prototype.firebaseAppInstance = ({ - installations: () => ({ getId, getToken }) - } as unknown) as FirebaseApp; + before(() => { + const getId = stub().resolves(IID); + const getToken = stub().resolves(AUTH_TOKEN); + SettingsService.prototype.installationsService = ({ + getId, + getToken + } as unknown) as FirebaseInstallations; + }); describe('getIidPromise', () => { it('provides iid', async () => { diff --git a/packages/performance/src/services/iid_service.ts b/packages/performance/src/services/iid_service.ts index c0d3b14edac..5750de0e8aa 100644 --- a/packages/performance/src/services/iid_service.ts +++ b/packages/performance/src/services/iid_service.ts @@ -14,17 +14,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -import '@firebase/installations'; import { SettingsService } from './settings_service'; let iid: string | undefined; let authToken: string | undefined; export function getIidPromise(): Promise { - const iidPromise = SettingsService.getInstance() - .firebaseAppInstance.installations() - .getId(); + const iidPromise = SettingsService.getInstance().installationsService.getId(); // eslint-disable-next-line @typescript-eslint/no-floating-promises iidPromise.then((iidVal: string) => { iid = iidVal; @@ -38,9 +34,7 @@ export function getIid(): string | undefined { } export function getAuthTokenPromise(): Promise { - const authTokenPromise = SettingsService.getInstance() - .firebaseAppInstance.installations() - .getToken(); + const authTokenPromise = SettingsService.getInstance().installationsService.getToken(); // eslint-disable-next-line @typescript-eslint/no-floating-promises authTokenPromise.then((authTokenVal: string) => { authToken = authTokenVal; diff --git a/packages/performance/src/services/settings_service.ts b/packages/performance/src/services/settings_service.ts index 4e0fafa9572..d1f48df7ef9 100644 --- a/packages/performance/src/services/settings_service.ts +++ b/packages/performance/src/services/settings_service.ts @@ -17,6 +17,7 @@ import { FirebaseApp } from '@firebase/app-types'; import { ERROR_FACTORY, ErrorCode } from '../utils/errors'; +import { FirebaseInstallations } from '@firebase/installations-types'; let settingsServiceInstance: SettingsService | undefined; @@ -46,6 +47,8 @@ export class SettingsService { firebaseAppInstance!: FirebaseApp; + installationsService!: FirebaseInstallations; + getAppId(): string { const appId = this.firebaseAppInstance && From cbee5cc62a7d778c05df731e7215457b8fc4670a Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Fri, 1 Nov 2019 11:59:14 -0700 Subject: [PATCH 2/3] [AUTOMATED]: Prettier Code Styling --- packages/app-types/index.d.ts | 4 ++-- packages/app/index.ts | 2 +- packages/app/src/firebaseNamespaceCore.ts | 2 +- packages/app/test/firebaseApp.test.ts | 4 ++-- 4 files changed, 6 insertions(+), 6 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.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 4ff023d59dd..607b7497edd 100644 --- a/packages/app/src/firebaseNamespaceCore.ts +++ b/packages/app/src/firebaseNamespaceCore.ts @@ -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, diff --git a/packages/app/test/firebaseApp.test.ts b/packages/app/test/firebaseApp.test.ts index e8999f5a2e4..671a050ddb7 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? @@ -342,4 +342,4 @@ function createTestComponent( ); component.setMultipleInstances(multiInstances); return component; -} \ No newline at end of file +} From 551dbb612368ddb7974af0d46dd22aad195fef0e Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Fri, 1 Nov 2019 12:03:21 -0700 Subject: [PATCH 3/3] removed 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.