Disallow reading entire StoreObject from EntityStore by ID.

One of the most important internal changes in Apollo Client 3.0 is that
the result caching system now tracks its dependencies at the field level,
rather than at the level of whole entity objects: #5617

As proof that this transformation is complete, we can finally remove the
ability to read entire entity objects from the EntityStore by ID, so that
the only way to read from the store is by providing both an ID and the
name of a field. The logic I improved in #5772 is now even simpler,
without the need for overloading multiple EntityStore#get signatures.

In the process, I noticed that the EntityStore#has(ID) method was
accidentally depending on any changes to the contents of the identified
entity, even though store.has only cares about the existence of the ID in
the store. This was only a problem if we called store.has during a larger
read operation, which currently only happens when InMemoryCache#read is
called with options.rootId. The symptoms of this oversight went unnoticed
because the caching of readFragment results was just a little more
sensitive to changes to the given entity. The results themselves were
still logically correct, but their caching was not as effective as it
could be. That's the beauty and the curse of caching: you might not notice
when it isn't working, because all your tests still pass, and nobody
complains that their application is broken.

Fortunately, we can model this dependency with an ID plus a fake field
name called "__exists", which is only dirtied when the ID is newly added
to or removed from the store.

src/cache/inmemory/__tests__/cache.ts

@@ -597,6 +597,127 @@ describe('Cache', () => {
         ).toEqual({ a: 1, b: 2, c: 3 });
       },
     );
+
+    it("should not accidentally depend on unrelated entity fields", () => {
+      const cache = new InMemoryCache({
+        resultCaching: true,
+      });
+
+      const firstNameFragment = gql`
+        fragment FirstNameFragment on Person {
+          firstName
+        }
+      `;
+
+      const lastNameFragment = gql`
+        fragment LastNameFragment on Person {
+          lastName
+        }
+      `;
+
+      const bothNamesData = {
+        __typename: "Person",
+        id: 123,
+        firstName: "Ben",
+        lastName: "Newman",
+      };
+
+      const id = cache.identify(bothNamesData);
+
+      cache.writeFragment({
+        id,
+        fragment: firstNameFragment,
+        data: bothNamesData,
+      });
+
+      expect(cache.extract()).toEqual({
+        "Person:123": {
+          __typename: "Person",
+          firstName: "Ben",
+        },
+      });
+
+      const firstNameResult = cache.readFragment({
+        id,
+        fragment: firstNameFragment,
+      });
+
+      expect(firstNameResult).toEqual({
+        __typename: "Person",
+        firstName: "Ben",
+      });
+
+      cache.writeFragment({
+        id,
+        fragment: lastNameFragment,
+        data: bothNamesData,
+      });
+
+      expect(cache.extract()).toEqual({
+        "Person:123": {
+          __typename: "Person",
+          firstName: "Ben",
+          lastName: "Newman",
+        },
+      });
+
+      // This is the crucial test: modifying the lastName field should not
+      // invalidate results that did not depend on the lastName field.
+      expect(cache.readFragment({
+        id,
+        fragment: firstNameFragment,
+      })).toBe(firstNameResult);
+
+      const lastNameResult = cache.readFragment({
+        id,
+        fragment: lastNameFragment,
+      });
+
+      expect(lastNameResult).toEqual({
+        __typename: "Person",
+        lastName: "Newman",
+      });
+
+      cache.writeFragment({
+        id,
+        fragment: firstNameFragment,
+        data: {
+          ...bothNamesData,
+          firstName: "Benjamin",
+        },
+      });
+
+      expect(cache.extract()).toEqual({
+        "Person:123": {
+          __typename: "Person",
+          firstName: "Benjamin",
+          lastName: "Newman",
+        },
+      });
+
+      const benjaminResult = cache.readFragment({
+        id,
+        fragment: firstNameFragment,
+      });
+
+      expect(benjaminResult).toEqual({
+        __typename: "Person",
+        firstName: "Benjamin",
+      });
+
+      // Still the same as it was?
+      expect(firstNameResult).toEqual({
+        __typename: "Person",
+        firstName: "Ben",
+      });
+
+      // Updating the firstName should not have invalidated the
+      // previously-read lastNameResult.
+      expect(cache.readFragment({
+        id,
+        fragment: lastNameFragment,
+      })).toBe(lastNameResult);
+    });
   });
 
   describe('writeQuery', () => {

src/cache/inmemory/__tests__/diffAgainstStore.ts

@@ -179,7 +179,7 @@ describe('diffing queries against the store', () => {
     });
 
     expect(complete).toBeTruthy();
-    expect(store.get('Person:{"id":"1"}')).toEqual({
+    expect((store as any).lookup('Person:{"id":"1"}')).toEqual({
       __typename: 'Person',
       id: '1',
       name: 'Luke Skywalker',

src/cache/inmemory/__tests__/recordingCache.ts

@@ -1,11 +1,15 @@
-import { NormalizedCacheObject } from '../types';
+import { NormalizedCacheObject, StoreObject } from '../types';
 import { EntityStore } from '../entityStore';
 
 describe('Optimistic EntityStore layering', () => {
   function makeLayer(root: EntityStore) {
     return root.addLayer('whatever', () => {});
   }
 
+  function lookup(store: EntityStore, dataId: string) {
+    return (store as any).lookup(dataId) as StoreObject;
+  }
+
   describe('returns correct values during recording', () => {
     const data = {
       Human: { __typename: 'Human', name: 'Mark' },
@@ -24,23 +28,23 @@ describe('Optimistic EntityStore layering', () => {
     });
 
     it('should passthrough values if not defined in recording', () => {
-      expect(store.get('Human')).toBe(data.Human);
-      expect(store.get('Animal')).toBe(data.Animal);
+      expect(lookup(store, 'Human')).toBe(data.Human);
+      expect(lookup(store, 'Animal')).toBe(data.Animal);
     });
 
     it('should return values defined during recording', () => {
       store.merge('Human', dataToRecord.Human);
-      expect(store.get('Human')).toEqual(dataToRecord.Human);
-      expect(underlyingStore.get('Human')).toBe(data.Human);
+      expect(lookup(store, 'Human')).toEqual(dataToRecord.Human);
+      expect(lookup(underlyingStore, 'Human')).toBe(data.Human);
     });
 
     it('should return undefined for values deleted during recording', () => {
-      expect(store.get('Animal')).toBe(data.Animal);
+      expect(lookup(store, 'Animal')).toBe(data.Animal);
       // delete should be registered in the recording:
       store.delete('Animal');
-      expect(store.get('Animal')).toBeUndefined();
+      expect(lookup(store, 'Animal')).toBeUndefined();
       expect(store.toObject()).toHaveProperty('Animal');
-      expect(underlyingStore.get('Animal')).toBe(data.Animal);
+      expect(lookup(underlyingStore, 'Animal')).toBe(data.Animal);
     });
   });
 

src/cache/inmemory/__tests__/writeToStore.ts

@@ -1507,7 +1507,7 @@ describe('writing to the store', () => {
     });
 
     Object.keys(store.toObject()).forEach(field => {
-      expect(store.get(field)).toEqual(newStore.get(field));
+      expect((store as any).lookup(field)).toEqual((newStore as any).lookup(field));
     });
   });
 
@@ -1543,7 +1543,7 @@ describe('writing to the store', () => {
         result,
       });
 
-      expect(newStore.get('1')).toEqual(result.todos[0]);
+      expect((newStore as any).lookup('1')).toEqual(result.todos[0]);
     });
 
     it('should warn when it receives the wrong data with non-union fragments', () => {
@@ -1569,7 +1569,7 @@ describe('writing to the store', () => {
           result,
         });
 
-        expect(newStore.get('1')).toEqual(result.todos[0]);
+        expect((newStore as any).lookup('1')).toEqual(result.todos[0]);
       }, /Missing field description/);
     });
 
@@ -1623,7 +1623,7 @@ describe('writing to the store', () => {
           result,
         });
 
-        expect(newStore.get('1')).toEqual(result.todos[0]);
+        expect((newStore as any).lookup('1')).toEqual(result.todos[0]);
       }, /Missing field price/);
     });
 
@@ -1651,7 +1651,7 @@ describe('writing to the store', () => {
           result,
         });
 
-        expect(newStore.get('1')).toEqual(result.todos[0]);
+        expect((newStore as any).lookup('1')).toEqual(result.todos[0]);
       }, /Missing field __typename/);
     });
 
@@ -1671,7 +1671,7 @@ describe('writing to the store', () => {
         result,
       });
 
-      expect(newStore.get('ROOT_QUERY')).toEqual({
+      expect((newStore as any).lookup('ROOT_QUERY')).toEqual({
         __typename: 'Query',
         todos: null,
       });
@@ -1700,7 +1700,7 @@ describe('writing to the store', () => {
         result,
       });
 
-      expect(newStore.get('ROOT_QUERY')).toEqual({ __typename: 'Query', id: 1 });
+      expect((newStore as any).lookup('ROOT_QUERY')).toEqual({ __typename: 'Query', id: 1 });
       expect(console.warn).not.toBeCalled();
       console.warn = originalWarn;
     });