Skip to content

Commit 224383f

Browse files
mikehardymikehardy
committed
fix(firestore): accept nested undefined array values
Fixes #5437
1 parent 0f18b75 commit 224383f

File tree

5 files changed

+133
-53
lines changed

5 files changed

+133
-53
lines changed

‎jest.setup.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ jest.doMock('react-native', () => {
4747
},
4848
RNFBFirestoreModule: {
4949
settings: jest.fn(),
50+
documentSet: jest.fn(),
5051
},
5152
RNFBPerfModule: {},
5253
RNFBStorageModule: {

‎packages/firestore/__tests__/firestore.test.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ describe('Storage', function () {
247247
}
248248
});
249249

250-
it('throws when nested undefined value provided and ignored undefined is false', async function () {
250+
it('throws when nested undefined object value provided and ignored undefined is false', async function () {
251251
await firebase.firestore().settings({ ignoreUndefinedProperties: false });
252252
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
253253
try {
@@ -262,5 +262,37 @@ describe('Storage', function () {
262262
return expect(e.message).toEqual('Unsupported field value: undefined');
263263
}
264264
});
265+
266+
it('throws when nested undefined array value provided and ignored undefined is false', async function () {
267+
await firebase.firestore().settings({ ignoreUndefinedProperties: false });
268+
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
269+
try {
270+
await docRef.set({
271+
myArray: [{ name: 'Tim', location: { state: undefined, country: 'United Kingdom' } }],
272+
});
273+
return Promise.reject(new Error('Expected set() to throw'));
274+
} catch (e) {
275+
return expect(e.message).toEqual('Unsupported field value: undefined');
276+
}
277+
});
278+
279+
it('does not throw when nested undefined array value provided and ignore undefined is true', async function () {
280+
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
281+
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
282+
await docRef.set({
283+
myArray: [{ name: 'Tim', location: { state: undefined, country: 'United Kingdom' } }],
284+
});
285+
});
286+
287+
it('does not throw when nested undefined object value provided and ignore undefined is true', async function () {
288+
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
289+
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
290+
await docRef.set({
291+
field1: 1,
292+
field2: {
293+
shouldNotWork: undefined,
294+
},
295+
});
296+
});
265297
});
266298
});

‎packages/firestore/e2e/DocumentReference/set.e2e.js

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,4 +156,86 @@ describe('firestore.doc().set()', function () {
156156
snapshot2.data().should.eql(jet.contextify(merged));
157157
await ref.delete();
158158
});
159+
160+
it('throws when nested undefined array value provided and ignored undefined is false', async function () {
161+
await firebase.firestore().settings({ ignoreUndefinedProperties: false });
162+
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
163+
try {
164+
await docRef.set({
165+
myArray: [{ name: 'Tim', location: { state: undefined, country: 'United Kingdom' } }],
166+
});
167+
return Promise.reject(new Error('Expected set() to throw'));
168+
} catch (error) {
169+
error.message.should.containEql('Unsupported field value: undefined');
170+
}
171+
});
172+
173+
it('accepts undefined nested array values if ignoreUndefined is true', async function () {
174+
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
175+
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
176+
await docRef.set({
177+
myArray: [{ name: 'Tim', location: { state: undefined, country: 'United Kingdom' } }],
178+
});
179+
});
180+
181+
it('does not throw when nested undefined object value provided and ignore undefined is true', async function () {
182+
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
183+
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
184+
await docRef.set({
185+
field1: 1,
186+
field2: {
187+
shouldNotWork: undefined,
188+
},
189+
});
190+
});
191+
192+
it('filters out undefined properties when setting enabled', async function () {
193+
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
194+
195+
const docRef = firebase.firestore().doc(`${COLLECTION}/ignoreUndefinedTrueProp`);
196+
await docRef.set({
197+
field1: 1,
198+
field2: undefined,
199+
});
200+
201+
const snap = await docRef.get();
202+
const snapData = snap.data();
203+
if (!snapData) {
204+
return Promise.reject(new Error('Snapshot not saved'));
205+
}
206+
207+
snapData.field1.should.eql(1);
208+
snapData.hasOwnProperty('field2').should.eql(false);
209+
});
210+
211+
it('filters out nested undefined properties when setting enabled', async function () {
212+
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
213+
214+
const docRef = firebase.firestore().doc(`${COLLECTION}/ignoreUndefinedTrueNestedProp`);
215+
await docRef.set({
216+
field1: 1,
217+
field2: {
218+
shouldBeMissing: undefined,
219+
},
220+
field3: [
221+
{
222+
shouldBeHere: 'Here',
223+
shouldBeMissing: undefined,
224+
},
225+
],
226+
});
227+
228+
const snap = await docRef.get();
229+
const snapData = snap.data();
230+
if (!snapData) {
231+
return Promise.reject(new Error('Snapshot not saved'));
232+
}
233+
234+
snapData.field1.should.eql(1);
235+
snapData.hasOwnProperty('field2').should.eql(true);
236+
snapData.field2.hasOwnProperty('shouldBeMissing').should.eql(false);
237+
snapData.hasOwnProperty('field3').should.eql(true);
238+
snapData.field3[0].shouldBeHere.should.eql('Here');
239+
snapData.field3[0].hasOwnProperty('shouldBeMissing').should.eql(false);
240+
});
159241
});

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

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -37,50 +37,6 @@ describe('firestore()', function () {
3737

3838
describe('collection()', function () {});
3939

40-
describe('doc()', function () {
41-
it('filters out undefined properties when setting enabled', async function () {
42-
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
43-
44-
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
45-
await docRef.set({
46-
field1: 1,
47-
field2: undefined,
48-
});
49-
50-
const snap = await docRef.get();
51-
const snapData = snap.data();
52-
if (!snapData) {
53-
return Promise.reject(new Error('Snapshot not saved'));
54-
}
55-
56-
snapData.field1.should.eql(1);
57-
snapData.hasOwnProperty('field2').should.eql(false);
58-
});
59-
60-
it('filters out nested undefined properties when setting enabled', async function () {
61-
await firebase.firestore().settings({ ignoreUndefinedProperties: true });
62-
63-
const docRef = firebase.firestore().doc(`${COLLECTION}/bar`);
64-
await docRef.set({
65-
field1: 1,
66-
field2: {
67-
shouldBeMissing: undefined,
68-
},
69-
});
70-
71-
const snap = await docRef.get();
72-
const snapData = snap.data();
73-
if (!snapData) {
74-
return Promise.reject(new Error('Snapshot not saved'));
75-
}
76-
77-
snapData.field1.should.eql(1);
78-
snapData.hasOwnProperty('field2').should.eql(true);
79-
80-
snapData.field2.hasOwnProperty('shouldBeMissing').should.eql(false);
81-
});
82-
});
83-
8440
describe('collectionGroup()', function () {
8541
it('performs a collection group query', async function () {
8642
const docRef1 = firebase.firestore().doc(`${COLLECTION}/collectionGroup1`);

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

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,14 @@ export function buildNativeMap(data, ignoreUndefined) {
5959
if (typeof data[key] === 'undefined') {
6060
if (!ignoreUndefined) {
6161
throw new Error('Unsupported field value: undefined');
62+
} else {
63+
continue;
6264
}
63-
} else {
64-
const typeMap = generateNativeData(data[key], ignoreUndefined);
65-
if (typeMap) {
66-
nativeData[key] = typeMap;
67-
}
65+
}
66+
67+
const typeMap = generateNativeData(data[key], ignoreUndefined);
68+
if (typeMap) {
69+
nativeData[key] = typeMap;
6870
}
6971
}
7072
}
@@ -76,12 +78,19 @@ export function buildNativeMap(data, ignoreUndefined) {
7678
* @param array
7779
* @returns {Array}
7880
*/
79-
export function buildNativeArray(array) {
81+
export function buildNativeArray(array, ignoreUndefined) {
8082
const nativeArray = [];
8183
if (array) {
8284
for (let i = 0; i < array.length; i++) {
8385
const value = array[i];
84-
const typeMap = generateNativeData(value);
86+
if (typeof value === 'undefined') {
87+
if (!ignoreUndefined) {
88+
throw new Error('Unsupported field value: undefined');
89+
} else {
90+
continue;
91+
}
92+
}
93+
const typeMap = generateNativeData(value, ignoreUndefined);
8594
if (typeMap) {
8695
nativeArray.push(typeMap);
8796
}
@@ -144,7 +153,7 @@ export function generateNativeData(value, ignoreUndefined) {
144153
}
145154

146155
if (isArray(value)) {
147-
return getTypeMapInt('array', buildNativeArray(value));
156+
return getTypeMapInt('array', buildNativeArray(value, ignoreUndefined));
148157
}
149158

150159
if (isObject(value)) {

0 commit comments

Comments
 (0)