Repository: urql-graphql/urql
Pull request #3665 (2024-09-09 07:35:03)

fix(graphcache): mutation would cause dependent operations and reexecuting operations become the same set


## Summary

The PR #2761 moved `reexecutingOperations = dependentOperations` assignment into `if (operation.kind === 'query')` guard. Thus after a completion of mutation, `dependentOperations = _reexecutingOperations` make reexecuting and dependent operations the same set object.

I noticed this issue after debugging the bug reported in discord https://discord.com/channels/1082378892523864074/1082386845553406002/1281542643670712334



changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: 85aff83f759a670999b525da52e58395194fb5ff

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/xuanduc987/urql/new/fix-graphcache-same-set?filename=.changeset/afraid-crabs-collect.md&value=---%0A%22%40urql%2Fexchange-graphcache%22%3A%20patch%0A---%0A%0Afix(graphcache)%3A%20mutation%20would%20cause%20dependent%20operations%20and%20reexecuting%20operations%20become%20the%20same%20set%0A)


JoviDeCroock: Mind adding a changeset and a test that shows this behavior?
xuanduc987: @JoviDeCroock I had added a new changeset.
About test, I took a look at `cacheExchange.test.ts` but still not sure how to test this.
xuanduc987: @JoviDeCroock I added test to assert that the number of reexec after a query would not change after executing a mutation

Patch:
diff --git a/.changeset/tidy-spies-turn.md b/.changeset/tidy-spies-turn.md
new file mode 100644
index 00000000..0eea5fda
--- /dev/null
+++ b/.changeset/tidy-spies-turn.md
@@ -0,0 +1,5 @@
+---
+'@urql/exchange-graphcache': patch
+---
+
+fix bug that mutation would cause dependent operations and reexecuting operations to become the same set
diff --git a/exchanges/graphcache/src/cacheExchange.ts b/exchanges/graphcache/src/cacheExchange.ts
index 260ca9db..0b14caee 100644
--- a/exchanges/graphcache/src/cacheExchange.ts
+++ b/exchanges/graphcache/src/cacheExchange.ts
@@ -136,8 +136,9 @@ export const cacheExchange =
         // Upon completion, all dependent operations become reexecuting operations, preventing
         // them from reexecuting prior operations again, causing infinite loops
         const _reexecutingOperations = reexecutingOperations;
+        reexecutingOperations = dependentOperations;
         if (operation.kind === 'query') {
-          (reexecutingOperations = dependentOperations).add(operation.key);
+          reexecutingOperations.add(operation.key);
         }
         (dependentOperations = _reexecutingOperations).clear();
       }


Test patch:
diff --git a/exchanges/graphcache/src/cacheExchange.test.ts b/exchanges/graphcache/src/cacheExchange.test.ts
index fb7b6c41..832d32f6 100644
--- a/exchanges/graphcache/src/cacheExchange.test.ts
+++ b/exchanges/graphcache/src/cacheExchange.test.ts
@@ -913,6 +913,148 @@ describe('data dependencies', () => {
     expect(reexecuteOperation).toHaveBeenCalledTimes(0);
     expect(result.mock.calls[0][0]).toHaveProperty('data.author', null);
   });
+
+  it('mutation does not change number of reexecute request after a query', () => {
+    const client = createClient({
+      url: 'http://0.0.0.0',
+      exchanges: [],
+    });
+
+    const { source: ops$, next: nextOp } = makeSubject<Operation>();
+
+    const reexec = vi
+      .spyOn(client, 'reexecuteOperation')
+      .mockImplementation(nextOp);
+
+    const mutation = gql`
+      mutation {
+        updateNode {
+          __typename
+          id
+        }
+      }
+    `;
+
+    const normalQuery = gql`
+      {
+        __typename
+        item {
+          __typename
+          id
+        }
+      }
+    `;
+
+    const extendedQuery = gql`
+      {
+        __typename
+        item {
+          __typename
+          extended: id
+          extra @_optional
+        }
+      }
+    `;
+
+    const mutationOp = client.createRequestOperation('mutation', {
+      key: 0,
+      query: mutation,
+      variables: undefined,
+    });
+
+    const normalOp = client.createRequestOperation(
+      'query',
+      {
+        key: 1,
+        query: normalQuery,
+        variables: undefined,
+      },
+      {
+        requestPolicy: 'cache-and-network',
+      }
+    );
+
+    const extendedOp = client.createRequestOperation(
+      'query',
+      {
+        key: 2,
+        query: extendedQuery,
+        variables: undefined,
+      },
+      {
+        requestPolicy: 'cache-only',
+      }
+    );
+
+    const response = vi.fn((forwardOp: Operation): OperationResult => {
+      if (forwardOp.key === 0) {
+        return {
+          operation: mutationOp,
+          data: {
+            __typename: 'Mutation',
+            updateNode: {
+              __typename: 'Node',
+              id: 'id',
+            },
+          },
+          stale: false,
+          hasNext: false,
+        };
+      } else if (forwardOp.key === 1) {
+        return {
+          operation: normalOp,
+          data: {
+            __typename: 'Query',
+            item: {
+              __typename: 'Node',
+              id: 'id',
+            },
+          },
+          stale: false,
+          hasNext: false,
+        };
+      } else if (forwardOp.key === 2) {
+        return {
+          operation: extendedOp,
+          data: {
+            __typename: 'Query',
+            item: {
+              __typename: 'Node',
+              extended: 'id',
+              extra: 'extra',
+            },
+          },
+          stale: false,
+          hasNext: false,
+        };
+      }
+
+      return undefined as any;
+    });
+
+    const forward = (ops$: Source<Operation>): Source<OperationResult> =>
+      pipe(ops$, map(response), share);
+
+    pipe(cacheExchange()({ forward, client, dispatchDebug })(ops$), publish);
+
+    nextOp(normalOp);
+    expect(reexec).toHaveBeenCalledTimes(0);
+
+    nextOp(extendedOp);
+    expect(reexec).toHaveBeenCalledTimes(0);
+
+    // re-execute first operation
+    reexec.mockClear();
+    nextOp(normalOp);
+    expect(reexec).toHaveBeenCalledTimes(4);
+
+    nextOp(mutationOp);
+
+    // re-execute first operation after mutation
+    reexec.mockClear();
+    nextOp(normalOp);
+    expect(reexec).toHaveBeenCalledTimes(4);
+  });
 });
 
 describe('directives', () => {