Make the _getInstance() instantiator a util for the whole SDK (#3369)

* Make the _getInstance() instantiator a util for the whole SDK

* PR feedback, added some debugAsserts

* Formatting

Author: sam-gc

packages-exp/auth-exp/src/core/auth/auth_impl.test.ts

@@ -26,10 +26,11 @@ import { FirebaseError } from '@firebase/util';
 import { testUser } from '../../../test/mock_auth';
 import { Auth } from '../../model/auth';
 import { User } from '../../model/user';
-import { _getInstance, Persistence } from '../persistence';
+import { Persistence } from '../persistence';
 import { browserLocalPersistence } from '../persistence/browser';
 import { inMemoryPersistence } from '../persistence/in_memory';
 import { PersistenceUserManager } from '../persistence/persistence_user_manager';
+import { _getInstance } from '../util/instantiator';
 import * as navigator from '../util/navigator';
 import { _getClientVersion, ClientPlatform } from '../util/version';
 import {
@@ -109,7 +110,7 @@ describe('core/auth/auth_impl', () => {
   describe('#setPersistence', () => {
     it('swaps underlying persistence', async () => {
       const newPersistence = browserLocalPersistence;
-      const newStub = sinon.stub(_getInstance(newPersistence));
+      const newStub = sinon.stub(_getInstance<Persistence>(newPersistence));
       persistenceStub.get.returns(
         Promise.resolve(testUser(auth, 'test').toPlainObject())
       );
@@ -308,7 +309,7 @@ describe('core/auth/initializeAuth', () => {
 
     it('pulls the user from storage', async () => {
       sinon
-        .stub(_getInstance(inMemoryPersistence), 'get')
+        .stub(_getInstance<Persistence>(inMemoryPersistence), 'get')
         .returns(Promise.resolve(testUser({}, 'uid').toPlainObject()));
       const auth = await initAndWait(inMemoryPersistence);
       expect(auth.currentUser!.uid).to.eq('uid');

packages-exp/auth-exp/src/core/auth/auth_impl.ts

@@ -31,9 +31,10 @@ import {
 import { Auth, Dependencies } from '../../model/auth';
 import { User } from '../../model/user';
 import { AuthErrorCode } from '../errors';
-import { _getInstance, Persistence } from '../persistence';
+import { Persistence } from '../persistence';
 import { PersistenceUserManager } from '../persistence/persistence_user_manager';
 import { assert } from '../util/assert';
+import { _getInstance } from '../util/instantiator';
 import { _getUserLanguage } from '../util/navigator';
 import { _getClientVersion, ClientPlatform } from '../util/version';
 
@@ -218,7 +219,7 @@ export function initializeAuth(
   const hierarchy = (Array.isArray(persistence)
     ? persistence
     : [persistence]
-  ).map(_getInstance);
+  ).map<Persistence>(_getInstance);
   const { apiKey, authDomain } = app.options;
 
   // TODO: platform needs to be determined using heuristics

packages-exp/auth-exp/src/core/persistence/browser.test.ts

@@ -19,7 +19,8 @@ import { expect } from 'chai';
 import * as sinon from 'sinon';
 
 import { testUser } from '../../../test/mock_auth';
-import { _getInstance, PersistedBlob, Persistence, PersistenceType } from './';
+import { _getInstance } from '../util/instantiator';
+import { PersistedBlob, Persistence, PersistenceType } from './';
 import { browserLocalPersistence, browserSessionPersistence } from './browser';
 
 describe('core/persistence/browser', () => {
@@ -74,7 +75,7 @@ describe('core/persistence/browser', () => {
   });
 
   describe('browserSessionPersistence', () => {
-    const persistence = _getInstance(browserSessionPersistence);
+    const persistence: Persistence = _getInstance(browserSessionPersistence);
 
     it('should work with persistence type', async () => {
       const key = 'my-super-special-persistence-type';

packages-exp/auth-exp/src/core/persistence/in_memory.test.ts

@@ -18,10 +18,11 @@
 import { expect } from 'chai';
 
 import { testUser } from '../../../test/mock_auth';
-import { _getInstance, PersistenceType } from './';
+import { _getInstance } from '../util/instantiator';
+import { Persistence, PersistenceType } from './';
 import { inMemoryPersistence } from './in_memory';
 
-const persistence = _getInstance(inMemoryPersistence);
+const persistence: Persistence = _getInstance(inMemoryPersistence);
 
 describe('core/persistence/in_memory', () => {
   it('should work with persistence type', async () => {

packages-exp/auth-exp/src/core/persistence/index.ts

@@ -15,8 +15,6 @@
  * limitations under the License.
  */
 
-import * as externs from '@firebase/auth-types-exp';
-
 export enum PersistenceType {
   SESSION = 'SESSION',
   LOCAL = 'LOCAL',
@@ -42,27 +40,3 @@ export interface Persistence {
   get<T extends PersistenceValue>(key: string): Promise<T | null>;
   remove(key: string): Promise<void>;
 }
-
-/**
- * We can't directly export all of the different types of persistence as
- * constants: this would cause tree-shaking libraries to keep all of the
- * various persistence classes in the bundle, even if they're not used, since
- * the system can't prove those constructors don't side-effect. Instead, the
- * persistence classes themselves all have a static method called _getInstance()
- * which does the instantiation.
- */
-export interface PersistenceInstantiator extends externs.Persistence {
-  new (): Persistence;
-}
-
-const persistenceCache: Map<externs.Persistence, Persistence> = new Map();
-
-export function _getInstance(cls: externs.Persistence): Persistence {
-  if (persistenceCache.has(cls)) {
-    return persistenceCache.get(cls)!;
-  }
-
-  const persistence = new (cls as PersistenceInstantiator)();
-  persistenceCache.set(cls, persistence);
-  return persistence;
-}

packages-exp/auth-exp/src/core/persistence/indexed_db.test.ts

@@ -19,10 +19,11 @@ import { expect } from 'chai';
 import * as sinon from 'sinon';
 
 import { testUser } from '../../../test/mock_auth';
-import { _getInstance, PersistenceType } from './';
+import { _getInstance } from '../util/instantiator';
+import { Persistence, PersistenceType } from './';
 import { indexedDBLocalPersistence } from './indexed_db';
 
-const persistence = _getInstance(indexedDBLocalPersistence);
+const persistence: Persistence = _getInstance(indexedDBLocalPersistence);
 
 describe('core/persistence/indexed_db', () => {
   afterEach(sinon.restore);

packages-exp/auth-exp/src/core/persistence/persistence_user_manager.test.ts

@@ -23,7 +23,8 @@ import * as sinonChai from 'sinon-chai';
 import { testAuth, testUser } from '../../../test/mock_auth';
 import { Auth } from '../../model/auth';
 import { UserImpl } from '../user/user_impl';
-import { _getInstance, Persistence, PersistenceType } from './';
+import { _getInstance } from '../util/instantiator';
+import { Persistence, PersistenceType } from './';
 import { inMemoryPersistence } from './in_memory';
 import { PersistenceUserManager } from './persistence_user_manager';
 

packages-exp/auth-exp/src/core/persistence/persistence_user_manager.ts

@@ -17,8 +17,9 @@
 
 import { ApiKey, AppName, Auth } from '../../model/auth';
 import { User } from '../../model/user';
-import { _getInstance, PersistedBlob, Persistence } from '../persistence';
+import { PersistedBlob, Persistence } from '../persistence';
 import { UserImpl } from '../user/user_impl';
+import { _getInstance } from '../util/instantiator';
 import { inMemoryPersistence } from './in_memory';
 
 export const _AUTH_USER_KEY_NAME = 'authUser';

packages-exp/auth-exp/src/core/persistence/react_native.test.ts

@@ -20,7 +20,8 @@ import { expect } from 'chai';
 import { ReactNativeAsyncStorage } from '@firebase/auth-types-exp';
 
 import { testUser } from '../../../test/mock_auth';
-import { _getInstance, PersistedBlob, PersistenceType } from './';
+import { _getInstance } from '../util/instantiator';
+import { PersistedBlob, Persistence, PersistenceType } from './';
 import { getReactNativePersistence } from './react_native';
 
 /**
@@ -48,7 +49,9 @@ class FakeAsyncStorage implements ReactNativeAsyncStorage {
 
 describe('core/persistence/react', () => {
   const fakeAsyncStorage = new FakeAsyncStorage();
-  const persistence = _getInstance(getReactNativePersistence(fakeAsyncStorage));
+  const persistence: Persistence = _getInstance(
+    getReactNativePersistence(fakeAsyncStorage)
+  );
 
   beforeEach(() => {
     fakeAsyncStorage.clear();

packages-exp/auth-exp/src/core/persistence/index.test.ts renamed to packages-exp/auth-exp/src/core/util/instantiator.test.ts

@@ -17,9 +17,9 @@
 
 import { expect } from 'chai';
 
-import { _getInstance } from './';
+import { _getInstance } from './instantiator';
 
-describe('src/core/persistence/index', () => {
+describe('src/core/util/instantiator', () => {
   context('_getInstance', () => {
     // All tests define their own classes since the Class object is used in the
     // global map.
@@ -66,5 +66,15 @@ describe('src/core/persistence/index', () => {
       expect(_getInstance(PersistenceA)).to.eq(a);
       expect(_getInstance(PersistenceB)).to.eq(b);
     });
+
+    it('instantiates any class', () => {
+      class Test {}
+
+      const a = _getInstance(Test);
+      const b = _getInstance(Test);
+
+      expect(a).to.be.instanceOf(Test);
+      expect(a).to.eq(b);
+    });
   });
 });

packages-exp/auth-exp/src/core/util/instantiator.ts

@@ -0,0 +1,48 @@
+/**
+ * @license
+ * Copyright 2020 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { debugAssert } from './assert';
+
+/**
+ * Our API has a lot of one-off constants that are used to do things.
+ * Unfortunately we can't export these as classes instantiated directly since
+ * the constructor may side effect and therefore can't be proven to be safely
+ * culled. Instead, we export these classes themselves as a lowerCamelCase
+ * constant, and instantiate them under the hood.
+ */
+export interface SingletonInstantiator<T> {
+  new (): T;
+}
+
+const instanceCache: Map<unknown, unknown> = new Map();
+
+export function _getInstance<T>(cls: unknown): T {
+  debugAssert(cls instanceof Function, 'Expected a class definition');
+  let instance = instanceCache.get(cls) as T | undefined;
+
+  if (instance) {
+    debugAssert(
+      instance instanceof cls,
+      'Instance stored in cache mismatched with class'
+    );
+    return instance;
+  }
+
+  instance = new (cls as SingletonInstantiator<T>)();
+  instanceCache.set(cls, instance);
+  return instance;
+}