From 756cfa6bea645ac6c18ad25bbae9cac5a3f5e379 Mon Sep 17 00:00:00 2001 From: jmif Date: Fri, 14 May 2021 13:47:46 -0400 Subject: [PATCH] feat(firestore)!: add support for ignoreUndefinedProperties BREAKING CHANGE: undefined values throw like firebase-js-sdk now. Use ignoreUndefinedProperties setting 'true' to behave as before --- packages/firestore/e2e/firestore.e2e.js | 90 +++++++++++++++++++ .../lib/FirestoreDocumentReference.js | 11 ++- .../firestore/lib/FirestoreQueryModifiers.js | 2 +- .../firestore/lib/FirestoreTransaction.js | 4 +- packages/firestore/lib/FirestoreWriteBatch.js | 4 +- packages/firestore/lib/index.d.ts | 5 ++ packages/firestore/lib/index.js | 18 +++- packages/firestore/lib/utils/serialize.js | 21 +++-- 8 files changed, 141 insertions(+), 14 deletions(-) diff --git a/packages/firestore/e2e/firestore.e2e.js b/packages/firestore/e2e/firestore.e2e.js index 4c210c138f..c97cdd9b17 100644 --- a/packages/firestore/e2e/firestore.e2e.js +++ b/packages/firestore/e2e/firestore.e2e.js @@ -120,6 +120,84 @@ describe('firestore()', function () { should.equal(docRef.constructor.name, 'FirestoreDocumentReference'); docRef.path.should.eql(`${COLLECTION}/bar`); }); + + it('filters out undefined properties when setting enabled', async function () { + await firebase.firestore().settings({ ignoreUndefinedProperties: true }); + + const docRef = firebase.firestore().doc(`${COLLECTION}/bar`); + await docRef.set({ + field1: 1, + field2: undefined, + }); + + const snap = await docRef.get(); + const snapData = snap.data(); + if (!snapData) { + return Promise.reject(new Error('Snapshot not saved')); + } + + snapData.field1.should.eql(1); + snapData.hasOwnProperty('field2').should.eql(false); + }); + + it('filters out nested undefined properties when setting enabled', async function () { + await firebase.firestore().settings({ ignoreUndefinedProperties: true }); + + const docRef = firebase.firestore().doc(`${COLLECTION}/bar`); + await docRef.set({ + field1: 1, + field2: { + shouldBeMissing: undefined, + }, + }); + + const snap = await docRef.get(); + const snapData = snap.data(); + if (!snapData) { + return Promise.reject(new Error('Snapshot not saved')); + } + + snapData.field1.should.eql(1); + snapData.hasOwnProperty('field2').should.eql(true); + + snapData.field2.hasOwnProperty('shouldBeMissing').should.eql(false); + }); + + it('throws when undefined value provided and ignored undefined is false', async function () { + await firebase.firestore().settings({ ignoreUndefinedProperties: false }); + + const docRef = firebase.firestore().doc(`${COLLECTION}/bar`); + + try { + await docRef.set({ + field1: 1, + field2: undefined, + }); + + return Promise.reject(new Error('Expected set() to throw')); + } catch (e) { + return Promise.resolve(); + } + }); + + it('throws when nested undefined value provided and ignored undefined is false', async function () { + await firebase.firestore().settings({ ignoreUndefinedProperties: false }); + + const docRef = firebase.firestore().doc(`${COLLECTION}/bar`); + + try { + await docRef.set({ + field1: 1, + field2: { + shouldNotWork: undefined, + }, + }); + + return Promise.reject(new Error('Expected set() to throw')); + } catch (e) { + return Promise.resolve(); + } + }); }); describe('collectionGroup()', function () { @@ -322,6 +400,18 @@ describe('firestore()', function () { } }); + it('throws if ignoreUndefinedProperties is not a boolean', function () { + try { + firebase.firestore().settings({ ignoreUndefinedProperties: 'true' }); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + "'settings.ignoreUndefinedProperties' must be a boolean value", + ); + return Promise.resolve(); + } + }); + it('throws if ssl is not a boolean', function () { try { firebase.firestore().settings({ ssl: 'true' }); diff --git a/packages/firestore/lib/FirestoreDocumentReference.js b/packages/firestore/lib/FirestoreDocumentReference.js index 2b024880b3..0ffe715f95 100644 --- a/packages/firestore/lib/FirestoreDocumentReference.js +++ b/packages/firestore/lib/FirestoreDocumentReference.js @@ -185,7 +185,11 @@ export default class FirestoreDocumentReference { throw new Error(`firebase.firestore().doc().set(_, *) ${e.message}.`); } - return this._firestore.native.documentSet(this.path, buildNativeMap(data), setOptions); + return this._firestore.native.documentSet( + this.path, + buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties), + setOptions, + ); } update(...args) { @@ -202,7 +206,10 @@ export default class FirestoreDocumentReference { throw new Error(`firebase.firestore().doc().update(*) ${e.message}`); } - return this._firestore.native.documentUpdate(this.path, buildNativeMap(data)); + return this._firestore.native.documentUpdate( + this.path, + buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties), + ); } } diff --git a/packages/firestore/lib/FirestoreQueryModifiers.js b/packages/firestore/lib/FirestoreQueryModifiers.js index d9facea881..696de2013a 100644 --- a/packages/firestore/lib/FirestoreQueryModifiers.js +++ b/packages/firestore/lib/FirestoreQueryModifiers.js @@ -204,7 +204,7 @@ export default class FirestoreQueryModifiers { const filter = { fieldPath, operator: OPERATORS[opStr], - value: generateNativeData(value), + value: generateNativeData(value, true), }; this._filters = this._filters.concat(filter); diff --git a/packages/firestore/lib/FirestoreTransaction.js b/packages/firestore/lib/FirestoreTransaction.js index 328d2f12a6..d6d710b974 100644 --- a/packages/firestore/lib/FirestoreTransaction.js +++ b/packages/firestore/lib/FirestoreTransaction.js @@ -85,7 +85,7 @@ export default class FirestoreTransaction { this._commandBuffer.push({ type: 'SET', path: documentRef.path, - data: buildNativeMap(data), + data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties), options: setOptions, }); @@ -111,7 +111,7 @@ export default class FirestoreTransaction { this._commandBuffer.push({ type: 'UPDATE', path: documentRef.path, - data: buildNativeMap(data), + data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties), }); return this; diff --git a/packages/firestore/lib/FirestoreWriteBatch.js b/packages/firestore/lib/FirestoreWriteBatch.js index 6a54ec0573..257b05fed9 100644 --- a/packages/firestore/lib/FirestoreWriteBatch.js +++ b/packages/firestore/lib/FirestoreWriteBatch.js @@ -94,7 +94,7 @@ export default class FirestoreWriteBatch { this._writes.push({ path: documentRef.path, type: 'SET', - data: buildNativeMap(data), + data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties), options: setOptions, }); @@ -131,7 +131,7 @@ export default class FirestoreWriteBatch { this._writes.push({ path: documentRef.path, type: 'UPDATE', - data: buildNativeMap(data), + data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties), }); return this; diff --git a/packages/firestore/lib/index.d.ts b/packages/firestore/lib/index.d.ts index 4e61835a71..0920181003 100644 --- a/packages/firestore/lib/index.d.ts +++ b/packages/firestore/lib/index.d.ts @@ -1419,6 +1419,11 @@ export namespace FirebaseFirestoreTypes { * Whether to use SSL when connecting. */ ssl?: boolean; + + /** + * When this parameter is set, Cloud Firestore ignores undefined properties inside objects. + */ + ignoreUndefinedProperties?: boolean; } /** diff --git a/packages/firestore/lib/index.js b/packages/firestore/lib/index.js index 227c1e00c5..bfe1600036 100644 --- a/packages/firestore/lib/index.js +++ b/packages/firestore/lib/index.js @@ -74,6 +74,10 @@ class FirebaseFirestoreModule extends FirebaseModule { event, ); }); + + this._settings = { + ignoreUndefinedProperties: false, + }; } batch() { @@ -185,7 +189,7 @@ class FirebaseFirestoreModule extends FirebaseModule { const keys = Object.keys(settings); - const opts = ['cacheSizeBytes', 'host', 'persistence', 'ssl']; + const opts = ['cacheSizeBytes', 'host', 'persistence', 'ssl', 'ignoreUndefinedProperties']; for (let i = 0; i < keys.length; i++) { const key = keys[i]; @@ -252,6 +256,18 @@ class FirebaseFirestoreModule extends FirebaseModule { throw new Error("firebase.firestore().settings(*) 'settings.ssl' must be a boolean value."); } + if (!isUndefined(settings.ignoreUndefinedProperties)) { + if (!isBoolean(settings.ignoreUndefinedProperties)) { + throw new Error( + "firebase.firestore().settings(*) 'settings.ignoreUndefinedProperties' must be a boolean value.", + ); + } else { + this._settings.ignoreUndefinedProperties = settings.ignoreUndefinedProperties; + } + + delete settings.ignoreUndefinedProperties; + } + return this.native.settings(settings); } } diff --git a/packages/firestore/lib/utils/serialize.js b/packages/firestore/lib/utils/serialize.js index d6737e1242..14a22b115b 100644 --- a/packages/firestore/lib/utils/serialize.js +++ b/packages/firestore/lib/utils/serialize.js @@ -47,16 +47,24 @@ export function provideFieldValueClass(fieldValue) { /** * * @param data + * @param ignoreUndefined */ -export function buildNativeMap(data) { +export function buildNativeMap(data, ignoreUndefined) { const nativeData = {}; if (data) { const keys = Object.keys(data); for (let i = 0; i < keys.length; i++) { const key = keys[i]; - const typeMap = generateNativeData(data[key]); - if (typeMap) { - nativeData[key] = typeMap; + + if (typeof data[key] === 'undefined') { + if (!ignoreUndefined) { + throw new Error('firebase.firestore() undefined values cannot be saved'); + } + } else { + const typeMap = generateNativeData(data[key], ignoreUndefined); + if (typeMap) { + nativeData[key] = typeMap; + } } } } @@ -90,9 +98,10 @@ export function buildNativeArray(array) { * Example: [7, 'some string']; * * @param value + * @param ignoreUndefined * @returns {*} */ -export function generateNativeData(value) { +export function generateNativeData(value, ignoreUndefined) { if (Number.isNaN(value)) { return getTypeMapInt('nan'); } @@ -165,7 +174,7 @@ export function generateNativeData(value) { return getTypeMapInt('fieldvalue', [value._type, value._elements]); } - return getTypeMapInt('object', buildNativeMap(value)); + return getTypeMapInt('object', buildNativeMap(value, ignoreUndefined)); } // eslint-disable-next-line no-console