From ce3a2c4b6f0e6ea7f7184ff40244f7f20cfe1346 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 27 Sep 2022 12:02:10 -0400 Subject: [PATCH 1/5] Re-write COUNT docs --- packages/firestore/src/api/aggregate.ts | 21 ++++++-- packages/firestore/src/lite-api/aggregate.ts | 30 ++++++++--- .../firestore/src/lite-api/aggregate_types.ts | 54 ++++++++++--------- 3 files changed, 68 insertions(+), 37 deletions(-) diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts index bd632d0ad6d..a4713ca4149 100644 --- a/packages/firestore/src/api/aggregate.ts +++ b/packages/firestore/src/api/aggregate.ts @@ -29,12 +29,25 @@ import { ExpUserDataWriter } from './reference_impl'; export { aggregateQuerySnapshotEqual } from '../lite-api/aggregate'; /** - * Executes the query and returns the results as a `AggregateQuerySnapshot` from the - * server. Returns an error if the network is not available. + * Calculates the number of documents in the result set of the given query, + * without actually downloading the documents. * - * @param query - The `Query` to execute. + * Using this function to count the documents is efficient because only the + * final count, not the documents' data, is downloaded. This function can even + * count the documents if the result set would be prohibitively large to + * download entirely (e.g. thousands of documents). * - * @returns A `Promise` that will be resolved with the results of the query. + * The result received from the server is presented, unaltered, without + * considering any local state. That is, documents in the local cache are not + * taken into consideration, neither are local modifications not yet + * synchronized with the server. Previously-downloaded results, if any, are not + * used: every request using this source necessarily involves a round trip to + * the server. + * + * @param query - The query whose result set size to calculate. + * @returns A Promise that will be resolved with the count; the count can be + * retrieved from `snapshot.data().count`, where `snapshot` is the + * `AggregateQuerySnapshot` to which the returned Promise resolves. */ export function getCountFromServer( query: Query diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts index 9532cec855c..d9977dc117e 100644 --- a/packages/firestore/src/lite-api/aggregate.ts +++ b/packages/firestore/src/lite-api/aggregate.ts @@ -31,14 +31,18 @@ import { Query, queryEqual } from './reference'; import { LiteUserDataWriter } from './reference_impl'; /** - * Counts the number of documents in the result set of the given query, ignoring - * any locally-cached data and any locally-pending writes and simply surfacing - * whatever the server returns. If the server cannot be reached then the - * returned promise will be rejected. + * Calculates the number of documents in the result set of the given query, + * without actually downloading the documents. * - * @param query - The `Query` to execute. + * Using this function to count the documents is efficient because only the + * final count, not the documents' data, is downloaded. This function can even + * count the documents if the result set would be prohibitively large to + * download entirely (e.g. thousands of documents). * - * @returns An `AggregateQuerySnapshot` that contains the number of documents. + * @param query - The query whose result set size to calculate. + * @returns A Promise that will be resolved with the count; the count can be + * retrieved from `snapshot.data().count`, where `snapshot` is the + * `AggregateQuerySnapshot` to which the returned Promise resolves. */ export function getCount( query: Query @@ -57,7 +61,19 @@ export function getCount( * @param left - The `AggregateQuerySnapshot` to compare. * @param right - The `AggregateQuerySnapshot` to compare. * - * @returns true if the AggregateQuerySnapshots are equal. + * @returns true if the AggregateQuerySnapshots are equal. */ + +/** + * Compares two `AggregateQuerySnapshot` instances for equality. + * + * Two `AggregateQuerySnapshot` instances are considered "equal" if they have + * underlying queries that compare equal, and the same data. + * + * @param left - The first `AggregateQuerySnapshot` to compare. + * @param right - The second `AggregateQuerySnapshot` to compare. + * + * @returns `true` if the objects are "equal", as defined above, or `false` + * otherwise. */ export function aggregateQuerySnapshotEqual( left: AggregateQuerySnapshot, diff --git a/packages/firestore/src/lite-api/aggregate_types.ts b/packages/firestore/src/lite-api/aggregate_types.ts index b7243f1062c..d71a36e86eb 100644 --- a/packages/firestore/src/lite-api/aggregate_types.ts +++ b/packages/firestore/src/lite-api/aggregate_types.ts @@ -18,64 +18,66 @@ import { Query } from './reference'; /** - * An `AggregateField`that captures input type T. + * Represents an aggregation that can be performed by Firestore. */ // eslint-disable-next-line @typescript-eslint/no-unused-vars export class AggregateField { + /** A type string to uniquely identify instances of this class. */ type = 'AggregateField'; } /** - * Creates and returns an aggregation field that counts the documents in the result set. - * @returns An `AggregateField` object with number input type. + * The union of all `AggregateField` types that are supported by Firestore. */ -export function count(): AggregateField { - return new AggregateField(); -} +export type AggregateFieldType = AggregateField; /** - * The union of all `AggregateField` types that are returned from the factory - * functions. - */ -export type AggregateFieldType = ReturnType; - -/** - * A type whose values are all `AggregateField` objects. - * This is used as an argument to the "getter" functions, and the snapshot will - * map the same names to the corresponding values. + * A type whose property values are all `AggregateField` objects. */ export interface AggregateSpec { [field: string]: AggregateFieldType; } /** - * A type whose keys are taken from an `AggregateSpec` type, and whose values - * are the result of the aggregation performed by the corresponding - * `AggregateField` from the input `AggregateSpec`. + * A type whose keys are taken from an `AggregateSpec`, and whose values are the + * result of the aggregation performed by the corresponding `AggregateField` + * from the input `AggregateSpec`. */ export type AggregateSpecData = { [P in keyof T]: T[P] extends AggregateField ? U : never; }; /** - * An `AggregateQuerySnapshot` contains the results of running an aggregate query. + * The results of executing an aggregation query. */ export class AggregateQuerySnapshot { + /** A type string to uniquely identify instances of this class. */ readonly type = 'AggregateQuerySnapshot'; + /** + * The underlying query over which the aggregations recorded in this + * `AggregateQuerySnapshot` were performed. + */ + readonly query: Query; + /** @hideconstructor */ constructor( - readonly query: Query, + query: Query, private readonly _data: AggregateSpecData - ) {} + ) { + this.query = query; + } /** - * The results of the requested aggregations. The keys of the returned object - * will be the same as those of the `AggregateSpec` object specified to the - * aggregation method, and the values will be the corresponding aggregation - * result. + * Returns the results of the aggregations performed over the underlying + * query. + * + * The keys of the returned object will be the same as those of the + * `AggregateSpec` object specified to the aggregation method, and the values + * will be the corresponding aggregation result. * - * @returns The aggregation statistics result of running a query. + * @returns The results of the aggregations performed over the underlying + * query. */ data(): AggregateSpecData { return this._data; From 352c53a73dc5127256d9ff1067388b672c0a86d3 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 27 Sep 2022 12:14:08 -0400 Subject: [PATCH 2/5] update exported APIs --- common/api-review/firestore-lite.api.md | 6 ------ common/api-review/firestore.api.md | 6 ------ packages/firestore/lite/index.ts | 3 +-- packages/firestore/src/api.ts | 3 +-- 4 files changed, 2 insertions(+), 16 deletions(-) diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index 52b8947344c..4be5982af59 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -19,7 +19,6 @@ export type AddPrefixToKeys { - // (undocumented) type: string; } @@ -29,9 +28,7 @@ export type AggregateFieldType = ReturnType; // @public export class AggregateQuerySnapshot { data(): AggregateSpecData; - // (undocumented) readonly query: Query; - // (undocumented) readonly type = "AggregateQuerySnapshot"; } @@ -95,9 +92,6 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por mockUserToken?: EmulatorMockTokenOptions | string; }): void; -// @public -export function count(): AggregateField; - // @public export function deleteDoc(reference: DocumentReference): Promise; diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 6fa63aa0d90..8f674b89f31 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -19,7 +19,6 @@ export type AddPrefixToKeys { - // (undocumented) type: string; } @@ -29,9 +28,7 @@ export type AggregateFieldType = ReturnType; // @public export class AggregateQuerySnapshot { data(): AggregateSpecData; - // (undocumented) readonly query: Query; - // (undocumented) readonly type = "AggregateQuerySnapshot"; } @@ -101,9 +98,6 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por mockUserToken?: EmulatorMockTokenOptions | string; }): void; -// @public -export function count(): AggregateField; - // @public export function deleteDoc(reference: DocumentReference): Promise; diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index 07451f0889b..5e3faa48e31 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -37,8 +37,7 @@ export { AggregateFieldType, AggregateSpec, AggregateSpecData, - AggregateQuerySnapshot, - count + AggregateQuerySnapshot } from '../src/lite-api/aggregate_types'; export { FirestoreSettings as Settings } from '../src/lite-api/settings'; diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 14c6c4b4cf1..f05db09f568 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -25,8 +25,7 @@ export { AggregateFieldType, AggregateSpec, AggregateSpecData, - AggregateQuerySnapshot, - count + AggregateQuerySnapshot } from './lite-api/aggregate_types'; export { FieldPath, documentId } from './api/field_path'; From 5862867e0f365a26ee831f92442b9529e54c8f4f Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 27 Sep 2022 12:21:28 -0400 Subject: [PATCH 3/5] lite-api/aggregate.ts: remove bogus comment that was accidentally left behind --- packages/firestore/src/lite-api/aggregate.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts index d9977dc117e..390c6af5ef8 100644 --- a/packages/firestore/src/lite-api/aggregate.ts +++ b/packages/firestore/src/lite-api/aggregate.ts @@ -53,16 +53,6 @@ export function getCount( return new CountQueryRunner(query, datastore, userDataWriter).run(); } -/** - * Compares two `AggregateQuerySnapshot` instances for equality. - * Two `AggregateQuerySnapshot` instances are considered "equal" if they have - * the same underlying query, and the same data. - * - * @param left - The `AggregateQuerySnapshot` to compare. - * @param right - The `AggregateQuerySnapshot` to compare. - * - * @returns true if the AggregateQuerySnapshots are equal. */ - /** * Compares two `AggregateQuerySnapshot` instances for equality. * From 9556997caa7868dddbc2b0cd0243f9088d80697b Mon Sep 17 00:00:00 2001 From: dconeybe Date: Tue, 27 Sep 2022 16:33:40 +0000 Subject: [PATCH 4/5] Update API reports --- common/api-review/firestore-lite.api.md | 2 +- common/api-review/firestore.api.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index 4be5982af59..b336e0d0c8b 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -23,7 +23,7 @@ export class AggregateField { } // @public -export type AggregateFieldType = ReturnType; +export type AggregateFieldType = AggregateField; // @public export class AggregateQuerySnapshot { diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 8f674b89f31..46ac63239c6 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -23,7 +23,7 @@ export class AggregateField { } // @public -export type AggregateFieldType = ReturnType; +export type AggregateFieldType = AggregateField; // @public export class AggregateQuerySnapshot { From b5af575e2646eff6b7d13d3e4868a28aefba0e23 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 27 Sep 2022 13:24:16 -0400 Subject: [PATCH 5/5] empty commit to trigger github actions