diff --git a/packages/firestore/index.console.ts b/packages/firestore/index.console.ts index 918e60fe8a5..afe870a0be7 100644 --- a/packages/firestore/index.console.ts +++ b/packages/firestore/index.console.ts @@ -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'; diff --git a/packages/firestore/src/api/blob.ts b/packages/firestore/src/api/blob.ts index ca7ad6f648b..f889f55240b 100644 --- a/packages/firestore/src/api/blob.ts +++ b/packages/firestore/src/api/blob.ts @@ -16,7 +16,6 @@ */ import { PlatformSupport } from '../platform/platform'; -import { makeConstructorPrivate } from '../util/api'; import { Code, FirestoreError } from '../util/error'; import { invalidClassError, @@ -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.' -); diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index a5e38ca87ec..4b82f0b0593 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -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'; @@ -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'; @@ -2529,39 +2527,3 @@ function applyFirestoreDataConverter( 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.() instead.' -); diff --git a/packages/firestore/src/platform/config.ts b/packages/firestore/src/platform/config.ts index c5be1a03709..770288a0a66 100644 --- a/packages/firestore/src/platform/config.ts +++ b/packages/firestore/src/platform/config.ts @@ -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.() instead.' +); +export const PublicBlob = makeConstructorPrivate( + Blob, + 'Use Blob.fromUint8Array() or Blob.fromBase64String() instead.' +); const firestoreNamespace = { Firestore: PublicFirestore, diff --git a/packages/firestore/test/integration/api/private_constructors.test.ts b/packages/firestore/test/integration/api/private_constructors.test.ts new file mode 100644 index 00000000000..4c9a8d4d80c --- /dev/null +++ b/packages/firestore/test/integration/api/private_constructors.test.ts @@ -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.() instead.' + ); + }); +}); diff --git a/packages/firestore/test/unit/api/blob.test.ts b/packages/firestore/test/unit/api/blob.test.ts index 10bb93c0605..0f9513bbd34 100644 --- a/packages/firestore/test/unit/api/blob.test.ts +++ b/packages/firestore/test/unit/api/blob.test.ts @@ -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', () => { @@ -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()', () => { diff --git a/packages/firestore/test/unit/api/field_value.test.ts b/packages/firestore/test/unit/api/field_value.test.ts index e1445cdd336..cb59025afd8 100644 --- a/packages/firestore/test/unit/api/field_value.test.ts +++ b/packages/firestore/test/unit/api/field_value.test.ts @@ -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', () => { diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index 8a9803d2129..c7a3ee3b6a6 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -18,7 +18,7 @@ import * as api from '../../../src/protos/firestore_proto_api'; import { expect } from 'chai'; -import { PublicFieldValue } from '../../../src/api/database'; +import { FieldValue } from '../../../src/api/field_value'; import { Timestamp } from '../../../src/api/timestamp'; import { User } from '../../../src/auth/user'; import { Query } from '../../../src/core/query'; @@ -1256,16 +1256,12 @@ function genericLocalStoreTests( doc('foo/bar', 0, { sum: 0 }, { hasLocalMutations: true }) ) .toContain(doc('foo/bar', 0, { sum: 0 }, { hasLocalMutations: true })) - .after( - transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) }) - ) + .after(transformMutation('foo/bar', { sum: FieldValue.increment(1) })) .toReturnChanged( doc('foo/bar', 0, { sum: 1 }, { hasLocalMutations: true }) ) .toContain(doc('foo/bar', 0, { sum: 1 }, { hasLocalMutations: true })) - .after( - transformMutation('foo/bar', { sum: PublicFieldValue.increment(2) }) - ) + .after(transformMutation('foo/bar', { sum: FieldValue.increment(2) })) .toReturnChanged( doc('foo/bar', 0, { sum: 3 }, { hasLocalMutations: true }) ) @@ -1290,9 +1286,7 @@ function genericLocalStoreTests( .toContain( doc('foo/bar', 1, { sum: 0 }, { hasCommittedMutations: true }) ) - .after( - transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) }) - ) + .after(transformMutation('foo/bar', { sum: FieldValue.increment(1) })) .toReturnChanged( doc('foo/bar', 1, { sum: 1 }, { hasLocalMutations: true }) ) @@ -1307,9 +1301,7 @@ function genericLocalStoreTests( .toContain( doc('foo/bar', 2, { sum: 1 }, { hasCommittedMutations: true }) ) - .after( - transformMutation('foo/bar', { sum: PublicFieldValue.increment(2) }) - ) + .after(transformMutation('foo/bar', { sum: FieldValue.increment(2) })) .toReturnChanged( doc('foo/bar', 2, { sum: 3 }, { hasLocalMutations: true }) ) @@ -1335,9 +1327,7 @@ function genericLocalStoreTests( .afterAcknowledgingMutation({ documentVersion: 1 }) .toReturnChanged(doc('foo/bar', 1, { sum: 0 })) .toContain(doc('foo/bar', 1, { sum: 0 })) - .after( - transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) }) - ) + .after(transformMutation('foo/bar', { sum: FieldValue.increment(1) })) .toReturnChanged( doc('foo/bar', 1, { sum: 1 }, { hasLocalMutations: true }) ) @@ -1353,9 +1343,7 @@ function genericLocalStoreTests( .toContain(doc('foo/bar', 2, { sum: 1 }, { hasLocalMutations: true })) // Add another increment. Note that we still compute the increment based // on the local value. - .after( - transformMutation('foo/bar', { sum: PublicFieldValue.increment(2) }) - ) + .after(transformMutation('foo/bar', { sum: FieldValue.increment(2) })) .toReturnChanged( doc('foo/bar', 2, { sum: 3 }, { hasLocalMutations: true }) ) @@ -1417,9 +1405,9 @@ function genericLocalStoreTests( ) .toReturnChanged(doc('foo/bar', 1, { sum: 0, arrayUnion: [] })) .afterMutations([ - transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) }), + transformMutation('foo/bar', { sum: FieldValue.increment(1) }), transformMutation('foo/bar', { - arrayUnion: PublicFieldValue.arrayUnion('foo') + arrayUnion: FieldValue.arrayUnion('foo') }) ]) .toReturnChanged( @@ -1458,7 +1446,7 @@ function genericLocalStoreTests( .toReturnTargetId(2) .afterMutations([ patchMutation('foo/bar', {}, Precondition.none()), - transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) }) + transformMutation('foo/bar', { sum: FieldValue.increment(1) }) ]) .toReturnChanged( doc('foo/bar', 0, { sum: 1 }, { hasLocalMutations: true }) @@ -1484,7 +1472,7 @@ function genericLocalStoreTests( .toReturnTargetId(2) .afterMutations([ patchMutation('foo/bar', {}), - transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) }) + transformMutation('foo/bar', { sum: FieldValue.increment(1) }) ]) .toReturnChanged(deletedDoc('foo/bar', 0)) .toNotContain('foo/bar') diff --git a/packages/firestore/test/unit/model/mutation.test.ts b/packages/firestore/test/unit/model/mutation.test.ts index 3914f07322e..10320234529 100644 --- a/packages/firestore/test/unit/model/mutation.test.ts +++ b/packages/firestore/test/unit/model/mutation.test.ts @@ -16,7 +16,7 @@ */ import { expect } from 'chai'; -import { PublicFieldValue as FieldValue } from '../../../src/api/database'; +import { FieldValue } from '../../../src/api/field_value'; import { Timestamp } from '../../../src/api/timestamp'; import { Document, MaybeDocument } from '../../../src/model/document'; import { serverTimestamp } from '../../../src/model/server_timestamps'; diff --git a/packages/firestore/test/unit/remote/serializer.helper.ts b/packages/firestore/test/unit/remote/serializer.helper.ts index 3bc718640a8..fa059581be1 100644 --- a/packages/firestore/test/unit/remote/serializer.helper.ts +++ b/packages/firestore/test/unit/remote/serializer.helper.ts @@ -18,10 +18,8 @@ import { expect } from 'chai'; import { Blob } from '../../../src/api/blob'; -import { - PublicFieldValue as FieldValue, - DocumentReference -} from '../../../src/api/database'; +import { DocumentReference } from '../../../src/api/database'; +import { FieldValue } from '../../../src/api/field_value'; import { GeoPoint } from '../../../src/api/geo_point'; import { Timestamp } from '../../../src/api/timestamp'; import { DatabaseId } from '../../../src/core/database_info';