Repository: FormidableLabs/urql
Pull request #1776 (2021-07-06 14:16:39)

(core) - Fix ssrExchange.restoreData adding invalidated results
Related #1775 
Resolve #1771

## Summary

Prevent `ssrExchange().restoreData()` from adding results to the exchange that have already been invalidated. This may happen when `restoreData()` is called repeatedly, e.g. per page. When a prior run has already invalidated an SSR result then the result is 'migrated' to the user's `cacheExchange`, which means that `restoreData()` should never attempt to re-add it again.

## Set of changes

- Invalidate results in `ssrExchange` with `null` rather than `delete`
- Use `null` entries to prevent overwriting invalidated results


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

Latest commit: 1bfd974a5765cd7731516b371d68bbf3d4eb23a1

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

<details><summary>This PR includes changesets to release 1 package</summary>

  | Name       | Type  |
| ---------- | ----- |
| @urql/core | Patch |

</details>

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/repeated-restore-ssr?filename=.changeset/old-eggs-obey.md&value=---%0A%22%40urql%2Fcore%22%3A%20patch%0A---%0A%0A(core)%20-%20Fix%20ssrExchange.restoreData%20adding%20invalidated%20results%0A)



Patch:
diff --git a/.changeset/cuddly-readers-walk.md b/.changeset/cuddly-readers-walk.md
new file mode 100644
index 00000000..c593ad74
--- /dev/null
+++ b/.changeset/cuddly-readers-walk.md
@@ -0,0 +1,5 @@
+---
+'@urql/core': patch
+---
+
+Prevent `ssrExchange().restoreData()` from adding results to the exchange that have already been invalidated. This may happen when `restoreData()` is called repeatedly, e.g. per page. When a prior run has already invalidated an SSR result then the result is 'migrated' to the user's `cacheExchange`, which means that `restoreData()` should never attempt to re-add it again.
diff --git a/packages/core/src/exchanges/ssr.ts b/packages/core/src/exchanges/ssr.ts
index 23df91a1..b8fc7abe 100644
--- a/packages/core/src/exchanges/ssr.ts
+++ b/packages/core/src/exchanges/ssr.ts
@@ -91,7 +91,7 @@ const deserializeResult = (
 
 /** The ssrExchange can be created to capture data during SSR and also to rehydrate it on the client */
 export const ssrExchange = (params?: SSRExchangeParams): SSRExchange => {
-  const data: SSRData = {};
+  const data: Record<string, SerializedResult | null> = {};
 
   // On the client-side, we delete results from the cache as they're resolved
   // this is delayed so that concurrent queries don't delete each other's data
@@ -101,13 +101,15 @@ export const ssrExchange = (params?: SSRExchangeParams): SSRExchange => {
     if (invalidateQueue.length === 1) {
       Promise.resolve().then(() => {
         let key: number | void;
-        while ((key = invalidateQueue.shift())) delete data[key];
+        while ((key = invalidateQueue.shift())) {
+          data[key] = null;
+        }
       });
     }
   };
 
   const isCached = (operation: Operation) => {
-    return !shouldSkip(operation) && data[operation.key] !== undefined;
+    return !shouldSkip(operation) && data[operation.key] != null;
   };
 
   // The SSR Exchange is a temporary cache that can populate results into data for suspense
@@ -134,7 +136,7 @@ export const ssrExchange = (params?: SSRExchangeParams): SSRExchange => {
       sharedOps$,
       filter(op => isCached(op)),
       map(op => {
-        const serialized = data[op.key];
+        const serialized = data[op.key]!;
         return deserializeResult(op, serialized);
       })
     );
@@ -159,8 +161,20 @@ export const ssrExchange = (params?: SSRExchangeParams): SSRExchange => {
     return merge([forwardedOps$, cachedOps$]);
   };
 
-  ssr.restoreData = (restore: SSRData) => Object.assign(data, restore);
-  ssr.extractData = () => Object.assign({}, data);
+  ssr.restoreData = (restore: SSRData) => {
+    for (const key in restore) {
+      // We only restore data that hasn't been previously invalidated
+      if (data[key] !== null) {
+        data[key] = restore[key];
+      }
+    }
+  };
+
+  ssr.extractData = () => {
+    const result: SSRData = {};
+    for (const key in data) if (data[key] != null) result[key] = data[key]!;
+    return result;
+  };
 
   if (params && params.initialState) {
     ssr.restoreData(params.initialState);


Test patch:
diff --git a/packages/core/src/exchanges/ssr.test.ts b/packages/core/src/exchanges/ssr.test.ts
index d2b3e161..6186327a 100644
--- a/packages/core/src/exchanges/ssr.test.ts
+++ b/packages/core/src/exchanges/ssr.test.ts
@@ -134,7 +134,7 @@ it('caches complex GraphQLErrors in query results correctly', () => {
   publish(exchange);
   next(queryOperation);
 
-  const error = ssr.extractData()[queryOperation.key].error;
+  const error = ssr.extractData()[queryOperation.key]!.error;
 
   expect(error).toHaveProperty('graphQLErrors.0.message', 'Oh no!');
   expect(error).toHaveProperty('graphQLErrors.0.path', ['Query']);
@@ -182,3 +182,34 @@ it('deletes cached results in non-suspense environments', async () => {
   // NOTE: The operation should not be duplicated
   expect(output).not.toHaveBeenCalled();
 });
+
+it('never allows restoration of invalidated results', async () => {
+  client.suspense = false;
+
+  const onPush = jest.fn();
+  const initialState = { [queryOperation.key]: serializedQueryResponse as any };
+
+  const ssr = ssrExchange({
+    isClient: true,
+    initialState: { ...initialState },
+  });
+
+  const { source: ops$, next } = input;
+  const exchange = ssr(exchangeInput)(ops$);
+
+  pipe(exchange, forEach(onPush));
+  next(queryOperation);
+
+  await Promise.resolve();
+
+  expect(Object.keys(ssr.extractData()).length).toBe(0);
+  expect(onPush).toHaveBeenCalledTimes(1);
+  expect(output).not.toHaveBeenCalled();
+
+  ssr.restoreData(initialState);
+  expect(Object.keys(ssr.extractData()).length).toBe(0);
+
+  next(queryOperation);
+  expect(onPush).toHaveBeenCalledTimes(2);
+  expect(output).toHaveBeenCalledTimes(1);
+});