Repository: FormidableLabs/urql (ts)

Issue:
(graphcache) - Stale result with suspense in development mode
This seems to be related to #1253. I've mounted a component that has a paginated list in the form of (last: 20, before: X) which renders properly. After the list is rendered I trigger a state change so that the component re-renders and fetches (last 20, before: Y). I can see in the network tab that the data is being fetched properly. Logging the output of the `useQuery` hook also shows the element rendering after the data is fetched with the correct data but then being re-rendered and using the previous data.

Disabling suspense causes the proper list (last: 20, before: Y) to be shown. Creating a production build disables React's double-render and will show the correct list with suspense enabled and suspense disabled.

urql version & exchanges: 1.11.5, [dedupExchange, Graphcache.cacheExchange, fetchExchange, subscriptionExchange]

I pulled and built the changes from #1300 locally and ran `yarn add /path/to/local/build` to use the latest fixes. This did not resolve the issue.

**Steps to reproduce**

1. In a React development build
2. Create an urql client with suspense enabled and the graphcache
3. Create a component that renders a list of a relay connection 
4. Change the cursor of the rendered list

**Expected behavior**

The component rerenders with the updated list twice

**Actual behavior**

The component rerenders with the updated list and then rerenders with the list for the old cursor

Reproduction sandbox is here: https://codesandbox.io/s/recursing-moser-nvfw5?file=/src/App.js


Pull request description:
(react) - Fix Suspense-on-update giving stale results in Concurrent Mode
Resolve #1303

## Summary

Fix edge cases related to Suspense triggering on an update in Concurrent Mode. Previously it was possible for stale state to be preserved across the Suspense update instead of the new state showing up. This has been fixed by preventing the suspending query source from closing prematurely.

See CodeSandbox for reproduction / fix in action: https://codesandbox.io/s/optimistic-andras-f2j1q?file=/src/useQuery.ts

For some next steps it'd be interesting to explore ways to remove the `toSuspenseSource` cache in `useQuery` for another solution that's more in-line with per-client shortterm caches, e.g. https://github.com/FormidableLabs/urql/tree/refactor/shared-sources

## Set of changes

- Don't close `toSuspenseSource` sources prematurely when throwing suspense promise
- Fix cached values in `toSuspenseSource` from being sent without a prior `Pull` event
- Remove delay on resolving the suspense promise
- Prevent leaks in `useSource` when the initial state getter throws a suspense promise


Patch:
diff --git a/.changeset/many-rats-buy.md b/.changeset/many-rats-buy.md
new file mode 100644
index 00000000..925a3c21
--- /dev/null
+++ b/.changeset/many-rats-buy.md
@@ -0,0 +1,5 @@
+---
+'urql': patch
+---
+
+Fix edge cases related to Suspense triggering on an update in Concurrent Mode. Previously it was possible for stale state to be preserved across the Suspense update instead of the new state showing up. This has been fixed by preventing the suspending query source from closing prematurely.
diff --git a/.changeset/strange-scissors-swim.md b/.changeset/strange-scissors-swim.md
new file mode 100644
index 00000000..65fbb71f
--- /dev/null
+++ b/.changeset/strange-scissors-swim.md
@@ -0,0 +1,5 @@
+---
+'@urql/exchange-graphcache': patch
+---
+
+Fix a Relay Pagination edge case where overlapping ends of pages queried using the `last` argument would be in reverse order.
diff --git a/exchanges/graphcache/src/extras/relayPagination.ts b/exchanges/graphcache/src/extras/relayPagination.ts
index 534223e5..db3a728c 100644
--- a/exchanges/graphcache/src/extras/relayPagination.ts
+++ b/exchanges/graphcache/src/extras/relayPagination.ts
@@ -240,8 +240,8 @@ export const relayPagination = (params: PaginationParams = {}): Resolver => {
         pageInfo.startCursor = page.pageInfo.startCursor;
         pageInfo.hasPreviousPage = page.pageInfo.hasPreviousPage;
       } else if (typeof args.last === 'number') {
-        endEdges = concatEdges(cache, endEdges, page.edges);
-        endNodes = concatNodes(endNodes, page.nodes);
+        endEdges = concatEdges(cache, page.edges, endEdges);
+        endNodes = concatNodes(page.nodes, endNodes);
         pageInfo = page.pageInfo;
       } else {
         startEdges = concatEdges(cache, startEdges, page.edges);
diff --git a/packages/react-urql/src/hooks/useQuery.ts b/packages/react-urql/src/hooks/useQuery.ts
index 40701997..c0d1138a 100644
--- a/packages/react-urql/src/hooks/useQuery.ts
+++ b/packages/react-urql/src/hooks/useQuery.ts
@@ -5,10 +5,11 @@ import {
   Source,
   pipe,
   share,
-  takeWhile,
   concat,
+  onPush,
   fromValue,
   switchMap,
+  filter,
   map,
   scan,
 } from 'wonka';
@@ -54,37 +55,35 @@ export type UseQueryResponse<Data = any, Variables = object> = [
 /** Convert the Source to a React Suspense source on demand */
 function toSuspenseSource<T>(source: Source<T>): Source<T> {
   const shared = share(source);
-  let cache: T | void;
+  let cache: T | undefined;
   let resolve: (value: T) => void;
 
-  return sink => {
-    let hasSuspended = false;
-
+  const suspend$: Source<T> = sink => {
     pipe(
       shared,
-      takeWhile(result => {
-        // The first result that is received will resolve the suspense
-        // promise after waiting for a microtick
-        if (cache === undefined) Promise.resolve(result).then(resolve);
+      onPush(result => {
+        // The first result that is received will resolve the Suspense promise
+        if (resolve && cache === undefined) resolve(result);
         cache = result;
-        return !hasSuspended;
       })
     )(sink);
 
-    // If we haven't got a previous result then start suspending
-    // otherwise issue the last known result immediately
-    if (cache !== undefined) {
-      const signal = [cache] as [T] & { tag: 1 };
-      signal.tag = 1;
-      sink(signal);
-    } else {
-      hasSuspended = true;
-      sink(0 /* End */);
+    // If we haven't got a previous result then throw a Suspense promise
+    if (cache === undefined) {
       throw new Promise<T>(_resolve => {
         resolve = _resolve;
       });
     }
   };
+
+  return concat([
+    pipe(
+      fromValue<T>(cache!),
+      map<T, T>(() => cache!),
+      filter(x => x !== undefined)
+    ),
+    suspend$,
+  ]);
 }
 
 const isSuspense = (client: Client, context?: Partial<OperationContext>) =>
@@ -186,9 +185,7 @@ export function useQuery<Data = any, Variables = object>(
     if (!isSuspense(client, args.context)) update(query$);
   }, [update, client, query$, request, args.context]);
 
-  if (isSuspense(client, args.context)) {
-    update(query$);
-  }
+  if (isSuspense(client, args.context)) update(query$);
 
   return [state, executeQuery];
 }
diff --git a/packages/react-urql/src/hooks/useSource.ts b/packages/react-urql/src/hooks/useSource.ts
index 1e95985d..f7fb35db 100644
--- a/packages/react-urql/src/hooks/useSource.ts
+++ b/packages/react-urql/src/hooks/useSource.ts
@@ -5,6 +5,7 @@ import {
   Source,
   fromValue,
   makeSubject,
+  takeWhile,
   pipe,
   map,
   concat,
@@ -58,6 +59,7 @@ export function useSource<T, R>(
     try {
       pipe(
         transform(input$),
+        takeWhile(() => currentInit),
         subscribe(value => {
           state = value;
         })


Test patch:
diff --git a/exchanges/graphcache/src/extras/relayPagination.test.ts b/exchanges/graphcache/src/extras/relayPagination.test.ts
index 466638be..2583ba1e 100644
--- a/exchanges/graphcache/src/extras/relayPagination.test.ts
+++ b/exchanges/graphcache/src/extras/relayPagination.test.ts
@@ -1156,3 +1156,166 @@ it('Works with nodes absent from query', () => {
   expect(res.partial).toBe(false);
   expect(res.data).toEqual(results);
 });
+
+it('handles subsequent queries with larger last values', () => {
+  const Pagination = gql`
+    query($last: Int!) {
+      __typename
+      items(last: $last) {
+        __typename
+        edges {
+          __typename
+          node {
+            __typename
+            id
+          }
+        }
+        nodes {
+          __typename
+          id
+        }
+        pageInfo {
+          __typename
+          hasPreviousPage
+          startCursor
+        }
+      }
+    }
+  `;
+
+  const store = new Store({
+    resolvers: {
+      Query: {
+        items: relayPagination(),
+      },
+    },
+  });
+
+  const pageOne = {
+    __typename: 'Query',
+    items: {
+      __typename: 'ItemsConnection',
+      edges: [itemEdge(2)],
+      nodes: [itemNode(2)],
+      pageInfo: {
+        __typename: 'PageInfo',
+        hasPreviousPage: true,
+        startCursor: '2',
+      },
+    },
+  };
+
+  const pageTwo = {
+    __typename: 'Query',
+    items: {
+      __typename: 'ItemsConnection',
+      edges: [itemEdge(1), itemEdge(2)],
+      nodes: [itemNode(1), itemNode(2)],
+      pageInfo: {
+        __typename: 'PageInfo',
+        hasPreviousPage: false,
+        startCursor: '1',
+      },
+    },
+  };
+
+  const pageThree = {
+    __typename: 'Query',
+    items: {
+      __typename: 'ItemsConnection',
+      edges: [itemEdge(0), itemEdge(1), itemEdge(2)],
+      nodes: [itemNode(0), itemNode(1), itemNode(2)],
+      pageInfo: {
+        __typename: 'PageInfo',
+        hasPreviousPage: false,
+        startCursor: '0',
+      },
+    },
+  };
+
+  write(store, { query: Pagination, variables: { last: 1 } }, pageOne);
+  write(store, { query: Pagination, variables: { last: 2 } }, pageTwo);
+
+  let res = query(store, { query: Pagination, variables: { last: 2 } });
+
+  expect(res.partial).toBe(false);
+  expect(res.data).toEqual(pageTwo);
+
+  write(store, { query: Pagination, variables: { last: 3 } }, pageThree);
+
+  res = query(store, { query: Pagination, variables: { last: 3 } });
+
+  expect(res.partial).toBe(false);
+  expect(res.data).toEqual(pageThree);
+});
+
+it('handles subsequent queries with larger first values', () => {
+  const Pagination = gql`
+    query($first: Int!) {
+      __typename
+      items(first: $first) {
+        __typename
+        edges {
+          __typename
+          node {
+            __typename
+            id
+          }
+        }
+        nodes {
+          __typename
+          id
+        }
+        pageInfo {
+          __typename
+          hasNextPage
+          endCursor
+        }
+      }
+    }
+  `;
+
+  const store = new Store({
+    resolvers: {
+      Query: {
+        items: relayPagination(),
+      },
+    },
+  });
+
+  const pageOne = {
+    __typename: 'Query',
+    items: {
+      __typename: 'ItemsConnection',
+      edges: [itemEdge(1)],
+      nodes: [itemNode(1)],
+      pageInfo: {
+        __typename: 'PageInfo',
+        hasNextPage: true,
+        endCursor: '1',
+      },
+    },
+  };
+
+  const pageTwo = {
+    __typename: 'Query',
+    items: {
+      __typename: 'ItemsConnection',
+      edges: [itemEdge(1), itemEdge(2)],
+      nodes: [itemNode(1), itemNode(2)],
+      pageInfo: {
+        __typename: 'PageInfo',
+        hasNextPage: false,
+        endCursor: '2',
+      },
+    },
+  };
+
+  write(store, { query: Pagination, variables: { first: 1 } }, pageOne);
+  write(store, { query: Pagination, variables: { first: 2 } }, pageTwo);
+
+  const res = query(store, { query: Pagination, variables: { first: 2 } });
+
+  expect(res.partial).toBe(false);
+  expect(res.data).toEqual(pageTwo);
+});