Repository: urql-graphql/urql (ts)

Issue:
Multiple requests when using react suspense
### Describe the bug

The bug is that urql is somehow spamming multiple requests.
Our application works like this basically. It has a set of modules and we iterate them returning lazy loaded components wrapped in React Suspense.

```
<Suspense>
  modules.map((m) => {
    component = lazy(m.component)
    return <Route 
      ...
    />
  })
</Suspense>
```

We don't know why this happens. But we suspect that it has something to do with how we structured one of our tree. We also suspect it has something to do with the lazy loading and react suspense In the example we have been able to reproduce the issue.

Consider this structure. It only happens when it is like this.
```
Query
  vehicles
Vehicles
  car
  cars
  bicycle
  bicycles
```

### Reproduction
https://github.com/danielkaxis/urqlsuspensetest/tree/multiplerequests

__mocked_generated__ folder has the schema and urql resolvers.

yarn
yarn serve
localhost:8000
Open developer tools
reload page

All logs show the query that has been called with console.count().
`Car:name` should be called 4 times but is called 14 times.

Why is it called so many times?

![image](https://github.com/urql-graphql/urql/assets/77433590/81f37e8e-678b-4c03-a38b-69eab378d4f7)

Comment out the mutation in App.tsx line 13.
```js
await client.mutation(userMutation, {}).toPromise()
```
reload the page
`Car:name` is now called only 8 times.

Why did that decrease the number of requests?

### Urql version

```
"@graphql-tools/schema": "10.0.3",
"@urql/devtools": "2.0.3",
"@urql/exchange-execute": "2.2.2",
"@urql/exchange-graphcache": "7.0.1",
"graphql": "16.8.1",
"urql": "4.0.7",
```

### Validations

- [X] I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which [GitHub Discussions](https://github.com/urql-graphql/urql/discussions) should be used
- [X] Read the [docs](https://formidable.com/open-source/urql/docs/).
- [X] Follow our [Code of Conduct](https://github.com/urql-graphql/urql/blob/main/CODE_OF_CONDUCT.md)

Pull request:
fix(client): eager unblocking of in-flight operations
Resolves #3565

## Summary

In the initial issue we were faced with the following issue, imagine a query that returns a list with values `[Todo:1, Todo:2]` we use this list to return components that in turn has its own query to fetch the details of this `Todo`. Both this list and the details have a shared parent field let's name it `Query.author`, this is important for graph-cache to re-dispatch operations.

This diagram of this looks like the following

```
Operation List
Completed Operation List
Operation Todo:1
Operation Todo:2
Completed Operation Todo:1
Operation Todo:2
Completed Operation Todo:2
Completed Operation Todo:2
```

Notice that we re-dispatch `Todo:2` even though it is already in-flight, this is problematic as in the cache it has not resolved to a response yet and hence we send off multiple network requests. This issue becomes more and more apparent given more queries being in-flight. The lines causing this issue can be found [here](https://github.com/urql-graphql/urql/blob/main/packages/core/src/client.ts#L701-L706) since there is no operation queued we will always remove the operation from our in-flight operations.

The solution here is to change this logic to only remove `dispatched` on a queued operation that either is already in `dispatched` or is `network-only`.

> I did notice that https://github.com/urql-graphql/urql/pull/3564 already makes this issue less severe due to dispatching less operations from graphcache.

The above fix however re-introduces the issue we faced in https://github.com/urql-graphql/urql/issues/3254 this due to our optimistic mutation re-triggering the operation and resulting in `stale` and then our real data re-triggering the query but it already having been dispatched. This because we have similar logic in the [`onPush`](https://github.com/urql-graphql/urql/blob/main/packages/core/src/client.ts#L645-L653) part of our `operation`. When it's stale we'll check the queue whether there is an operation in there, however there won't be as we are dealing with an optimistic mutation re-trigger, to fix this I added a check whether the stale result has a next payload coming, if not it gets deleted form dispatched either way.

I think the above `stale` flag fix is the root cause that needed to be addressed in #3254 but I am not a 100% sure as I got a bit overloaded while leveraging `console.log` in all of these reproductions 😅 


Patch:
diff --git a/.changeset/tiny-pots-allow.md b/.changeset/tiny-pots-allow.md
new file mode 100644
index 00000000..f1e3555a
--- /dev/null
+++ b/.changeset/tiny-pots-allow.md
@@ -0,0 +1,6 @@
+---
+'@urql/core': patch
+---
+
+Fix issue where a reexecute on an in-flight operation would lead to multiple network-requests.
+For example, this issue presents itself when Graphcache is concurrently updating multiple, inter-dependent queries with shared entities. One query completing while others are still in-flight may lead to duplicate operations being issued.
diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts
index c8e3b153..0cf9b74f 100755
--- a/packages/core/src/client.ts
+++ b/packages/core/src/client.ts
@@ -643,12 +643,17 @@ export const Client: new (opts: ClientOptions) => Client = function Client(
         // Store replay result
         onPush(result => {
           if (result.stale) {
-            // If the current result has queued up an operation of the same
-            // key, then `stale` refers to it
-            for (const operation of queue) {
-              if (operation.key === result.operation.key) {
-                dispatched.delete(operation.key);
-                break;
+            if (!result.hasNext) {
+              // we are dealing with an optimistic mutation or a partial result
+              dispatched.delete(operation.key);
+            } else {
+              // If the current result has queued up an operation of the same
+              // key, then `stale` refers to it
+              for (const operation of queue) {
+                if (operation.key === result.operation.key) {
+                  dispatched.delete(operation.key);
+                  break;
+                }
               }
             }
           } else if (!result.hasNext) {
@@ -697,13 +702,29 @@ export const Client: new (opts: ClientOptions) => Client = function Client(
       // operation's exchange results
       if (operation.kind === 'teardown') {
         dispatchOperation(operation);
-      } else if (operation.kind === 'mutation' || active.has(operation.key)) {
-        let queued = false;
-        for (let i = 0; i < queue.length; i++)
-          queued = queued || queue[i].key === operation.key;
-        if (!queued) dispatched.delete(operation.key);
+      } else if (operation.kind === 'mutation') {
         queue.push(operation);
         Promise.resolve().then(dispatchOperation);
+      } else if (active.has(operation.key)) {
+        let queued = false;
+        for (let i = 0; i < queue.length; i++) {
+          if (queue[i].key === operation.key) {
+            queue[i] = operation;
+            queued = true;
+          }
+        }
+
+        if (
+          !queued &&
+          (!dispatched.has(operation.key) ||
+            operation.context.requestPolicy === 'network-only')
+        ) {
+          queue.push(operation);
+          Promise.resolve().then(dispatchOperation);
+        } else {
+          dispatched.delete(operation.key);
+          Promise.resolve().then(dispatchOperation);
+        }
       }
     },
 


Test patch:
diff --git a/packages/core/src/client.test.ts b/packages/core/src/client.test.ts
index fcd403ba..956851ba 100755
--- a/packages/core/src/client.test.ts
+++ b/packages/core/src/client.test.ts
@@ -19,13 +19,20 @@ import {
   onPush,
   tap,
   take,
+  fromPromise,
+  fromValue,
+  mergeMap,
 } from 'wonka';
 
 import { gql } from './gql';
 import { Exchange, Operation, OperationResult } from './types';
 import { makeOperation } from './utils';
 import { Client, createClient } from './client';
-import { queryOperation, subscriptionOperation } from './test-utils';
+import {
+  mutationOperation,
+  queryOperation,
+  subscriptionOperation,
+} from './test-utils';
 
 const url = 'https://hostname.com';
 
@@ -701,7 +708,7 @@ describe('deduplication behavior', () => {
     expect(onOperation).toHaveBeenCalledTimes(2);
   });
 
-  it('unblocks operations on call to reexecuteOperation', async () => {
+  it('unblocks mutation operations on call to reexecuteOperation', async () => {
     const onOperation = vi.fn();
     const onResult = vi.fn();
 
@@ -724,6 +731,46 @@ describe('deduplication behavior', () => {
       exchanges: [exchange],
     });
 
+    const operation = makeOperation('mutation', mutationOperation, {
+      ...mutationOperation.context,
+      requestPolicy: 'cache-first',
+    });
+
+    pipe(client.executeRequestOperation(operation), subscribe(onResult));
+
+    expect(onOperation).toHaveBeenCalledTimes(1);
+    expect(onResult).toHaveBeenCalledTimes(0);
+
+    client.reexecuteOperation(operation);
+    await Promise.resolve();
+
+    expect(onOperation).toHaveBeenCalledTimes(2);
+    expect(onResult).toHaveBeenCalledTimes(1);
+  });
+
+  // See https://github.com/urql-graphql/urql/issues/3254
+  it('unblocks stale operations', async () => {
+    const onOperation = vi.fn();
+    const onResult = vi.fn();
+
+    let sends = 0;
+    const exchange: Exchange = () => ops$ =>
+      pipe(
+        ops$,
+        onPush(onOperation),
+        map(op => ({
+          hasNext: false,
+          stale: sends++ ? false : true,
+          data: 'test',
+          operation: op,
+        }))
+      );
+
+    const client = createClient({
+      url: 'test',
+      exchanges: [exchange],
+    });
+
     const operation = makeOperation('query', queryOperation, {
       ...queryOperation.context,
       requestPolicy: 'cache-first',
@@ -732,13 +779,86 @@ describe('deduplication behavior', () => {
     pipe(client.executeRequestOperation(operation), subscribe(onResult));
 
     expect(onOperation).toHaveBeenCalledTimes(1);
-    expect(onResult).toHaveBeenCalledTimes(0);
+    expect(onResult).toHaveBeenCalledTimes(1);
 
     client.reexecuteOperation(operation);
     await Promise.resolve();
 
+    expect(onOperation).toHaveBeenCalledTimes(2);
+    expect(onResult).toHaveBeenCalledTimes(2);
+  });
+
+  // See https://github.com/urql-graphql/urql/issues/3565
+  it('blocks reexecuting operations that are in-flight', async () => {
+    const onOperation = vi.fn();
+    const onResult = vi.fn();
+
+    let resolve;
+    const exchange: Exchange =
+      ({ client }) =>
+      ops$ =>
+        pipe(
+          ops$,
+          onPush(onOperation),
+          mergeMap(op => {
+            if (op.key === queryOperation.key) {
+              const promise = new Promise<OperationResult>(res => {
+                resolve = res;
+              });
+              return fromPromise(
+                promise.then(() => {
+                  return {
+                    hasNext: false,
+                    stale: false,
+                    data: 'test',
+                    operation: op,
+                  };
+                })
+              );
+            } else {
+              client.reexecuteOperation(queryOperation);
+              return fromValue({
+                hasNext: false,
+                stale: false,
+                data: 'test',
+                operation: op,
+              });
+            }
+          })
+        );
+
+    const client = createClient({
+      url: 'test',
+      exchanges: [exchange],
+    });
+
+    const operation = makeOperation('query', queryOperation, {
+      ...queryOperation.context,
+      requestPolicy: 'cache-first',
+    });
+
+    const mutation = makeOperation('mutation', mutationOperation, {
+      ...mutationOperation.context,
+      requestPolicy: 'cache-first',
+    });
+
+    pipe(client.executeRequestOperation(operation), subscribe(onResult));
+
+    expect(onOperation).toHaveBeenCalledTimes(1);
+    expect(onResult).toHaveBeenCalledTimes(0);
+
+    pipe(client.executeRequestOperation(mutation), subscribe(onResult));
+    await Promise.resolve();
+
     expect(onOperation).toHaveBeenCalledTimes(2);
     expect(onResult).toHaveBeenCalledTimes(1);
+
+    resolve();
+    await Promise.resolve();
+    await Promise.resolve();
+    await Promise.resolve();
+    expect(onOperation).toHaveBeenCalledTimes(2);
+    expect(onResult).toHaveBeenCalledTimes(2);
   });
 });