From bbc2cea85923da9127b189446fd8db661f3b649e Mon Sep 17 00:00:00 2001 From: Grzegorz Rozdzialik Date: Fri, 19 May 2023 13:21:27 +0200 Subject: [PATCH] feat(cache): type inference for cache `Modifiers` type Problem: The `fields` property in `cache.modify` did not offer ways to infer the field names or values based on the field type. It accepted any field names and the field values were `any`. Solution: Add a generic type parameter for the object type to the `Modifiers` type. The code can now use the `satisfies Modifiers<...>` operator to inform TypeScript about the possible field names and values. Field values include `Reference`s for objects and arrays. The consuming code is then responsible for checking (or asserting) that the field value is or is not a reference. --- .changeset/afraid-zebras-punch.md | 27 +++++++ src/cache/core/types/Cache.ts | 2 +- src/cache/core/types/common.ts | 17 +++- src/cache/inmemory/__tests__/cache.ts | 110 +++++++++++++++++++------- src/cache/inmemory/entityStore.ts | 4 +- src/cache/inmemory/types.ts | 2 +- 6 files changed, 126 insertions(+), 36 deletions(-) create mode 100644 .changeset/afraid-zebras-punch.md diff --git a/.changeset/afraid-zebras-punch.md b/.changeset/afraid-zebras-punch.md new file mode 100644 index 00000000000..37885b9af9f --- /dev/null +++ b/.changeset/afraid-zebras-punch.md @@ -0,0 +1,27 @@ +--- +"@apollo/client": minor +--- + +Add generic type parameter for the cache `Modifiers` type. Improves TypeScript +type inference for that type's fields and values of those fields. + +Example: + +```ts +cache.modify({ + id: cache.identify(someBook), + fields: { + title: (title) => { + // title has type `string`. + // It used to be `any`. + }, + author: (author) => { + // author has type `Reference | Book["author"]`. + // It used to be `any`. + }, + } satisfies Modifiers, +}); +``` + +To take advantage of the type inference, use [the `satisfies Modifiers<...>` +operator](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-9.html). diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts index 4086d34a45e..79704962b25 100644 --- a/src/cache/core/types/Cache.ts +++ b/src/cache/core/types/Cache.ts @@ -59,7 +59,7 @@ export namespace Cache { export interface ModifyOptions { id?: string; - fields: Modifiers | Modifier; + fields: Modifiers> | Modifier; optimistic?: boolean; broadcast?: boolean; } diff --git a/src/cache/core/types/common.ts b/src/cache/core/types/common.ts index 359f461429c..7448e366a07 100644 --- a/src/cache/core/types/common.ts +++ b/src/cache/core/types/common.ts @@ -98,6 +98,17 @@ export type Modifier = ( details: ModifierDetails ) => T | DeleteModifier | InvalidateModifier; -export type Modifiers = { - [fieldName: string]: Modifier; -}; +type StoreObjectValueMaybeReference = StoreVal extends Record< + string, + unknown +>[] + ? StoreVal | Reference[] + : StoreVal extends Record + ? StoreVal | Reference + : StoreVal; + +export type Modifiers> = Partial<{ + [FieldName in keyof T]: Modifier< + StoreObjectValueMaybeReference> + >; +}>; diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts index 8d978c4ce6c..985989dc466 100644 --- a/src/cache/inmemory/__tests__/cache.ts +++ b/src/cache/inmemory/__tests__/cache.ts @@ -2,7 +2,7 @@ import gql, { disableFragmentWarnings } from 'graphql-tag'; import { cloneDeep } from '../../../utilities/common/cloneDeep'; import { makeReference, Reference, makeVar, TypedDocumentNode, isReference, DocumentNode } from '../../../core'; -import { Cache } from '../../../cache'; +import { Cache, Modifiers } from '../../../cache'; import { InMemoryCache } from '../inMemoryCache'; import { InMemoryCacheConfig } from '../types'; @@ -3918,18 +3918,43 @@ describe('TypedDocumentNode', () => { } `; - it('should determine Data and Variables types of {write,read}{Query,Fragment}', () => { - const cache = new InMemoryCache({ + // We need to define these objects separately from calling writeQuery, + // because passing them directly to writeQuery will trigger excess property + // warnings due to the extra __typename and isbn fields. Internally, we + // almost never pass object literals to writeQuery or writeFragment, so + // excess property checks should not be a problem in practice. + const jcmAuthor = { + __typename: "Author", + name: "John C. Mitchell", + }; + + const ffplBook = { + __typename: "Book", + isbn: "0262133210", + title: "Foundations for Programming Languages", + author: jcmAuthor, + }; + + const ffplVariables = { + isbn: "0262133210", + }; + + function getBookCache() { + return new InMemoryCache({ typePolicies: { Query: { fields: { book(existing, { args, toReference }) { - return existing ?? (args && toReference({ - __typename: "Book", - isbn: args.isbn, - })); - } - } + return ( + existing ?? + (args && + toReference({ + __typename: "Book", + isbn: args.isbn, + })) + ); + }, + }, }, Book: { @@ -3941,27 +3966,10 @@ describe('TypedDocumentNode', () => { }, }, }); + } - // We need to define these objects separately from calling writeQuery, - // because passing them directly to writeQuery will trigger excess property - // warnings due to the extra __typename and isbn fields. Internally, we - // almost never pass object literals to writeQuery or writeFragment, so - // excess property checks should not be a problem in practice. - const jcmAuthor = { - __typename: "Author", - name: "John C. Mitchell", - }; - - const ffplBook = { - __typename: "Book", - isbn: "0262133210", - title: "Foundations for Programming Languages", - author: jcmAuthor, - }; - - const ffplVariables = { - isbn: "0262133210", - }; + it("should determine Data and Variables types of {write,read}{Query,Fragment}", () => { + const cache = getBookCache(); cache.writeQuery({ query, @@ -4041,4 +4049,48 @@ describe('TypedDocumentNode', () => { }, }); }); + + it("should infer the types of modifier fields", () => { + const cache = getBookCache(); + + cache.writeQuery({ + query, + variables: ffplVariables, + data: { + book: ffplBook, + }, + }); + + /** Asserts the inferred TypeScript type of a value matches the expected type */ + const assertType = + () => + ( + _value: ActualType + ): ExpectedType extends ActualType ? void : never => + void 0 as any; + + cache.modify({ + id: cache.identify(ffplBook), + fields: { + isbn: (value) => { + assertType()(value); + return value; + }, + title: (value, { INVALIDATE }) => { + assertType()(value); + return INVALIDATE; + }, + author: (value, { DELETE, isReference }) => { + assertType()(value); + if (isReference(value)) { + assertType()(value); + } else { + assertType()(value); + } + + return DELETE; + }, + } satisfies Modifiers, + }); + }); }); diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts index 9bf0a764a1e..a7ab2b8bfe3 100644 --- a/src/cache/inmemory/entityStore.ts +++ b/src/cache/inmemory/entityStore.ts @@ -193,7 +193,7 @@ export abstract class EntityStore implements NormalizedCache { public modify( dataId: string, - fields: Modifier | Modifiers, + fields: Modifier | Modifiers>, ): boolean { const storeObject = this.lookup(dataId); @@ -224,7 +224,7 @@ export abstract class EntityStore implements NormalizedCache { const fieldName = fieldNameFromStoreName(storeFieldName); let fieldValue = storeObject[storeFieldName]; if (fieldValue === void 0) return; - const modify: Modifier = typeof fields === "function" + const modify: Modifier | undefined = typeof fields === "function" ? fields : fields[storeFieldName] || fields[fieldName]; if (modify) { diff --git a/src/cache/inmemory/types.ts b/src/cache/inmemory/types.ts index d6a52ef809e..57437436e63 100644 --- a/src/cache/inmemory/types.ts +++ b/src/cache/inmemory/types.ts @@ -49,7 +49,7 @@ export interface NormalizedCache { merge(olderId: string, newerObject: StoreObject): void; merge(olderObject: StoreObject, newerId: string): void; - modify(dataId: string, fields: Modifiers | Modifier): boolean; + modify(dataId: string, fields: Modifiers> | Modifier): boolean; delete(dataId: string, fieldName?: string): boolean; clear(): void;