Repository: FormidableLabs/urql (ts)

Issue:
exchange-request-policy saves last access time, not last time of network request
**urql version & exchanges:**

"urql": "^2.0.3"
"@urql/core": "^2.1.1"
"@urql/exchange-request-policy": "^0.1.2"

**Steps to reproduce**

Switch between two pages faster than the TTL

**Expected behavior**

Queries don't trigger `shouldUpgrade` (and subsequently a network request)

**Actual behavior**

Queries should trigger `shouldUpgrade` after TTL has been expired based on last network request, not after last access of cached data

**Reproduction**
https://codesandbox.io/s/vibrant-bird-5pi8t?file=/src/index.js

Open the console and use the toggle button faster than every two seconds and "shouldUpgrade" won't show in the console. Wait for two seconds and then toggle, then you will see "shouldUpgrade".

It will still not make a network request, but that is handled in this issue: https://github.com/FormidableLabs/urql/issues/1334

requestPolicyExchange upgrade does not result in network request
https://github.com/FormidableLabs/urql/issues/1334 is probably still not solved.

**Reproduction:**

1. Go to this URL: https://codesandbox.io/s/vibrant-bird-5pi8t?file=/src/index.js
2. Open your network tab
3. Click "toggle" twice
4. You will see "shouldUpgrade" in your console
5. Still, no network request is being made

Pull request description:
(request-policy) - Fix TTL being erased and renewed on cache hits
Resolves #1636
Resolves #1640

Patch:
diff --git a/.changeset/polite-laws-brake.md b/.changeset/polite-laws-brake.md
new file mode 100644
index 00000000..e4f902aa
--- /dev/null
+++ b/.changeset/polite-laws-brake.md
@@ -0,0 +1,5 @@
+---
+'@urql/exchange-request-policy': patch
+---
+
+Fix TTL being updated to a newer timestamp when a cached result comes in, and prevent TTL from being deleted on our React binding's cache probes. Instead we now never delete the TTL and update it on incoming cache miss results.
diff --git a/exchanges/request-policy/src/requestPolicyExchange.ts b/exchanges/request-policy/src/requestPolicyExchange.ts
index 5e211e86..3b18e5aa 100644
--- a/exchanges/request-policy/src/requestPolicyExchange.ts
+++ b/exchanges/request-policy/src/requestPolicyExchange.ts
@@ -23,19 +23,17 @@ export const requestPolicyExchange = (options: Options): Exchange => ({
     if (
       operation.kind !== 'query' ||
       (operation.context.requestPolicy !== 'cache-first' &&
-        operation.context.requestPolicy !== 'cache-only') ||
-      !operations.has(operation.key)
+        operation.context.requestPolicy !== 'cache-only')
     ) {
       return operation;
     }
 
-    const lastOccurrence = operations.get(operation.key);
     const currentTime = new Date().getTime();
+    const lastOccurrence = operations.get(operation.key) || 0;
     if (
       currentTime - lastOccurrence > TTL &&
-      (options.shouldUpgrade ? options.shouldUpgrade(operation) : true)
+      (!options.shouldUpgrade || options.shouldUpgrade(operation))
     ) {
-      operations.delete(operation.key);
       return makeOperation(operation.kind, operation, {
         ...operation.context,
         requestPolicy: 'cache-and-network',
@@ -46,7 +44,14 @@ export const requestPolicyExchange = (options: Options): Exchange => ({
   };
 
   const processIncomingResults = (result: OperationResult): void => {
-    operations.set(result.operation.key, new Date().getTime());
+    const meta = result.operation.context.meta;
+    const isMiss =
+      !operations.has(result.operation.key) ||
+      !meta ||
+      meta.cacheOutcome === 'miss';
+    if (isMiss) {
+      operations.set(result.operation.key, new Date().getTime());
+    }
   };
 
   return ops$ => {


Test patch:
diff --git a/exchanges/request-policy/src/requestPolicyExchange.test.ts b/exchanges/request-policy/src/requestPolicyExchange.test.ts
index 182b27fe..bd33ee86 100644
--- a/exchanges/request-policy/src/requestPolicyExchange.test.ts
+++ b/exchanges/request-policy/src/requestPolicyExchange.test.ts
@@ -74,7 +74,7 @@ it(`upgrades to cache-and-network`, done => {
 
   expect(response).toHaveBeenCalledTimes(1);
   expect(response.mock.calls[0][0].context.requestPolicy).toEqual(
-    'cache-first'
+    'cache-and-network'
   );
   expect(result).toHaveBeenCalledTimes(1);
 
@@ -130,7 +130,7 @@ it(`doesn't upgrade when shouldUpgrade returns false`, done => {
       'cache-first'
     );
     expect(result).toHaveBeenCalledTimes(2);
-    expect(shouldUpgrade).toBeCalledTimes(1);
+    expect(shouldUpgrade).toBeCalledTimes(2);
     done();
   }, 10);
 });