Tree shakeable persistence (#3288)

* Make the persistence objects tree-shakeable

* Formatting

* Updated to stop using casts everywhere

* Formatting

* Make the browser impls tree shakeable

* Formatting

* Fix the react native implementation in auth-compat-exp

* Formatting

* Make the react native class wrapped in a closure, to avoid the prototype nonsense

* Formatting

* Fix index.rn.ts

* Formatting

* PR feedback

* Fix ordering in index.ts

* Formatting

* Fix test

* Formatting

Author: sam-gc

packages-exp/auth-compat-exp/index.rn.ts

@@ -23,7 +23,8 @@
  */
 
 import { AsyncStorage } from 'react-native';
-import { ReactNativePersistence } from '@firebase/auth-exp/src/core/persistence/react_native';
+
+import { getReactNativePersistence } from '@firebase/auth-exp/src/core/persistence/react_native';
 
 // eslint-disable-next-line @typescript-eslint/no-unused-vars
-const reactNativeLocalPersistence = new ReactNativePersistence(AsyncStorage);
+const reactNativeLocalPersistence = getReactNativePersistence(AsyncStorage);

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

@@ -26,18 +26,18 @@ import { FirebaseError } from '@firebase/util';
 import { testUser } from '../../../test/mock_auth';
 import { Auth } from '../../model/auth';
 import { User } from '../../model/user';
-import { Persistence } from '../persistence';
+import { _getInstance, Persistence } from '../persistence';
 import { browserLocalPersistence } from '../persistence/browser';
 import { inMemoryPersistence } from '../persistence/in_memory';
 import { PersistenceUserManager } from '../persistence/persistence_user_manager';
+import * as navigator from '../util/navigator';
 import { _getClientVersion, ClientPlatform } from '../util/version';
 import {
   DEFAULT_API_HOST,
   DEFAULT_API_SCHEME,
   DEFAULT_TOKEN_API_HOST,
   initializeAuth
 } from './auth_impl';
-import * as navigator from '../util/navigator';
 
 use(sinonChai);
 
@@ -55,7 +55,7 @@ describe('core/auth/auth_impl', () => {
   let persistenceStub: sinon.SinonStubbedInstance<Persistence>;
 
   beforeEach(() => {
-    persistenceStub = sinon.stub(inMemoryPersistence as Persistence);
+    persistenceStub = sinon.stub(_getInstance(inMemoryPersistence));
     auth = initializeAuth(FAKE_APP, {
       persistence: inMemoryPersistence
     }) as Auth;
@@ -108,8 +108,8 @@ describe('core/auth/auth_impl', () => {
 
   describe('#setPersistence', () => {
     it('swaps underlying persistence', async () => {
-      const newPersistence = browserLocalPersistence as Persistence;
-      const newStub = sinon.stub(newPersistence);
+      const newPersistence = browserLocalPersistence;
+      const newStub = sinon.stub(_getInstance(newPersistence));
       persistenceStub.get.returns(
         Promise.resolve(testUser(auth, 'test').toPlainObject())
       );
@@ -302,13 +302,13 @@ describe('core/auth/initializeAuth', () => {
     it('converts single persistence to array', async () => {
       const auth = await initAndWait(inMemoryPersistence);
       expect(createManagerStub).to.have.been.calledWith(auth, [
-        inMemoryPersistence
+        _getInstance(inMemoryPersistence)
       ]);
     });
 
     it('pulls the user from storage', async () => {
       sinon
-        .stub(inMemoryPersistence as Persistence, 'get')
+        .stub(_getInstance(inMemoryPersistence), 'get')
         .returns(Promise.resolve(testUser({}, 'uid').toPlainObject()));
       const auth = await initAndWait(inMemoryPersistence);
       expect(auth.currentUser!.uid).to.eq('uid');
@@ -320,8 +320,8 @@ describe('core/auth/initializeAuth', () => {
         browserLocalPersistence
       ]);
       expect(createManagerStub).to.have.been.calledWith(auth, [
-        inMemoryPersistence,
-        browserLocalPersistence
+        _getInstance(inMemoryPersistence),
+        _getInstance(browserLocalPersistence)
       ]);
     });
 

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

@@ -31,11 +31,11 @@ import {
 import { Auth, Dependencies } from '../../model/auth';
 import { User } from '../../model/user';
 import { AuthErrorCode } from '../errors';
-import { Persistence } from '../persistence';
+import { _getInstance, Persistence } from '../persistence';
 import { PersistenceUserManager } from '../persistence/persistence_user_manager';
 import { assert } from '../util/assert';
-import { _getClientVersion, ClientPlatform } from '../util/version';
 import { _getUserLanguage } from '../util/navigator';
+import { _getClientVersion, ClientPlatform } from '../util/version';
 
 interface AsyncAction {
   (): Promise<void>;
@@ -101,9 +101,9 @@ export class AuthImpl implements Auth {
     return this.updateCurrentUser(null);
   }
 
-  setPersistence(persistence: Persistence): Promise<void> {
+  setPersistence(persistence: externs.Persistence): Promise<void> {
     return this.queue(async () => {
-      await this.assertedPersistence.setPersistence(persistence);
+      await this.assertedPersistence.setPersistence(_getInstance(persistence));
     });
   }
 
@@ -215,7 +215,10 @@ export function initializeAuth(
   deps?: Dependencies
 ): externs.Auth {
   const persistence = deps?.persistence || [];
-  const hierarchy = Array.isArray(persistence) ? persistence : [persistence];
+  const hierarchy = (Array.isArray(persistence)
+    ? persistence
+    : [persistence]
+  ).map(_getInstance);
   const { apiKey, authDomain } = app.options;
 
   // TODO: platform needs to be determined using heuristics
@@ -234,7 +237,7 @@ export function initializeAuth(
   // This promise is intended to float; auth initialization happens in the
   // background, meanwhile the auth object may be used by the app.
   // eslint-disable-next-line @typescript-eslint/no-floating-promises
-  auth._initializeWithPersistence(hierarchy as Persistence[]);
+  auth._initializeWithPersistence(hierarchy);
 
   return auth;
 }

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

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

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

@@ -24,41 +24,72 @@ import {
   STORAGE_AVAILABLE_KEY
 } from './';
 
-class BrowserPersistence implements Persistence {
-  type: PersistenceType = PersistenceType.LOCAL;
+// There are two different browser persistence types: local and session.
+// Both have the same implementation but use a different underlying storage
+// object. Using class inheritance compiles down to an es5 polyfill, which
+// prevents rollup from tree shaking. By making these "methods" free floating
+// functions bound to the classes, the two different types can share the
+// implementation without subclassing.
 
-  constructor(private readonly storage: Storage) {}
+interface BrowserPersistenceClass extends Persistence {
+  storage: Storage;
+}
 
-  async isAvailable(): Promise<boolean> {
-    try {
-      if (!this.storage) {
-        return false;
-      }
-      this.storage.setItem(STORAGE_AVAILABLE_KEY, '1');
-      this.storage.removeItem(STORAGE_AVAILABLE_KEY);
-      return true;
-    } catch {
-      return false;
+function isAvailable(this: BrowserPersistenceClass): Promise<boolean> {
+  try {
+    if (!this.storage) {
+      return Promise.resolve(false);
     }
+    this.storage.setItem(STORAGE_AVAILABLE_KEY, '1');
+    this.storage.removeItem(STORAGE_AVAILABLE_KEY);
+    return Promise.resolve(true);
+  } catch {
+    return Promise.resolve(false);
   }
+}
 
-  async set(key: string, value: PersistenceValue): Promise<void> {
-    this.storage.setItem(key, JSON.stringify(value));
-  }
+function set(
+  this: BrowserPersistenceClass,
+  key: string,
+  value: PersistenceValue
+): Promise<void> {
+  this.storage.setItem(key, JSON.stringify(value));
+  return Promise.resolve();
+}
 
-  async get<T extends PersistenceValue>(key: string): Promise<T | null> {
-    const json = this.storage.getItem(key);
-    return json ? JSON.parse(json) : null;
-  }
+function get<T extends PersistenceValue>(
+  this: BrowserPersistenceClass,
+  key: string
+): Promise<T | null> {
+  const json = this.storage.getItem(key);
+  return Promise.resolve(json ? JSON.parse(json) : null);
+}
 
-  async remove(key: string): Promise<void> {
-    this.storage.removeItem(key);
-  }
+function remove(this: BrowserPersistenceClass, key: string): Promise<void> {
+  this.storage.removeItem(key);
+  return Promise.resolve();
+}
+
+class BrowserLocalPersistence implements BrowserPersistenceClass {
+  static type: 'LOCAL' = 'LOCAL';
+  type = PersistenceType.LOCAL;
+  isAvailable = isAvailable;
+  set = set;
+  get = get;
+  remove = remove;
+  storage = localStorage;
 }
 
-export const browserLocalPersistence: externs.Persistence = new BrowserPersistence(
-  localStorage
-);
-export const browserSessionPersistence: externs.Persistence = new BrowserPersistence(
-  sessionStorage
-);
+class BrowserSessionPersistence implements BrowserPersistenceClass {
+  static type: 'SESSION' = 'SESSION';
+  type = PersistenceType.SESSION;
+  isAvailable = isAvailable;
+  set = set;
+  get = get;
+  remove = remove;
+  storage = sessionStorage;
+}
+
+export const browserLocalPersistence: externs.Persistence = BrowserLocalPersistence;
+
+export const browserSessionPersistence: externs.Persistence = BrowserSessionPersistence;

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

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

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

@@ -20,10 +20,9 @@ import * as externs from '@firebase/auth-types-exp';
 import { Persistence, PersistenceType, PersistenceValue } from '../persistence';
 
 export class InMemoryPersistence implements Persistence {
-  type: PersistenceType = PersistenceType.NONE;
-  storage: {
-    [key: string]: PersistenceValue;
-  } = {};
+  static type: 'NONE' = 'NONE';
+  readonly type = PersistenceType.NONE;
+  storage: Record<string, PersistenceValue> = {};
 
   async isAvailable(): Promise<boolean> {
     return true;
@@ -43,4 +42,4 @@ export class InMemoryPersistence implements Persistence {
   }
 }
 
-export const inMemoryPersistence: externs.Persistence = new InMemoryPersistence();
+export const inMemoryPersistence: externs.Persistence = InMemoryPersistence;

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

@@ -0,0 +1,70 @@
+/**
+ * @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 { expect } from 'chai';
+
+import { _getInstance } from './';
+
+describe('src/core/persistence/index', () => {
+  context('_getInstance', () => {
+    // All tests define their own classes since the Class object is used in the
+    // global map.
+
+    it('instantiates a new class', () => {
+      let classInstantiated = false;
+      class Persistence {
+        static type: 'LOCAL' = 'LOCAL';
+        constructor() {
+          classInstantiated = true;
+        }
+      }
+
+      _getInstance(Persistence);
+      expect(classInstantiated).to.be.true;
+    });
+
+    it('instantiates a class only once', () => {
+      let instantiationCount = 0;
+      class Persistence {
+        static type: 'LOCAL' = 'LOCAL';
+        constructor() {
+          instantiationCount++;
+        }
+      }
+
+      _getInstance(Persistence);
+      _getInstance(Persistence);
+      _getInstance(Persistence);
+
+      expect(instantiationCount).to.eq(1);
+    });
+
+    it('caches correctly', () => {
+      class PersistenceA {
+        static type: 'LOCAL' = 'LOCAL';
+      }
+      class PersistenceB {
+        static type: 'LOCAL' = 'LOCAL';
+      }
+
+      const a = _getInstance(PersistenceA);
+      const b = _getInstance(PersistenceB);
+      expect(_getInstance(PersistenceA)).to.eq(a);
+      expect(_getInstance(PersistenceB)).to.eq(b);
+    });
+  });
+});

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

@@ -15,6 +15,8 @@
  * limitations under the License.
  */
 
+import * as externs from '@firebase/auth-types-exp';
+
 export enum PersistenceType {
   SESSION = 'SESSION',
   LOCAL = 'LOCAL',
@@ -40,3 +42,27 @@ 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;
+}