Change timestampsInSnapshots default to true. (#1464)

Also update log messages instructing folks what to do if they're using the
soon-to-be-removed timestampsInSnapshots setting.

Author: mikelehen

packages/firebase/index.d.ts

@@ -774,22 +774,24 @@ declare namespace firebase.firestore {
     ssl?: boolean;
 
     /**
-     * Enables the use of `Timestamp`s for timestamp fields in
-     * `DocumentSnapshot`s.
-     *
-     * Currently, Firestore returns timestamp fields as `Date` but `Date` only
-     * supports millisecond precision, which leads to truncation and causes
-     * unexpected behavior when using a timestamp from a snapshot as a part
-     * of a subsequent query.
-     *
-     * Setting `timestampsInSnapshots` to true will cause Firestore to return
-     * `Timestamp` values instead of `Date` avoiding this kind of problem. To make
-     * this work you must also change any code that uses `Date` to use `Timestamp`
-     * instead.
-     *
-     * NOTE: in the future `timestampsInSnapshots: true` will become the
-     * default and this option will be removed so you should change your code to
-     * use Timestamp now and opt-in to this new behavior as soon as you can.
+     * Specifies whether to use `Timestamp` objects for timestamp fields in
+     * `DocumentSnapshot`s. This is enabled by default and should not be
+     * disabled.
+     *
+     * Previously, Firestore returned timestamp fields as `Date` but `Date`
+     * only supports millisecond precision, which leads to truncation and
+     * 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.
+     *
+     * @deprecated This setting will be removed in a future release. You should
+     * update your code to expect `Timestamp` objects and stop using the
+     * `timestampsInSnapshots` setting.
      */
     timestampsInSnapshots?: boolean;
 

packages/firestore-types/index.d.ts

@@ -44,22 +44,24 @@ export interface Settings {
   ssl?: boolean;
 
   /**
-   * Enables the use of `Timestamp`s for timestamp fields in
-   * `DocumentSnapshot`s.
+   * Specifies whether to use `Timestamp` objects for timestamp fields in
+   * `DocumentSnapshot`s. This is enabled by default and should not be
+   * disabled.
    *
-   * Currently, Firestore returns timestamp fields as `Date` but `Date` only
+   * Previously, Firestore returned timestamp fields as `Date` but `Date` only
    * supports millisecond precision, which leads to truncation and causes
-   * unexpected behavior when using a timestamp from a snapshot as a part
-   * of a subsequent query.
+   * unexpected behavior when using a timestamp from a snapshot as a part of a
+   * subsequent query.
    *
-   * Setting `timestampsInSnapshots` to true will cause Firestore to return
-   * `Timestamp` values instead of `Date` avoiding this kind of problem. To make
-   * this work you must also change any code that uses `Date` to use `Timestamp`
-   * instead.
+   * So now Firestore returns `Timestamp` values instead of `Date`, avoiding
+   * this kind of problem.
    *
-   * NOTE: in the future `timestampsInSnapshots: true` will become the
-   * default and this option will be removed so you should change your code to
-   * use Timestamp now and opt-in to this new behavior as soon as you can.
+   * To opt into the old behavior of returning `Date` objects, you can
+   * temporarily set `timestampsInSnapshots` to false.
+   *
+   * @deprecated This setting will be removed in a future release. You should
+   * update your code to expect `Timestamp` objects and stop using the
+   * `timestampsInSnapshots` setting.
    */
   timestampsInSnapshots?: boolean;
 

packages/firestore/CHANGELOG.md

@@ -1,4 +1,10 @@
 # Unreleased
+- [changed] The `timestampsInSnapshots` setting is now enabled by default
+  so timestamp fields read from a `DocumentSnapshot` will be returned as
+  `Timestamp` objects instead of `Date`. Any code expecting to receive a
+  `Date` object must be updated.
+
+# 0.9.2
 - [fixed] Fixed a regression introduced in 5.7.0 that caused apps using
   experimentalTabSynchronization to hit an exception for "Failed to obtain
   primary lease for action 'Collect garbage'".
@@ -48,7 +54,7 @@
 - [changed] Eliminated superfluous update events for locally cached documents
   that are known to lag behind the server version. Instead, we buffer these
   events until the client has caught up with the server.
-  
+
 # 0.7.2
 - [fixed] Fixed a regression that prevented use of Firestore on ReactNative's
   Expo platform (#1138).

packages/firestore/karma.conf.js

@@ -61,13 +61,11 @@ function getFirestoreSettings(argv) {
     return {
       host: 'localhost:8080',
       ssl: false,
-      timestampsInSnapshots: true
     };
   } else {
     return {
       host: 'firestore.googleapis.com',
       ssl: true,
-      timestampsInSnapshots: true
     };
   }
 }

packages/firestore/src/api/database.ts

@@ -106,7 +106,7 @@ import {
 // settings() defaults:
 const DEFAULT_HOST = 'firestore.googleapis.com';
 const DEFAULT_SSL = true;
-const DEFAULT_TIMESTAMPS_IN_SNAPSHOTS = false;
+const DEFAULT_TIMESTAMPS_IN_SNAPSHOTS = true;
 
 /**
  * Constant used to indicate the LRU garbage collection should be disabled.
@@ -192,6 +192,37 @@ class FirestoreSettings {
       'timestampsInSnapshots',
       settings.timestampsInSnapshots
     );
+
+    // Nobody should set timestampsInSnapshots anymore, but the error depends on
+    // whether they set it to true or false...
+    if (settings.timestampsInSnapshots === true) {
+      log.error(`
+  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.`);
+    } else if (settings.timestampsInSnapshots === false) {
+      log.error(`
+  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.`);
+    }
     this.timestampsInSnapshots = objUtils.defaulted(
       settings.timestampsInSnapshots,
       DEFAULT_TIMESTAMPS_IN_SNAPSHOTS
@@ -373,32 +404,6 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
       'FirestoreSettings.host cannot be falsey'
     );
 
-    if (!this._config.settings.timestampsInSnapshots) {
-      log.error(`
-The behavior for Date objects stored in Firestore is going to change
-AND YOUR APP MAY BREAK.
-To hide this warning and ensure your app does not break, you need to add the
-following code to your app before calling any other Cloud Firestore methods:
-
-  const firestore = firebase.firestore();
-  const settings = {/* your settings... */ timestampsInSnapshots: true};
-  firestore.settings(settings);
-
-With this change, 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. In a
-future release, the behavior will change to the new behavior, so if you do not
-follow these steps, YOUR APP MAY BREAK.`);
-    }
-
     assert(!this._firestoreClient, 'configureClient() called multiple times');
 
     const databaseInfo = new DatabaseInfo(

packages/firestore/src/model/field_value.ts

@@ -329,7 +329,7 @@ export class TimestampValue extends FieldValue {
   }
 
   value(options?: FieldValueOptions): Date | Timestamp {
-    if (options && options.timestampsInSnapshots) {
+    if (!options || options.timestampsInSnapshots) {
       return this.internalValue;
     } else {
       return this.internalValue.toDate();

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

@@ -351,7 +351,7 @@ apiDescribe('Timestamp Fields in snapshots', persistence => {
     return { timestamp: ts, nested: { timestamp2: ts } };
   };
 
-  it('are returned as native dates if timestampsInSnapshots is not set', () => {
+  it('are returned as native dates if timestampsInSnapshots set to false', () => {
     // Avoid the verbose log message triggered by timestampsInSnapshots ==
     // false.
     const logLevel = log.getLogLevel();
@@ -386,8 +386,6 @@ apiDescribe('Timestamp Fields in snapshots', persistence => {
   });
 
   it('are returned as Timestamps', () => {
-    expect(DEFAULT_SETTINGS['timestampsInSnapshots']).to.equal(true);
-
     const timestamp = new Timestamp(100, 123456000);
     // Timestamps are currently truncated to microseconds after being written to
     // the database, so a truncated version of the timestamp is needed for

packages/firestore/test/integration/util/helpers.ts

@@ -34,13 +34,11 @@ export const USE_EMULATOR = !!EMULATOR_PORT;
 const EMULATOR_FIRESTORE_SETTING = {
   host: `localhost:${EMULATOR_PORT}`,
   ssl: false,
-  timestampsInSnapshots: true
 };
 
 const PROD_FIRESTORE_SETTING = {
   host: 'firestore.googleapis.com',
   ssl: true,
-  timestampsInSnapshots: true
 };
 
 export const DEFAULT_SETTINGS = getDefaultSettings();

packages/firestore/test/unit/model/field_value.test.ts

@@ -118,8 +118,8 @@ describe('FieldValue', () => {
     expect(dateValue1).to.be.an.instanceof(fieldValue.TimestampValue);
     expect(dateValue2).to.be.an.instanceof(fieldValue.TimestampValue);
 
-    expect(dateValue1.value()).to.deep.equal(date1);
-    expect(dateValue2.value()).to.deep.equal(date2);
+    expect(dateValue1.value()).to.deep.equal(Timestamp.fromDate(date1));
+    expect(dateValue2.value()).to.deep.equal(Timestamp.fromDate(date2));
   });
 
   it('can parse geo points', () => {

packages/firestore/test/util/api_helpers.ts

@@ -47,7 +47,6 @@ export const FIRESTORE = new Firestore({
 });
 
 export function firestore(): Firestore {
-  FIRESTORE.settings({ timestampsInSnapshots: true });
   return FIRESTORE;
 }
 

packages/rxfire/test/firestore.test.ts

@@ -83,7 +83,6 @@ describe('RxFire Firestore', () => {
   beforeEach(() => {
     app = initializeApp({ projectId: TEST_PROJECT.projectId });
     firestore = app.firestore();
-    firestore.settings({ timestampsInSnapshots: true });
     firestore.disableNetwork();
   });
 

packages/testing/src/api/index.ts

@@ -132,7 +132,6 @@ function initializeApp(
     app.firestore().settings({
       host: FIRESTORE_ADDRESS,
       ssl: false,
-      timestampsInSnapshots: true
     });
   }
   /**