Skip to content

Commit

Permalink
feat!: remove deprecated timestampInSnapshots setting (#808)
Browse files Browse the repository at this point in the history
  • Loading branch information
@schmidt-sebastian
schmidt-sebastian committed Dec 9, 2019
1 parent 1f4eb0a commit f37fffc
Show file tree
Hide file tree
Showing 7 changed files with 6 additions and 156 deletions.
64 changes: 1 addition & 63 deletions dev/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ export class Firestore {
* The configuration options for the GAPIC client.
* @private
*/
_settings: Settings = {};
private _settings: Settings = {};

/**
* Whether the initialization settings can still be changed by invoking
Expand Down Expand Up @@ -317,20 +317,6 @@ export class Firestore {
* can specify a `keyFilename` instead.
* @param {string=} settings.host The host to connect to.
* @param {boolean=} settings.ssl Whether to use SSL when connecting.
* @param {boolean=} settings.timestampsInSnapshots Specifies whether to use
* `Timestamp` objects for timestamp fields in `DocumentSnapshot`s. This is
* enabled by default and should not be disabled.
* <br/>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.
* <br/>So now Firestore returns `Timestamp` values instead of `Date`,
* avoiding this kind of problem.
* <br/>To opt into the old behavior of returning `Date` objects, you can
* temporarily set `timestampsInSnapshots` to false.
* <br/>WARNING: This setting will be removed in a future release. You should
* update your code to expect `Timestamp` objects and stop using the
* `timestampsInSnapshots` setting.
*/
constructor(settings?: Settings) {
const libraryHeader = {
Expand Down Expand Up @@ -420,12 +406,6 @@ export class Firestore {
settings(settings: Settings): void {
validateObject('settings', settings);
validateString('settings.projectId', settings.projectId, {optional: true});
validateBoolean(
'settings.timestampsInSnapshots',
// tslint:disable-next-line deprecation
settings.timestampsInSnapshots,
{optional: true}
);

if (this._settingsFrozen) {
throw new Error(
Expand All @@ -441,13 +421,6 @@ export class Firestore {
}

private validateAndApplySettings(settings: Settings): void {
validateBoolean(
'settings.timestampsInSnapshots',
// tslint:disable-next-line deprecation
settings.timestampsInSnapshots,
{optional: true}
);

if (settings.projectId !== undefined) {
validateString('settings.projectId', settings.projectId);
this._projectId = settings.projectId;
Expand Down Expand Up @@ -1043,41 +1016,6 @@ export class Firestore {
* @return A Promise that resolves when the client is initialized.
*/
async initializeIfNeeded(requestTag: string): Promise<void> {
if (!this._settingsFrozen) {
// Nobody should set timestampsInSnapshots anymore, but the error depends
// on whether they set it to true or false...
// tslint:disable-next-line deprecation
if (this._settings.timestampsInSnapshots === true) {
console.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.`);
// tslint:disable-next-line deprecation
} else if (this._settings.timestampsInSnapshots === false) {
console.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._settingsFrozen = true;

if (this._projectId === undefined) {
Expand Down
10 changes: 1 addition & 9 deletions dev/src/serializer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,13 @@ export interface Serializable {
* @private
*/
export class Serializer {
private timestampsInSnapshots: boolean;
private createReference: (path: string) => DocumentReference;

constructor(firestore: Firestore) {
// Instead of storing the `firestore` object, we store just a reference to
// its `.doc()` method. This avoid a circular reference, which breaks
// JSON.stringify().
this.createReference = path => firestore.doc(path);
// tslint:disable-next-line deprecation
if (firestore._settings.timestampsInSnapshots === undefined) {
this.timestampsInSnapshots = true;
} else {
// tslint:disable-next-line deprecation
this.timestampsInSnapshots = firestore._settings.timestampsInSnapshots;
}
}

/**
Expand Down Expand Up @@ -218,7 +210,7 @@ export class Serializer {
}
case 'timestampValue': {
const timestamp = Timestamp.fromProto(proto.timestampValue!);
return this.timestampsInSnapshots ? timestamp : timestamp.toDate();
return timestamp;
}
case 'referenceValue': {
const resourcePath = QualifiedResourcePath.fromSlashSeparatedString(
Expand Down
21 changes: 0 additions & 21 deletions dev/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,27 +69,6 @@ export interface Settings {
*/
credentials?: {client_email?: string; private_key?: string};

/**
* 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;

/** Whether to use SSL when connecting. */
ssl?: boolean;

Expand Down
23 changes: 4 additions & 19 deletions dev/test/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,10 @@ describe('instantiation', () => {
const firestore = new Firestore.Firestore(DEFAULT_SETTINGS);
firestore.settings({foo: 'bar'});

expect(firestore._settings.projectId).to.equal(PROJECT_ID);
expect(firestore._settings.foo).to.equal('bar');
/* tslint:disable:no-any */
expect((firestore as any)._settings.projectId).to.equal(PROJECT_ID);
expect((firestore as any)._settings.foo).to.equal('bar');
/* tslint:enable:no-any */
});

it('can only call settings() once', () => {
Expand Down Expand Up @@ -340,23 +342,6 @@ describe('instantiation', () => {
);
});

it('validates timestampsInSnapshots is boolean', () => {
expect(() => {
const settings = {...DEFAULT_SETTINGS, timestampsInSnapshots: 1337};
new Firestore.Firestore(settings as InvalidApiUsage);
}).to.throw(
'Value for argument "settings.timestampsInSnapshots" is not a valid boolean.'
);

expect(() => {
new Firestore.Firestore(DEFAULT_SETTINGS).settings({
timestampsInSnapshots: 1337 as InvalidApiUsage,
});
}).to.throw(
'Value for argument "settings.timestampsInSnapshots" is not a valid boolean.'
);
});

it('validates ssl is a boolean', () => {
const invalidValues = ['true', 1337];

Expand Down
21 changes: 0 additions & 21 deletions dev/test/timestamp.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,27 +67,6 @@ describe('timestamps', () => {
});
});

it('converted to dates when disabled', () => {
const oldErrorLog = console.error;
// Prevent error message that prompts to enable `timestampsInSnapshots`
// behavior.
console.error = () => {};

return createInstance(
{timestampsInSnapshots: false},
DOCUMENT_WITH_TIMESTAMP
).then(firestore => {
return firestore
.doc('collectionId/documentId')
.get()
.then(res => {
expect(res.data()!['moonLanding']).to.be.instanceOf(Date);
expect(res.get('moonLanding')).to.be.instanceOf(Date);
console.error = oldErrorLog;
});
});
});

it('retain seconds and nanoseconds', () => {
return createInstance({}, DOCUMENT_WITH_TIMESTAMP).then(firestore => {
return firestore
Expand Down
2 changes: 0 additions & 2 deletions dev/test/typescript.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ xdescribe('firestore.d.ts', () => {
const firestore: Firestore = new Firestore({
keyFilename: 'foo',
projectId: 'foo',
timestampsInSnapshots: true,
host: 'localhost',
ssl: false,
otherOption: 'foo',
Expand All @@ -59,7 +58,6 @@ xdescribe('firestore.d.ts', () => {
firestore.settings({
keyFilename: 'foo',
projectId: 'foo',
timestampsInSnapshots: true,
otherOption: 'foo',
});
const collRef: CollectionReference = firestore.collection('coll');
Expand Down
21 changes: 0 additions & 21 deletions types/firestore.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,27 +77,6 @@ declare namespace FirebaseFirestore {
*/
credentials?: {client_email?:string, private_key?:string};

/**
* 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;

/** Whether to use SSL when connecting. */
ssl?: boolean;

Expand Down

0 comments on commit f37fffc

Please sign in to comment.