Skip to content

Commit 7c0ff71

Browse files
committed
resolve comments
1 parent cc93b3b commit 7c0ff71

File tree

7 files changed

+67
-109
lines changed

7 files changed

+67
-109
lines changed

packages/firebase/compat/index.d.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8227,7 +8227,10 @@ declare namespace firebase.storage {
82278227
}
82288228

82298229
declare namespace firebase.firestore {
8230+
8231+
/** Alias dynamic document field value types to any */
82308232
export type DocumentFieldValue = any;
8233+
82318234
/**
82328235
* Document data (for use with `DocumentReference.set()`) consists of fields
82338236
* mapped to values.

packages/firestore/src/api/aggregate.ts

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,9 @@
1414
* See the License for the specific language governing permissions and
1515
* limitations under the License.
1616
*/
17-
import { Query, SnapshotOptions } from '../api';
17+
import { Query } from '../api';
1818
import { firestoreClientRunCountQuery } from '../core/firestore_client';
19-
import {
20-
AggregateField,
21-
AggregateQuerySnapshot as AggregateQuerySnapshotLite,
22-
AggregateSpec,
23-
AggregateSpecData
24-
} from '../lite-api/aggregate';
19+
import { AggregateField, AggregateQuerySnapshot } from '../lite-api/aggregate';
2520
import { cast } from '../util/input_validation';
2621

2722
import { ensureFirestoreConfigured, Firestore } from './database';
@@ -31,29 +26,15 @@ export {
3126
AggregateSpec,
3227
AggregateSpecData,
3328
AggregateQuerySnapshot,
34-
aggregateSnapshotEqual,
35-
aggregateFieldEqual
29+
aggregateSnapshotEqual
3630
} from '../lite-api/aggregate';
3731

3832
/**
39-
* A `AggregateQuerySnapshot` contains the result of running an aggregation query.
40-
* The result data can be extracted with `.data()`.
41-
*/
42-
class AggregateQuerySnapshot<
43-
T extends AggregateSpec
44-
> extends AggregateQuerySnapshotLite<T> {
45-
constructor(query: Query<unknown>, _data: AggregateSpecData<T>) {
46-
super(query, _data);
47-
}
48-
data(options: SnapshotOptions = {}): AggregateSpecData<T> {
49-
return super.data();
50-
}
51-
}
52-
53-
/**
54-
* Executes the query and returns the results as a `QuerySnapshot` from the
33+
* Executes the query and returns the results as a `AggregateQuerySnapshot` from the
5534
* server. Returns an error if the network is not available.
5635
*
36+
* @param query - The `Query` to execute.
37+
*
5738
* @returns A `Promise` that will be resolved with the results of the query.
5839
*/
5940
export function getCountFromServer(

packages/firestore/src/core/firestore_client.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,17 @@
1717

1818
import { GetOptions } from '@firebase/firestore-types';
1919

20-
import {
21-
AggregateField,
22-
AggregateSpec,
23-
AggregateQuerySnapshot
24-
} from '../api/aggregate';
20+
import { AggregateField } from '../api/aggregate';
2521
import { LoadBundleTask } from '../api/bundle';
2622
import {
2723
CredentialChangeListener,
2824
CredentialsProvider
2925
} from '../api/credentials';
3026
import { User } from '../auth/user';
31-
import { getCountFromServer } from '../lite-api/aggregate';
27+
import {
28+
getCount as getCountFromServer,
29+
AggregateQuerySnapshot as LiteAggregateQuerySnapshot
30+
} from '../lite-api/aggregate';
3231
import { Query as LiteQuery } from '../lite-api/reference';
3332
import { LocalStore } from '../local/local_store';
3433
import {
@@ -512,9 +511,9 @@ export function firestoreClientTransaction<T>(
512511
export function firestoreClientRunCountQuery(
513512
client: FirestoreClient,
514513
query: LiteQuery<unknown>
515-
): Promise<AggregateQuerySnapshot<{ count: AggregateField<number> }>> {
514+
): Promise<LiteAggregateQuerySnapshot<{ count: AggregateField<number> }>> {
516515
const deferred = new Deferred<
517-
AggregateQuerySnapshot<{ count: AggregateField<number> }>
516+
LiteAggregateQuerySnapshot<{ count: AggregateField<number> }>
518517
>();
519518
client.asyncQueue.enqueueAndForget(async () => {
520519
const remoteStore = await getRemoteStore(client);

packages/firestore/src/lite-api/aggregate.ts

Lines changed: 25 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,30 @@ import { cast } from '../util/input_validation';
2424

2525
import { getDatastore } from './components';
2626
import { Firestore } from './database';
27-
import { DocumentFieldValue, Query, queryEqual } from './reference';
27+
import { Query, queryEqual } from './reference';
2828
import { LiteUserDataWriter } from './reference_impl';
2929

3030
/**
31-
* An `AggregateField` computes some aggregation statistics from the result set of
32-
* an aggregation query.
31+
* An `AggregateField`that captures input type T.
3332
*/
3433
export class AggregateField<T> {
3534
type = 'AggregateField';
36-
_datum?: T;
35+
}
3736

38-
constructor(readonly subType: string) {}
37+
/**
38+
* Creates and returns an aggregation field that counts the documents in the result set.
39+
*
40+
* @returns An `AggregateField` object with number input type.
41+
*/
42+
export function count(): AggregateField<number> {
43+
return new AggregateField<number>();
3944
}
4045

4146
/**
4247
* The union of all `AggregateField` types that are returned from the factory
4348
* functions.
4449
*/
45-
export type AggregateFieldType =
46-
| AggregateField<number>
47-
| AggregateField<DocumentFieldValue | undefined>
48-
| AggregateField<number | undefined>;
50+
type AggregateFieldType = ReturnType<typeof count>;
4951

5052
/**
5153
* A type whose values are all `AggregateField` objects.
@@ -60,46 +62,28 @@ export type AggregateSpec = { [field: string]: AggregateFieldType };
6062
* `AggregateField` from the input `AggregateSpec`.
6163
*/
6264
export type AggregateSpecData<T extends AggregateSpec> = {
63-
[Property in keyof T]-?: T[Property]['_datum'];
65+
[P in keyof T]: T[P] extends AggregateField<infer U> ? U : never;
6466
};
6567

66-
/**
67-
* Creates and returns an aggregation field that counts the documents in the result set.
68-
* @returns An `AggregateField` object that includes number of documents.
69-
*/
70-
export function count(): AggregateField<number> {
71-
return new AggregateField<number>('count');
72-
}
73-
74-
/**
75-
* Compares two `AggregateField` instances for equality.
76-
* The two `AggregateField` instances are considered "equal" if and only if
77-
* they were created by the same factory function (e.g. `count()`, `min()`, and
78-
* `sum()`) with "equal" arguments.
79-
*/
80-
export function aggregateFieldEqual(
81-
left: AggregateField<unknown>,
82-
right: AggregateField<unknown>
83-
): boolean {
84-
return typeof left === typeof right && left.subType === right.subType;
85-
}
86-
8768
/**
8869
* An `AggregateQuerySnapshot` contains the results of running an aggregate query.
8970
*/
9071
export class AggregateQuerySnapshot<T extends AggregateSpec> {
9172
readonly type = 'AggregateQuerySnapshot';
9273

74+
/** @hideconstructor */
9375
constructor(
9476
readonly query: Query<unknown>,
95-
protected readonly _data: AggregateSpecData<T>
77+
private readonly _data: AggregateSpecData<T>
9678
) {}
9779

9880
/**
9981
* The results of the requested aggregations. The keys of the returned object
10082
* will be the same as those of the `AggregateSpec` object specified to the
10183
* aggregation method, and the values will be the corresponding aggregation
10284
* result.
85+
*
86+
* @returns The aggregation statistics result of running a query.
10387
*/
10488
data(): AggregateSpecData<T> {
10589
return this._data;
@@ -112,10 +96,11 @@ export class AggregateQuerySnapshot<T extends AggregateSpec> {
11296
* whatever the server returns. If the server cannot be reached then the
11397
* returned promise will be rejected.
11498
*
115-
* This is a convenience shorthand for:
116-
* getAggregateFromServer(query, { count: count() }).
99+
* @param query - The `Query` to execute.
100+
*
101+
* @returns An `AggregateQuerySnapshot` that contains the number of documents.
117102
*/
118-
export function getCountFromServer(
103+
export function getCount(
119104
query: Query<unknown>
120105
): Promise<AggregateQuerySnapshot<{ count: AggregateField<number> }>> {
121106
const firestore = cast(query.firestore, Firestore);
@@ -150,6 +135,11 @@ export function getCountFromServer(
150135
* Compares two `AggregateQuerySnapshot` instances for equality.
151136
* Two `AggregateQuerySnapshot` instances are considered "equal" if they have
152137
* the same underlying query, the same metadata, and the same data.
138+
*
139+
* @param left - The `AggregateQuerySnapshot` to compare.
140+
* @param right - The `AggregateQuerySnapshot` to compare.
141+
*
142+
* @returns true if the AggregateQuerySnapshos are equal.
153143
*/
154144
export function aggregateSnapshotEqual<T extends AggregateSpec>(
155145
left: AggregateQuerySnapshot<T>,

packages/firestore/src/lite-api/reference.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import { FieldValue } from './field_value';
4040
import { FirestoreDataConverter } from './snapshot';
4141
import { NestedUpdateFields, Primitive } from './types';
4242

43+
/** Alias dynamic document field value types to any */
4344
// eslint-disable-next-line @typescript-eslint/no-explicit-any
4445
export type DocumentFieldValue = any;
4546

0 commit comments

Comments
 (0)