feat(firestore)!: add support for ignoreUndefinedProperties

BREAKING CHANGE: undefined values throw like firebase-js-sdk now. Use ignoreUndefinedProperties setting 'true' to behave as before
hmhm2292 · May 16, 2021 · 756cfa6 · 756cfa6
1 parent 01c94cc
commit 756cfa6
Show file tree

Hide file tree

Showing 8 changed files with 141 additions and 14 deletions.
diff --git a/packages/firestore/e2e/firestore.e2e.js b/packages/firestore/e2e/firestore.e2e.js
@@ -120,6 +120,84 @@ describe('firestore()', function () {
       should.equal(docRef.constructor.name, 'FirestoreDocumentReference');
       docRef.path.should.eql(`${COLLECTION}/bar`);
     });
+
+    it('filters out undefined properties when setting enabled', async function () {
+      await firebase.firestore().settings({ ignoreUndefinedProperties: true });
+
+      const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
+      await docRef.set({
+        field1: 1,
+        field2: undefined,
+      });
+
+      const snap = await docRef.get();
+      const snapData = snap.data();
+      if (!snapData) {
+        return Promise.reject(new Error('Snapshot not saved'));
+      }
+
+      snapData.field1.should.eql(1);
+      snapData.hasOwnProperty('field2').should.eql(false);
+    });
+
+    it('filters out nested undefined properties when setting enabled', async function () {
+      await firebase.firestore().settings({ ignoreUndefinedProperties: true });
+
+      const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
+      await docRef.set({
+        field1: 1,
+        field2: {
+          shouldBeMissing: undefined,
+        },
+      });
+
+      const snap = await docRef.get();
+      const snapData = snap.data();
+      if (!snapData) {
+        return Promise.reject(new Error('Snapshot not saved'));
+      }
+
+      snapData.field1.should.eql(1);
+      snapData.hasOwnProperty('field2').should.eql(true);
+
+      snapData.field2.hasOwnProperty('shouldBeMissing').should.eql(false);
+    });
+
+    it('throws when undefined value provided and ignored undefined is false', async function () {
+      await firebase.firestore().settings({ ignoreUndefinedProperties: false });
+
+      const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
+
+      try {
+        await docRef.set({
+          field1: 1,
+          field2: undefined,
+        });
+
+        return Promise.reject(new Error('Expected set() to throw'));
+      } catch (e) {
+        return Promise.resolve();
+      }
+    });
+
+    it('throws when nested undefined value provided and ignored undefined is false', async function () {
+      await firebase.firestore().settings({ ignoreUndefinedProperties: false });
+
+      const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
+
+      try {
+        await docRef.set({
+          field1: 1,
+          field2: {
+            shouldNotWork: undefined,
+          },
+        });
+
+        return Promise.reject(new Error('Expected set() to throw'));
+      } catch (e) {
+        return Promise.resolve();
+      }
+    });
   });
 
   describe('collectionGroup()', function () {
@@ -322,6 +400,18 @@ describe('firestore()', function () {
       }
     });
 
+    it('throws if ignoreUndefinedProperties is not a boolean', function () {
+      try {
+        firebase.firestore().settings({ ignoreUndefinedProperties: 'true' });
+        return Promise.reject(new Error('Did not throw an Error.'));
+      } catch (error) {
+        error.message.should.containEql(
+          "'settings.ignoreUndefinedProperties' must be a boolean value",
+        );
+        return Promise.resolve();
+      }
+    });
+
     it('throws if ssl is not a boolean', function () {
       try {
         firebase.firestore().settings({ ssl: 'true' });

diff --git a/packages/firestore/lib/FirestoreDocumentReference.js b/packages/firestore/lib/FirestoreDocumentReference.js
@@ -185,7 +185,11 @@ export default class FirestoreDocumentReference {
       throw new Error(`firebase.firestore().doc().set(_, *) ${e.message}.`);
     }
 
-    return this._firestore.native.documentSet(this.path, buildNativeMap(data), setOptions);
+    return this._firestore.native.documentSet(
+      this.path,
+      buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
+      setOptions,
+    );
   }
 
   update(...args) {
@@ -202,7 +206,10 @@ export default class FirestoreDocumentReference {
       throw new Error(`firebase.firestore().doc().update(*) ${e.message}`);
     }
 
-    return this._firestore.native.documentUpdate(this.path, buildNativeMap(data));
+    return this._firestore.native.documentUpdate(
+      this.path,
+      buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
+    );
   }
 }
 

diff --git a/packages/firestore/lib/FirestoreQueryModifiers.js b/packages/firestore/lib/FirestoreQueryModifiers.js
@@ -204,7 +204,7 @@ export default class FirestoreQueryModifiers {
     const filter = {
       fieldPath,
       operator: OPERATORS[opStr],
-      value: generateNativeData(value),
+      value: generateNativeData(value, true),
     };
 
     this._filters = this._filters.concat(filter);

diff --git a/packages/firestore/lib/FirestoreTransaction.js b/packages/firestore/lib/FirestoreTransaction.js
@@ -85,7 +85,7 @@ export default class FirestoreTransaction {
     this._commandBuffer.push({
       type: 'SET',
       path: documentRef.path,
-      data: buildNativeMap(data),
+      data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
       options: setOptions,
     });
 
@@ -111,7 +111,7 @@ export default class FirestoreTransaction {
     this._commandBuffer.push({
       type: 'UPDATE',
       path: documentRef.path,
-      data: buildNativeMap(data),
+      data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
     });
 
     return this;

diff --git a/packages/firestore/lib/FirestoreWriteBatch.js b/packages/firestore/lib/FirestoreWriteBatch.js
@@ -94,7 +94,7 @@ export default class FirestoreWriteBatch {
     this._writes.push({
       path: documentRef.path,
       type: 'SET',
-      data: buildNativeMap(data),
+      data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
       options: setOptions,
     });
 
@@ -131,7 +131,7 @@ export default class FirestoreWriteBatch {
     this._writes.push({
       path: documentRef.path,
       type: 'UPDATE',
-      data: buildNativeMap(data),
+      data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
     });
 
     return this;

diff --git a/packages/firestore/lib/index.d.ts b/packages/firestore/lib/index.d.ts
@@ -1419,6 +1419,11 @@ export namespace FirebaseFirestoreTypes {
      * Whether to use SSL when connecting.
      */
     ssl?: boolean;
+
+    /**
+     * When this parameter is set, Cloud Firestore ignores undefined properties inside objects.
+     */
+    ignoreUndefinedProperties?: boolean;
   }
 
   /**

diff --git a/packages/firestore/lib/index.js b/packages/firestore/lib/index.js
@@ -74,6 +74,10 @@ class FirebaseFirestoreModule extends FirebaseModule {
         event,
       );
     });
+
+    this._settings = {
+      ignoreUndefinedProperties: false,
+    };
   }
 
   batch() {
@@ -185,7 +189,7 @@ class FirebaseFirestoreModule extends FirebaseModule {
 
     const keys = Object.keys(settings);
 
-    const opts = ['cacheSizeBytes', 'host', 'persistence', 'ssl'];
+    const opts = ['cacheSizeBytes', 'host', 'persistence', 'ssl', 'ignoreUndefinedProperties'];
 
     for (let i = 0; i < keys.length; i++) {
       const key = keys[i];
@@ -252,6 +256,18 @@ class FirebaseFirestoreModule extends FirebaseModule {
       throw new Error("firebase.firestore().settings(*) 'settings.ssl' must be a boolean value.");
     }
 
+    if (!isUndefined(settings.ignoreUndefinedProperties)) {
+      if (!isBoolean(settings.ignoreUndefinedProperties)) {
+        throw new Error(
+          "firebase.firestore().settings(*) 'settings.ignoreUndefinedProperties' must be a boolean value.",
+        );
+      } else {
+        this._settings.ignoreUndefinedProperties = settings.ignoreUndefinedProperties;
+      }
+
+      delete settings.ignoreUndefinedProperties;
+    }
+
     return this.native.settings(settings);
   }
 }

diff --git a/packages/firestore/lib/utils/serialize.js b/packages/firestore/lib/utils/serialize.js
@@ -47,16 +47,24 @@ export function provideFieldValueClass(fieldValue) {
 /**
  *
  * @param data
+ * @param ignoreUndefined
  */
-export function buildNativeMap(data) {
+export function buildNativeMap(data, ignoreUndefined) {
   const nativeData = {};
   if (data) {
     const keys = Object.keys(data);
     for (let i = 0; i < keys.length; i++) {
       const key = keys[i];
-      const typeMap = generateNativeData(data[key]);
-      if (typeMap) {
-        nativeData[key] = typeMap;
+
+      if (typeof data[key] === 'undefined') {
+        if (!ignoreUndefined) {
+          throw new Error('firebase.firestore() undefined values cannot be saved');
+        }
+      } else {
+        const typeMap = generateNativeData(data[key], ignoreUndefined);
+        if (typeMap) {
+          nativeData[key] = typeMap;
+        }
       }
     }
   }
@@ -90,9 +98,10 @@ export function buildNativeArray(array) {
  * Example: [7, 'some string'];
  *
  * @param value
+ * @param ignoreUndefined
  * @returns {*}
  */
-export function generateNativeData(value) {
+export function generateNativeData(value, ignoreUndefined) {
   if (Number.isNaN(value)) {
     return getTypeMapInt('nan');
   }
@@ -165,7 +174,7 @@ export function generateNativeData(value) {
       return getTypeMapInt('fieldvalue', [value._type, value._elements]);
     }
 
-    return getTypeMapInt('object', buildNativeMap(value));
+    return getTypeMapInt('object', buildNativeMap(value, ignoreUndefined));
   }
 
   // eslint-disable-next-line no-console