Require Cache.EvictOptions when calling cache.evict.

CHANGELOG.md

@@ -59,8 +59,9 @@
 - `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)
 
-- The `cache.evict` method can optionally take an arguments object as its third parameter (following the entity ID and field name), to delete only those field values with specific arguments. <br/>
+- **[beta-BREAKING]** Please note that the `cache.evict` method now requires `Cache.EvictOptions`, though it previously supported positional arguments as well. <br/>
   [@danReynolds](https://github.com/danReynolds) in [#6141](https://github.com/apollographql/apollo-client/pull/6141)
+  [@benjamn](https://github.com/benjamn) in [#6364](https://github.com/apollographql/apollo-client/pull/6364)
 
 - Cache methods that would normally trigger a broadcast, like `cache.evict`, `cache.writeQuery`, and `cache.writeFragment`, can now be called with a named options object, which supports a `broadcast: boolean` property that can be used to silence the broadcast, for situations where you want to update the cache multiple times without triggering a broadcast each time. <br/>
   [@benjamn](https://github.com/benjamn) in [#6288](https://github.com/apollographql/apollo-client/pull/6288)

src/__tests__/client.ts

@@ -2913,7 +2913,7 @@ describe('@connection', () => {
     expect(checkLastResult(abResults, a456bOyez)).toBe(a456bOyez);
 
     // Now invalidate the ROOT_QUERY.a field.
-    client.cache.evict("ROOT_QUERY", "a");
+    client.cache.evict({ fieldName: "a" });
     await wait();
 
     // The results are structurally the same, but the result objects have
@@ -2957,7 +2957,7 @@ describe('@connection', () => {
     checkLastResult(abResults, a456bOyez);
     checkLastResult(cResults, { c: "saw" });
 
-    client.cache.evict("ROOT_QUERY", "c");
+    client.cache.evict({ fieldName: "c" });
     await wait();
 
     checkLastResult(aResults, a456);

src/cache/core/cache.ts

@@ -28,14 +28,6 @@ export abstract class ApolloCache<TSerialized> implements DataProxy {
   // removed from the cache.
   public abstract evict(options: Cache.EvictOptions): boolean;
 
-  // For backwards compatibility, evict can also take positional
-  // arguments. Please prefer the Cache.EvictOptions style (above).
-  public abstract evict(
-    id: string,
-    field?: string,
-    args?: Record<string, any>,
-  ): boolean;
-
   // intializer / offline / ssr API
   /**
    * Replaces existing state in the cache (if any) with the values expressed by

src/cache/core/types/Cache.ts

@@ -28,7 +28,7 @@ export namespace Cache {
   }
 
   export interface EvictOptions {
-    id: string;
+    id?: string;
     fieldName?: string;
     args?: Record<string, any>;
     broadcast?: boolean;

src/cache/inmemory/__tests__/entityStore.ts

@@ -620,7 +620,7 @@ describe('EntityStore', () => {
       },
     });
 
-    expect(cache.evict("Author:J.K. Rowling")).toBe(false);
+    expect(cache.evict({ id: "Author:J.K. Rowling" })).toBe(false);
 
     const bookAuthorFragment = gql`
       fragment BookAuthor on Book {
@@ -689,7 +689,7 @@ describe('EntityStore', () => {
 
     expect(cache.gc()).toEqual([]);
 
-    expect(cache.evict("Author:Robert Galbraith")).toBe(true);
+    expect(cache.evict({ id: "Author:Robert Galbraith" })).toBe(true);
 
     expect(cache.gc()).toEqual([]);
 
@@ -754,9 +754,27 @@ describe('EntityStore', () => {
 
     expect(cache.gc()).toEqual([]);
 
-    // If you're ever tempted to do this, you probably want to use cache.clear()
-    // instead, but evicting the ROOT_QUERY should work at least.
-    expect(cache.evict("ROOT_QUERY")).toBe(true);
+    function checkFalsyEvictId(id: any) {
+      expect(id).toBeFalsy();
+      expect(cache.evict({
+        // Accidentally passing a falsy/undefined options.id to
+        // cache.evict (perhaps because cache.identify failed) should
+        // *not* cause the ROOT_QUERY object to be evicted! In order for
+        // cache.evict to default to ROOT_QUERY, the options.id property
+        // must be *absent* (not just undefined).
+        id,
+      })).toBe(false);
+    }
+    checkFalsyEvictId(void 0);
+    checkFalsyEvictId(null);
+    checkFalsyEvictId(false);
+    checkFalsyEvictId(0);
+    checkFalsyEvictId("");
+
+    // In other words, this is how you evict the entire ROOT_QUERY
+    // object. If you're ever tempted to do this, you probably want to use
+    // cache.clear() instead, but evicting the ROOT_QUERY should work.
+    expect(cache.evict({})).toBe(true);
 
     expect(cache.extract(true)).toEqual({
       "Book:031648637X": {
@@ -1177,7 +1195,10 @@ describe('EntityStore', () => {
       },
     });
 
-    cache.evict('ROOT_QUERY', 'authorOfBook', { isbn: "1" });
+    cache.evict({
+      fieldName: 'authorOfBook',
+      args: { isbn: "1" },
+    });
 
     expect(cache.extract()).toEqual({
       ROOT_QUERY: {
@@ -1195,7 +1216,10 @@ describe('EntityStore', () => {
       },
     });
 
-    cache.evict('ROOT_QUERY', 'authorOfBook', { isbn: '3' });
+    cache.evict({
+      fieldName: 'authorOfBook',
+      args: { isbn: '3' },
+    });
 
     expect(cache.extract()).toEqual({
       ROOT_QUERY: {
@@ -1213,7 +1237,10 @@ describe('EntityStore', () => {
       },
     });
 
-    cache.evict('ROOT_QUERY', 'authorOfBook', {});
+    cache.evict({
+      fieldName: 'authorOfBook',
+      args: {},
+    });
 
     expect(cache.extract()).toEqual({
       ROOT_QUERY: {
@@ -1226,7 +1253,9 @@ describe('EntityStore', () => {
       },
     });
 
-    cache.evict('ROOT_QUERY', 'authorOfBook');;
+    cache.evict({
+      fieldName: 'authorOfBook',
+    });
 
     expect(cache.extract()).toEqual({
       ROOT_QUERY: {
@@ -1506,12 +1535,14 @@ describe('EntityStore', () => {
       query: queryWithoutAliases,
     })).toBe(resultWithoutAliases);
 
-    cache.evict(cache.identify({
-      __typename: "ABCs",
-      a: "ay",
-      b: "bee",
-      c: "see",
-    })!);
+    cache.evict({
+      id: cache.identify({
+        __typename: "ABCs",
+        a: "ay",
+        b: "bee",
+        c: "see",
+      }),
+    });
 
     expect(cache.extract()).toEqual({
       ROOT_QUERY: {
@@ -1590,7 +1621,7 @@ describe('EntityStore', () => {
       id: 2,
     })!;
 
-    expect(cache.evict(authorId)).toBe(true);
+    expect(cache.evict({ id: authorId })).toBe(true);
 
     expect(cache.extract(true)).toEqual({
       "Book:1": {
@@ -1604,7 +1635,7 @@ describe('EntityStore', () => {
       },
     });
 
-    expect(cache.evict(authorId)).toBe(false);
+    expect(cache.evict({ id: authorId })).toBe(false);
 
     const missing = [
       new MissingFieldError(

src/cache/inmemory/__tests__/policies.ts

@@ -2513,7 +2513,9 @@ describe("type policies", function () {
 
       expect(cache.gc()).toEqual([]);
 
-      expect(cache.evict("ROOT_QUERY", "book")).toBe(true);
+      expect(cache.evict({
+        fieldName: "book",
+      })).toBe(true);
 
       expect(cache.gc().sort()).toEqual([
         'Book:{"isbn":"0393354326"}',

src/cache/inmemory/entityStore.ts

@@ -214,17 +214,19 @@ export abstract class EntityStore implements NormalizedCache {
 
   public evict(options: Cache.EvictOptions): boolean {
     let evicted = false;
-    if (hasOwn.call(this.data, options.id)) {
-      evicted = this.delete(options.id, options.fieldName, options.args);
-    }
-    if (this instanceof Layer) {
-      evicted = this.parent.evict(options) || evicted;
+    if (options.id) {
+      if (hasOwn.call(this.data, options.id)) {
+        evicted = this.delete(options.id, options.fieldName, options.args);
+      }
+      if (this instanceof Layer) {
+        evicted = this.parent.evict(options) || evicted;
+      }
+      // Always invalidate the field to trigger rereading of watched
+      // queries, even if no cache data was modified by the eviction,
+      // because queries may depend on computed fields with custom read
+      // functions, whose values are not stored in the EntityStore.
+      this.group.dirty(options.id, options.fieldName || "__exists");
     }
-    // Always invalidate the field to trigger rereading of watched
-    // queries, even if no cache data was modified by the eviction,
-    // because queries may depend on computed fields with custom read
-    // functions, whose values are not stored in the EntityStore.
-    this.group.dirty(options.id, options.fieldName || "__exists");
     return evicted;
   }
 

src/cache/inmemory/inMemoryCache.ts

@@ -236,28 +236,24 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
     return this.policies.identify(object)[0];
   }
 
-  public evict(
-    idOrOptions: string | Cache.EvictOptions,
-    fieldName?: string,
-    args?: Record<string, any>,
-  ): boolean {
+  public evict(options: Cache.EvictOptions): boolean {
+    if (!options.id) {
+      if (hasOwn.call(options, "id")) {
+        // See comment in modify method about why we return false when
+        // options.id exists but is falsy/undefined.
+        return false;
+      }
+      options = { ...options, id: "ROOT_QUERY" };
+    }
     try {
       // It's unlikely that the eviction will end up invoking any other
       // cache update operations while it's running, but {in,de}crementing
       // this.txCount still seems like a good idea, for uniformity with
       // the other update methods.
       ++this.txCount;
-      return this.optimisticData.evict(
-        typeof idOrOptions === "string" ? {
-          id: idOrOptions,
-          fieldName,
-          args,
-        } : idOrOptions,
-      );
+      return this.optimisticData.evict(options);
     } finally {
-      if (!--this.txCount &&
-          (typeof idOrOptions === "string" ||
-           idOrOptions.broadcast !== false)) {
+      if (!--this.txCount && options.broadcast !== false) {
         this.broadcastWatches();
       }
     }