Remove error messages for deprecated APIs (#2857)

Author: schmidt-sebastian

packages/firebase/index.d.ts

@@ -7682,11 +7682,23 @@ declare namespace firebase.firestore {
      * causes unexpected behavior when using a timestamp from a snapshot as a
      * part of a subsequent query.
      *
-     * So now Firestore returns `Timestamp` values instead of `Date`, avoiding
-     * this kind of problem.
-     *
-     * To opt into the old behavior of returning `Date` objects, you can
-     * temporarily set `timestampsInSnapshots` to false.
+     * Now, Firestore returns `Timestamp` values for all timestamp values stored
+     * in Cloud Firestore instead of system `Date` objects, avoiding this kind
+     * of problem. Consequently, you must update your code to handle `Timestamp`
+     * objects instead of `Date` objects.
+     *
+     * If you want to **temporarily** opt into the old behavior of returning
+     * `Date` objects, you may **temporarily** set `timestampsInSnapshots` to
+     * false. Opting into this behavior will no longer be possible in the next
+     * major release of Firestore, after which code that expects Date objects
+     * **will break**.
+     *
+     * @example **Using Date objects in Firestore.**
+     * // With deprecated setting `timestampsInSnapshot: true`:
+     * const date : Date = snapshot.get('created_at');
+     * // With new default behavior:
+     * const timestamp : Timestamp = snapshot.get('created_at');
+     * const date : Date = timestamp.toDate();
      *
      * @deprecated This setting will be removed in a future release. You should
      * update your code to expect `Timestamp` objects and stop using the

packages/firestore/src/api/database.ts

@@ -194,32 +194,15 @@ class FirestoreSettings {
     // Nobody should set timestampsInSnapshots anymore, but the error depends on
     // whether they set it to true or false...
     if (settings.timestampsInSnapshots === true) {
-      logError(`
-  The timestampsInSnapshots setting now defaults to true and you no
-  longer need to explicitly set it. In a future release, the setting
-  will be removed entirely and so it is recommended that you remove it
-  from your firestore.settings() call now.`);
+      logError(
+        "The setting 'timestampsInSnapshots: true' is no longer required " +
+          'and should be removed.'
+      );
     } else if (settings.timestampsInSnapshots === false) {
-      logError(`
-  The timestampsInSnapshots setting will soon be removed. YOU MUST UPDATE
-  YOUR CODE.
-
-  To hide this warning, stop using the timestampsInSnapshots setting in your
-  firestore.settings({ ... }) call.
-
-  Once you remove the setting, Timestamps stored in Cloud Firestore will be
-  read back as Firebase Timestamp objects instead of as system Date objects.
-  So you will also need to update code expecting a Date to instead expect a
-  Timestamp. For example:
-
-  // Old:
-  const date = snapshot.get('created_at');
-  // New:
-  const timestamp = snapshot.get('created_at'); const date =
-  timestamp.toDate();
-
-  Please audit all existing usages of Date when you enable the new
-  behavior.`);
+      logError(
+        "Support for 'timestampsInSnapshots: false' will be removed soon. " +
+          'You must update your code to handle Timestamp objects.'
+      );
     }
     this.timestampsInSnapshots =
       settings.timestampsInSnapshots ?? DEFAULT_TIMESTAMPS_IN_SNAPSHOTS;
@@ -338,14 +321,6 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
     validateExactNumberOfArgs('Firestore.settings', arguments, 1);
     validateArgType('Firestore.settings', 'object', 1, settingsLiteral);
 
-    if (contains(settingsLiteral, 'persistence')) {
-      throw new FirestoreError(
-        Code.INVALID_ARGUMENT,
-        '"persistence" is now specified with a separate call to ' +
-          'firestore.enablePersistence().'
-      );
-    }
-
     const newSettings = new FirestoreSettings(settingsLiteral);
     if (this._firestoreClient && !this._settings.isEqual(newSettings)) {
       throw new FirestoreError(
@@ -387,10 +362,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
     if (settings) {
       if (settings.experimentalTabSynchronization !== undefined) {
         logError(
-          "The 'experimentalTabSynchronization' setting has been renamed to " +
-            "'synchronizeTabs'. In a future release, the setting will be removed " +
-            'and it is recommended that you update your ' +
-            "firestore.enablePersistence() call to use 'synchronizeTabs'."
+          "The 'experimentalTabSynchronization' setting will be removed. Use 'synchronizeTabs' instead."
         );
       }
       synchronizeTabs =
@@ -2305,42 +2277,6 @@ export class QuerySnapshot<T = firestore.DocumentData>
   }
 }
 
-// TODO(2018/11/01): As of 2018/04/17 we're changing docChanges from an array
-// into a method. Because this is a runtime breaking change and somewhat subtle
-// (both Array and Function have a .length, etc.), we'll replace commonly-used
-// properties (including Symbol.iterator) to throw a custom error message. In
-// ~6 months we can delete the custom error as most folks will have hopefully
-// migrated.
-function throwDocChangesMethodError(): never {
-  throw new FirestoreError(
-    Code.INVALID_ARGUMENT,
-    'QuerySnapshot.docChanges has been changed from a property into a ' +
-      'method, so usages like "querySnapshot.docChanges" should become ' +
-      '"querySnapshot.docChanges()"'
-  );
-}
-
-const docChangesPropertiesToOverride = [
-  'length',
-  'forEach',
-  'map',
-  ...(typeof Symbol !== 'undefined' ? [Symbol.iterator] : [])
-];
-docChangesPropertiesToOverride.forEach(property => {
-  /**
-   * We are (re-)defining properties on QuerySnapshot.prototype.docChanges which
-   * is a Function. This could fail, in particular in the case of 'length' which
-   * already exists on Function.prototype and on IE11 is improperly defined with
-   * `{ configurable: false }`. So we wrap this in a try/catch to ensure that we
-   * still have a functional SDK.
-   */
-  try {
-    Object.defineProperty(QuerySnapshot.prototype.docChanges, property, {
-      get: () => throwDocChangesMethodError()
-    });
-  } catch (err) {} // Ignore this failure intentionally
-});
-
 export class CollectionReference<T = firestore.DocumentData> extends Query<T>
   implements firestore.CollectionReference<T> {
   constructor(

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

@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2017 Google Inc.
+ * Copyright 2017 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -784,22 +784,6 @@ apiDescribe('Queries', (persistence: boolean) => {
     });
   });
 
-  it('throws custom error when using docChanges as property', () => {
-    return withTestCollection(persistence, {}, async coll => {
-      const snapshot = await coll.get();
-      const expectedError =
-        'QuerySnapshot.docChanges has been changed from a property into a method';
-      // We are testing invalid API usage.
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      const docChange = snapshot.docChanges as any;
-      expect(() => docChange.length).to.throw(expectedError);
-      expect(() => {
-        for (const _ of docChange) {
-        }
-      }).to.throw(expectedError);
-    });
-  });
-
   it('can query collection groups', async () => {
     await withTestDb(persistence, async db => {
       // Use .doc() to get a random collection group name to use but ensure it starts with 'b' for