Skip to content

Commit fc6930f

Browse files
schmidt-sebastianschmidt-sebastian
committed
Remove static references to API types
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 da3582b commit fc6930f

File tree

9 files changed

+161
-103
lines changed

9 files changed

+161
-103
lines changed

‎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+
const PublicFirestore = makeConstructorPrivate(
44+
Firestore,
45+
'Use firebase.firestore() instead.'
46+
);
47+
const PublicTransaction = makeConstructorPrivate(
48+
Transaction,
49+
'Use firebase.firestore().runTransaction() instead.'
50+
);
51+
const PublicWriteBatch = makeConstructorPrivate(
52+
WriteBatch,
53+
'Use firebase.firestore().batch() instead.'
54+
);
55+
const PublicDocumentReference = makeConstructorPrivate(
56+
DocumentReference,
57+
'Use firebase.firestore().doc() instead.'
58+
);
59+
const PublicDocumentSnapshot = makeConstructorPrivate(DocumentSnapshot);
60+
const PublicQueryDocumentSnapshot = makeConstructorPrivate(
61+
QueryDocumentSnapshot
62+
);
63+
const PublicQuery = makeConstructorPrivate(Query);
64+
const PublicQuerySnapshot = makeConstructorPrivate(QuerySnapshot);
65+
const PublicCollectionReference = makeConstructorPrivate(
66+
CollectionReference,
67+
'Use firebase.firestore().collection() instead.'
68+
);
69+
const PublicFieldValue = makeConstructorPrivate(
70+
FieldValue,
71+
'Use FieldValue.<field>() instead.'
72+
);
73+
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', () => {

‎packages/firestore/test/unit/local/local_store.test.ts

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import * as api from '../../../src/protos/firestore_proto_api';
1919

2020
import { expect } from 'chai';
21-
import { PublicFieldValue } from '../../../src/api/database';
21+
import { FieldValue } from '../../../src/api/field_value';
2222
import { Timestamp } from '../../../src/api/timestamp';
2323
import { User } from '../../../src/auth/user';
2424
import { Query } from '../../../src/core/query';
@@ -1256,16 +1256,12 @@ function genericLocalStoreTests(
12561256
doc('foo/bar', 0, { sum: 0 }, { hasLocalMutations: true })
12571257
)
12581258
.toContain(doc('foo/bar', 0, { sum: 0 }, { hasLocalMutations: true }))
1259-
.after(
1260-
transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) })
1261-
)
1259+
.after(transformMutation('foo/bar', { sum: FieldValue.increment(1) }))
12621260
.toReturnChanged(
12631261
doc('foo/bar', 0, { sum: 1 }, { hasLocalMutations: true })
12641262
)
12651263
.toContain(doc('foo/bar', 0, { sum: 1 }, { hasLocalMutations: true }))
1266-
.after(
1267-
transformMutation('foo/bar', { sum: PublicFieldValue.increment(2) })
1268-
)
1264+
.after(transformMutation('foo/bar', { sum: FieldValue.increment(2) }))
12691265
.toReturnChanged(
12701266
doc('foo/bar', 0, { sum: 3 }, { hasLocalMutations: true })
12711267
)
@@ -1290,9 +1286,7 @@ function genericLocalStoreTests(
12901286
.toContain(
12911287
doc('foo/bar', 1, { sum: 0 }, { hasCommittedMutations: true })
12921288
)
1293-
.after(
1294-
transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) })
1295-
)
1289+
.after(transformMutation('foo/bar', { sum: FieldValue.increment(1) }))
12961290
.toReturnChanged(
12971291
doc('foo/bar', 1, { sum: 1 }, { hasLocalMutations: true })
12981292
)
@@ -1307,9 +1301,7 @@ function genericLocalStoreTests(
13071301
.toContain(
13081302
doc('foo/bar', 2, { sum: 1 }, { hasCommittedMutations: true })
13091303
)
1310-
.after(
1311-
transformMutation('foo/bar', { sum: PublicFieldValue.increment(2) })
1312-
)
1304+
.after(transformMutation('foo/bar', { sum: FieldValue.increment(2) }))
13131305
.toReturnChanged(
13141306
doc('foo/bar', 2, { sum: 3 }, { hasLocalMutations: true })
13151307
)
@@ -1335,9 +1327,7 @@ function genericLocalStoreTests(
13351327
.afterAcknowledgingMutation({ documentVersion: 1 })
13361328
.toReturnChanged(doc('foo/bar', 1, { sum: 0 }))
13371329
.toContain(doc('foo/bar', 1, { sum: 0 }))
1338-
.after(
1339-
transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) })
1340-
)
1330+
.after(transformMutation('foo/bar', { sum: FieldValue.increment(1) }))
13411331
.toReturnChanged(
13421332
doc('foo/bar', 1, { sum: 1 }, { hasLocalMutations: true })
13431333
)
@@ -1353,9 +1343,7 @@ function genericLocalStoreTests(
13531343
.toContain(doc('foo/bar', 2, { sum: 1 }, { hasLocalMutations: true }))
13541344
// Add another increment. Note that we still compute the increment based
13551345
// on the local value.
1356-
.after(
1357-
transformMutation('foo/bar', { sum: PublicFieldValue.increment(2) })
1358-
)
1346+
.after(transformMutation('foo/bar', { sum: FieldValue.increment(2) }))
13591347
.toReturnChanged(
13601348
doc('foo/bar', 2, { sum: 3 }, { hasLocalMutations: true })
13611349
)
@@ -1417,9 +1405,9 @@ function genericLocalStoreTests(
14171405
)
14181406
.toReturnChanged(doc('foo/bar', 1, { sum: 0, arrayUnion: [] }))
14191407
.afterMutations([
1420-
transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) }),
1408+
transformMutation('foo/bar', { sum: FieldValue.increment(1) }),
14211409
transformMutation('foo/bar', {
1422-
arrayUnion: PublicFieldValue.arrayUnion('foo')
1410+
arrayUnion: FieldValue.arrayUnion('foo')
14231411
})
14241412
])
14251413
.toReturnChanged(
@@ -1458,7 +1446,7 @@ function genericLocalStoreTests(
14581446
.toReturnTargetId(2)
14591447
.afterMutations([
14601448
patchMutation('foo/bar', {}, Precondition.none()),
1461-
transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) })
1449+
transformMutation('foo/bar', { sum: FieldValue.increment(1) })
14621450
])
14631451
.toReturnChanged(
14641452
doc('foo/bar', 0, { sum: 1 }, { hasLocalMutations: true })
@@ -1484,7 +1472,7 @@ function genericLocalStoreTests(
14841472
.toReturnTargetId(2)
14851473
.afterMutations([
14861474
patchMutation('foo/bar', {}),
1487-
transformMutation('foo/bar', { sum: PublicFieldValue.increment(1) })
1475+
transformMutation('foo/bar', { sum: FieldValue.increment(1) })
14881476
])
14891477
.toReturnChanged(deletedDoc('foo/bar', 0))
14901478
.toNotContain('foo/bar')

‎packages/firestore/test/unit/model/mutation.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
*/
1717

1818
import { expect } from 'chai';
19-
import { PublicFieldValue as FieldValue } from '../../../src/api/database';
19+
import { FieldValue } from '../../../src/api/field_value';
2020
import { Timestamp } from '../../../src/api/timestamp';
2121
import { Document, MaybeDocument } from '../../../src/model/document';
2222
import { serverTimestamp } from '../../../src/model/server_timestamps';

0 commit comments

Comments
 (0)