Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions packages-exp/auth-exp/src/core/auth/auth_impl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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())
);
Expand Down Expand Up @@ -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');
Expand Down
5 changes: 3 additions & 2 deletions packages-exp/auth-exp/src/core/auth/auth_impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions packages-exp/auth-exp/src/core/persistence/browser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
26 changes: 0 additions & 26 deletions packages-exp/auth-exp/src/core/persistence/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
* limitations under the License.
*/

import * as externs from '@firebase/auth-types-exp';

export enum PersistenceType {
SESSION = 'SESSION',
LOCAL = 'LOCAL',
Expand All @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
});
});
});
48 changes: 48 additions & 0 deletions packages-exp/auth-exp/src/core/util/instantiator.ts
Original file line number Diff line number Diff line change
@@ -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;
}