Make Document.field() nullable (#1927)

Author: schmidt-sebastian

packages/firestore/src/api/database.ts

@@ -1294,7 +1294,7 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot {
       const value = this._document.data.field(
         fieldPathFromArgument('DocumentSnapshot.get', fieldPath)
       );
-      if (value !== undefined) {
+      if (value !== null) {
         return this.convertValue(
           value,
           FieldValueOptions.fromSnapshotOptions(
@@ -1648,7 +1648,7 @@ export class Query implements firestore.Query {
               '" is an uncommitted server timestamp. (Since the value of ' +
               'this field is unknown, you cannot start/end a query with it.)'
           );
-        } else if (value !== undefined) {
+        } else if (value !== null) {
           components.push(value);
         } else {
           const field = orderBy.field.canonicalString();

packages/firestore/src/core/query.ts

@@ -401,10 +401,7 @@ export class Query {
   private matchesOrderBy(doc: Document): boolean {
     for (const orderBy of this.explicitOrderBy) {
       // order by key always matches
-      if (
-        !orderBy.field.isKeyField() &&
-        doc.field(orderBy.field) === undefined
-      ) {
+      if (!orderBy.field.isKeyField() && doc.field(orderBy.field) === null) {
         return false;
       }
     }
@@ -572,7 +569,7 @@ export class FieldFilter extends Filter {
 
     // Only compare types with matching backend order (such as double and int).
     return (
-      other !== undefined &&
+      other !== null &&
       this.value.typeOrder === other.typeOrder &&
       this.matchesComparison(other.compareTo(this.value))
     );
@@ -676,7 +673,7 @@ export class InFilter extends FieldFilter {
   matches(doc: Document): boolean {
     const arrayValue = this.value;
     const other = doc.field(this.field);
-    return other !== undefined && arrayValue.contains(other);
+    return other !== null && arrayValue.contains(other);
   }
 }
 

packages/firestore/src/model/document.ts

@@ -75,7 +75,7 @@ export class Document extends MaybeDocument {
     this.hasCommittedMutations = !!options.hasCommittedMutations;
   }
 
-  field(path: FieldPath): FieldValue | undefined {
+  field(path: FieldPath): FieldValue | null {
     return this.data.field(path);
   }
 
@@ -114,7 +114,7 @@ export class Document extends MaybeDocument {
   static compareByField(field: FieldPath, d1: Document, d2: Document): number {
     const v1 = d1.field(field);
     const v2 = d2.field(field);
-    if (v1 !== undefined && v2 !== undefined) {
+    if (v1 !== null && v2 !== null) {
       return v1.compareTo(v2);
     } else {
       return fail("Trying to compare documents on fields that don't exist");

packages/firestore/src/model/field_value.ts

@@ -589,17 +589,17 @@ export class ObjectValue extends FieldValue {
   }
 
   contains(path: FieldPath): boolean {
-    return this.field(path) !== undefined;
+    return this.field(path) !== null;
   }
 
-  field(path: FieldPath): FieldValue | undefined {
+  field(path: FieldPath): FieldValue | null {
     assert(!path.isEmpty(), "Can't get field of empty path");
-    let field: FieldValue | undefined = this;
+    let field: FieldValue | null = this;
     path.forEach((pathSegment: string) => {
       if (field instanceof ObjectValue) {
-        field = field.internalValue.get(pathSegment) || undefined;
+        field = field.internalValue.get(pathSegment);
       } else {
-        field = undefined;
+        field = null;
       }
     });
     return field;

packages/firestore/src/model/mutation.ts

@@ -85,7 +85,7 @@ export class FieldMask {
         return data;
       } else {
         const newValue = data.field(fieldMaskPath);
-        if (newValue !== undefined) {
+        if (newValue !== null) {
           filteredObject = filteredObject.set(fieldMaskPath, newValue);
         }
       }
@@ -511,7 +511,7 @@ export class PatchMutation extends Mutation {
     this.fieldMask.fields.forEach(fieldPath => {
       if (!fieldPath.isEmpty()) {
         const newValue = this.data.field(fieldPath);
-        if (newValue !== undefined) {
+        if (newValue !== null) {
           data = data.set(fieldPath, newValue);
         } else {
           data = data.delete(fieldPath);
@@ -671,7 +671,7 @@ export class TransformMutation extends Mutation {
       const transform = fieldTransform.transform;
       let previousValue: FieldValue | null = null;
       if (baseDoc instanceof Document) {
-        previousValue = baseDoc.field(fieldTransform.field) || null;
+        previousValue = baseDoc.field(fieldTransform.field);
       }
       transformResults.push(
         transform.applyToRemoteDocument(
@@ -703,7 +703,7 @@ export class TransformMutation extends Mutation {
 
       let previousValue: FieldValue | null = null;
       if (baseDoc instanceof Document) {
-        previousValue = baseDoc.field(fieldTransform.field) || null;
+        previousValue = baseDoc.field(fieldTransform.field);
       }
 
       transformResults.push(

packages/firestore/test/unit/model/field_value.test.ts

@@ -193,9 +193,9 @@ describe('FieldValue', () => {
       fieldValue.StringValue
     );
 
-    expect(objValue.field(field('foo.a.b'))).to.equal(undefined);
-    expect(objValue.field(field('bar'))).to.equal(undefined);
-    expect(objValue.field(field('bar.a'))).to.equal(undefined);
+    expect(objValue.field(field('foo.a.b'))).to.be.null;
+    expect(objValue.field(field('bar'))).to.be.null;
+    expect(objValue.field(field('bar.a'))).to.be.null;
 
     expect(objValue.field(field('foo'))!.value()).to.deep.equal({
       a: 1,