Skip to content

Commit 756cfa6

Browse files
jmifmikehardy
jmif
authored and
mikehardy
committed
feat(firestore)!: add support for ignoreUndefinedProperties
BREAKING CHANGE: undefined values throw like firebase-js-sdk now. Use ignoreUndefinedProperties setting 'true' to behave as before
1 parent 01c94cc commit 756cfa6

File tree

8 files changed

+141
-14
lines changed

8 files changed

+141
-14
lines changed

‎packages/firestore/e2e/firestore.e2e.js

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,84 @@ describe('firestore()', function () {
120120
should.equal(docRef.constructor.name, 'FirestoreDocumentReference');
121121
docRef.path.should.eql(`${COLLECTION}/bar`);
122122
});
123+
124+
it('filters out undefined properties when setting enabled', async function () {
125+
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
126+
127+
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
128+
await docRef.set({
129+
field1: 1,
130+
field2: undefined,
131+
});
132+
133+
const snap = await docRef.get();
134+
const snapData = snap.data();
135+
if (!snapData) {
136+
return Promise.reject(new Error('Snapshot not saved'));
137+
}
138+
139+
snapData.field1.should.eql(1);
140+
snapData.hasOwnProperty('field2').should.eql(false);
141+
});
142+
143+
it('filters out nested undefined properties when setting enabled', async function () {
144+
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
145+
146+
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
147+
await docRef.set({
148+
field1: 1,
149+
field2: {
150+
shouldBeMissing: undefined,
151+
},
152+
});
153+
154+
const snap = await docRef.get();
155+
const snapData = snap.data();
156+
if (!snapData) {
157+
return Promise.reject(new Error('Snapshot not saved'));
158+
}
159+
160+
snapData.field1.should.eql(1);
161+
snapData.hasOwnProperty('field2').should.eql(true);
162+
163+
snapData.field2.hasOwnProperty('shouldBeMissing').should.eql(false);
164+
});
165+
166+
it('throws when undefined value provided and ignored undefined is false', async function () {
167+
await firebase.firestore().settings({ ignoreUndefinedProperties: false });
168+
169+
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
170+
171+
try {
172+
await docRef.set({
173+
field1: 1,
174+
field2: undefined,
175+
});
176+
177+
return Promise.reject(new Error('Expected set() to throw'));
178+
} catch (e) {
179+
return Promise.resolve();
180+
}
181+
});
182+
183+
it('throws when nested undefined value provided and ignored undefined is false', async function () {
184+
await firebase.firestore().settings({ ignoreUndefinedProperties: false });
185+
186+
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
187+
188+
try {
189+
await docRef.set({
190+
field1: 1,
191+
field2: {
192+
shouldNotWork: undefined,
193+
},
194+
});
195+
196+
return Promise.reject(new Error('Expected set() to throw'));
197+
} catch (e) {
198+
return Promise.resolve();
199+
}
200+
});
123201
});
124202

125203
describe('collectionGroup()', function () {
@@ -322,6 +400,18 @@ describe('firestore()', function () {
322400
}
323401
});
324402

403+
it('throws if ignoreUndefinedProperties is not a boolean', function () {
404+
try {
405+
firebase.firestore().settings({ ignoreUndefinedProperties: 'true' });
406+
return Promise.reject(new Error('Did not throw an Error.'));
407+
} catch (error) {
408+
error.message.should.containEql(
409+
"'settings.ignoreUndefinedProperties' must be a boolean value",
410+
);
411+
return Promise.resolve();
412+
}
413+
});
414+
325415
it('throws if ssl is not a boolean', function () {
326416
try {
327417
firebase.firestore().settings({ ssl: 'true' });

‎packages/firestore/lib/FirestoreDocumentReference.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,11 @@ export default class FirestoreDocumentReference {
185185
throw new Error(`firebase.firestore().doc().set(_, *) ${e.message}.`);
186186
}
187187

188-
return this._firestore.native.documentSet(this.path, buildNativeMap(data), setOptions);
188+
return this._firestore.native.documentSet(
189+
this.path,
190+
buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
191+
setOptions,
192+
);
189193
}
190194

191195
update(...args) {
@@ -202,7 +206,10 @@ export default class FirestoreDocumentReference {
202206
throw new Error(`firebase.firestore().doc().update(*) ${e.message}`);
203207
}
204208

205-
return this._firestore.native.documentUpdate(this.path, buildNativeMap(data));
209+
return this._firestore.native.documentUpdate(
210+
this.path,
211+
buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
212+
);
206213
}
207214
}
208215

‎packages/firestore/lib/FirestoreQueryModifiers.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ export default class FirestoreQueryModifiers {
204204
const filter = {
205205
fieldPath,
206206
operator: OPERATORS[opStr],
207-
value: generateNativeData(value),
207+
value: generateNativeData(value, true),
208208
};
209209

210210
this._filters = this._filters.concat(filter);

‎packages/firestore/lib/FirestoreTransaction.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export default class FirestoreTransaction {
8585
this._commandBuffer.push({
8686
type: 'SET',
8787
path: documentRef.path,
88-
data: buildNativeMap(data),
88+
data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
8989
options: setOptions,
9090
});
9191

@@ -111,7 +111,7 @@ export default class FirestoreTransaction {
111111
this._commandBuffer.push({
112112
type: 'UPDATE',
113113
path: documentRef.path,
114-
data: buildNativeMap(data),
114+
data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
115115
});
116116

117117
return this;

‎packages/firestore/lib/FirestoreWriteBatch.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ export default class FirestoreWriteBatch {
9494
this._writes.push({
9595
path: documentRef.path,
9696
type: 'SET',
97-
data: buildNativeMap(data),
97+
data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
9898
options: setOptions,
9999
});
100100

@@ -131,7 +131,7 @@ export default class FirestoreWriteBatch {
131131
this._writes.push({
132132
path: documentRef.path,
133133
type: 'UPDATE',
134-
data: buildNativeMap(data),
134+
data: buildNativeMap(data, this._firestore._settings.ignoreUndefinedProperties),
135135
});
136136

137137
return this;

‎packages/firestore/lib/index.d.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1419,6 +1419,11 @@ export namespace FirebaseFirestoreTypes {
14191419
* Whether to use SSL when connecting.
14201420
*/
14211421
ssl?: boolean;
1422+
1423+
/**
1424+
* When this parameter is set, Cloud Firestore ignores undefined properties inside objects.
1425+
*/
1426+
ignoreUndefinedProperties?: boolean;
14221427
}
14231428

14241429
/**

‎packages/firestore/lib/index.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ class FirebaseFirestoreModule extends FirebaseModule {
7474
event,
7575
);
7676
});
77+
78+
this._settings = {
79+
ignoreUndefinedProperties: false,
80+
};
7781
}
7882

7983
batch() {
@@ -185,7 +189,7 @@ class FirebaseFirestoreModule extends FirebaseModule {
185189

186190
const keys = Object.keys(settings);
187191

188-
const opts = ['cacheSizeBytes', 'host', 'persistence', 'ssl'];
192+
const opts = ['cacheSizeBytes', 'host', 'persistence', 'ssl', 'ignoreUndefinedProperties'];
189193

190194
for (let i = 0; i < keys.length; i++) {
191195
const key = keys[i];
@@ -252,6 +256,18 @@ class FirebaseFirestoreModule extends FirebaseModule {
252256
throw new Error("firebase.firestore().settings(*) 'settings.ssl' must be a boolean value.");
253257
}
254258

259+
if (!isUndefined(settings.ignoreUndefinedProperties)) {
260+
if (!isBoolean(settings.ignoreUndefinedProperties)) {
261+
throw new Error(
262+
"firebase.firestore().settings(*) 'settings.ignoreUndefinedProperties' must be a boolean value.",
263+
);
264+
} else {
265+
this._settings.ignoreUndefinedProperties = settings.ignoreUndefinedProperties;
266+
}
267+
268+
delete settings.ignoreUndefinedProperties;
269+
}
270+
255271
return this.native.settings(settings);
256272
}
257273
}

‎packages/firestore/lib/utils/serialize.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,24 @@ export function provideFieldValueClass(fieldValue) {
4747
/**
4848
*
4949
* @param data
50+
* @param ignoreUndefined
5051
*/
51-
export function buildNativeMap(data) {
52+
export function buildNativeMap(data, ignoreUndefined) {
5253
const nativeData = {};
5354
if (data) {
5455
const keys = Object.keys(data);
5556
for (let i = 0; i < keys.length; i++) {
5657
const key = keys[i];
57-
const typeMap = generateNativeData(data[key]);
58-
if (typeMap) {
59-
nativeData[key] = typeMap;
58+
59+
if (typeof data[key] === 'undefined') {
60+
if (!ignoreUndefined) {
61+
throw new Error('firebase.firestore() undefined values cannot be saved');
62+
}
63+
} else {
64+
const typeMap = generateNativeData(data[key], ignoreUndefined);
65+
if (typeMap) {
66+
nativeData[key] = typeMap;
67+
}
6068
}
6169
}
6270
}
@@ -90,9 +98,10 @@ export function buildNativeArray(array) {
9098
* Example: [7, 'some string'];
9199
*
92100
* @param value
101+
* @param ignoreUndefined
93102
* @returns {*}
94103
*/
95-
export function generateNativeData(value) {
104+
export function generateNativeData(value, ignoreUndefined) {
96105
if (Number.isNaN(value)) {
97106
return getTypeMapInt('nan');
98107
}
@@ -165,7 +174,7 @@ export function generateNativeData(value) {
165174
return getTypeMapInt('fieldvalue', [value._type, value._elements]);
166175
}
167176

168-
return getTypeMapInt('object', buildNativeMap(value));
177+
return getTypeMapInt('object', buildNativeMap(value, ignoreUndefined));
169178
}
170179

171180
// eslint-disable-next-line no-console

0 commit comments

Comments
 (0)