From 718c99c77326c1ded663e5fdd2638f052385fec7 Mon Sep 17 00:00:00 2001 From: Josh Crowther Date: Wed, 17 May 2017 19:57:55 -0700 Subject: [PATCH 1/9] WIP: add lazy module instantiation --- src/app/firebase_app.ts | 80 +++++++++++------------------ tests/app/unit/firebase_app.test.ts | 40 +++++++++++++++ 2 files changed, 71 insertions(+), 49 deletions(-) diff --git a/src/app/firebase_app.ts b/src/app/firebase_app.ts index f57a4fe5cc4..961670b096b 100644 --- a/src/app/firebase_app.ts +++ b/src/app/firebase_app.ts @@ -229,8 +229,12 @@ class FirebaseAppImpl implements FirebaseApp { private options_: FirebaseOptions; private name_: string; private isDeleted_ = false; - private services_: {[name: string]: - {[instance: string]: FirebaseService}} = {}; + private _registeredServices: { + [name: string]: boolean + } = {}; + private services_: { + [name: string]: FirebaseService + } = {}; public INTERNAL: FirebaseAppInternals; constructor(options: FirebaseOptions, @@ -238,18 +242,6 @@ class FirebaseAppImpl implements FirebaseApp { private firebase_: FirebaseNamespace) { this.name_ = name; this.options_ = deepCopy(options); - - Object.keys(firebase_.INTERNAL.factories).forEach((serviceName) => { - // Ignore virtual services - let factoryName = firebase_.INTERNAL.useAsService(this, serviceName); - if (factoryName === null) { - return; - } - - // Defer calling createService until service is accessed. - let getService = this.getService.bind(this, factoryName); - patchProperty(this, serviceName, getService); - }); } get name(): string { @@ -288,24 +280,17 @@ class FirebaseAppImpl implements FirebaseApp { /** * Return the service instance associated with this app (creating it * on demand). + * @internal */ - private getService(name: string, instanceString?: string): FirebaseService - |null { + _getService(name: string): FirebaseService { this.checkDestroyed_(); - if (typeof this.services_[name] === 'undefined') { - this.services_[name] = {}; + if (!this.services_[name]) { + let service = this.firebase_.INTERNAL.factories[name](this, this.extendApp.bind(this)); + this.services_[name] = service; } - let instanceSpecifier = instanceString || DEFAULT_ENTRY_NAME; - if (typeof this.services_[name]![instanceSpecifier] === 'undefined') { - let firebaseService = this.firebase_.INTERNAL.factories[name]( - this, this.extendApp.bind(this), instanceString); - this.services_[name]![instanceSpecifier] = firebaseService; - return firebaseService; - } else { - return this.services_[name]![instanceSpecifier] as FirebaseService | null; - } + return this.services_[name]; } /** @@ -425,8 +410,9 @@ export function createFirebaseNamespace(): FirebaseNamespace { if (apps_[name!] !== undefined) { error('duplicate-app', {'name': name}); } - let app = new FirebaseAppImpl(options, name!, - ((namespace as any) as FirebaseNamespace)); + + let app = new FirebaseAppImpl(options, name!, namespace as FirebaseNamespace); + apps_[name!] = app; callAppHooks(app, 'create'); @@ -474,36 +460,27 @@ export function createFirebaseNamespace(): FirebaseNamespace { if (factories[name]) { error('duplicate-service', {'name': name}); } - if (!!allowMultipleInstances) { - // Check if the service allows multiple instances per app - factories[name] = createService; - } else { - // If not, always return the same instance when a service is instantiated - // with an instanceString different than the default. - factories[name] = - (app: FirebaseApp, extendApp?: (props: {[prop: string]: any}) => void, - instanceString?: string) => { - // If a new instance is requested for a service that does not allow - // multiple instances, return the default instance - return createService(app, extendApp, DEFAULT_ENTRY_NAME); - }; - } + + /** + * If multiple instances are allowed, return the true create service + * otherwise, return a proxied reference to the same service + */ + factories[name] = allowMultipleInstances ? createService : + (...args) => createService.apply(this, args); + + // Capture the appHook, if passed if (appHook) { appHooks[name] = appHook; } - let serviceNamespace: FirebaseServiceNamespace; - // The Service namespace is an accessor function ... - serviceNamespace = (appArg?: FirebaseApp) => { - if (appArg === undefined) { - appArg = app(); - } + const serviceNamespace = (appArg: FirebaseApp = app()) => { if (typeof(appArg as any)[name] !== 'function') { // Invalid argument. // This happens in the following case: firebase.storage('gs:/') error('invalid-app-argument', {'name': name}); } + // Forward service instance lookup to the FirebaseApp. return (appArg as any)[name](); }; @@ -516,6 +493,11 @@ export function createFirebaseNamespace(): FirebaseNamespace { // Monkey-patch the serviceNamespace onto the firebase namespace (namespace as any)[name] = serviceNamespace; + // Patch the FirebaseAppImpl prototype + FirebaseAppImpl.prototype[name] = function() { + return this._getService(name); + } + return serviceNamespace; } diff --git a/tests/app/unit/firebase_app.test.ts b/tests/app/unit/firebase_app.test.ts index 086b495bff3..8ea66634ef2 100644 --- a/tests/app/unit/firebase_app.test.ts +++ b/tests/app/unit/firebase_app.test.ts @@ -166,6 +166,46 @@ describe("Firebase App Class", () => { assert.equal(registrations, 2); }); + it("Can lazy load a service", () => { + let registrations = 0; + + const app1 = firebase.initializeApp({}); + assert.isUndefined((app1 as any).lazyService); + + firebase.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); + + // 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); + }); + describe("Check for bad app names", () => { let tests = ["", 123, false, null]; for (let data of tests) { From 1413770efa3288a895e16e1622bfac2328626eee Mon Sep 17 00:00:00 2001 From: Josh Crowther Date: Wed, 17 May 2017 20:03:10 -0700 Subject: [PATCH 2/9] feat(app): allow for lazy loading of services --- src/app/firebase_app.ts | 3 +++ tests/app/unit/firebase_app.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/app/firebase_app.ts b/src/app/firebase_app.ts index 961670b096b..24ace9ebebc 100644 --- a/src/app/firebase_app.ts +++ b/src/app/firebase_app.ts @@ -471,6 +471,9 @@ export function createFirebaseNamespace(): FirebaseNamespace { // Capture the appHook, if passed if (appHook) { appHooks[name] = appHook; + getApps().forEach(app => { + appHook('create', app); + }); } // The Service namespace is an accessor function ... diff --git a/tests/app/unit/firebase_app.test.ts b/tests/app/unit/firebase_app.test.ts index 8ea66634ef2..6774789f94e 100644 --- a/tests/app/unit/firebase_app.test.ts +++ b/tests/app/unit/firebase_app.test.ts @@ -206,6 +206,28 @@ describe("Firebase App Class", () => { assert.equal(registrations, 2); }); + it("Can lazy register App Hook", (done) => { + let events = ['create', 'delete']; + let hookEvents = 0; + const app = firebase.initializeApp({}); + firebase.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); + app.delete(); + }); + describe("Check for bad app names", () => { let tests = ["", 123, false, null]; for (let data of tests) { From f26cfc54eb26fa14afde81d5b0366e2f850dd869 Mon Sep 17 00:00:00 2001 From: Josh Crowther Date: Wed, 17 May 2017 20:13:08 -0700 Subject: [PATCH 3/9] test(app): add integration test for webpack/browserify for lazy instantiation --- .../bundlers/browser/lazy-app.test.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 tests-integration/bundlers/browser/lazy-app.test.js diff --git a/tests-integration/bundlers/browser/lazy-app.test.js b/tests-integration/bundlers/browser/lazy-app.test.js new file mode 100644 index 00000000000..a9f2a07c4cc --- /dev/null +++ b/tests-integration/bundlers/browser/lazy-app.test.js @@ -0,0 +1,21 @@ +var assert = require('chai').assert; +// Partial inclusion is a browser-only feature +var firebase = require('../../../dist/package/app'); +var helper = require('./test-helper.js'); + +describe("Lazy Firebase App (" + helper.getPackagerName() + ")", function() { + it("firebase namespace", function() { + assert.isDefined(firebase); + }); + + it("SDK_VERSION", function() { + assert.isDefined(firebase.SDK_VERSION); + }); + + it('Should allow for lazy component init', function() { + assert.isUndefined(firebase.database); + firebase.initializeApp({}); + require('../../../dist/package/database'); + assert.isDefined(firebase.database); + }); +}); From 3a38558aef7e9948cd5903475ed9f3c583f33fc4 Mon Sep 17 00:00:00 2001 From: Josh Crowther Date: Thu, 18 May 2017 13:01:29 -0700 Subject: [PATCH 4/9] fix(app): fix issue where auth listeners weren't being patched properly --- src/app/firebase_app.ts | 48 +++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/src/app/firebase_app.ts b/src/app/firebase_app.ts index 24ace9ebebc..64e83856b15 100644 --- a/src/app/firebase_app.ts +++ b/src/app/firebase_app.ts @@ -221,6 +221,10 @@ let LocalPromise = local.Promise as typeof Promise; const DEFAULT_ENTRY_NAME = '[DEFAULT]'; +// An array to capture listeners before the true auth functions +// exist +let tokenListeners = []; + /** * Global context object for a collection of services using * a shared authentication state. @@ -235,7 +239,8 @@ class FirebaseAppImpl implements FirebaseApp { private services_: { [name: string]: FirebaseService } = {}; - public INTERNAL: FirebaseAppInternals; + + public INTERNAL; constructor(options: FirebaseOptions, name: string, @@ -298,7 +303,17 @@ class FirebaseAppImpl implements FirebaseApp { * of service instance creation. */ private extendApp(props: {[name: string]: any}): void { - deepExtend(this, props); + // Copy the object onto the FirebaseAppImpl prototype + deepExtend(FirebaseAppImpl.prototype, props); + + // If the app has overwritten the addAuthTokenListener stub, forward + // the active token listeners on to the true fxn. + if (props.INTERNAL && props.INTERNAL.addAuthTokenListener) { + tokenListeners.forEach(listener => { + this.INTERNAL.addAuthTokenListener(listener); + }); + tokenListeners = []; + } } /** @@ -312,6 +327,19 @@ class FirebaseAppImpl implements FirebaseApp { } }; +FirebaseAppImpl.prototype.INTERNAL = { + 'getUid': () => null, + 'getToken': () => LocalPromise.resolve(null), + 'addAuthTokenListener': (callback: (token: string|null) => void) => { + tokenListeners.push(callback); + // Make sure callback is called, asynchronously, in the absence of the auth module + setTimeout(() => callback(null), 0); + }, + 'removeAuthTokenListener': (callback) => { + tokenListeners = tokenListeners.filter(listener => listener !== callback); + }, +} + // Prevent dead-code elimination of these methods w/o invalid property // copying. FirebaseAppImpl.prototype.name && @@ -416,22 +444,6 @@ export function createFirebaseNamespace(): FirebaseNamespace { apps_[name!] = app; callAppHooks(app, 'create'); - // Ensure that getUid, getToken, addAuthListener and removeAuthListener - // have a default implementation if no service has patched the App - // (i.e., Auth is not present). - if (app.INTERNAL == undefined || app.INTERNAL.getToken == undefined) { - deepExtend(app, { - INTERNAL: { - 'getUid': () => null, - 'getToken': () => LocalPromise.resolve(null), - 'addAuthTokenListener': (callback: (token: string|null) => void) => { - // Make sure callback is called, asynchronously, in the absence of the auth module - setTimeout(() => callback(null), 0); - }, - 'removeAuthTokenListener': () => { /*_*/ }, - } - }); - } return app; } From 47d618ae94990c1a59c97d80c3f9020a3e0d84f2 Mon Sep 17 00:00:00 2001 From: Josh Crowther Date: Mon, 22 May 2017 14:03:49 -0600 Subject: [PATCH 5/9] docs(app): adding doc explaining Firebase Auth specific code in firebase_app.ts --- src/app/firebase_app.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/app/firebase_app.ts b/src/app/firebase_app.ts index 64e83856b15..354f8db9fd5 100644 --- a/src/app/firebase_app.ts +++ b/src/app/firebase_app.ts @@ -306,8 +306,15 @@ class FirebaseAppImpl implements FirebaseApp { // Copy the object onto the FirebaseAppImpl prototype deepExtend(FirebaseAppImpl.prototype, props); - // If the app has overwritten the addAuthTokenListener stub, forward - // the active token listeners on to the true fxn. + /** + * 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 && props.INTERNAL.addAuthTokenListener) { tokenListeners.forEach(listener => { this.INTERNAL.addAuthTokenListener(listener); From 9baf65f749d81515acbdab9aff75a5e01a0cb8db Mon Sep 17 00:00:00 2001 From: Josh Crowther Date: Tue, 23 May 2017 14:51:32 -0600 Subject: [PATCH 6/9] fix(app): revert code refactor to _getService function --- src/app/firebase_app.ts | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/app/firebase_app.ts b/src/app/firebase_app.ts index 354f8db9fd5..1e51caca9e3 100644 --- a/src/app/firebase_app.ts +++ b/src/app/firebase_app.ts @@ -233,11 +233,10 @@ class FirebaseAppImpl implements FirebaseApp { private options_: FirebaseOptions; private name_: string; private isDeleted_ = false; - private _registeredServices: { - [name: string]: boolean - } = {}; private services_: { - [name: string]: FirebaseService + [name: string]: { + [serviceName: string]: FirebaseService + } } = {}; public INTERNAL; @@ -283,19 +282,32 @@ class FirebaseAppImpl implements FirebaseApp { } /** - * Return the service instance associated with this app (creating it - * on demand). + * Return a service instance associated with this app (creating it + * on demand), identified by the passed instanceIdentifier. + * + * NOTE: Currently storage is the only one that is leveraging this + * functionality. They invoke it by calling: + * + * ```javascript + * firebase.app().storage('STORAGE BUCKET ID') + * ``` + * + * The service name is passed to this already * @internal */ - _getService(name: string): FirebaseService { + _getService(name: string, instanceIdentifier: string = DEFAULT_ENTRY_NAME): FirebaseService { this.checkDestroyed_(); if (!this.services_[name]) { + this.services_[name] = {}; + } + + if (!this.services_[name][instanceIdentifier]) { let service = this.firebase_.INTERNAL.factories[name](this, this.extendApp.bind(this)); - this.services_[name] = service; + this.services_[name][instanceIdentifier] = service; } - return this.services_[name]; + return this.services_[name][instanceIdentifier]; } /** From 99820ef1a8f2504c949eeb3a1c3b51511e81df22 Mon Sep 17 00:00:00 2001 From: Josh Crowther Date: Tue, 23 May 2017 16:04:13 -0600 Subject: [PATCH 7/9] test(app): add tests and fix issue with multiple instances of a service There was an issue where we weren't properly limiting mulitple service instances. Added a test and refactored the code to support that use case --- src/app/firebase_app.ts | 5 ++-- tests/app/unit/firebase_app.test.ts | 45 +++++++++++++++++++++++++++++ tsconfig.test.json | 1 + 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/app/firebase_app.ts b/src/app/firebase_app.ts index 1e51caca9e3..bed6373c70a 100644 --- a/src/app/firebase_app.ts +++ b/src/app/firebase_app.ts @@ -528,8 +528,9 @@ export function createFirebaseNamespace(): FirebaseNamespace { (namespace as any)[name] = serviceNamespace; // Patch the FirebaseAppImpl prototype - FirebaseAppImpl.prototype[name] = function() { - return this._getService(name); + FirebaseAppImpl.prototype[name] = function(...args) { + const serviceFxn = this._getService.bind(this, name); + return serviceFxn.apply(this, allowMultipleInstances ? args : []); } return serviceNamespace; diff --git a/tests/app/unit/firebase_app.test.ts b/tests/app/unit/firebase_app.test.ts index 6774789f94e..e9733589395 100644 --- a/tests/app/unit/firebase_app.test.ts +++ b/tests/app/unit/firebase_app.test.ts @@ -228,6 +228,51 @@ describe("Firebase App Class", () => { app.delete(); }); + it('Can register multiple instances of some services', () => { + // Register Multi Instance Service + firebase.INTERNAL.registerService( + 'multiInstance', + app => new TestService(app), + null, + null, + true + ); + firebase.initializeApp({}); + + // Capture a given service ref + const service = (firebase.app() as any).multiInstance(); + assert.strictEqual(service, (firebase.app() as any).multiInstance()); + + // 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)); + + // Ensure that the two services **are not equal** + assert.notStrictEqual(service, service2); + assert.notStrictEqual((firebase.app() as any).multiInstance(), (firebase.app() as any).multiInstance(serviceIdentifier)); + }); + + it(`Should return the same instance of a service if a service doesn't support multi instance`, () => { + // Register Multi Instance Service + firebase.INTERNAL.registerService( + 'singleInstance', + app => new TestService(app), + null, + null, + false // <-- multi instance flag + ); + firebase.initializeApp({}); + + // 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, service2); + }); + describe("Check for bad app names", () => { let tests = ["", 123, false, null]; for (let data of tests) { diff --git a/tsconfig.test.json b/tsconfig.test.json index 1288876dffd..df5542946d6 100644 --- a/tsconfig.test.json +++ b/tsconfig.test.json @@ -7,6 +7,7 @@ "allowJs": true, "declaration": false }, + "compileOnSave": true, "include": [ "src/**/*.ts", "tests/**/*.ts" From 3e211b300b5862bb4d2221b9b715ac1dc9c65d19 Mon Sep 17 00:00:00 2001 From: Josh Crowther Date: Wed, 24 May 2017 13:15:46 -0600 Subject: [PATCH 8/9] refactor(app): remove unneeded ternary --- src/app/firebase_app.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/app/firebase_app.ts b/src/app/firebase_app.ts index bed6373c70a..05a72a326e9 100644 --- a/src/app/firebase_app.ts +++ b/src/app/firebase_app.ts @@ -488,20 +488,19 @@ export function createFirebaseNamespace(): FirebaseNamespace { appHook?: AppHook, allowMultipleInstances?: boolean): FirebaseServiceNamespace { + // Cannot re-register a service that already exists if (factories[name]) { error('duplicate-service', {'name': name}); } - /** - * If multiple instances are allowed, return the true create service - * otherwise, return a proxied reference to the same service - */ - factories[name] = allowMultipleInstances ? createService : - (...args) => createService.apply(this, args); + // 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); }); From c75fb59aab4007a6e82f5fe8f3ccc4b415045a36 Mon Sep 17 00:00:00 2001 From: Josh Crowther Date: Thu, 25 May 2017 09:55:34 -0600 Subject: [PATCH 9/9] fix(app): fix issue where instanceIdentifier wasn't being passed Fixed an issue where instance identifiers weren't being passed to their creation factories. Added some tests to validate that we don't accidentally remove that in the future. --- gulp/tasks/test.integration.js | 2 +- src/app/firebase_app.ts | 2 +- tests/app/unit/firebase_app.test.ts | 16 +++++++++++----- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/gulp/tasks/test.integration.js b/gulp/tasks/test.integration.js index c2dd4a1ed1e..cffbaaa8310 100644 --- a/gulp/tasks/test.integration.js +++ b/gulp/tasks/test.integration.js @@ -48,7 +48,7 @@ function compileTypescript() { function compileWebpack() { return gulp.src('tests-integration/bundlers/**/*.test.js') .pipe(named()) - .pipe(webpackStream()) + .pipe(webpackStream({}, webpack)) .pipe(rename(path => { const rawPath = path.basename.replace('.test', ''); path.basename = `${rawPath}.webpack.test`; diff --git a/src/app/firebase_app.ts b/src/app/firebase_app.ts index 05a72a326e9..f66606f3774 100644 --- a/src/app/firebase_app.ts +++ b/src/app/firebase_app.ts @@ -303,7 +303,7 @@ class FirebaseAppImpl implements FirebaseApp { } if (!this.services_[name][instanceIdentifier]) { - let service = this.firebase_.INTERNAL.factories[name](this, this.extendApp.bind(this)); + let service = this.firebase_.INTERNAL.factories[name](this, this.extendApp.bind(this), instanceIdentifier); this.services_[name][instanceIdentifier] = service; } diff --git a/tests/app/unit/firebase_app.test.ts b/tests/app/unit/firebase_app.test.ts index e9733589395..2098ebd2fea 100644 --- a/tests/app/unit/firebase_app.test.ts +++ b/tests/app/unit/firebase_app.test.ts @@ -232,7 +232,10 @@ describe("Firebase App Class", () => { // Register Multi Instance Service firebase.INTERNAL.registerService( 'multiInstance', - app => new TestService(app), + (...args) => { + const [app,,instanceIdentifier] = args; + return new TestService(app, instanceIdentifier); + }, null, null, true @@ -249,6 +252,7 @@ describe("Firebase App Class", () => { assert.strictEqual(service2, (firebase.app() as any).multiInstance(serviceIdentifier)); // 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)); }); @@ -257,7 +261,10 @@ describe("Firebase App Class", () => { // Register Multi Instance Service firebase.INTERNAL.registerService( 'singleInstance', - app => new TestService(app), + (...args) => { + const [app,,instanceIdentifier] = args; + return new TestService(app, instanceIdentifier) + }, null, null, false // <-- multi instance flag @@ -270,6 +277,7 @@ describe("Firebase App Class", () => { 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); }); @@ -286,9 +294,7 @@ describe("Firebase App Class", () => { }); class TestService implements FirebaseService { - constructor(private app_: FirebaseApp) { - // empty - } + constructor(private app_: FirebaseApp, public instanceIdentifier?: string) {} // TODO(koss): Shouldn't this just be an added method on // the service instance?