Skip to content

Commit e67affb

Browse files
schmidt-sebastianschmidt-sebastian
authored
Remove static references to API types (#3023)
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
1 parent 8877803 commit e67affb

File tree

10 files changed

+166
-106
lines changed

10 files changed

+166
-106
lines changed

‎packages/firestore/index.console.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,16 @@ import './src/platform_browser/browser_init';
2020
export {
2121
Firestore,
2222
FirestoreDatabase,
23+
} from './src/api/database';
24+
export {
2325
PublicCollectionReference as CollectionReference,
2426
PublicDocumentReference as DocumentReference,
2527
PublicDocumentSnapshot as DocumentSnapshot,
2628
PublicQuerySnapshot as QuerySnapshot,
27-
PublicFieldValue as FieldValue
28-
} from './src/api/database';
29+
PublicFieldValue as FieldValue,
30+
PublicBlob as Blob
31+
} from './src/platform/config';
2932
export { GeoPoint } from './src/api/geo_point';
30-
export { PublicBlob as Blob } from './src/api/blob';
3133
export { FirstPartyCredentialsSettings } from './src/api/credentials';
3234
export { FieldPath } from './src/api/field_path';
3335
export { Timestamp } from './src/api/timestamp';

‎packages/firestore/src/api/blob.ts

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
*/
1717

1818
import { PlatformSupport } from '../platform/platform';
19-
import { makeConstructorPrivate } from '../util/api';
2019
import { Code, FirestoreError } from '../util/error';
2120
import {
2221
invalidClassError,
@@ -105,15 +104,3 @@ export class Blob {
105104
return this._byteString.isEqual(other._byteString);
106105
}
107106
}
108-
109-
// Public instance that disallows construction at runtime. This constructor is
110-
// used when exporting Blob on firebase.firestore.Blob and will be called Blob
111-
// publicly. Internally we still use Blob which has a type checked private
112-
// constructor. Note that Blob and PublicBlob can be used interchangeably in
113-
// instanceof checks.
114-
// For our internal TypeScript code PublicBlob doesn't exist as a type, and so
115-
// we need to use Blob as type and export it too.
116-
export const PublicBlob = makeConstructorPrivate(
117-
Blob,
118-
'Use Blob.fromUint8Array() or Blob.fromBase64String() instead.'
119-
);

‎packages/firestore/src/api/database.ts

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ import { FieldPath, ResourcePath } from '../model/path';
4747
import { isServerTimestamp } from '../model/server_timestamps';
4848
import { refValue } from '../model/values';
4949
import { PlatformSupport } from '../platform/platform';
50-
import { makeConstructorPrivate } from '../util/api';
5150
import { debugAssert, fail } from '../util/assert';
5251
import { AsyncObserver } from '../util/async_observer';
5352
import { AsyncQueue } from '../util/async_queue';
@@ -93,7 +92,6 @@ import { fieldPathFromArgument, UserDataReader } from './user_data_reader';
9392
import { UserDataWriter } from './user_data_writer';
9493
import { FirebaseAuthInternalName } from '@firebase/auth-interop-types';
9594
import { Provider } from '@firebase/component';
96-
import { FieldValue } from './field_value';
9795

9896
// settings() defaults:
9997
const DEFAULT_HOST = 'firestore.googleapis.com';
@@ -2529,39 +2527,3 @@ function applyFirestoreDataConverter<T>(
25292527
function contains(obj: object, key: string): obj is { key: unknown } {
25302528
return Object.prototype.hasOwnProperty.call(obj, key);
25312529
}
2532-
2533-
// Export the classes with a private constructor (it will fail if invoked
2534-
// at runtime). Note that this still allows instanceof checks.
2535-
2536-
// We're treating the variables as class names, so disable checking for lower
2537-
// case variable names.
2538-
export const PublicFirestore = makeConstructorPrivate(
2539-
Firestore,
2540-
'Use firebase.firestore() instead.'
2541-
);
2542-
export const PublicTransaction = makeConstructorPrivate(
2543-
Transaction,
2544-
'Use firebase.firestore().runTransaction() instead.'
2545-
);
2546-
export const PublicWriteBatch = makeConstructorPrivate(
2547-
WriteBatch,
2548-
'Use firebase.firestore().batch() instead.'
2549-
);
2550-
export const PublicDocumentReference = makeConstructorPrivate(
2551-
DocumentReference,
2552-
'Use firebase.firestore().doc() instead.'
2553-
);
2554-
export const PublicDocumentSnapshot = makeConstructorPrivate(DocumentSnapshot);
2555-
export const PublicQueryDocumentSnapshot = makeConstructorPrivate(
2556-
QueryDocumentSnapshot
2557-
);
2558-
export const PublicQuery = makeConstructorPrivate(Query);
2559-
export const PublicQuerySnapshot = makeConstructorPrivate(QuerySnapshot);
2560-
export const PublicCollectionReference = makeConstructorPrivate(
2561-
CollectionReference,
2562-
'Use firebase.firestore().collection() instead.'
2563-
);
2564-
export const PublicFieldValue = makeConstructorPrivate(
2565-
FieldValue,
2566-
'Use FieldValue.<field>() instead.'
2567-
);

‎packages/firestore/src/platform/config.ts

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,61 @@ import { FirebaseApp, FirebaseNamespace } from '@firebase/app-types';
1919
import { FirebaseAuthInternalName } from '@firebase/auth-interop-types';
2020
import { _FirebaseNamespace } from '@firebase/app-types/private';
2121
import { Component, ComponentType, Provider } from '@firebase/component';
22-
import { PublicBlob } from '../api/blob';
2322
import {
2423
CACHE_SIZE_UNLIMITED,
2524
Firestore,
26-
PublicCollectionReference,
27-
PublicDocumentReference,
28-
PublicDocumentSnapshot,
29-
PublicFirestore,
30-
PublicQuery,
31-
PublicQueryDocumentSnapshot,
32-
PublicQuerySnapshot,
33-
PublicTransaction,
34-
PublicWriteBatch,
35-
PublicFieldValue
25+
DocumentReference,
26+
DocumentSnapshot,
27+
QueryDocumentSnapshot,
28+
Query,
29+
QuerySnapshot,
30+
CollectionReference,
31+
Transaction,
32+
WriteBatch
3633
} from '../api/database';
34+
import { Blob } from '../api/blob';
3735
import { FieldPath } from '../api/field_path';
3836
import { GeoPoint } from '../api/geo_point';
3937
import { Timestamp } from '../api/timestamp';
38+
import { makeConstructorPrivate } from '../util/api';
39+
import { FieldValue } from '../api/field_value';
40+
41+
// Public instance that disallows construction at runtime. Note that this still
42+
// allows instanceof checks.
43+
export const PublicFirestore = makeConstructorPrivate(
44+
Firestore,
45+
'Use firebase.firestore() instead.'
46+
);
47+
export const PublicTransaction = makeConstructorPrivate(
48+
Transaction,
49+
'Use firebase.firestore().runTransaction() instead.'
50+
);
51+
export const PublicWriteBatch = makeConstructorPrivate(
52+
WriteBatch,
53+
'Use firebase.firestore().batch() instead.'
54+
);
55+
export const PublicDocumentReference = makeConstructorPrivate(
56+
DocumentReference,
57+
'Use firebase.firestore().doc() instead.'
58+
);
59+
export const PublicDocumentSnapshot = makeConstructorPrivate(DocumentSnapshot);
60+
export const PublicQueryDocumentSnapshot = makeConstructorPrivate(
61+
QueryDocumentSnapshot
62+
);
63+
export const PublicQuery = makeConstructorPrivate(Query);
64+
export const PublicQuerySnapshot = makeConstructorPrivate(QuerySnapshot);
65+
export const PublicCollectionReference = makeConstructorPrivate(
66+
CollectionReference,
67+
'Use firebase.firestore().collection() instead.'
68+
);
69+
export const PublicFieldValue = makeConstructorPrivate(
70+
FieldValue,
71+
'Use FieldValue.<field>() instead.'
72+
);
73+
export const PublicBlob = makeConstructorPrivate(
74+
Blob,
75+
'Use Blob.fromUint8Array() or Blob.fromBase64String() instead.'
76+
);
4077

4178
const firestoreNamespace = {
4279
Firestore: PublicFirestore,

‎packages/firestore/test/integration/api/private_constructors.test.ts

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/**
2+
* @license
3+
* Copyright 2017 Google Inc.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
import { expect } from 'chai';
19+
20+
import firebase from '../util/firebase_export';
21+
22+
// allow using constructor with any
23+
/* eslint-disable @typescript-eslint/no-explicit-any */
24+
25+
describe('Constructors', () => {
26+
it('are private for Firestore', () => {
27+
expect(() => new (firebase.firestore!.Firestore as any)('')).to.throw(
28+
'This constructor is private. Use firebase.firestore() instead.'
29+
);
30+
});
31+
32+
it('are private for Transaction', () => {
33+
expect(() => new (firebase.firestore!.Transaction as any)('')).to.throw(
34+
'This constructor is private. Use firebase.firestore().runTransaction() instead.'
35+
);
36+
});
37+
38+
it('are private for WriteBatch', () => {
39+
expect(() => new (firebase.firestore!.WriteBatch as any)('')).to.throw(
40+
'This constructor is private. Use firebase.firestore().batch() instead.'
41+
);
42+
});
43+
44+
it('are private for DocumentReference', () => {
45+
expect(
46+
() => new (firebase.firestore!.DocumentReference as any)('')
47+
).to.throw(
48+
'This constructor is private. Use firebase.firestore().doc() instead.'
49+
);
50+
});
51+
52+
it('are private for Query', () => {
53+
expect(() => new (firebase.firestore!.Query as any)('')).to.throw(
54+
'This constructor is private.'
55+
);
56+
});
57+
58+
it('are private for CollectionReference', () => {
59+
expect(
60+
() => new (firebase.firestore!.CollectionReference as any)('')
61+
).to.throw(
62+
'This constructor is private. Use firebase.firestore().collection() instead.'
63+
);
64+
});
65+
66+
it('are private for QuerySnapshot', () => {
67+
expect(() => new (firebase.firestore!.QuerySnapshot as any)('')).to.throw(
68+
'This constructor is private.'
69+
);
70+
});
71+
72+
it('are private for DocumentSnapshot', () => {
73+
expect(
74+
() => new (firebase.firestore!.DocumentSnapshot as any)('')
75+
).to.throw('This constructor is private.');
76+
});
77+
78+
it('are private for QueryDocumentSnapshot', () => {
79+
expect(
80+
() => new (firebase.firestore!.QueryDocumentSnapshot as any)('')
81+
).to.throw('This constructor is private.');
82+
});
83+
84+
it('are private for Blob', () => {
85+
expect(() => new (firebase.firestore!.Blob as any)('')).to.throw(
86+
'This constructor is private.'
87+
);
88+
});
89+
90+
it('are private for FieldValue', () => {
91+
expect(() => new (firebase.firestore!.FieldValue as any)('')).to.throw(
92+
'This constructor is private. Use FieldValue.<field>() instead.'
93+
);
94+
});
95+
});

‎packages/firestore/test/unit/api/blob.test.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
*/
1717

1818
import { expect } from 'chai';
19-
import { Blob, PublicBlob } from '../../../src/api/blob';
19+
import { Blob } from '../../../src/api/blob';
2020
import { blob, expectEqual, expectNotEqual } from '../../util/helpers';
2121

2222
describe('Blob', () => {
@@ -52,17 +52,8 @@ describe('Blob', () => {
5252
);
5353
});
5454

55-
it('Blob throws on using the public constructor', () => {
56-
// allow using constructor with any
57-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
58-
expect(() => new (PublicBlob as any)('')).to.throw(
59-
'This constructor is private. Use Blob.fromUint8Array() or ' +
60-
'Blob.fromBase64String() instead.'
61-
);
62-
});
63-
64-
it('PublicBlob works with instanceof checks', () => {
65-
expect(Blob.fromBase64String('') instanceof PublicBlob).to.equal(true);
55+
it('works with instanceof checks', () => {
56+
expect(Blob.fromBase64String('') instanceof Blob).to.equal(true);
6657
});
6758

6859
it('support equality checking with isEqual()', () => {

‎packages/firestore/test/unit/api/field_value.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* limitations under the License.
1616
*/
1717

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

2121
describe('FieldValue', () => {

0 commit comments

Comments
 (0)