feat(graphcache): only add dependency if we are changing the value (#…

…3564)

.changeset/nine-walls-behave.md

@@ -0,0 +1,5 @@
+---
+'@urql/exchange-graphcache': patch
+---
+
+Only record dependencies that are changing data, this will reduce the amount of operations we re-invoke due to network-only/cache-and-network queries and mutations

exchanges/graphcache/src/cacheExchange.test.ts

@@ -480,6 +480,167 @@ describe('data dependencies', () => {
     );
   });
 
+  it('does not notify related queries when a mutation update does not change the data', () => {
+    vi.useFakeTimers();
+
+    const balanceFragment = gql`
+      fragment BalanceFragment on Author {
+        id
+        balance {
+          amount
+        }
+      }
+    `;
+
+    const queryById = gql`
+      query ($id: ID!) {
+        author(id: $id) {
+          id
+          name
+          ...BalanceFragment
+        }
+      }
+
+      ${balanceFragment}
+    `;
+
+    const queryByIdDataA = {
+      __typename: 'Query',
+      author: {
+        __typename: 'Author',
+        id: '1',
+        name: 'Author 1',
+        balance: {
+          __typename: 'Balance',
+          amount: 100,
+        },
+      },
+    };
+
+    const queryByIdDataB = {
+      __typename: 'Query',
+      author: {
+        __typename: 'Author',
+        id: '2',
+        name: 'Author 2',
+        balance: {
+          __typename: 'Balance',
+          amount: 200,
+        },
+      },
+    };
+
+    const mutation = gql`
+      mutation ($userId: ID!, $amount: Int!) {
+        updateBalance(userId: $userId, amount: $amount) {
+          userId
+          balance {
+            amount
+          }
+        }
+      }
+    `;
+
+    const mutationData = {
+      __typename: 'Mutation',
+      updateBalance: {
+        __typename: 'UpdateBalanceResult',
+        userId: '1',
+        balance: {
+          __typename: 'Balance',
+          amount: 100,
+        },
+      },
+    };
+
+    const client = createClient({
+      url: 'http://0.0.0.0',
+      exchanges: [],
+    });
+    const { source: ops$, next } = makeSubject<Operation>();
+
+    const reexec = vi
+      .spyOn(client, 'reexecuteOperation')
+      .mockImplementation(next);
+
+    const opOne = client.createRequestOperation('query', {
+      key: 1,
+      query: queryById,
+      variables: { id: 1 },
+    });
+
+    const opTwo = client.createRequestOperation('query', {
+      key: 2,
+      query: queryById,
+      variables: { id: 2 },
+    });
+
+    const opMutation = client.createRequestOperation('mutation', {
+      key: 3,
+      query: mutation,
+      variables: { userId: '1', amount: 1000 },
+    });
+
+    const response = vi.fn((forwardOp: Operation): OperationResult => {
+      if (forwardOp.key === 1) {
+        return { ...queryResponse, operation: opOne, data: queryByIdDataA };
+      } else if (forwardOp.key === 2) {
+        return { ...queryResponse, operation: opTwo, data: queryByIdDataB };
+      } else if (forwardOp.key === 3) {
+        return {
+          ...queryResponse,
+          operation: opMutation,
+          data: mutationData,
+        };
+      }
+
+      return undefined as any;
+    });
+
+    const result = vi.fn();
+    const forward: ExchangeIO = ops$ =>
+      pipe(ops$, delay(1), map(response), share);
+
+    const updates = {
+      Mutation: {
+        updateBalance: vi.fn((result, _args, cache) => {
+          const {
+            updateBalance: { userId, balance },
+          } = result;
+          cache.writeFragment(balanceFragment, { id: userId, balance });
+        }),
+      },
+    };
+
+    const keys = {
+      Balance: () => null,
+    };
+
+    pipe(
+      cacheExchange({ updates, keys })({ forward, client, dispatchDebug })(
+        ops$
+      ),
+      tap(result),
+      publish
+    );
+
+    next(opTwo);
+    vi.runAllTimers();
+    expect(response).toHaveBeenCalledTimes(1);
+
+    next(opOne);
+    vi.runAllTimers();
+    expect(response).toHaveBeenCalledTimes(2);
+
+    next(opMutation);
+    vi.runAllTimers();
+
+    expect(response).toHaveBeenCalledTimes(3);
+    expect(updates.Mutation.updateBalance).toHaveBeenCalledTimes(1);
+
+    expect(reexec).toHaveBeenCalledTimes(0);
+  });
+
   it('does nothing when no related queries have changed', () => {
     const queryUnrelated = gql`
       {
@@ -1448,15 +1609,17 @@ describe('optimistic updates', () => {
 
     expect(response).toHaveBeenCalledTimes(1);
     expect(optimistic.concealAuthor).toHaveBeenCalledTimes(2);
-    expect(reexec).toHaveBeenCalledTimes(2);
+    expect(reexec).toHaveBeenCalledTimes(1);
     expect(result).toHaveBeenCalledTimes(0);
 
     vi.advanceTimersByTime(2);
     expect(response).toHaveBeenCalledTimes(2);
-    expect(result).toHaveBeenCalledTimes(0);
+    expect(reexec).toHaveBeenCalledTimes(2);
+    expect(result).toHaveBeenCalledTimes(1);
 
     vi.runAllTimers();
     expect(response).toHaveBeenCalledTimes(3);
+    expect(reexec).toHaveBeenCalledTimes(2);
     expect(result).toHaveBeenCalledTimes(2);
   });
 

exchanges/graphcache/src/operations/write.ts

@@ -343,6 +343,7 @@ const writeSelection = (
             ? InMemoryData.readLink(entityKey || typename, fieldKey)
             : undefined
         );
+
         InMemoryData.writeLink(entityKey || typename, fieldKey, link);
       } else {
         writeField(ctx, getSelectionSet(node), ensureData(fieldValue));

exchanges/graphcache/src/store/data.test.ts

@@ -98,8 +98,10 @@ describe('garbage collection', () => {
     InMemoryData.gc();
 
     expect(InMemoryData.readRecord('Todo:1', 'id')).toBe('1');
+    // TODO: is it a problem that this fails, we are reading from Todo
+    // but we are not updating anything
     expect(InMemoryData.getCurrentDependencies()).toEqual(
-      new Set(['Query.todo', 'Todo:1'])
+      new Set(['Query.todo'])
     );
   });
 

exchanges/graphcache/src/store/data.ts

@@ -458,7 +458,9 @@ export const readRecord = (
   entityKey: string,
   fieldKey: string
 ): EntityField => {
-  updateDependencies(entityKey, fieldKey);
+  if (currentOperation === 'read') {
+    updateDependencies(entityKey, fieldKey);
+  }
   return getNode(currentData!.records, entityKey, fieldKey);
 };
 
@@ -467,7 +469,9 @@ export const readLink = (
   entityKey: string,
   fieldKey: string
 ): Link | undefined => {
-  updateDependencies(entityKey, fieldKey);
+  if (currentOperation === 'read') {
+    updateDependencies(entityKey, fieldKey);
+  }
   return getNode(currentData!.links, entityKey, fieldKey);
 };
 
@@ -508,8 +512,12 @@ export const writeRecord = (
   fieldKey: string,
   value?: EntityField
 ) => {
-  updateDependencies(entityKey, fieldKey);
-  updatePersist(entityKey, fieldKey);
+  const existing = getNode(currentData!.records, entityKey, fieldKey);
+  if (!isEqualLinkOrScalar(existing, value)) {
+    updateDependencies(entityKey, fieldKey);
+    updatePersist(entityKey, fieldKey);
+  }
+
   setNode(currentData!.records, entityKey, fieldKey, value);
 };
 
@@ -533,9 +541,12 @@ export const writeLink = (
     updateRCForLink(entityLinks && entityLinks[fieldKey], -1);
     updateRCForLink(link, 1);
   }
-  // Update persistence batch and dependencies
-  updateDependencies(entityKey, fieldKey);
-  updatePersist(entityKey, fieldKey);
+  const existing = getNode(currentData!.links, entityKey, fieldKey);
+  if (!isEqualLinkOrScalar(existing, link)) {
+    updateDependencies(entityKey, fieldKey);
+    updatePersist(entityKey, fieldKey);
+  }
+
   // Update the link
   setNode(currentData!.links, entityKey, fieldKey, link);
 };
@@ -629,8 +640,9 @@ const squashLayer = (layerKey: number) => {
     for (const entry of links.entries()) {
       const entityKey = entry[0];
       const keyMap = entry[1];
-      for (const fieldKey in keyMap)
+      for (const fieldKey in keyMap) {
         writeLink(entityKey, fieldKey, keyMap[fieldKey]);
+      }
     }
   }
 
@@ -639,8 +651,9 @@ const squashLayer = (layerKey: number) => {
     for (const entry of records.entries()) {
       const entityKey = entry[0];
       const keyMap = entry[1];
-      for (const fieldKey in keyMap)
+      for (const fieldKey in keyMap) {
         writeRecord(entityKey, fieldKey, keyMap[fieldKey]);
+      }
     }
   }
 
@@ -710,3 +723,17 @@ export const hydrateData = (
   data.hydrating = false;
   clearDataState();
 };
+
+function isEqualLinkOrScalar(
+  a: Link | EntityField | undefined,
+  b: Link | EntityField | undefined
+) {
+  if (typeof a !== typeof b) return false;
+  if (a !== b) return false;
+  if (Array.isArray(a) && Array.isArray(b)) {
+    if (a.length !== b.length) return false;
+    return !a.some((el, index) => el !== b[index]);
+  }
+
+  return true;
+}