Skip to content

Commit

Permalink
Remove static references to API types (#3023)
Browse files Browse the repository at this point in the history
The pattern in api/database.ts breaks tree-shaking since it instatiates static references to all public API types. I moved this code to platform/config.ts which is used to register the non Tree-Shakeable API
  • Loading branch information
schmidt-sebastian authored May 6, 2020
1 parent 8877803 commit e67affb
Show file tree
Hide file tree
Showing 10 changed files with 166 additions and 106 deletions.
8 changes: 5 additions & 3 deletions packages/firestore/index.console.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@ import './src/platform_browser/browser_init';
export {
Firestore,
FirestoreDatabase,
} from './src/api/database';
export {
PublicCollectionReference as CollectionReference,
PublicDocumentReference as DocumentReference,
PublicDocumentSnapshot as DocumentSnapshot,
PublicQuerySnapshot as QuerySnapshot,
PublicFieldValue as FieldValue
} from './src/api/database';
PublicFieldValue as FieldValue,
PublicBlob as Blob
} from './src/platform/config';
export { GeoPoint } from './src/api/geo_point';
export { PublicBlob as Blob } from './src/api/blob';
export { FirstPartyCredentialsSettings } from './src/api/credentials';
export { FieldPath } from './src/api/field_path';
export { Timestamp } from './src/api/timestamp';
13 changes: 0 additions & 13 deletions packages/firestore/src/api/blob.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/

import { PlatformSupport } from '../platform/platform';
import { makeConstructorPrivate } from '../util/api';
import { Code, FirestoreError } from '../util/error';
import {
invalidClassError,
Expand Down Expand Up @@ -105,15 +104,3 @@ export class Blob {
return this._byteString.isEqual(other._byteString);
}
}

// Public instance that disallows construction at runtime. This constructor is
// used when exporting Blob on firebase.firestore.Blob and will be called Blob
// publicly. Internally we still use Blob which has a type checked private
// constructor. Note that Blob and PublicBlob can be used interchangeably in
// instanceof checks.
// For our internal TypeScript code PublicBlob doesn't exist as a type, and so
// we need to use Blob as type and export it too.
export const PublicBlob = makeConstructorPrivate(
Blob,
'Use Blob.fromUint8Array() or Blob.fromBase64String() instead.'
);
38 changes: 0 additions & 38 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import { FieldPath, ResourcePath } from '../model/path';
import { isServerTimestamp } from '../model/server_timestamps';
import { refValue } from '../model/values';
import { PlatformSupport } from '../platform/platform';
import { makeConstructorPrivate } from '../util/api';
import { debugAssert, fail } from '../util/assert';
import { AsyncObserver } from '../util/async_observer';
import { AsyncQueue } from '../util/async_queue';
Expand Down Expand Up @@ -93,7 +92,6 @@ import { fieldPathFromArgument, UserDataReader } from './user_data_reader';
import { UserDataWriter } from './user_data_writer';
import { FirebaseAuthInternalName } from '@firebase/auth-interop-types';
import { Provider } from '@firebase/component';
import { FieldValue } from './field_value';

// settings() defaults:
const DEFAULT_HOST = 'firestore.googleapis.com';
Expand Down Expand Up @@ -2529,39 +2527,3 @@ function applyFirestoreDataConverter<T>(
function contains(obj: object, key: string): obj is { key: unknown } {
return Object.prototype.hasOwnProperty.call(obj, key);
}

// Export the classes with a private constructor (it will fail if invoked
// at runtime). Note that this still allows instanceof checks.

// We're treating the variables as class names, so disable checking for lower
// case variable names.
export const PublicFirestore = makeConstructorPrivate(
Firestore,
'Use firebase.firestore() instead.'
);
export const PublicTransaction = makeConstructorPrivate(
Transaction,
'Use firebase.firestore().runTransaction() instead.'
);
export const PublicWriteBatch = makeConstructorPrivate(
WriteBatch,
'Use firebase.firestore().batch() instead.'
);
export const PublicDocumentReference = makeConstructorPrivate(
DocumentReference,
'Use firebase.firestore().doc() instead.'
);
export const PublicDocumentSnapshot = makeConstructorPrivate(DocumentSnapshot);
export const PublicQueryDocumentSnapshot = makeConstructorPrivate(
QueryDocumentSnapshot
);
export const PublicQuery = makeConstructorPrivate(Query);
export const PublicQuerySnapshot = makeConstructorPrivate(QuerySnapshot);
export const PublicCollectionReference = makeConstructorPrivate(
CollectionReference,
'Use firebase.firestore().collection() instead.'
);
export const PublicFieldValue = makeConstructorPrivate(
FieldValue,
'Use FieldValue.<field>() instead.'
);
59 changes: 48 additions & 11 deletions packages/firestore/src/platform/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,61 @@ import { FirebaseApp, FirebaseNamespace } from '@firebase/app-types';
import { FirebaseAuthInternalName } from '@firebase/auth-interop-types';
import { _FirebaseNamespace } from '@firebase/app-types/private';
import { Component, ComponentType, Provider } from '@firebase/component';
import { PublicBlob } from '../api/blob';
import {
CACHE_SIZE_UNLIMITED,
Firestore,
PublicCollectionReference,
PublicDocumentReference,
PublicDocumentSnapshot,
PublicFirestore,
PublicQuery,
PublicQueryDocumentSnapshot,
PublicQuerySnapshot,
PublicTransaction,
PublicWriteBatch,
PublicFieldValue
DocumentReference,
DocumentSnapshot,
QueryDocumentSnapshot,
Query,
QuerySnapshot,
CollectionReference,
Transaction,
WriteBatch
} from '../api/database';
import { Blob } from '../api/blob';
import { FieldPath } from '../api/field_path';
import { GeoPoint } from '../api/geo_point';
import { Timestamp } from '../api/timestamp';
import { makeConstructorPrivate } from '../util/api';
import { FieldValue } from '../api/field_value';

// Public instance that disallows construction at runtime. Note that this still
// allows instanceof checks.
export const PublicFirestore = makeConstructorPrivate(
Firestore,
'Use firebase.firestore() instead.'
);
export const PublicTransaction = makeConstructorPrivate(
Transaction,
'Use firebase.firestore().runTransaction() instead.'
);
export const PublicWriteBatch = makeConstructorPrivate(
WriteBatch,
'Use firebase.firestore().batch() instead.'
);
export const PublicDocumentReference = makeConstructorPrivate(
DocumentReference,
'Use firebase.firestore().doc() instead.'
);
export const PublicDocumentSnapshot = makeConstructorPrivate(DocumentSnapshot);
export const PublicQueryDocumentSnapshot = makeConstructorPrivate(
QueryDocumentSnapshot
);
export const PublicQuery = makeConstructorPrivate(Query);
export const PublicQuerySnapshot = makeConstructorPrivate(QuerySnapshot);
export const PublicCollectionReference = makeConstructorPrivate(
CollectionReference,
'Use firebase.firestore().collection() instead.'
);
export const PublicFieldValue = makeConstructorPrivate(
FieldValue,
'Use FieldValue.<field>() instead.'
);
export const PublicBlob = makeConstructorPrivate(
Blob,
'Use Blob.fromUint8Array() or Blob.fromBase64String() instead.'
);

const firestoreNamespace = {
Firestore: PublicFirestore,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/**
* @license
* Copyright 2017 Google Inc.
*
* 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 firebase from '../util/firebase_export';

// allow using constructor with any
/* eslint-disable @typescript-eslint/no-explicit-any */

describe('Constructors', () => {
it('are private for Firestore', () => {
expect(() => new (firebase.firestore!.Firestore as any)('')).to.throw(
'This constructor is private. Use firebase.firestore() instead.'
);
});

it('are private for Transaction', () => {
expect(() => new (firebase.firestore!.Transaction as any)('')).to.throw(
'This constructor is private. Use firebase.firestore().runTransaction() instead.'
);
});

it('are private for WriteBatch', () => {
expect(() => new (firebase.firestore!.WriteBatch as any)('')).to.throw(
'This constructor is private. Use firebase.firestore().batch() instead.'
);
});

it('are private for DocumentReference', () => {
expect(
() => new (firebase.firestore!.DocumentReference as any)('')
).to.throw(
'This constructor is private. Use firebase.firestore().doc() instead.'
);
});

it('are private for Query', () => {
expect(() => new (firebase.firestore!.Query as any)('')).to.throw(
'This constructor is private.'
);
});

it('are private for CollectionReference', () => {
expect(
() => new (firebase.firestore!.CollectionReference as any)('')
).to.throw(
'This constructor is private. Use firebase.firestore().collection() instead.'
);
});

it('are private for QuerySnapshot', () => {
expect(() => new (firebase.firestore!.QuerySnapshot as any)('')).to.throw(
'This constructor is private.'
);
});

it('are private for DocumentSnapshot', () => {
expect(
() => new (firebase.firestore!.DocumentSnapshot as any)('')
).to.throw('This constructor is private.');
});

it('are private for QueryDocumentSnapshot', () => {
expect(
() => new (firebase.firestore!.QueryDocumentSnapshot as any)('')
).to.throw('This constructor is private.');
});

it('are private for Blob', () => {
expect(() => new (firebase.firestore!.Blob as any)('')).to.throw(
'This constructor is private.'
);
});

it('are private for FieldValue', () => {
expect(() => new (firebase.firestore!.FieldValue as any)('')).to.throw(
'This constructor is private. Use FieldValue.<field>() instead.'
);
});
});
15 changes: 3 additions & 12 deletions packages/firestore/test/unit/api/blob.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/

import { expect } from 'chai';
import { Blob, PublicBlob } from '../../../src/api/blob';
import { Blob } from '../../../src/api/blob';
import { blob, expectEqual, expectNotEqual } from '../../util/helpers';

describe('Blob', () => {
Expand Down Expand Up @@ -52,17 +52,8 @@ describe('Blob', () => {
);
});

it('Blob throws on using the public constructor', () => {
// allow using constructor with any
// eslint-disable-next-line @typescript-eslint/no-explicit-any
expect(() => new (PublicBlob as any)('')).to.throw(
'This constructor is private. Use Blob.fromUint8Array() or ' +
'Blob.fromBase64String() instead.'
);
});

it('PublicBlob works with instanceof checks', () => {
expect(Blob.fromBase64String('') instanceof PublicBlob).to.equal(true);
it('works with instanceof checks', () => {
expect(Blob.fromBase64String('') instanceof Blob).to.equal(true);
});

it('support equality checking with isEqual()', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/test/unit/api/field_value.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

import { PublicFieldValue as FieldValue } from '../../../src/api/database';
import { FieldValue } from '../../../src/api/field_value';
import { expectEqual, expectNotEqual } from '../../util/helpers';

describe('FieldValue', () => {
Expand Down
Loading

0 comments on commit e67affb

Please sign in to comment.