Add ignoreUndefinedProperties (#3077)

Author: schmidt-sebastian

packages/firebase/index.d.ts

@@ -6920,15 +6920,15 @@ declare namespace firebase.database.ServerValue {
    * ```
    */
   var TIMESTAMP: Object;
-  
+
   /**
-   * Returns a placeholder value that can be used to atomically increment the 
+   * Returns a placeholder value that can be used to atomically increment the
    * current database value by the provided delta.
    *
    * @param delta the amount to modify the current value atomically.
    * @return a placeholder value for modifying data atomically server-side.
    */
-  function increment(delta: number) : Object;
+  function increment(delta: number): Object;
 }
 
 /**
@@ -7743,6 +7743,14 @@ declare namespace firebase.firestore {
      * @webonly
      */
     experimentalForceLongPolling?: boolean;
+
+    /**
+     * Whether to skip nested properties that are set to `undefined` during
+     * object serialization. If set to `true`, these properties are skipped
+     * and not written to Firestore. If set `false` or omitted, the SDK throws
+     * an exception when it encounters properties of type `undefined`.
+     */
+    ignoreUndefinedProperties?: boolean;
   }
 
   /**

packages/firestore-types/index.d.ts

@@ -29,6 +29,7 @@ export interface Settings {
   timestampsInSnapshots?: boolean;
   cacheSizeBytes?: number;
   experimentalForceLongPolling?: boolean;
+  ignoreUndefinedProperties?: boolean;
 }
 
 export interface PersistenceSettings {

packages/firestore/CHANGELOG.md

@@ -1,4 +1,9 @@
 # Unreleased
+- [feature] Added support for calling  `FirebaseFiresore.settings` with 
+  `{ ignoreUndefinedProperties: true }`. When set, Firestore ignores 
+  undefined properties inside objects rather than rejecting the API call.
+
+# Released
 - [fixed] Fixed a regression introduced in v7.14.2 that incorrectly applied
   a `FieldValue.increment` in combination with `set({...}, {merge: true})`.
 - [fixed] Firestore now rejects `onSnapshot()` listeners if they cannot be

packages/firestore/src/api/database.ts

@@ -98,6 +98,7 @@ const DEFAULT_HOST = 'firestore.googleapis.com';
 const DEFAULT_SSL = true;
 const DEFAULT_TIMESTAMPS_IN_SNAPSHOTS = true;
 const DEFAULT_FORCE_LONG_POLLING = false;
+const DEFAULT_IGNORE_UNDEFINED_PROPERTIES = false;
 
 /**
  * Constant used to indicate the LRU garbage collection should be disabled.
@@ -142,6 +143,8 @@ class FirestoreSettings {
 
   readonly forceLongPolling: boolean;
 
+  readonly ignoreUndefinedProperties: boolean;
+
   // Can be a google-auth-library or gapi client.
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   credentials?: any;
@@ -169,7 +172,8 @@ class FirestoreSettings {
       'credentials',
       'timestampsInSnapshots',
       'cacheSizeBytes',
-      'experimentalForceLongPolling'
+      'experimentalForceLongPolling',
+      'ignoreUndefinedProperties'
     ]);
 
     validateNamedOptionalType(
@@ -187,6 +191,13 @@ class FirestoreSettings {
       settings.timestampsInSnapshots
     );
 
+    validateNamedOptionalType(
+      'settings',
+      'boolean',
+      'ignoreUndefinedProperties',
+      settings.ignoreUndefinedProperties
+    );
+
     // Nobody should set timestampsInSnapshots anymore, but the error depends on
     // whether they set it to true or false...
     if (settings.timestampsInSnapshots === true) {
@@ -202,6 +213,8 @@ class FirestoreSettings {
     }
     this.timestampsInSnapshots =
       settings.timestampsInSnapshots ?? DEFAULT_TIMESTAMPS_IN_SNAPSHOTS;
+    this.ignoreUndefinedProperties =
+      settings.ignoreUndefinedProperties ?? DEFAULT_IGNORE_UNDEFINED_PROPERTIES;
 
     validateNamedOptionalType(
       'settings',
@@ -232,9 +245,7 @@ class FirestoreSettings {
       settings.experimentalForceLongPolling
     );
     this.forceLongPolling =
-      settings.experimentalForceLongPolling === undefined
-        ? DEFAULT_FORCE_LONG_POLLING
-        : settings.experimentalForceLongPolling;
+      settings.experimentalForceLongPolling ?? DEFAULT_FORCE_LONG_POLLING;
   }
 
   isEqual(other: FirestoreSettings): boolean {
@@ -244,7 +255,8 @@ class FirestoreSettings {
       this.timestampsInSnapshots === other.timestampsInSnapshots &&
       this.credentials === other.credentials &&
       this.cacheSizeBytes === other.cacheSizeBytes &&
-      this.forceLongPolling === other.forceLongPolling
+      this.forceLongPolling === other.forceLongPolling &&
+      this.ignoreUndefinedProperties === other.ignoreUndefinedProperties
     );
   }
 }
@@ -275,7 +287,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
   // TODO(mikelehen): Use modularized initialization instead.
   readonly _queue = new AsyncQueue();
 
-  readonly _dataReader: UserDataReader;
+  _userDataReader: UserDataReader | undefined;
 
   // Note: We are using `MemoryComponentProvider` as a default
   // ComponentProvider to ensure backwards compatibility with the format
@@ -310,7 +322,21 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
 
     this._componentProvider = componentProvider;
     this._settings = new FirestoreSettings({});
-    this._dataReader = new UserDataReader(this._databaseId);
+  }
+
+  get _dataReader(): UserDataReader {
+    debugAssert(
+      !!this._firestoreClient,
+      'Cannot obtain UserDataReader before instance is intitialized'
+    );
+    if (!this._userDataReader) {
+      // Lazy initialize UserDataReader once the settings are frozen
+      this._userDataReader = new UserDataReader(
+        this._databaseId,
+        this._settings.ignoreUndefinedProperties
+      );
+    }
+    return this._userDataReader;
   }
 
   settings(settingsLiteral: firestore.Settings): void {

packages/firestore/src/api/field_value.ts

@@ -109,7 +109,8 @@ export class ArrayUnionFieldValueImpl extends FieldValueImpl {
         arrayElement: true
       },
       context.databaseId,
-      context.serializer
+      context.serializer,
+      context.ignoreUndefinedProperties
     );
     const parsedElements = this._elements.map(
       element => parseData(element, parseContext)!
@@ -140,7 +141,8 @@ export class ArrayRemoveFieldValueImpl extends FieldValueImpl {
         arrayElement: true
       },
       context.databaseId,
-      context.serializer
+      context.serializer,
+      context.ignoreUndefinedProperties
     );
     const parsedElements = this._elements.map(
       element => parseData(element, parseContext)!
@@ -167,7 +169,8 @@ export class NumericIncrementFieldValueImpl extends FieldValueImpl {
         methodName: this._methodName
       },
       context.databaseId,
-      context.serializer
+      context.serializer,
+      context.ignoreUndefinedProperties
     );
     const operand = parseData(this._operand, parseContext)!;
     const numericIncrement = new NumericIncrementTransformOperation(

packages/firestore/src/api/user_data_reader.ts

@@ -158,6 +158,8 @@ export class ParseContext {
    * @param settings The settings for the parser.
    * @param databaseId The database ID of the Firestore instance.
    * @param serializer The serializer to use to generate the Value proto.
+   * @param ignoreUndefinedProperties Whether to ignore undefined properties
+   * rather than throw.
    * @param fieldTransforms A mutable list of field transforms encountered while
    *     parsing the data.
    * @param fieldMask A mutable list of field paths encountered while parsing
@@ -172,6 +174,7 @@ export class ParseContext {
     readonly settings: ContextSettings,
     readonly databaseId: DatabaseId,
     readonly serializer: JsonProtoSerializer,
+    readonly ignoreUndefinedProperties: boolean,
     fieldTransforms?: FieldTransform[],
     fieldMask?: FieldPath[]
   ) {
@@ -198,6 +201,7 @@ export class ParseContext {
       { ...this.settings, ...configuration },
       this.databaseId,
       this.serializer,
+      this.ignoreUndefinedProperties,
       this.fieldTransforms,
       this.fieldMask
     );
@@ -276,6 +280,7 @@ export class UserDataReader {
 
   constructor(
     private readonly databaseId: DatabaseId,
+    private readonly ignoreUndefinedProperties: boolean,
     serializer?: JsonProtoSerializer
   ) {
     this.serializer =
@@ -458,7 +463,8 @@ export class UserDataReader {
         arrayElement: false
       },
       this.databaseId,
-      this.serializer
+      this.serializer,
+      this.ignoreUndefinedProperties
     );
   }
 
@@ -613,7 +619,10 @@ function parseSentinelFieldValue(
  *
  * @return The parsed value
  */
-function parseScalarValue(value: unknown, context: ParseContext): api.Value {
+function parseScalarValue(
+  value: unknown,
+  context: ParseContext
+): api.Value | null {
   if (value === null) {
     return { nullValue: 'NULL_VALUE' };
   } else if (typeof value === 'number') {
@@ -659,6 +668,8 @@ function parseScalarValue(value: unknown, context: ParseContext): api.Value {
         value.firestore._databaseId
       )
     };
+  } else if (value === undefined && context.ignoreUndefinedProperties) {
+    return null;
   } else {
     throw context.createError(
       `Unsupported field value: ${valueDescription(value)}`

packages/firestore/src/util/input_validation.ts

@@ -461,7 +461,7 @@ export function validatePositiveNumber(
   if (n <= 0) {
     throw new FirestoreError(
       Code.INVALID_ARGUMENT,
-      `Function "${functionName}()" requires its ${ordinal(
+      `Function ${functionName}() requires its ${ordinal(
         position
       )} argument to be a positive number, but it was: ${n}.`
     );

packages/firestore/test/integration/api/fields.test.ts

@@ -23,7 +23,8 @@ import {
   toDataArray,
   withTestCollection,
   withTestCollectionSettings,
-  withTestDoc
+  withTestDoc,
+  withTestDocAndSettings
 } from '../util/helpers';
 
 const FieldPath = firebase.firestore!.FieldPath;
@@ -433,3 +434,60 @@ apiDescribe('Timestamp Fields in snapshots', (persistence: boolean) => {
     });
   });
 });
+
+apiDescribe('`undefined` properties', (persistence: boolean) => {
+  const settings = { ...DEFAULT_SETTINGS };
+  settings.ignoreUndefinedProperties = true;
+
+  it('are ignored in set()', () => {
+    return withTestDocAndSettings(persistence, settings, async doc => {
+      await doc.set({ foo: 'foo', 'bar': undefined });
+      const docSnap = await doc.get();
+      expect(docSnap.data()).to.deep.equal({ foo: 'foo' });
+    });
+  });
+
+  it('are ignored in update()', () => {
+    return withTestDocAndSettings(persistence, settings, async doc => {
+      await doc.set({});
+      await doc.update({ a: { foo: 'foo', 'bar': undefined } });
+      await doc.update('b', { foo: 'foo', 'bar': undefined });
+      const docSnap = await doc.get();
+      expect(docSnap.data()).to.deep.equal({
+        a: { foo: 'foo' },
+        b: { foo: 'foo' }
+      });
+    });
+  });
+
+  it('are ignored in Query.where()', () => {
+    return withTestCollectionSettings(
+      persistence,
+      settings,
+      { 'doc1': { nested: { foo: 'foo' } } },
+      async coll => {
+        const query = coll.where('nested', '==', {
+          foo: 'foo',
+          'bar': undefined
+        });
+        const querySnap = await query.get();
+        expect(querySnap.size).to.equal(1);
+      }
+    );
+  });
+
+  it('are ignored in Query.startAt()', () => {
+    return withTestCollectionSettings(
+      persistence,
+      settings,
+      { 'doc1': { nested: { foo: 'foo' } } },
+      async coll => {
+        const query = coll
+          .orderBy('nested')
+          .startAt({ foo: 'foo', 'bar': undefined });
+        const querySnap = await query.get();
+        expect(querySnap.size).to.equal(1);
+      }
+    );
+  });
+});