Remove experimental cache.modify method. (#6289)

The `cache.modify` API was first introduced in #5909 as a quick way to
transform the values of specific existing fields in the cache.

At the time, `cache.modify` seemed promising as an alternative to the
`readQuery`-transform-`writeQuery` pattern, but feedback has been mixed,
most importantly from our developer experience team, who helped me
understand why `cache.modify` would be difficult to learn and to teach.

While my refactoring in #6221 addressed concerns about broadcasting
`cache.modify` updates automatically, the bigger problem with
`cache.modify` is simply that it requires knowledge of the internal
workings of the cache, making it nearly impossible to explain to
developers who are not already Apollo Client 3 experts.

As much as I wanted `cache.modify` to be a selling point for Apollo Client
3, it simply wasn't safe to use without a firm understanding of concepts
like cache normalization, references, field identity, read/merge functions
and their options API, and the `options.toReference(object, true)` helper
function (#5970).

By contrast, the `readQuery`-transform-`writeQuery` pattern may be a bit
more verbose, but it has none of these problems, because these older
methods work in terms of GraphQL result objects, rather than exposing the
internal data format of the `EntityStore`.

Since cache.modify was motivated to some extent by vague power-user
performance concerns, it's worth noting that we recently made writeQuery
and writeFragment even more efficient when rewriting unchanged results (#6274),
so whatever performance gap there might have been between cache.modify
and readQuery/writeQuery should now be even less noticeable.

One final reason that `cache.modify` seemed like a good idea was that
custom `merge` functions can interfere with certain `writeQuery` patterns,
such as deleting an item from a paginated list. If you run into trouble
with `merge` functions running when you don't want them to, we recommend
calling `cache.evict` before `cache.writeQuery` to ensure your `merge`
function won't be confused by existing field data when you write your
modified data back into the cache.

CHANGELOG.md

@@ -41,7 +41,7 @@
 - **[BREAKING]** `InMemoryCache` now _throws_ when data with missing or undefined query fields is written into the cache, rather than just warning in development. <br/>
   [@benjamn](https://github.com/benjamn) in [#6055](https://github.com/apollographql/apollo-client/pull/6055)
 
-- **[BREAKING]** `client|cache.writeData` have been fully removed. `writeData` usage is one of the easiest ways to turn faulty assumptions about how the cache represents data internally, into cache inconsistency and corruption. `client|cache.writeQuery`, `client|cache.writeFragment`, and/or `cache.modify` can be used to update the cache.  <br/>
+- **[BREAKING]** The `client|cache.writeData` methods have been fully removed, as `writeData` is one of the easiest ways to turn faulty assumptions about how the cache represents data internally into cache inconsistency and corruption. Instead, use `client|cache.writeQuery` or `client|cache.writeFragment` to update the cache. <br/>
   [@benjamn](https://github.com/benjamn) in [#5923](https://github.com/apollographql/apollo-client/pull/5923)
 
 - **[BREAKING]** Apollo Client will no longer deliver "stale" results to `ObservableQuery` consumers, but will instead log more helpful errors about which cache fields were missing. <br/>
@@ -56,6 +56,9 @@
 - **[BREAKING?]** Refactor `QueryManager` to make better use of observables and enforce `fetchPolicy` more reliably. <br/>
   [@benjamn](https://github.com/benjamn) in [#6221](https://github.com/apollographql/apollo-client/pull/6221)
 
+- **[beta-BREAKING]** The experimental `cache.modify` method, first introduced in [PR #5909](https://github.com/apollographql/apollo-client/pull/5909), has been removed. <br/>
+  [@benjamn](https://github.com/benjamn) in [#6289](https://github.com/apollographql/apollo-client/pull/6289)
+
 - `InMemoryCache` now supports tracing garbage collection and eviction. Note that the signature of the `evict` method has been simplified in a potentially backwards-incompatible way. <br/>
   [@benjamn](https://github.com/benjamn) in [#5310](https://github.com/apollographql/apollo-client/pull/5310)
 
@@ -86,18 +89,6 @@
 - The result caching system (introduced in [#3394](https://github.com/apollographql/apollo-client/pull/3394)) now tracks dependencies at the field level, rather than at the level of whole entity objects, allowing the cache to return identical (`===`) results much more often than before. <br/>
   [@benjamn](https://github.com/benjamn) in [#5617](https://github.com/apollographql/apollo-client/pull/5617)
 
-- `InMemoryCache` now has a method called `modify` which can be used to update the value of a specific field within a specific entity object:
-  ```ts
-  cache.modify({
-    comments(comments: Reference[], { readField }) {
-      return comments.filter(comment => idToRemove !== readField("id", comment));
-    },
-  }, cache.identify(post));
-  ```
-  This API gracefully handles cases where multiple field values are associated with a single field name, and also removes the need for updating the cache by reading a query or fragment, modifying the result, and writing the modified result back into the cache. Behind the scenes, the `cache.evict` method is now implemented in terms of `cache.modify`. <br/>
-  [@benjamn](https://github.com/benjamn) in [#5909](https://github.com/apollographql/apollo-client/pull/5909)
-  and [#6178](https://github.com/apollographql/apollo-client/pull/6178)
-
 - `InMemoryCache` provides a new API for storing client state that can be updated from anywhere:
   ```ts
   const v = cache.makeVar(123)
@@ -150,7 +141,7 @@
 - Make sure `ApolloContext` plays nicely with IE11 when storing the shared context.  <br/>
   [@ms](https://github.com/ms) in [#5840](https://github.com/apollographql/apollo-client/pull/5840)
 
-- Expose cache `modify` and `identify` to the mutate `update` function.  <br/>
+- Expose `cache.identify` to the mutation `update` function. <br/>
   [@hwillson](https://github.com/hwillson) in [#5956](https://github.com/apollographql/apollo-client/pull/5956)
 
 - Add a default `gc` implementation to `ApolloCache`.  <br/>

src/__tests__/client.ts

@@ -2942,19 +2942,6 @@ describe('@connection', () => {
     checkLastResult(abResults, a456bOyez);
     checkLastResult(cResults, { c: "see" });
 
-    cache.modify({
-      c(value) {
-        expect(value).toBe("see");
-        return "saw";
-      },
-    });
-    await wait();
-
-    checkLastResult(aResults, a456);
-    checkLastResult(bResults, bOyez);
-    checkLastResult(abResults, a456bOyez);
-    checkLastResult(cResults, { c: "saw" });
-
     client.cache.evict("ROOT_QUERY", "c");
     await wait();
 
@@ -2991,7 +2978,6 @@ describe('@connection', () => {
     expect(cResults).toEqual([
       {},
       { c: "see" },
-      { c: "saw" },
       {},
     ]);
 

src/__tests__/local-state/resolvers.ts

@@ -546,19 +546,22 @@ describe('Writing cache data from resolvers', () => {
       resolvers: {
         Mutation: {
           start() {
+            const obj = {
+              __typename: 'Object',
+              id: 'uniqueId',
+              field: 1,
+            };
+
             cache.writeQuery({
               query,
-              data: {
-                obj: { field: 1, id: 'uniqueId', __typename: 'Object' },
-              },
+              data: { obj },
             });
 
-            cache.modify({
-              field(value) {
-                expect(value).toBe(1);
-                return 2;
-              },
-            }, 'Object:uniqueId');
+            cache.writeFragment({
+              id: cache.identify(obj)!,
+              fragment: gql`fragment Field on Object { field }`,
+              data: { field: 2 },
+            });
 
             return { start: true };
           },
@@ -574,7 +577,7 @@ describe('Writing cache data from resolvers', () => {
       });
   });
 
-  it('should not overwrite __typename when writing to the cache with an id', () => {
+  itAsync('should not overwrite __typename when writing to the cache with an id', (resolve, reject) => {
     const query = gql`
       {
         obj @client {
@@ -600,22 +603,35 @@ describe('Writing cache data from resolvers', () => {
       resolvers: {
         Mutation: {
           start() {
+            const obj = {
+              __typename: 'Object',
+              id: 'uniqueId',
+              field: {
+                __typename: 'Field',
+                field2: 1,
+              },
+            };
+
             cache.writeQuery({
               query,
+              data: { obj },
+            });
+
+            cache.writeFragment({
+              id: cache.identify(obj)!,
+              fragment: gql`fragment FieldField2 on Object {
+                field {
+                  field2
+                }
+              }`,
               data: {
-                obj: {
-                  field: { field2: 1, __typename: 'Field' },
-                  id: 'uniqueId',
-                  __typename: 'Object',
+                field: {
+                  __typename: 'Field',
+                  field2: 2,
                 },
               },
             });
-            cache.modify({
-              field(value: { field2: number }) {
-                expect(value.field2).toBe(1);
-                return { ...value, field2: 2 };
-              },
-            }, 'Object:uniqueId');
+
             return { start: true };
           },
         },
@@ -628,8 +644,7 @@ describe('Writing cache data from resolvers', () => {
       .then(({ data }: any) => {
         expect(data.obj.__typename).toEqual('Object');
         expect(data.obj.field.__typename).toEqual('Field');
-      })
-      .catch(e => console.log(e));
+      }).then(resolve, reject);
   });
 });
 

src/cache/core/cache.ts

@@ -5,7 +5,6 @@ import { getFragmentQueryDocument } from '../../utilities/graphql/fragments';
 import { StoreObject } from '../../utilities/graphql/storeUtils';
 import { DataProxy } from './types/DataProxy';
 import { Cache } from './types/Cache';
-import { Modifier, Modifiers } from './types/common';
 
 export type Transaction<T> = (c: ApolloCache<T>) => void;
 
@@ -78,14 +77,6 @@ export abstract class ApolloCache<TSerialized> implements DataProxy {
   public identify(object: StoreObject): string | undefined {
     return;
   }
-
-  public modify(
-    modifiers: Modifier<any> | Modifiers,
-    dataId?: string,
-    optimistic?: boolean,
-  ): boolean {
-    return false;
-  }
 
   public gc(): string[] {
     return [];

src/cache/core/types/common.ts

@@ -1,14 +1,5 @@
 import { DocumentNode } from 'graphql';
 
-import {
-  isReference,
-  StoreValue,
-  StoreObject,
-  Reference
-} from '../../../utilities/graphql/storeUtils';
-
-import { ToReferenceFunction } from '../../inmemory/entityStore';
-
 // The Readonly<T> type only really works for object types, since it marks
 // all of the object's properties as readonly, but there are many cases when
 // a generic type parameter like TExisting might be a string or some other
@@ -18,22 +9,6 @@ import { ToReferenceFunction } from '../../inmemory/entityStore';
 // Readonly<any>, somewhat surprisingly.
 export type SafeReadonly<T> = T extends object ? Readonly<T> : T;
 
-export type Modifier<T> = (value: T, details: {
-  DELETE: any;
-  fieldName: string;
-  storeFieldName: string;
-  isReference: typeof isReference;
-  toReference: ToReferenceFunction;
-  readField<V = StoreValue>(
-    fieldName: string,
-    objOrRef?: StoreObject | Reference,
-  ): SafeReadonly<V>;
-}) => T;
-
-export type Modifiers = {
-  [fieldName: string]: Modifier<any>;
-}
-
 export class MissingFieldError {
   constructor(
     public readonly message: string,