Repository: FormidableLabs/urql
Pull request #1011 (2020-09-29 14:14:03)

(graphcache) - Fix offline queries not flowing through the entire cacheExchange
Test against: https://codesandbox.io/s/urql-crud-router-doesnt-update-forked-3nf19

This change updates when queries that have received an offline / network error are dispatched as `cache-only` operations again, and adds support for them being retried automatically (in case they're still active) when the client comes back online.

A small change has also been made to `@urql/core` to allow `client.reexecuteOperation` to be called with mutations.

changeset-bot[bot]: ###  🦋  Changeset detected

Latest commit: 8f590a211c8156297a45ad78fe42ee1b61cec452

**The changes in this PR will be included in the next version bump.**



Not sure what this means? [Click here  to learn what changesets are](https://github.com/changesets/changesets/blob/master/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add another changeset to this PR](https://github.com/FormidableLabs/urql/new/fix/offline-query-fallback?filename=.changeset/many-jars-marry.md&value=---%0A%22%40fake-scope%2Ffake-pkg%22%3A%20patch%0A---%0A%0A(graphcache)%20-%20Fix%20offline%20queries%20not%20flowing%20through%20the%20entire%20cacheExchange%0A)


kitten: Tested against: https://codesandbox.io/s/urql-crud-router-doesnt-update-forked-7c622?file=/src/urql.js

**Working as intended!** The todos page starts fetching and receives an empty result with the passed operation as expected.

![Screenshot 2020-09-29 at 16 17 20](https://user-images.githubusercontent.com/2041385/94578154-452af280-026f-11eb-84f0-c8b78fbbf67b.png)


Patch:
diff --git a/.changeset/strange-waves-confess.md b/.changeset/strange-waves-confess.md
new file mode 100644
index 00000000..9a5a0294
--- /dev/null
+++ b/.changeset/strange-waves-confess.md
@@ -0,0 +1,6 @@
+---
+'@urql/exchange-graphcache': patch
+---
+
+Fix queries that have erroed with a `NetworkError` (`isOfflineError`) not flowing back completely through the `cacheExchange`.
+These queries should also now be reexecuted when the client comes back online.
diff --git a/.changeset/tidy-bobcats-sniff.md b/.changeset/tidy-bobcats-sniff.md
new file mode 100644
index 00000000..bde72fcf
--- /dev/null
+++ b/.changeset/tidy-bobcats-sniff.md
@@ -0,0 +1,5 @@
+---
+'@urql/core': patch
+---
+
+Allow `client.reexecuteOperation` to be called with mutations which skip the active operation minimums.
diff --git a/exchanges/graphcache/src/offlineExchange.ts b/exchanges/graphcache/src/offlineExchange.ts
index ed6b89e2..147ce5c6 100644
--- a/exchanges/graphcache/src/offlineExchange.ts
+++ b/exchanges/graphcache/src/offlineExchange.ts
@@ -3,8 +3,8 @@ import { print, SelectionNode } from 'graphql';
 
 import {
   Operation,
-  GraphQLRequest,
   Exchange,
+  ExchangeIO,
   CombinedError,
   createRequest,
 } from '@urql/core';
@@ -20,7 +20,11 @@ import {
 } from './ast';
 
 import { makeDict } from './helpers/dict';
-import { OptimisticMutationConfig, Variables } from './types';
+import {
+  SerializedRequest,
+  OptimisticMutationConfig,
+  Variables,
+} from './types';
 import { cacheExchange, CacheExchangeOpts } from './cacheExchange';
 import { toRequestPolicy } from './helpers/operation';
 
@@ -59,14 +63,8 @@ const isOfflineError = (error: undefined | CombinedError) =>
       error.networkError.message
     ));
 
-export const offlineExchange = (opts: CacheExchangeOpts): Exchange => ({
-  forward: outerForward,
-  client,
-  dispatchDebug,
-}) => ops$ => {
+export const offlineExchange = (opts: CacheExchangeOpts): Exchange => input => {
   const { storage } = opts;
-  const { source: reboundOps$, next } = makeSubject<Operation>();
-  let forward = outerForward;
 
   if (
     storage &&
@@ -74,77 +72,99 @@ export const offlineExchange = (opts: CacheExchangeOpts): Exchange => ({
     storage.readMetadata &&
     storage.writeMetadata
   ) {
+    const { forward: outerForward, client, dispatchDebug } = input;
     const optimisticMutations = opts.optimistic || {};
-    const failedQueue: GraphQLRequest[] = [];
+    const failedQueue: Operation[] = [];
 
     const updateMetadata = () => {
-      storage.writeMetadata!(
-        failedQueue.map(op => ({
-          query: print(op.query),
-          variables: op.variables,
-        }))
-      );
+      const requests: SerializedRequest[] = [];
+      for (let i = 0; i < failedQueue.length; i++) {
+        const op = failedQueue[i];
+        if (op.operationName === 'mutation') {
+          requests.push({
+            query: print(op.query),
+            variables: op.variables,
+          });
+        }
+      }
+      storage.writeMetadata!(requests);
     };
 
     let _flushing = false;
     const flushQueue = () => {
-      let request: void | GraphQLRequest;
-      while (!_flushing && (request = failedQueue.shift())) {
+      if (!_flushing) {
         _flushing = true;
-        client.dispatchOperation(
-          client.createRequestOperation('mutation', request)
-        );
+        let operation: void | Operation;
+        while ((operation = failedQueue.shift()))
+          client.reexecuteOperation(operation);
+        updateMetadata();
         _flushing = false;
       }
-
-      updateMetadata();
     };
 
-    forward = ops$ => {
+    const forward: ExchangeIO = ops$ => {
       return pipe(
         outerForward(ops$),
         filter(res => {
           if (
-            res.operation.operationName === 'subscription' ||
-            !isOfflineError(res.error)
+            res.operation.operationName === 'mutation' &&
+            isOfflineError(res.error) &&
+            isOptimisticMutation(optimisticMutations, res.operation)
           ) {
-            return true;
-          } else if (res.operation.operationName === 'mutation') {
-            if (isOptimisticMutation(optimisticMutations, res.operation)) {
-              failedQueue.push(res.operation);
-              updateMetadata();
-              return false;
-            }
-
-            return true;
+            failedQueue.push(res.operation);
+            updateMetadata();
+            return false;
           }
 
-          Promise.resolve().then(() =>
-            next(toRequestPolicy(res.operation, 'cache-only'))
-          );
-
-          return false;
+          return true;
         })
       );
     };
 
     storage.onOnline(flushQueue);
     storage.readMetadata().then(mutations => {
-      if (mutations)
-        for (let i = 0; i < mutations.length; i++)
+      if (mutations) {
+        for (let i = 0; i < mutations.length; i++) {
           failedQueue.push(
-            createRequest(mutations[i].query, mutations[i].variables)
+            client.createRequestOperation(
+              'mutation',
+              createRequest(mutations[i].query, mutations[i].variables)
+            )
           );
-      flushQueue();
+        }
+
+        flushQueue();
+      }
+    });
+
+    const cacheResults$ = cacheExchange(opts)({
+      client,
+      dispatchDebug,
+      forward,
     });
-  }
 
-  const sharedOps$ = share(ops$);
-  const opsAndRebound$ = merge([reboundOps$, sharedOps$]);
+    return ops$ => {
+      const sharedOps$ = share(ops$);
+      const { source: reboundOps$, next } = makeSubject<Operation>();
+      const opsAndRebound$ = merge([reboundOps$, sharedOps$]);
+
+      return pipe(
+        cacheResults$(opsAndRebound$),
+        filter(res => {
+          if (
+            res.operation.operationName === 'query' &&
+            isOfflineError(res.error)
+          ) {
+            next(toRequestPolicy(res.operation, 'cache-only'));
+            failedQueue.push(res.operation);
+            return false;
+          }
+
+          return true;
+        })
+      );
+    };
+  }
 
-  return cacheExchange(opts)({
-    forward,
-    client,
-    dispatchDebug,
-  })(opsAndRebound$);
+  return cacheExchange(opts)(input);
 };
diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts
index 8d4a3805..b3c6dda8 100755
--- a/packages/core/src/client.ts
+++ b/packages/core/src/client.ts
@@ -135,7 +135,10 @@ export class Client {
     this.reexecuteOperation = (operation: Operation) => {
       // Reexecute operation only if any subscribers are still subscribed to the
       // operation's exchange results
-      if ((this.activeOperations[operation.key] || 0) > 0) {
+      if (
+        operation.operationName === 'mutation' ||
+        (this.activeOperations[operation.key] || 0) > 0
+      ) {
         this.queue.push(operation);
         if (!isOperationBatchActive) {
           Promise.resolve().then(this.dispatchOperation);


Test patch:
diff --git a/exchanges/graphcache/src/offlineExchange.test.ts b/exchanges/graphcache/src/offlineExchange.test.ts
index 57ffc9c1..048e23a0 100644
--- a/exchanges/graphcache/src/offlineExchange.test.ts
+++ b/exchanges/graphcache/src/offlineExchange.test.ts
@@ -61,7 +61,9 @@ describe('storage', () => {
 
   it('should read the metadata and dispatch operations on initialization', () => {
     const client = createClient({ url: 'http://0.0.0.0' });
-    const dispatchOperationSpy = jest.spyOn(client, 'dispatchOperation');
+    const reexecuteOperation = jest
+      .spyOn(client, 'reexecuteOperation')
+      .mockImplementation(() => undefined);
     const op = client.createRequestOperation('mutation', {
       key: 1,
       query: mutationOne,
@@ -81,7 +83,7 @@ describe('storage', () => {
 
     storage.readData.mockReturnValueOnce({ then: () => undefined });
     storage.readMetadata.mockReturnValueOnce({ then: cb => cb([op]) });
-    dispatchOperationSpy.mockImplementation(() => undefined);
+    reexecuteOperation.mockImplementation(() => undefined);
 
     jest.useFakeTimers();
     pipe(
@@ -92,8 +94,8 @@ describe('storage', () => {
     jest.runAllTimers();
 
     expect(storage.readMetadata).toBeCalledTimes(1);
-    expect(dispatchOperationSpy).toBeCalledTimes(1);
-    expect(dispatchOperationSpy).toBeCalledWith({
+    expect(reexecuteOperation).toBeCalledTimes(1);
+    expect(reexecuteOperation).toBeCalledWith({
       ...op,
       key: expect.any(Number),
     });
@@ -229,10 +231,7 @@ describe('offline', () => {
     );
 
     next(queryOp);
-    expect(result).toBeCalledTimes(0);
-    expect(response).toBeCalledTimes(1);
 
-    await Promise.resolve();
     expect(result).toBeCalledTimes(1);
     expect(response).toBeCalledTimes(1);
 
@@ -258,8 +257,8 @@ describe('offline', () => {
     const onlineSpy = jest.spyOn(navigator, 'onLine', 'get');
 
     const client = createClient({ url: 'http://0.0.0.0' });
-    const dispatchOperationSpy = jest
-      .spyOn(client, 'dispatchOperation')
+    const reexecuteOperation = jest
+      .spyOn(client, 'reexecuteOperation')
       .mockImplementation(() => undefined);
 
     const mutationOp = client.createRequestOperation('mutation', {
@@ -319,9 +318,9 @@ describe('offline', () => {
     ]);
 
     flush!();
-    expect(dispatchOperationSpy).toHaveBeenCalledTimes(1);
-    expect((dispatchOperationSpy.mock.calls[0][0] as any).key).toEqual(1);
-    expect((dispatchOperationSpy.mock.calls[0][0] as any).query).toEqual(
+    expect(reexecuteOperation).toHaveBeenCalledTimes(1);
+    expect((reexecuteOperation.mock.calls[0][0] as any).key).toEqual(1);
+    expect((reexecuteOperation.mock.calls[0][0] as any).query).toEqual(
       formatDocument(mutationOp.query)
     );
   });