From e90304c8ac4341d8b23b55da784eb21348b04025 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Fri, 26 Jun 2020 15:51:08 -0700 Subject: [PATCH] Remove makeConstructorPrivate (#3309) --- .changeset/small-ghosts-visit.md | 6 ++ packages/firestore/src/api/field_value.ts | 4 + packages/firestore/src/config.ts | 62 +++--------- packages/firestore/src/util/api.ts | 57 ----------- .../api/private_constructors.test.ts | 95 ------------------- packages/firestore/test/unit/util/api.test.ts | 72 -------------- 6 files changed, 22 insertions(+), 274 deletions(-) create mode 100644 .changeset/small-ghosts-visit.md delete mode 100644 packages/firestore/src/util/api.ts delete mode 100644 packages/firestore/test/integration/api/private_constructors.test.ts delete mode 100644 packages/firestore/test/unit/util/api.test.ts diff --git a/.changeset/small-ghosts-visit.md b/.changeset/small-ghosts-visit.md new file mode 100644 index 00000000000..adefdffa616 --- /dev/null +++ b/.changeset/small-ghosts-visit.md @@ -0,0 +1,6 @@ +--- +"firebase": patch +"@firebase/firestore": patch +--- + +Removed internal wrapper around our public API that was meant to prevent incorrect SDK usage for JavaScript users, but caused our SDK to stop working in IE11. diff --git a/packages/firestore/src/api/field_value.ts b/packages/firestore/src/api/field_value.ts index ce1237cafce..d1dd98db81a 100644 --- a/packages/firestore/src/api/field_value.ts +++ b/packages/firestore/src/api/field_value.ts @@ -210,6 +210,10 @@ export class NumericIncrementFieldValueImpl extends SerializableFieldValue { /** The public FieldValue class of the lite API. */ export abstract class FieldValue extends SerializableFieldValue implements firestore.FieldValue { + protected constructor() { + super(); + } + static delete(): firestore.FieldValue { validateNoArgs('FieldValue.delete', arguments); return new FieldValueDelegate( diff --git a/packages/firestore/src/config.ts b/packages/firestore/src/config.ts index 8631ad590f6..c73ce45a165 100644 --- a/packages/firestore/src/config.ts +++ b/packages/firestore/src/config.ts @@ -21,13 +21,13 @@ import { _FirebaseNamespace } from '@firebase/app-types/private'; import { Component, ComponentType, Provider } from '@firebase/component'; import { CACHE_SIZE_UNLIMITED, - Firestore, + CollectionReference, DocumentReference, DocumentSnapshot, - QueryDocumentSnapshot, + Firestore, Query, + QueryDocumentSnapshot, QuerySnapshot, - CollectionReference, Transaction, WriteBatch } from './api/database'; @@ -35,61 +35,23 @@ 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( +const firestoreNamespace = { Firestore, - 'Use firebase.firestore() instead.' -); -export const PublicTransaction = makeConstructorPrivate( + GeoPoint, + Timestamp, + Blob, 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( + DocumentSnapshot, + Query, + QueryDocumentSnapshot, + QuerySnapshot, 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, - GeoPoint, - Timestamp, - Blob: PublicBlob, - Transaction: PublicTransaction, - WriteBatch: PublicWriteBatch, - DocumentReference: PublicDocumentReference, - DocumentSnapshot: PublicDocumentSnapshot, - Query: PublicQuery, - QueryDocumentSnapshot: PublicQueryDocumentSnapshot, - QuerySnapshot: PublicQuerySnapshot, - CollectionReference: PublicCollectionReference, FieldPath, - FieldValue: PublicFieldValue, + FieldValue, setLogLevel: Firestore.setLogLevel, CACHE_SIZE_UNLIMITED }; diff --git a/packages/firestore/src/util/api.ts b/packages/firestore/src/util/api.ts deleted file mode 100644 index 8df5d3c1079..00000000000 --- a/packages/firestore/src/util/api.ts +++ /dev/null @@ -1,57 +0,0 @@ -/** - * @license - * Copyright 2017 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 { Code, FirestoreError } from './error'; - -/** List of JavaScript builtins that cannot be reassigned. */ -const RESERVED_READONLY_PROPS = ['length', 'name']; - -/** - * Helper function to prevent instantiation through the constructor. - * - * This method creates a new constructor that throws when it's invoked. - * The prototype of that constructor is then set to the prototype of the hidden - * "class" to expose all the prototype methods and allow for instanceof - * checks. - * - * To also make all the static methods available, all properties of the - * original constructor are copied to the new constructor. - */ -export function makeConstructorPrivate( - cls: T, - optionalMessage?: string -): T { - function PublicConstructor(): never { - let error = 'This constructor is private.'; - if (optionalMessage) { - error += ' '; - error += optionalMessage; - } - throw new FirestoreError(Code.INVALID_ARGUMENT, error); - } - - // Copy static members and prototype - for (const staticProp of Object.getOwnPropertyNames(cls)) { - if (RESERVED_READONLY_PROPS.indexOf(staticProp) === -1) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (PublicConstructor as any)[staticProp] = (cls as any)[staticProp]; - } - } - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return PublicConstructor as any; -} diff --git a/packages/firestore/test/integration/api/private_constructors.test.ts b/packages/firestore/test/integration/api/private_constructors.test.ts deleted file mode 100644 index 53cfde74287..00000000000 --- a/packages/firestore/test/integration/api/private_constructors.test.ts +++ /dev/null @@ -1,95 +0,0 @@ -/** - * @license - * Copyright 2017 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 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/util/api.test.ts b/packages/firestore/test/unit/util/api.test.ts deleted file mode 100644 index db35b886c99..00000000000 --- a/packages/firestore/test/unit/util/api.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -/** - * @license - * Copyright 2017 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 { makeConstructorPrivate } from '../../../src/util/api'; - -describe('makeConstructorPrivate', () => { - class PrivateClass { - x = 'x-value'; - private _y = 'y-value'; - - getY(): string { - return this._y; - } - - setY(newValue: string): void { - this._y = newValue; - } - - static foobar(): string { - return 'foobar-value'; - } - - static make(): PrivateClass { - return new PrivateClass(); - } - } - - // tslint:disable-next-line:variable-name We're treating this as a class name. - const PublicClass = makeConstructorPrivate(PrivateClass); - - it('throws on instantiation', () => { - expect(() => new PublicClass()).to.throw(); - }); - - it('still exposes static methods', () => { - expect(PublicClass.foobar()).to.equal('foobar-value'); - }); - - it('still works with instanceof methods', () => { - const x = PublicClass.make(); - const y = new PrivateClass(); - expect(x instanceof PublicClass).to.equal(true); - expect(x instanceof PrivateClass).to.equal(true); - expect(y instanceof PublicClass).to.equal(true); - expect(y instanceof PrivateClass).to.equal(true); - }); - - it('still works with class members', () => { - const instance = PublicClass.make(); - expect(instance.x).to.equal('x-value'); - expect(instance.getY()).to.equal('y-value'); - instance.x = 'new-x-value'; - instance.setY('new-y-value'); - expect(instance.x).to.equal('new-x-value'); - expect(instance.getY()).to.equal('new-y-value'); - }); -});