Merge 9d6d28d into 8877803

Author: schmidt-sebastian

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;

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");
   }
 }
 

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)

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);
   }
 }