diff --git a/packages/firestore/src/core/query.ts b/packages/firestore/src/core/query.ts index 7f3e2a6820c..50e540430e0 100644 --- a/packages/firestore/src/core/query.ts +++ b/packages/firestore/src/core/query.ts @@ -17,7 +17,7 @@ import * as api from '../protos/firestore_proto_api'; -import { Document } from '../model/document'; +import { compareDocumentsByField, Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { canonicalId, @@ -808,8 +808,8 @@ export class OrderBy { compare(d1: Document, d2: Document): number { const comparison = this.isKeyOrderBy - ? Document.compareByKey(d1, d2) - : Document.compareByField(this.field, d1, d2); + ? DocumentKey.comparator(d1.key, d2.key) + : compareDocumentsByField(this.field, d1, d2); switch (this.dir) { case Direction.ASCENDING: return comparison; diff --git a/packages/firestore/src/model/document.ts b/packages/firestore/src/model/document.ts index ce8600f33dc..64a43b5a06c 100644 --- a/packages/firestore/src/model/document.ts +++ b/packages/firestore/src/model/document.ts @@ -37,10 +37,6 @@ export interface DocumentOptions { export abstract class MaybeDocument { constructor(readonly key: DocumentKey, readonly version: SnapshotVersion) {} - static compareByKey(d1: MaybeDocument, d2: MaybeDocument): number { - return DocumentKey.comparator(d1.key, d2.key); - } - /** * Whether this document had a local mutation applied that has not yet been * acknowledged by Watch. @@ -107,15 +103,23 @@ export class Document extends MaybeDocument { get hasPendingWrites(): boolean { return this.hasLocalMutations || this.hasCommittedMutations; } +} - static compareByField(field: FieldPath, d1: Document, d2: Document): number { - const v1 = d1.field(field); - const v2 = d2.field(field); - if (v1 !== null && v2 !== null) { - return valueCompare(v1, v2); - } else { - return fail("Trying to compare documents on fields that don't exist"); - } +/** + * Compares the value for field `field` in the provided documents. Throws if + * the field does not exist in both documents. + */ +export function compareDocumentsByField( + field: FieldPath, + d1: Document, + d2: Document +): number { + const v1 = d1.field(field); + const v2 = d2.field(field); + if (v1 !== null && v2 !== null) { + return valueCompare(v1, v2); + } else { + return fail("Trying to compare documents on fields that don't exist"); } } diff --git a/packages/firestore/test/unit/model/document_set.test.ts b/packages/firestore/test/unit/model/document_set.test.ts index 49bf1815c06..13cc4a98f60 100644 --- a/packages/firestore/test/unit/model/document_set.test.ts +++ b/packages/firestore/test/unit/model/document_set.test.ts @@ -54,7 +54,7 @@ describe('DocumentSet', () => { }); it('adds and deletes elements', () => { - const set = new DocumentSet(Document.compareByKey) + const set = new DocumentSet() // Compares by key by default .add(d1) .add(d2) .add(d3) diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index ad52e043257..e2de8705e72 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -51,6 +51,7 @@ import { maybeDocumentMap } from '../../src/model/collections'; import { + compareDocumentsByField, Document, DocumentOptions, MaybeDocument, @@ -640,7 +641,7 @@ export function documentSetAsArray(docs: DocumentSet): Document[] { export class DocComparator { static byField(...fields: string[]): DocumentComparator { const path = new FieldPath(fields); - return Document.compareByField.bind(this, path); + return (doc1, doc2) => compareDocumentsByField(path, doc1, doc2); } }