From 0faf2222f9a14e082600424a6e52fa5a59434e81 Mon Sep 17 00:00:00 2001
From: Grzegorz Rozdzialik <voreny.gelio@gmail.com>
Date: Fri, 19 May 2023 12:11:57 +0200
Subject: [PATCH 01/10] feat(cache): accept DELETE and INVALIDATE modifiers in
 the return type

Problem: The `DELETE` and `INVALIDATE` modifiers type is `any`.
Returning them from the `Modifier<T>` function triggers the
`@typescript-eslint/no-unsafe-return` ESLint rule [0],
since the modifier is then returning a value with type `any`.

Solution: Set the types of these modifiers to two different `unique
symbol`s. These serve as unique opaque types. The consuming code should
not peek into them. These 2 types are permitted in the return type of
the `Modifier<T>` function, since they are not `any` anymore, and thus,
are not assignable to `T`.

This change only affects TypeScript types. It has no runtime impact.

[0]: https://typescript-eslint.io/rules/no-unsafe-return/
---
 .changeset/cold-tips-accept.md    |  9 +++++++++
 src/cache/core/types/common.ts    | 14 +++++++++++---
 src/cache/inmemory/entityStore.ts |  9 ++++++---
 3 files changed, 26 insertions(+), 6 deletions(-)
 create mode 100644 .changeset/cold-tips-accept.md

diff --git a/.changeset/cold-tips-accept.md b/.changeset/cold-tips-accept.md
new file mode 100644
index 00000000000..0e8775403a9
--- /dev/null
+++ b/.changeset/cold-tips-accept.md
@@ -0,0 +1,9 @@
+---
+"@apollo/client": minor
+---
+
+Use unique opaque types for the `DELETE` and `INVALIDATE` Apollo cache modifiers.
+
+This increases type safety, since these 2 modifiers no longer have the `any` type.
+Moreover, it no longer triggers [the `@typescript-eslint/no-unsafe-return`
+rule](https://typescript-eslint.io/rules/no-unsafe-return/).
diff --git a/src/cache/core/types/common.ts b/src/cache/core/types/common.ts
index 4e45b84ad82..d6020d00d02 100644
--- a/src/cache/core/types/common.ts
+++ b/src/cache/core/types/common.ts
@@ -76,9 +76,14 @@ export type ToReferenceFunction = (
 
 export type CanReadFunction = (value: StoreValue) => boolean;
 
+declare const _deleteModifier: unique symbol;
+export type DeleteModifier = typeof _deleteModifier
+declare const _invalidateModifier: unique symbol;
+export type InvalidateModifier = typeof _invalidateModifier
+
 export type ModifierDetails = {
-  DELETE: any;
-  INVALIDATE: any;
+  DELETE: DeleteModifier;
+  INVALIDATE: InvalidateModifier;
   fieldName: string;
   storeFieldName: string;
   readField: ReadFieldFunction;
@@ -88,7 +93,10 @@ export type ModifierDetails = {
   storage: StorageType;
 }
 
-export type Modifier<T> = (value: T, details: ModifierDetails) => T;
+export type Modifier<T> = (
+  value: T,
+  details: ModifierDetails
+) => T | DeleteModifier | InvalidateModifier;
 
 export type Modifiers = {
   [fieldName: string]: Modifier<any>;
diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts
index 9631ca615ef..30ee5b3e528 100644
--- a/src/cache/inmemory/entityStore.ts
+++ b/src/cache/inmemory/entityStore.ts
@@ -27,11 +27,14 @@ import type {
   ReadFieldOptions,
   ToReferenceFunction,
   CanReadFunction,
+  InvalidateModifier,
+  DeleteModifier,
+  ModifierDetails,
 } from '../core/types/common';
 
-const DELETE: any = Object.create(null);
+const DELETE: DeleteModifier = Object.create(null);
 const delModifier: Modifier<any> = () => DELETE;
-const INVALIDATE: any = Object.create(null);
+const INVALIDATE: InvalidateModifier = Object.create(null);
 
 export abstract class EntityStore implements NormalizedCache {
   protected data: NormalizedCacheObject = Object.create(null);
@@ -217,7 +220,7 @@ export abstract class EntityStore implements NormalizedCache {
           } : fieldNameOrOptions,
           { store: this },
         ),
-      };
+      } satisfies Partial<ModifierDetails>;
 
       Object.keys(storeObject).forEach(storeFieldName => {
         const fieldName = fieldNameFromStoreName(storeFieldName);

From 5f84a0bd3db1a9ca9fe2c02d5b2c3332e4a5df96 Mon Sep 17 00:00:00 2001
From: Grzegorz Rozdzialik <voreny.gelio@gmail.com>
Date: Fri, 19 May 2023 13:21:27 +0200
Subject: [PATCH 02/10] 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<Book>,
+});
+```
+
+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 cb89169930b..7ffe6609a4f 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<any>;
+    fields: Modifiers<Record<string, any>> | Modifier<any>;
     optimistic?: boolean;
     broadcast?: boolean;
   }
diff --git a/src/cache/core/types/common.ts b/src/cache/core/types/common.ts
index d6020d00d02..e1bab708b09 100644
--- a/src/cache/core/types/common.ts
+++ b/src/cache/core/types/common.ts
@@ -98,6 +98,17 @@ export type Modifier<T> = (
   details: ModifierDetails
 ) => T | DeleteModifier | InvalidateModifier;
 
-export type Modifiers = {
-  [fieldName: string]: Modifier<any>;
-};
+type StoreObjectValueMaybeReference<StoreVal> = StoreVal extends Record<
+  string,
+  unknown
+>[]
+  ? StoreVal | Reference[]
+  : StoreVal extends Record<string, unknown>
+  ? StoreVal | Reference
+  : StoreVal;
+
+export type Modifiers<T extends Record<string, unknown>> = Partial<{
+  [FieldName in keyof T]: Modifier<
+    StoreObjectValueMaybeReference<Exclude<T[FieldName], undefined>>
+  >;
+}>;
diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts
index 470247db539..f8d9d9fd17a 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';
 
@@ -3905,18 +3905,43 @@ describe('TypedDocumentNode<Data, Variables>', () => {
     }
   `;
 
-  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: {
@@ -3928,27 +3953,10 @@ describe('TypedDocumentNode<Data, Variables>', () => {
         },
       },
     });
+  }
 
-    // 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,
@@ -4028,4 +4036,48 @@ describe('TypedDocumentNode<Data, Variables>', () => {
       },
     });
   });
+
+  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 =
+      <ExpectedType>() =>
+      <ActualType extends ExpectedType>(
+        _value: ActualType
+      ): ExpectedType extends ActualType ? void : never =>
+        void 0 as any;
+
+    cache.modify({
+      id: cache.identify(ffplBook),
+      fields: {
+        isbn: (value) => {
+          assertType<string>()(value);
+          return value;
+        },
+        title: (value, { INVALIDATE }) => {
+          assertType<string>()(value);
+          return INVALIDATE;
+        },
+        author: (value, { DELETE, isReference }) => {
+          assertType<Reference | Book["author"]>()(value);
+          if (isReference(value)) {
+            assertType<Reference>()(value);
+          } else {
+            assertType<Book["author"]>()(value);
+          }
+
+          return DELETE;
+        },
+      } satisfies Modifiers<Book>,
+    });
+  });
 });
diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts
index 30ee5b3e528..a7dd96be059 100644
--- a/src/cache/inmemory/entityStore.ts
+++ b/src/cache/inmemory/entityStore.ts
@@ -195,7 +195,7 @@ export abstract class EntityStore implements NormalizedCache {
 
   public modify(
     dataId: string,
-    fields: Modifier<any> | Modifiers,
+    fields: Modifier<any> | Modifiers<Record<string, any>>,
   ): boolean {
     const storeObject = this.lookup(dataId);
 
@@ -226,7 +226,7 @@ export abstract class EntityStore implements NormalizedCache {
         const fieldName = fieldNameFromStoreName(storeFieldName);
         let fieldValue = storeObject[storeFieldName];
         if (fieldValue === void 0) return;
-        const modify: Modifier<StoreValue> = typeof fields === "function"
+        const modify: Modifier<StoreValue> | 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 31a06caa20a..aff52a31ed4 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<any>): boolean;
+  modify(dataId: string, fields: Modifiers<Record<string, any>> | Modifier<any>): boolean;
   delete(dataId: string, fieldName?: string): boolean;
   clear(): void;
 

From 4568407063bd7822580177c61e94a239bc161259 Mon Sep 17 00:00:00 2001
From: Grzegorz Rozdzialik <voreny.gelio@gmail.com>
Date: Tue, 30 May 2023 17:59:28 +0200
Subject: [PATCH 03/10] refactor(cache): add default value for Modifiers
 generic parameter

Makes the change backwards-compatible.
---
 src/cache/core/types/common.ts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/cache/core/types/common.ts b/src/cache/core/types/common.ts
index e1bab708b09..44a99e956c2 100644
--- a/src/cache/core/types/common.ts
+++ b/src/cache/core/types/common.ts
@@ -107,7 +107,9 @@ type StoreObjectValueMaybeReference<StoreVal> = StoreVal extends Record<
   ? StoreVal | Reference
   : StoreVal;
 
-export type Modifiers<T extends Record<string, unknown>> = Partial<{
+export type Modifiers<
+  T extends Record<string, unknown> = Record<string, unknown>
+> = Partial<{
   [FieldName in keyof T]: Modifier<
     StoreObjectValueMaybeReference<Exclude<T[FieldName], undefined>>
   >;

From 9f7d536a24b1f7dd20c977491518faab409d05d4 Mon Sep 17 00:00:00 2001
From: Grzegorz Rozdzialik <voreny.gelio@gmail.com>
Date: Tue, 30 May 2023 17:59:49 +0200
Subject: [PATCH 04/10] refactor(cache): use interfaces for invalidate and
 delete modifier types

Interfaces retain their names in error messages and hover dialogs. Type
aliases did not.
---
 src/cache/core/types/common.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/cache/core/types/common.ts b/src/cache/core/types/common.ts
index 44a99e956c2..8be9c8c2c88 100644
--- a/src/cache/core/types/common.ts
+++ b/src/cache/core/types/common.ts
@@ -77,9 +77,9 @@ export type ToReferenceFunction = (
 export type CanReadFunction = (value: StoreValue) => boolean;
 
 declare const _deleteModifier: unique symbol;
-export type DeleteModifier = typeof _deleteModifier
+export interface DeleteModifier { [_deleteModifier]: true }
 declare const _invalidateModifier: unique symbol;
-export type InvalidateModifier = typeof _invalidateModifier
+export interface InvalidateModifier { [_invalidateModifier]: true}
 
 export type ModifierDetails = {
   DELETE: DeleteModifier;

From 1cfb03b12bbc485a4cd018bab0f4837659ec6c33 Mon Sep 17 00:00:00 2001
From: Grzegorz Rozdzialik <voreny.gelio@gmail.com>
Date: Tue, 30 May 2023 18:28:10 +0200
Subject: [PATCH 05/10] refactor(cache): add generic parameter for
 ModifyOptions

A generic parameter is easier to discover and set than
`fields: { ... } satisfies Modifiers<...>`
---
 src/cache/core/cache.ts               | 2 +-
 src/cache/core/types/Cache.ts         | 6 ++++--
 src/cache/inmemory/__tests__/cache.ts | 4 ++--
 src/cache/inmemory/inMemoryCache.ts   | 2 +-
 src/cache/inmemory/types.ts           | 3 ++-
 5 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/cache/core/cache.ts b/src/cache/core/cache.ts
index 30069aa89be..77d0dc3948a 100644
--- a/src/cache/core/cache.ts
+++ b/src/cache/core/cache.ts
@@ -120,7 +120,7 @@ export abstract class ApolloCache<TSerialized> implements DataProxy {
     return [];
   }
 
-  public modify(options: Cache.ModifyOptions): boolean {
+  public modify<Entity = any>(options: Cache.ModifyOptions<Entity>): boolean {
     return false;
   }
 
diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts
index 7ffe6609a4f..c79c37c6914 100644
--- a/src/cache/core/types/Cache.ts
+++ b/src/cache/core/types/Cache.ts
@@ -57,9 +57,11 @@ export namespace Cache {
     discardWatches?: boolean;
   }
 
-  export interface ModifyOptions {
+  export interface ModifyOptions<Entity = any> {
     id?: string;
-    fields: Modifiers<Record<string, any>> | Modifier<any>;
+    fields: Entity extends Record<string, unknown>
+      ? Modifiers<Entity> | Modifier<Entity>
+      : Modifier<Entity>;
     optimistic?: boolean;
     broadcast?: boolean;
   }
diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts
index f8d9d9fd17a..9ead45a9ea4 100644
--- a/src/cache/inmemory/__tests__/cache.ts
+++ b/src/cache/inmemory/__tests__/cache.ts
@@ -4056,7 +4056,7 @@ describe('TypedDocumentNode<Data, Variables>', () => {
       ): ExpectedType extends ActualType ? void : never =>
         void 0 as any;
 
-    cache.modify({
+    cache.modify<Book>({
       id: cache.identify(ffplBook),
       fields: {
         isbn: (value) => {
@@ -4077,7 +4077,7 @@ describe('TypedDocumentNode<Data, Variables>', () => {
 
           return DELETE;
         },
-      } satisfies Modifiers<Book>,
+      },
     });
   });
 });
diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts
index aa4d42713f3..24403e34ec4 100644
--- a/src/cache/inmemory/inMemoryCache.ts
+++ b/src/cache/inmemory/inMemoryCache.ts
@@ -214,7 +214,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
     }
   }
 
-  public modify(options: Cache.ModifyOptions): boolean {
+  public modify<Entity = any>(options: Cache.ModifyOptions<Entity>): boolean {
     if (hasOwn.call(options, "id") && !options.id) {
       // To my knowledge, TypeScript does not currently provide a way to
       // enforce that an optional property?:type must *not* be undefined
diff --git a/src/cache/inmemory/types.ts b/src/cache/inmemory/types.ts
index aff52a31ed4..29a74b87740 100644
--- a/src/cache/inmemory/types.ts
+++ b/src/cache/inmemory/types.ts
@@ -49,7 +49,8 @@ export interface NormalizedCache {
   merge(olderId: string, newerObject: StoreObject): void;
   merge(olderObject: StoreObject, newerId: string): void;
 
-  modify(dataId: string, fields: Modifiers<Record<string, any>> | Modifier<any>): boolean;
+  modify<Entity extends Record<string, any>>(dataId: string, fields: Modifiers<Entity>): boolean;
+  modify<Entity>(dataId: string, modifier: Modifier<Entity>): boolean;
   delete(dataId: string, fieldName?: string): boolean;
   clear(): void;
 

From 014f17d9d76775e06104cdfda8602c78a419daa7 Mon Sep 17 00:00:00 2001
From: Grzegorz Rozdzialik <voreny.gelio@gmail.com>
Date: Fri, 2 Jun 2023 11:02:19 +0200
Subject: [PATCH 06/10] refactor(cache): use Record<string, any> as the default
 Entity type

Use a more appropriate real-life default type for `cache.modify`.
---
 src/cache/core/cache.ts             | 2 +-
 src/cache/core/types/Cache.ts       | 2 +-
 src/cache/inmemory/inMemoryCache.ts | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/cache/core/cache.ts b/src/cache/core/cache.ts
index 77d0dc3948a..742fd959921 100644
--- a/src/cache/core/cache.ts
+++ b/src/cache/core/cache.ts
@@ -120,7 +120,7 @@ export abstract class ApolloCache<TSerialized> implements DataProxy {
     return [];
   }
 
-  public modify<Entity = any>(options: Cache.ModifyOptions<Entity>): boolean {
+  public modify<Entity = Record<string, any>>(options: Cache.ModifyOptions<Entity>): boolean {
     return false;
   }
 
diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts
index c79c37c6914..4efc0dd08a3 100644
--- a/src/cache/core/types/Cache.ts
+++ b/src/cache/core/types/Cache.ts
@@ -57,7 +57,7 @@ export namespace Cache {
     discardWatches?: boolean;
   }
 
-  export interface ModifyOptions<Entity = any> {
+  export interface ModifyOptions<Entity = Record<string, any>> {
     id?: string;
     fields: Entity extends Record<string, unknown>
       ? Modifiers<Entity> | Modifier<Entity>
diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts
index 24403e34ec4..f4cbaaa181d 100644
--- a/src/cache/inmemory/inMemoryCache.ts
+++ b/src/cache/inmemory/inMemoryCache.ts
@@ -214,7 +214,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
     }
   }
 
-  public modify<Entity = any>(options: Cache.ModifyOptions<Entity>): boolean {
+  public modify<Entity = Record<string, any>>(options: Cache.ModifyOptions<Entity>): boolean {
     if (hasOwn.call(options, "id") && !options.id) {
       // To my knowledge, TypeScript does not currently provide a way to
       // enforce that an optional property?:type must *not* be undefined

From 6973cd474bfe7b90ce56a7c142ec9ee7e62c43bd Mon Sep 17 00:00:00 2001
From: Lenz Weber-Tronic <lorenz.weber-tronic@apollographql.com>
Date: Mon, 12 Jun 2023 15:13:36 +0200
Subject: [PATCH 07/10] code review adjustments

---
 src/cache/core/__tests__/cache.ts     | 141 +++++++++++++++++++++++++-
 src/cache/core/cache.ts               |   2 +-
 src/cache/core/types/Cache.ts         |   8 +-
 src/cache/core/types/common.ts        |  18 ++--
 src/cache/inmemory/__tests__/cache.ts |   5 +-
 src/cache/inmemory/inMemoryCache.ts   |   2 +-
 6 files changed, 159 insertions(+), 17 deletions(-)

diff --git a/src/cache/core/__tests__/cache.ts b/src/cache/core/__tests__/cache.ts
index 5f07bb28e97..48660ffb715 100644
--- a/src/cache/core/__tests__/cache.ts
+++ b/src/cache/core/__tests__/cache.ts
@@ -2,7 +2,7 @@ import gql from 'graphql-tag';
 import { ApolloCache } from '../cache';
 import { Cache, DataProxy } from '../..';
 import { Reference } from '../../../utilities/graphql/storeUtils';
-
+import { expectTypeOf } from 'expect-type'
 class TestCache extends ApolloCache<unknown> {
   constructor() {
     super();
@@ -308,3 +308,142 @@ describe('abstract cache', () => {
     });
   });
 });
+
+describe.skip('Cache type tests', () => {
+  describe('modify', () => {
+    test('field types are inferred correctly from passed entity type', () => {
+      const cache = new TestCache();
+      cache.modify<{
+        prop1: string;
+        prop2: number;
+        child: {
+          someObject: true
+        },
+        children: {
+          anotherObject: false
+        }[]
+      }>({
+        fields: {
+          prop1(field) {
+            expectTypeOf(field).toEqualTypeOf<string>();
+            return field;
+          },
+          prop2(field) {
+            expectTypeOf(field).toEqualTypeOf<number>();
+            return field;
+          },
+          child(field) {
+            expectTypeOf(field).toEqualTypeOf<{ someObject: true } | Reference>();
+            return field;
+          },
+          children(field) {
+            expectTypeOf(field).toEqualTypeOf<(ReadonlyArray<{ anotherObject: false }>) | ReadonlyArray<Reference>>();
+            return field;
+          }
+        }
+      })
+    })
+    test('field method needs to return a value of the correct type', () => {
+      const cache = new TestCache();
+      cache.modify<{ p1: string, p2: string, p3: string, p4: string, p5: string }>({
+        fields: {
+          p1() { return "" },
+          // @ts-expect-error returns wrong type
+          p2() { return 1 },
+          // @ts-expect-error needs return statement
+          p3() {},
+          p4(_, { DELETE }) { return DELETE },
+          p5(_, { INVALIDATE }) { return INVALIDATE },
+        }
+      })
+    })
+    test('passing a function as `field` should infer all entity properties as possible input (interfaces)', () => {
+      interface ParentEntity {
+        prop1: string;
+        prop2: number;
+        child: ChildEntity;
+      }
+      interface ChildEntity {
+        prop1: boolean;
+        prop2: symbol;
+        children: OtherChildEntry[];
+      }
+      interface OtherChildEntry {
+        foo: false
+      }
+
+      const cache = new TestCache();
+      // with reference
+      cache.modify<ParentEntity>({
+        id: 'foo',
+        fields(field) {
+          expectTypeOf(field).toEqualTypeOf<string | number | ChildEntity | Reference>();
+          return field;
+        }
+      })
+      // without reference
+      cache.modify<ChildEntity>({
+        id: 'foo',
+        fields(field) {
+          expectTypeOf(field).toEqualTypeOf<boolean | symbol | readonly OtherChildEntry[] | readonly Reference[]>();
+          return field;
+        }
+      })
+    })
+    test('passing a function as `field` should infer all entity properties as possible input (types)', () => {
+      type ParentEntity = {
+        prop1: string;
+        prop2: number;
+        child: ChildEntity;
+      }
+      type ChildEntity = {
+        prop1: boolean;
+        prop2: symbol;
+        children: OtherChildEntry[];
+      }
+      type OtherChildEntry = {
+        foo: false
+      }
+
+      const cache = new TestCache();
+      // with reference
+      cache.modify<ParentEntity>({
+        id: 'foo',
+        fields(field) {
+          expectTypeOf(field).toEqualTypeOf<string | number | ChildEntity | Reference>();
+          return field;
+        }
+      })
+      // without reference
+      cache.modify<ChildEntity>({
+        id: 'foo',
+        fields(field) {
+          expectTypeOf(field).toEqualTypeOf<boolean | symbol | readonly OtherChildEntry[] | readonly Reference[]>();
+          return field;
+        }
+      })
+    })
+    test('passing a function as `field` w/o specifying an entity type', () => {
+      const cache = new TestCache();
+      cache.modify({
+        id: 'foo',
+        fields(field) {
+          expectTypeOf(field).toEqualTypeOf<any>();
+          return field;
+        }
+      });
+    });
+    test('passing a function as `field` property w/o specifying an entity type', () => {
+      const cache = new TestCache();
+      cache.modify({
+        id: 'foo',
+        fields: {
+          p1(field) {
+            expectTypeOf(field).toEqualTypeOf<any>();
+            return field;
+          }
+        }
+      });
+    });
+  });
+});
diff --git a/src/cache/core/cache.ts b/src/cache/core/cache.ts
index 742fd959921..e48f12ccaec 100644
--- a/src/cache/core/cache.ts
+++ b/src/cache/core/cache.ts
@@ -120,7 +120,7 @@ export abstract class ApolloCache<TSerialized> implements DataProxy {
     return [];
   }
 
-  public modify<Entity = Record<string, any>>(options: Cache.ModifyOptions<Entity>): boolean {
+  public modify<Entity extends Record<string, any> = Record<string, any>>(options: Cache.ModifyOptions<Entity>): boolean {
     return false;
   }
 
diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts
index 4efc0dd08a3..47504c287c7 100644
--- a/src/cache/core/types/Cache.ts
+++ b/src/cache/core/types/Cache.ts
@@ -1,5 +1,5 @@
 import { DataProxy } from './DataProxy';
-import type { Modifier, Modifiers } from './common';
+import type { AllFieldsModifier, Modifiers } from './common';;
 import type { ApolloCache } from '../cache';
 
 export namespace Cache {
@@ -57,11 +57,9 @@ export namespace Cache {
     discardWatches?: boolean;
   }
 
-  export interface ModifyOptions<Entity = Record<string, any>> {
+  export interface ModifyOptions<Entity extends Record<string, any> = Record<string, any>> {
     id?: string;
-    fields: Entity extends Record<string, unknown>
-      ? Modifiers<Entity> | Modifier<Entity>
-      : Modifier<Entity>;
+    fields: Modifiers<Entity> | AllFieldsModifier<Entity>;
     optimistic?: boolean;
     broadcast?: boolean;
   }
diff --git a/src/cache/core/types/common.ts b/src/cache/core/types/common.ts
index 8be9c8c2c88..3a1cdcf4df6 100644
--- a/src/cache/core/types/common.ts
+++ b/src/cache/core/types/common.ts
@@ -98,17 +98,21 @@ export type Modifier<T> = (
   details: ModifierDetails
 ) => T | DeleteModifier | InvalidateModifier;
 
-type StoreObjectValueMaybeReference<StoreVal> = StoreVal extends Record<
-  string,
-  unknown
->[]
-  ? StoreVal | Reference[]
-  : StoreVal extends Record<string, unknown>
+type StoreObjectValueMaybeReference<StoreVal> = 
+  StoreVal extends Record<string, any>[]
+  ? Readonly<StoreVal> | readonly Reference[]
+  : StoreVal extends Record<string, any>
   ? StoreVal | Reference
   : StoreVal;
 
+export type AllFieldsModifier<
+  Entity extends Record<string, any>
+> = Modifier<Entity[keyof Entity] extends infer Value ? 
+  StoreObjectValueMaybeReference<Exclude<Value, undefined>>
+  : never>;
+
 export type Modifiers<
-  T extends Record<string, unknown> = Record<string, unknown>
+  T extends Record<string, any> = Record<string, unknown>
 > = Partial<{
   [FieldName in keyof T]: Modifier<
     StoreObjectValueMaybeReference<Exclude<T[FieldName], undefined>>
diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts
index 9ead45a9ea4..84d8a42bd7b 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, Modifiers } from '../../../cache';
+import { Cache } from '../../../cache';
 import { InMemoryCache } from '../inMemoryCache';
 import { InMemoryCacheConfig } from '../types';
 
@@ -2817,7 +2817,7 @@ describe("InMemoryCache#modify", () => {
 
     cache.modify({
       fields: {
-        comments(comments: Reference[], { readField }) {
+        comments(comments: readonly Reference[], { readField }) {
           expect(Object.isFrozen(comments)).toBe(true);
           expect(comments.length).toBe(3);
           const filtered = comments.filter(comment => {
@@ -2902,6 +2902,7 @@ describe("InMemoryCache#modify", () => {
         expect(fieldName).not.toBe("b");
         if (fieldName === "a") expect(value).toBe(1);
         if (fieldName === "c") expect(value).toBe(3);
+        return value;
       },
       optimistic: true,
     });
diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts
index f4cbaaa181d..49b33c59397 100644
--- a/src/cache/inmemory/inMemoryCache.ts
+++ b/src/cache/inmemory/inMemoryCache.ts
@@ -214,7 +214,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
     }
   }
 
-  public modify<Entity = Record<string, any>>(options: Cache.ModifyOptions<Entity>): boolean {
+  public modify<Entity extends Record<string, any> = Record<string, any>>(options: Cache.ModifyOptions<Entity>): boolean {
     if (hasOwn.call(options, "id") && !options.id) {
       // To my knowledge, TypeScript does not currently provide a way to
       // enforce that an optional property?:type must *not* be undefined

From 4c126268a02603dc9d86e18dcd2b359c22e9c047 Mon Sep 17 00:00:00 2001
From: Grzegorz Rozdzialik <voreny.gelio@gmail.com>
Date: Mon, 12 Jun 2023 19:58:26 +0200
Subject: [PATCH 08/10] refactor(cache-test): replace assertType with
 expect-type library

Use an already installed library instead of introducing a new utility
function.
---
 src/cache/inmemory/__tests__/cache.ts | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts
index 84d8a42bd7b..cfc0d86afd8 100644
--- a/src/cache/inmemory/__tests__/cache.ts
+++ b/src/cache/inmemory/__tests__/cache.ts
@@ -1,4 +1,5 @@
 import gql, { disableFragmentWarnings } from 'graphql-tag';
+import { expectTypeOf } from 'expect-type'
 
 import { cloneDeep } from '../../../utilities/common/cloneDeep';
 import { makeReference, Reference, makeVar, TypedDocumentNode, isReference, DocumentNode } from '../../../core';
@@ -4038,7 +4039,7 @@ describe('TypedDocumentNode<Data, Variables>', () => {
     });
   });
 
-  it("should infer the types of modifier fields", () => {
+  it.skip("should infer the types of modifier fields", () => {
     const cache = getBookCache();
 
     cache.writeQuery({
@@ -4049,31 +4050,23 @@ describe('TypedDocumentNode<Data, Variables>', () => {
       },
     });
 
-    /** Asserts the inferred TypeScript type of a value matches the expected type */
-    const assertType =
-      <ExpectedType>() =>
-      <ActualType extends ExpectedType>(
-        _value: ActualType
-      ): ExpectedType extends ActualType ? void : never =>
-        void 0 as any;
-
     cache.modify<Book>({
       id: cache.identify(ffplBook),
       fields: {
         isbn: (value) => {
-          assertType<string>()(value);
+          expectTypeOf(value).toEqualTypeOf<string>();
           return value;
         },
         title: (value, { INVALIDATE }) => {
-          assertType<string>()(value);
+          expectTypeOf(value).toEqualTypeOf<string>();
           return INVALIDATE;
         },
         author: (value, { DELETE, isReference }) => {
-          assertType<Reference | Book["author"]>()(value);
+          expectTypeOf(value).toEqualTypeOf<Reference | Book["author"]>();
           if (isReference(value)) {
-            assertType<Reference>()(value);
+            expectTypeOf(value).toEqualTypeOf<Reference>();
           } else {
-            assertType<Book["author"]>()(value);
+            expectTypeOf(value).toEqualTypeOf<Book["author"]>();
           }
 
           return DELETE;

From 18e8bf9bd868d7acff9593f271c227cf83995b5a Mon Sep 17 00:00:00 2001
From: Grzegorz Rozdzialik <voreny.gelio@gmail.com>
Date: Mon, 12 Jun 2023 19:59:26 +0200
Subject: [PATCH 09/10] chore: use generic parameter instead of satisfies in
 changeset

Promote the use of a generic parameter of `cache.modify` over using
`satisfies` which requires importing the `Modifers` type.
---
 .changeset/afraid-zebras-punch.md | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/.changeset/afraid-zebras-punch.md b/.changeset/afraid-zebras-punch.md
index 37885b9af9f..8869d5b5934 100644
--- a/.changeset/afraid-zebras-punch.md
+++ b/.changeset/afraid-zebras-punch.md
@@ -2,13 +2,13 @@
 "@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.
+Add generic type parameter for the entity modified in `cache.modify`. Improves
+TypeScript type inference for that type's fields and values of those fields.
 
 Example:
 
 ```ts
-cache.modify({
+cache.modify<Book>({
   id: cache.identify(someBook),
   fields: {
     title: (title) => {
@@ -19,9 +19,6 @@ cache.modify({
       // author has type `Reference | Book["author"]`.
       // It used to be `any`.
     },
-  } satisfies Modifiers<Book>,
+  },
 });
 ```
-
-To take advantage of the type inference, use [the `satisfies Modifiers<...>`
-operator](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-9.html).

From fc50711d9a4983bb124682b44a894921c5e9b714 Mon Sep 17 00:00:00 2001
From: Grzegorz Rozdzialik <voreny.gelio@gmail.com>
Date: Mon, 12 Jun 2023 20:06:14 +0200
Subject: [PATCH 10/10] fix(cache): use AllFieldsModifier type in cache.modify
 parameter

---
 src/cache/inmemory/types.ts | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/cache/inmemory/types.ts b/src/cache/inmemory/types.ts
index 29a74b87740..286649c8239 100644
--- a/src/cache/inmemory/types.ts
+++ b/src/cache/inmemory/types.ts
@@ -15,10 +15,10 @@ import type {
   FieldMergeFunction,
 } from './policies';
 import type {
-  Modifier,
   Modifiers,
   ToReferenceFunction,
   CanReadFunction,
+  AllFieldsModifier,
 } from '../core/types/common';
 
 import type { FragmentRegistryAPI } from './fragmentRegistry';
@@ -49,8 +49,7 @@ export interface NormalizedCache {
   merge(olderId: string, newerObject: StoreObject): void;
   merge(olderObject: StoreObject, newerId: string): void;
 
-  modify<Entity extends Record<string, any>>(dataId: string, fields: Modifiers<Entity>): boolean;
-  modify<Entity>(dataId: string, modifier: Modifier<Entity>): boolean;
+  modify<Entity extends Record<string, any>>(dataId: string, fields: Modifiers<Entity> | AllFieldsModifier<Entity>): boolean;
   delete(dataId: string, fieldName?: string): boolean;
   clear(): void;