Throw when user data parsing fails

Author: schmidt-sebastian

packages/firestore/lite/src/api/reference.ts

@@ -39,6 +39,7 @@ import { hardAssert } from '../../../src/util/assert';
 import { DeleteMutation, Precondition } from '../../../src/model/mutation';
 import { PlatformSupport } from '../../../src/platform/platform';
 import { applyFirestoreDataConverter } from '../../../src/api/database';
+import { DatabaseId } from '../../../src/core/database_info';
 
 /**
  * A reference to a particular document in a collection in the database.
@@ -312,21 +313,27 @@ export function setDoc<T>(
   options?: firestore.SetOptions
 ): Promise<void> {
   const ref = tryCast(reference, DocumentReference);
-  return ref.firestore._ensureClientConfigured().then(firestore => {
-    const dataReader = newUserDataReader(firestore);
 
-    const [convertedValue] = applyFirestoreDataConverter(
-      ref._converter,
-      data,
-      'setDoc'
-    );
+  const [convertedValue] = applyFirestoreDataConverter(
+    ref._converter,
+    data,
+    'setDoc'
+  );
 
-    const parsed = isMerge(options)
-      ? dataReader.parseMergeData('setDoc', convertedValue)
-      : isMergeFields(options)
-      ? dataReader.parseMergeData('setDoc', convertedValue, options.mergeFields)
-      : dataReader.parseSetData('setDoc', convertedValue);
+  // Kick off configuring the client, which freezes the settings.
+  const configureClient = ref.firestore._ensureClientConfigured();
+  const dataReader = newUserDataReader(
+    ref.firestore._databaseId,
+    ref.firestore._settings!
+  );
 
+  const parsed = isMerge(options)
+    ? dataReader.parseMergeData('setDoc', convertedValue)
+    : isMergeFields(options)
+    ? dataReader.parseMergeData('setDoc', convertedValue, options.mergeFields)
+    : dataReader.parseSetData('setDoc', convertedValue);
+
+  return configureClient.then(firestore => {
     return invokeCommitRpc(
       firestore._datastore,
       parsed.toMutations(ref._key, Precondition.none())
@@ -363,13 +370,14 @@ function isMergeFields(
   );
 }
 
-function newUserDataReader(firestore: Required<Firestore>): UserDataReader {
-  const serializer = PlatformSupport.getPlatform().newSerializer(
-    firestore._databaseId
-  );
+function newUserDataReader(
+  databaseId: DatabaseId,
+  settings: firestore.Settings
+): UserDataReader {
+  const serializer = PlatformSupport.getPlatform().newSerializer(databaseId);
   return new UserDataReader(
-    firestore._databaseId,
-    !!firestore._settings.ignoreUndefinedProperties,
+    databaseId,
+    !!settings.ignoreUndefinedProperties,
     serializer
   );
 }

packages/firestore/lite/test/integration.test.ts

@@ -23,7 +23,12 @@ import {
   getFirestore,
   initializeFirestore
 } from '../src/api/database';
-import { withTestDb, withTestDoc, withTestDocAndInitialData } from './helpers';
+import {
+  withTestDb,
+  withTestDbSettings,
+  withTestDoc,
+  withTestDocAndInitialData
+} from './helpers';
 import {
   parent,
   collection,
@@ -35,6 +40,10 @@ import {
   setDoc
 } from '../src/api/reference';
 import { FieldPath } from '../src/api/field_path';
+import {
+  DEFAULT_PROJECT_ID,
+  DEFAULT_SETTINGS
+} from '../../test/integration/util/settings';
 
 describe('Firestore', () => {
   it('can provide setting', () => {
@@ -222,6 +231,29 @@ describe('setDoc()', () => {
       });
     });
   });
+
+  it("rejects 'undefined' by default", () => {
+    return withTestDoc(async docRef => {
+      expect(() => {
+        setDoc(docRef, { val: undefined });
+      }).to.throw(
+        'Function setDoc() called with invalid data. Unsupported field value: undefined (found in field val)'
+      );
+    });
+  });
+
+  it("ignores 'undefined' when enabled", () => {
+    return withTestDbSettings(
+      DEFAULT_PROJECT_ID,
+      { ...DEFAULT_SETTINGS, ignoreUndefinedProperties: true },
+      async db => {
+        const docRef = doc(collection(db, 'test-collection'));
+        await setDoc(docRef, { val: undefined });
+        const docSnap = await getDoc(docRef);
+        expect(docSnap.data()).to.deep.equal({});
+      }
+    );
+  });
 });
 
 describe('DocumentSnapshot', () => {