diff --git a/.changeset/short-mice-itch.md b/.changeset/short-mice-itch.md new file mode 100644 index 00000000000..f414f7bdfd6 --- /dev/null +++ b/.changeset/short-mice-itch.md @@ -0,0 +1,6 @@ +--- +'@firebase/firestore': minor +'firebase': minor +--- + +Expanded `Firestore.WithFieldValue` to include `T`. This allows developers to delegate `WithFieldValue` inside wrappers of type `T` to avoid exposing Firebase types beyond Firebase-specific logic. diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index 83eebeb488d..666c72047d4 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -204,9 +204,9 @@ export function orderBy(fieldPath: string | FieldPath, directionStr?: OrderByDir export type OrderByDirection = 'desc' | 'asc'; // @public -export type PartialWithFieldValue = T extends Primitive ? T : T extends {} ? { +export type PartialWithFieldValue = Partial | (T extends Primitive ? T : T extends {} ? { [K in keyof T]?: PartialWithFieldValue | FieldValue; -} : Partial; +} : never); // @public export type Primitive = string | number | boolean | undefined | null; @@ -352,9 +352,9 @@ export function where(fieldPath: string | FieldPath, opStr: WhereFilterOp, value export type WhereFilterOp = '<' | '<=' | '==' | '!=' | '>=' | '>' | 'array-contains' | 'in' | 'array-contains-any' | 'not-in'; // @public -export type WithFieldValue = T extends Primitive ? T : T extends {} ? { +export type WithFieldValue = T | (T extends Primitive ? T : T extends {} ? { [K in keyof T]: WithFieldValue | FieldValue; -} : Partial; +} : never); // @public export class WriteBatch { diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 2d3fee14117..e892ed38209 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -328,9 +328,9 @@ export function orderBy(fieldPath: string | FieldPath, directionStr?: OrderByDir export type OrderByDirection = 'desc' | 'asc'; // @public -export type PartialWithFieldValue = T extends Primitive ? T : T extends {} ? { +export type PartialWithFieldValue = Partial | (T extends Primitive ? T : T extends {} ? { [K in keyof T]?: PartialWithFieldValue | FieldValue; -} : Partial; +} : never); // @public export interface PersistenceSettings { @@ -504,9 +504,9 @@ export function where(fieldPath: string | FieldPath, opStr: WhereFilterOp, value export type WhereFilterOp = '<' | '<=' | '==' | '!=' | '>=' | '>' | 'array-contains' | 'in' | 'array-contains-any' | 'not-in'; // @public -export type WithFieldValue = T extends Primitive ? T : T extends {} ? { +export type WithFieldValue = T | (T extends Primitive ? T : T extends {} ? { [K in keyof T]: WithFieldValue | FieldValue; -} : Partial; +} : never); // @public export class WriteBatch { diff --git a/packages/firestore/src/lite-api/reference.ts b/packages/firestore/src/lite-api/reference.ts index ae46bf1e49d..c359d1a53a3 100644 --- a/packages/firestore/src/lite-api/reference.ts +++ b/packages/firestore/src/lite-api/reference.ts @@ -54,21 +54,25 @@ export interface DocumentData { * Similar to Typescript's `Partial`, but allows nested fields to be * omitted and FieldValues to be passed in as property values. */ -export type PartialWithFieldValue = T extends Primitive - ? T - : T extends {} - ? { [K in keyof T]?: PartialWithFieldValue | FieldValue } - : Partial; +export type PartialWithFieldValue = + | Partial + | (T extends Primitive + ? T + : T extends {} + ? { [K in keyof T]?: PartialWithFieldValue | FieldValue } + : never); /** * Allows FieldValues to be passed in as a property value while maintaining * type safety. */ -export type WithFieldValue = T extends Primitive - ? T - : T extends {} - ? { [K in keyof T]: WithFieldValue | FieldValue } - : Partial; +export type WithFieldValue = + | T + | (T extends Primitive + ? T + : T extends {} + ? { [K in keyof T]: WithFieldValue | FieldValue } + : never); /** * Update data (for use with {@link (updateDoc:1)}) that consists of field paths diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts index 04dd47c99d6..5f6056bdf2a 100644 --- a/packages/firestore/test/lite/integration.test.ts +++ b/packages/firestore/test/lite/integration.test.ts @@ -1286,7 +1286,7 @@ describe('withConverter() support', () => { } }; - describe('NestedPartial', () => { + describe('nested partial support', () => { const testConverterMerge = { toFirestore( testObj: PartialWithFieldValue, @@ -1399,6 +1399,44 @@ describe('withConverter() support', () => { ); }); }); + + it('allows omitting fields', async () => { + return withTestDoc(async doc => { + const ref = doc.withConverter(testConverterMerge); + + // Omit outer fields + await setDoc( + ref, + { + outerString: deleteField(), + nested: { + innerNested: { + innerNestedNum: increment(1) + }, + innerArr: arrayUnion(2), + timestamp: serverTimestamp() + } + }, + { merge: true } + ); + + // Omit inner fields + await setDoc( + ref, + { + outerString: deleteField(), + outerArr: [], + nested: { + innerNested: { + innerNestedNum: increment(1) + }, + timestamp: serverTimestamp() + } + }, + { merge: true } + ); + }); + }); }); describe('WithFieldValue', () => { @@ -1421,7 +1459,7 @@ describe('withConverter() support', () => { }); }); - it('requires all fields to be present', async () => { + it('requires all outer fields to be present', async () => { return withTestDoc(async doc => { const ref = doc.withConverter(testConverter); @@ -1440,6 +1478,24 @@ describe('withConverter() support', () => { }); }); + it('requires all nested fields to be present', async () => { + return withTestDoc(async doc => { + const ref = doc.withConverter(testConverter); + + await setDoc(ref, { + outerString: 'foo', + outerArr: [], + // @ts-expect-error + nested: { + innerNested: { + innerNestedNum: increment(1) + }, + timestamp: serverTimestamp() + } + }); + }); + }); + it('validates inner and outer fields', async () => { return withTestDoc(async doc => { const ref = doc.withConverter(testConverter); @@ -1496,6 +1552,77 @@ describe('withConverter() support', () => { }); }); }); + + it('allows certain types but not others', () => { + const withTryCatch = async (fn: () => Promise): Promise => { + try { + await fn(); + } catch {} + }; + + // These tests exist to establish which object types are allowed to be + // passed in by default when `T = DocumentData`. Some objects extend + // the Javascript `{}`, which is why they're allowed whereas others + // throw an error. + return withTestDoc(async doc => { + // @ts-expect-error + await withTryCatch(() => setDoc(doc, 1)); + // @ts-expect-error + await withTryCatch(() => setDoc(doc, 'foo')); + // @ts-expect-error + await withTryCatch(() => setDoc(doc, false)); + await withTryCatch(() => setDoc(doc, undefined)); + await withTryCatch(() => setDoc(doc, null)); + await withTryCatch(() => setDoc(doc, [0])); + await withTryCatch(() => setDoc(doc, new Set())); + await withTryCatch(() => setDoc(doc, new Map())); + }); + }); + + describe('used as a type', () => { + class ObjectWrapper { + withFieldValueT(value: WithFieldValue): WithFieldValue { + return value; + } + + withPartialFieldValueT( + value: PartialWithFieldValue + ): PartialWithFieldValue { + return value; + } + + // Wrapper to avoid having Firebase types in non-Firebase code. + withT(value: T): void { + this.withFieldValueT(value); + } + + // Wrapper to avoid having Firebase types in non-Firebase code. + withPartialT(value: Partial): void { + this.withPartialFieldValueT(value); + } + } + + it('supports passing in the object as `T`', () => { + interface Foo { + id: string; + foo: number; + } + const foo = new ObjectWrapper(); + foo.withFieldValueT({ id: '', foo: increment(1) }); + foo.withPartialFieldValueT({ foo: increment(1) }); + foo.withT({ id: '', foo: 1 }); + foo.withPartialT({ foo: 1 }); + }); + + it('does not allow primitive types to use FieldValue', () => { + type Bar = number; + const bar = new ObjectWrapper(); + // @ts-expect-error + bar.withFieldValueT(increment(1)); + // @ts-expect-error + bar.withPartialFieldValueT(increment(1)); + }); + }); }); describe('UpdateData', () => {