Skip to content

Commit 082d185

Browse files
author
Brian Chen
authored
Merge f2acddc into 6160497 
2 parents 6160497 + f2acddc commit 082d185

File tree

5 files changed

+157
-20
lines changed

5 files changed

+157
-20
lines changed

‎.changeset/short-mice-itch.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@firebase/firestore': minor
3+
'firebase': minor
4+
---
5+
6+
Expanded `Firestore.WithFieldValue<T>` to include `T`. This allows developers to delegate `WithFieldValue<T>` inside wrappers of type `T` to avoid exposing Firebase types beyond Firebase-specific logic.

‎common/api-review/firestore-lite.api.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,9 @@ export function orderBy(fieldPath: string | FieldPath, directionStr?: OrderByDir
204204
export type OrderByDirection = 'desc' | 'asc';
205205

206206
// @public
207-
export type PartialWithFieldValue<T> = T extends Primitive ? T : T extends {} ? {
207+
export type PartialWithFieldValue<T> = Partial<T> | (T extends Primitive ? T : T extends {} ? {
208208
[K in keyof T]?: PartialWithFieldValue<T[K]> | FieldValue;
209-
} : Partial<T>;
209+
} : never);
210210

211211
// @public
212212
export type Primitive = string | number | boolean | undefined | null;
@@ -352,9 +352,9 @@ export function where(fieldPath: string | FieldPath, opStr: WhereFilterOp, value
352352
export type WhereFilterOp = '<' | '<=' | '==' | '!=' | '>=' | '>' | 'array-contains' | 'in' | 'array-contains-any' | 'not-in';
353353

354354
// @public
355-
export type WithFieldValue<T> = T extends Primitive ? T : T extends {} ? {
355+
export type WithFieldValue<T> = T | (T extends Primitive ? T : T extends {} ? {
356356
[K in keyof T]: WithFieldValue<T[K]> | FieldValue;
357-
} : Partial<T>;
357+
} : never);
358358

359359
// @public
360360
export class WriteBatch {

‎common/api-review/firestore.api.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,9 +328,9 @@ export function orderBy(fieldPath: string | FieldPath, directionStr?: OrderByDir
328328
export type OrderByDirection = 'desc' | 'asc';
329329

330330
// @public
331-
export type PartialWithFieldValue<T> = T extends Primitive ? T : T extends {} ? {
331+
export type PartialWithFieldValue<T> = Partial<T> | (T extends Primitive ? T : T extends {} ? {
332332
[K in keyof T]?: PartialWithFieldValue<T[K]> | FieldValue;
333-
} : Partial<T>;
333+
} : never);
334334

335335
// @public
336336
export interface PersistenceSettings {
@@ -504,9 +504,9 @@ export function where(fieldPath: string | FieldPath, opStr: WhereFilterOp, value
504504
export type WhereFilterOp = '<' | '<=' | '==' | '!=' | '>=' | '>' | 'array-contains' | 'in' | 'array-contains-any' | 'not-in';
505505

506506
// @public
507-
export type WithFieldValue<T> = T extends Primitive ? T : T extends {} ? {
507+
export type WithFieldValue<T> = T | (T extends Primitive ? T : T extends {} ? {
508508
[K in keyof T]: WithFieldValue<T[K]> | FieldValue;
509-
} : Partial<T>;
509+
} : never);
510510

511511
// @public
512512
export class WriteBatch {

‎packages/firestore/src/lite-api/reference.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,21 +54,25 @@ export interface DocumentData {
5454
* Similar to Typescript's `Partial<T>`, but allows nested fields to be
5555
* omitted and FieldValues to be passed in as property values.
5656
*/
57-
export type PartialWithFieldValue<T> = T extends Primitive
58-
? T
59-
: T extends {}
60-
? { [K in keyof T]?: PartialWithFieldValue<T[K]> | FieldValue }
61-
: Partial<T>;
57+
export type PartialWithFieldValue<T> =
58+
| Partial<T>
59+
| (T extends Primitive
60+
? T
61+
: T extends {}
62+
? { [K in keyof T]?: PartialWithFieldValue<T[K]> | FieldValue }
63+
: never);
6264

6365
/**
6466
* Allows FieldValues to be passed in as a property value while maintaining
6567
* type safety.
6668
*/
67-
export type WithFieldValue<T> = T extends Primitive
68-
? T
69-
: T extends {}
70-
? { [K in keyof T]: WithFieldValue<T[K]> | FieldValue }
71-
: Partial<T>;
69+
export type WithFieldValue<T> =
70+
| T
71+
| (T extends Primitive
72+
? T
73+
: T extends {}
74+
? { [K in keyof T]: WithFieldValue<T[K]> | FieldValue }
75+
: never);
7276

7377
/**
7478
* Update data (for use with {@link (updateDoc:1)}) that consists of field paths

‎packages/firestore/test/lite/integration.test.ts

Lines changed: 129 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,7 +1286,7 @@ describe('withConverter() support', () => {
12861286
}
12871287
};
12881288

1289-
describe('NestedPartial', () => {
1289+
describe('nested partial support', () => {
12901290
const testConverterMerge = {
12911291
toFirestore(
12921292
testObj: PartialWithFieldValue<TestObject>,
@@ -1399,6 +1399,44 @@ describe('withConverter() support', () => {
13991399
);
14001400
});
14011401
});
1402+
1403+
it('allows omitting fields', async () => {
1404+
return withTestDoc(async doc => {
1405+
const ref = doc.withConverter(testConverterMerge);
1406+
1407+
// Omit outer fields
1408+
await setDoc(
1409+
ref,
1410+
{
1411+
outerString: deleteField(),
1412+
nested: {
1413+
innerNested: {
1414+
innerNestedNum: increment(1)
1415+
},
1416+
innerArr: arrayUnion(2),
1417+
timestamp: serverTimestamp()
1418+
}
1419+
},
1420+
{ merge: true }
1421+
);
1422+
1423+
// Omit inner fields
1424+
await setDoc(
1425+
ref,
1426+
{
1427+
outerString: deleteField(),
1428+
outerArr: [],
1429+
nested: {
1430+
innerNested: {
1431+
innerNestedNum: increment(1)
1432+
},
1433+
timestamp: serverTimestamp()
1434+
}
1435+
},
1436+
{ merge: true }
1437+
);
1438+
});
1439+
});
14021440
});
14031441

14041442
describe('WithFieldValue', () => {
@@ -1421,7 +1459,7 @@ describe('withConverter() support', () => {
14211459
});
14221460
});
14231461

1424-
it('requires all fields to be present', async () => {
1462+
it('requires all outer fields to be present', async () => {
14251463
return withTestDoc(async doc => {
14261464
const ref = doc.withConverter(testConverter);
14271465

@@ -1440,6 +1478,24 @@ describe('withConverter() support', () => {
14401478
});
14411479
});
14421480

1481+
it('requires all nested fields to be present', async () => {
1482+
return withTestDoc(async doc => {
1483+
const ref = doc.withConverter(testConverter);
1484+
1485+
await setDoc(ref, {
1486+
outerString: 'foo',
1487+
outerArr: [],
1488+
// @ts-expect-error
1489+
nested: {
1490+
innerNested: {
1491+
innerNestedNum: increment(1)
1492+
},
1493+
timestamp: serverTimestamp()
1494+
}
1495+
});
1496+
});
1497+
});
1498+
14431499
it('validates inner and outer fields', async () => {
14441500
return withTestDoc(async doc => {
14451501
const ref = doc.withConverter(testConverter);
@@ -1496,6 +1552,77 @@ describe('withConverter() support', () => {
14961552
});
14971553
});
14981554
});
1555+
1556+
it('allows certain types but not others', () => {
1557+
const withTryCatch = async (fn: () => Promise<void>): Promise<void> => {
1558+
try {
1559+
await fn();
1560+
} catch {}
1561+
};
1562+
1563+
// These tests exist to establish which object types are allowed to be
1564+
// passed in by default when `T = DocumentData`. Some objects extend
1565+
// the Javascript `{}`, which is why they're allowed whereas others
1566+
// throw an error.
1567+
return withTestDoc(async doc => {
1568+
// @ts-expect-error
1569+
await withTryCatch(() => setDoc(doc, 1));
1570+
// @ts-expect-error
1571+
await withTryCatch(() => setDoc(doc, 'foo'));
1572+
// @ts-expect-error
1573+
await withTryCatch(() => setDoc(doc, false));
1574+
await withTryCatch(() => setDoc(doc, undefined));
1575+
await withTryCatch(() => setDoc(doc, null));
1576+
await withTryCatch(() => setDoc(doc, [0]));
1577+
await withTryCatch(() => setDoc(doc, new Set<string>()));
1578+
await withTryCatch(() => setDoc(doc, new Map<string, number>()));
1579+
});
1580+
});
1581+
1582+
describe('used as a type', () => {
1583+
class ObjectWrapper<T> {
1584+
withFieldValueT(value: WithFieldValue<T>): WithFieldValue<T> {
1585+
return value;
1586+
}
1587+
1588+
withPartialFieldValueT(
1589+
value: PartialWithFieldValue<T>
1590+
): PartialWithFieldValue<T> {
1591+
return value;
1592+
}
1593+
1594+
// Wrapper to avoid having Firebase types in non-Firebase code.
1595+
withT(value: T): void {
1596+
this.withFieldValueT(value);
1597+
}
1598+
1599+
// Wrapper to avoid having Firebase types in non-Firebase code.
1600+
withPartialT(value: Partial<T>): void {
1601+
this.withPartialFieldValueT(value);
1602+
}
1603+
}
1604+
1605+
it('supports passing in the object as `T`', () => {
1606+
interface Foo {
1607+
id: string;
1608+
foo: number;
1609+
}
1610+
const foo = new ObjectWrapper<Foo>();
1611+
foo.withFieldValueT({ id: '', foo: increment(1) });
1612+
foo.withPartialFieldValueT({ foo: increment(1) });
1613+
foo.withT({ id: '', foo: 1 });
1614+
foo.withPartialT({ foo: 1 });
1615+
});
1616+
1617+
it('does not allow primitive types to use FieldValue', () => {
1618+
type Bar = number;
1619+
const bar = new ObjectWrapper<Bar>();
1620+
// @ts-expect-error
1621+
bar.withFieldValueT(increment(1));
1622+
// @ts-expect-error
1623+
bar.withPartialFieldValueT(increment(1));
1624+
});
1625+
});
14991626
});
15001627

15011628
describe('UpdateData', () => {

0 commit comments

Comments
 (0)