fix: support Objects created with Object.create({}) (#842)

dev/src/document.ts

@@ -23,10 +23,10 @@ import {google} from '../protos/firestore_v1_proto_api';
 import {FieldTransform} from './field-value';
 import {FieldPath, validateFieldPath} from './path';
 import {DocumentReference} from './reference';
-import {isPlainObject, Serializer} from './serializer';
+import {Serializer} from './serializer';
 import {Timestamp} from './timestamp';
 import {ApiMapValue, DocumentData, UpdateMap} from './types';
-import {isEmpty, isObject} from './util';
+import {isEmpty, isObject, isPlainObject} from './util';
 
 import api = google.firestore.v1;
 

dev/src/serializer.ts

@@ -24,7 +24,7 @@ import {DocumentReference, Firestore} from './index';
 import {FieldPath, QualifiedResourcePath} from './path';
 import {Timestamp} from './timestamp';
 import {ApiMapValue, DocumentData, ValidationOptions} from './types';
-import {isEmpty, isObject} from './util';
+import {isEmpty, isObject, isPlainObject} from './util';
 import {customObjectMessage, invalidArgumentMessage} from './validate';
 
 import api = proto.google.firestore.v1;
@@ -256,22 +256,6 @@ export class Serializer {
   }
 }
 
-/**
- * Verifies that 'obj' is a plain JavaScript object that can be encoded as a
- * 'Map' in Firestore.
- *
- * @private
- * @param input The argument to verify.
- * @returns 'true' if the input can be a treated as a plain object.
- */
-export function isPlainObject(input: unknown): input is DocumentData {
-  return (
-    isObject(input) &&
-    (Object.getPrototypeOf(input) === Object.prototype ||
-      Object.getPrototypeOf(input) === null)
-  );
-}
-
 /**
  * Validates a JavaScript value for usage as a Firestore value.
  *

dev/src/transaction.ts

@@ -25,15 +25,14 @@ import {
   QuerySnapshot,
   validateDocumentReference,
 } from './reference';
-import {isPlainObject} from './serializer';
 import {
   DocumentData,
   Precondition as PublicPrecondition,
   ReadOptions,
   SetOptions,
   UpdateData,
 } from './types';
-import {isObject, requestTag} from './util';
+import {isObject, isPlainObject} from './util';
 import {
   invalidArgumentMessage,
   RequiredArgumentOptions,

dev/src/util.ts

@@ -16,6 +16,8 @@
 
 import {GoogleError, ServiceConfig, Status} from 'google-gax';
 
+import {DocumentData} from './types';
+
 /**
  * A Promise implementation that supports deferred resolution.
  * @private
@@ -77,6 +79,23 @@ export function isObject(value: unknown): value is {[k: string]: unknown} {
   return Object.prototype.toString.call(value) === '[object Object]';
 }
 
+/**
+ * Verifies that 'obj' is a plain JavaScript object that can be encoded as a
+ * 'Map' in Firestore.
+ *
+ * @private
+ * @param input The argument to verify.
+ * @returns 'true' if the input can be a treated as a plain object.
+ */
+export function isPlainObject(input: unknown): input is DocumentData {
+  return (
+    isObject(input) &&
+    (Object.getPrototypeOf(input) === Object.prototype ||
+      Object.getPrototypeOf(input) === null ||
+      input.constructor.name === 'Object')
+  );
+}
+
 /**
  * Returns whether `value` has no custom properties.
  *

dev/src/validate.ts

@@ -37,22 +37,6 @@ export interface NumericRangeOptions {
   maxValue?: number;
 }
 
-/**
- * Returns the name of the base class (ignoring Object).
- *
- * @private
- * @param value The object whose base class name to extract.
- * @returns The name of the base class constructor or "Object" if value does not
- * extend a custom class.
- */
-function extractBaseClassName(value: object): string {
-  let constructorName = 'Object';
-  while (Object.getPrototypeOf(value) !== Object.prototype) {
-    value = Object.getPrototypeOf(value);
-    constructorName = value.constructor.name;
-  }
-  return constructorName;
-}
 /**
  * Generates an error message to use with custom objects that cannot be
  * serialized.
@@ -73,7 +57,7 @@ export function customObjectMessage(
     // We use the base class name as the type name as the sentinel classes
     // returned by the public FieldValue API are subclasses of FieldValue. By
     // using the base name, we reduce the number of special cases below.
-    const typeName = extractBaseClassName(value);
+    const typeName = value.constructor.name;
     switch (typeName) {
       case 'DocumentReference':
       case 'FieldPath':

dev/src/write-batch.ts

@@ -28,7 +28,7 @@ import {Firestore} from './index';
 import {logger} from './logger';
 import {FieldPath, validateFieldPath} from './path';
 import {DocumentReference, validateDocumentReference} from './reference';
-import {isPlainObject, Serializer, validateUserInput} from './serializer';
+import {Serializer, validateUserInput} from './serializer';
 import {Timestamp} from './timestamp';
 import {
   Precondition as PublicPrecondition,
@@ -37,7 +37,7 @@ import {
   UpdateMap,
 } from './types';
 import {DocumentData} from './types';
-import {isObject, requestTag} from './util';
+import {isObject, isPlainObject, requestTag} from './util';
 import {
   customObjectMessage,
   invalidArgumentMessage,

dev/test/document.ts

@@ -166,18 +166,24 @@ describe('serialize document', () => {
     }).to.throw(
       'Value for argument "data" is not a valid Firestore document. Couldn\'t serialize object of type "Foo". Firestore doesn\'t support JavaScript objects with custom prototypes (i.e. objects that were created via the "new" operator).'
     );
+
+    expect(() => {
+      class Foo {}
+      class Bar extends Foo {}
+      firestore
+        .doc('collectionId/documentId')
+        .set(new Bar() as InvalidApiUsage);
+    }).to.throw(
+      'Value for argument "data" is not a valid Firestore document. Couldn\'t serialize object of type "Bar". Firestore doesn\'t support JavaScript objects with custom prototypes (i.e. objects that were created via the "new" operator).'
+    );
   });
 
   it('provides custom error for objects from different Firestore instance', () => {
     class FieldPath {}
-    class CustomFieldPath extends FieldPath {}
-    class VeryCustomFieldPath extends CustomFieldPath {}
+    class GeoPoint {}
+    class Timestamp {}
 
-    const customClasses = [
-      new FieldPath(),
-      new CustomFieldPath(),
-      new VeryCustomFieldPath(),
-    ];
+    const customClasses = [new FieldPath(), new GeoPoint(), new Timestamp()];
 
     for (const customClass of customClasses) {
       expect(() => {
@@ -186,7 +192,7 @@ describe('serialize document', () => {
           .set(customClass as InvalidApiUsage);
       }).to.throw(
         'Value for argument "data" is not a valid Firestore document. ' +
-          'Detected an object of type "FieldPath" that doesn\'t match the expected instance.'
+          `Detected an object of type "${customClass.constructor.name}" that doesn't match the expected instance.`
       );
     }
   });

dev/test/util.ts

@@ -0,0 +1,35 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import {expect} from 'chai';
+import {isPlainObject} from '../src/util';
+
+describe('isPlainObject()', () => {
+  it('allows Object.create()', () => {
+    expect(isPlainObject(Object.create({}))).to.be.true;
+    expect(isPlainObject(Object.create(Object.prototype))).to.be.true;
+    expect(isPlainObject(Object.create(null))).to.be.true;
+  });
+
+  it(' allows plain types', () => {
+    expect(isPlainObject({foo: 'bar'})).to.be.true;
+    expect(isPlainObject({})).to.be.true;
+  });
+
+  it('rejects custom types', () => {
+    class Foo {}
+    expect(isPlainObject(new Foo())).to.be.false;
+    expect(isPlainObject(Object.create(new Foo()))).to.be.false;
+  });
+});