Repository: FormidableLabs/urql (ts)

Issue:
RFC: preserving startCursor / endCursor in cases of empty pagination results
## Summary

When using relay style cursor pagination to fetch the latest result

```
connectionField(first: 10)
```

We get a result that looks like this:

```
pageInfo: { startCursor: 'ABC', endCursor: 'DEF' }
nodes: [{ ... }, { ... }, { ... }, ...]
```

If we want to poll for items earlier newer than `startCursor`, we might want to query for something like:

```
connectionField(last: 10, before $startCursor)
```

The issue is that when there is an empty result set: 

```
pageInfo: { startCursor: null, endCursor: null }
nodes: []
```

The `relayPagination` will then merge in & return the `null` cursor values for this latest result set. Ideally we'd like to keep the latest known `startCursor` so we can continue to re-issue the query for items before the `startCursor`.

## Proposed Solution

Add a new option, maybe named something like `skipEmpty` or `excludeEmpty`, which would skip over results that don't have any nodes/edges while [looping through the pages](https://github.com/FormidableLabs/urql/blob/e1278c91b8bb601e57835179c202ac4526c6f1bf/exchanges/graphcache/src/extras/relayPagination.ts#L207-L217) in the `relayPagination` helper, so as to not clobber the cursors for the existing paginated list.

```ts
if (params.excludeEmpty && (page.edges.length === 0 && page.nodes.length === 0 && typename)) {
  continue
}
```


Pull request:
fix: ignore empty relay pagination pages
Resolves #2430 

## Summary

Ignores empty pages in the `relayPagination` helper when there is at least one result, so as to preserve the start & end cursor when an empty result is returned with null cursor values.

## Set of changes

- Ignore empty pages when we already have at least one page in the cursor
- Adds test for the intended behavior

Patch:
diff --git a/.changeset/fluffy-toys-live.md b/.changeset/fluffy-toys-live.md
new file mode 100644
index 00000000..099670d7
--- /dev/null
+++ b/.changeset/fluffy-toys-live.md
@@ -0,0 +1,5 @@
+---
+"@urql/exchange-graphcache": patch
+---
+
+Fix ignore empty relay edges when using the `relayPagination` resolver
diff --git a/.changeset/shiny-bags-serve.md b/.changeset/shiny-bags-serve.md
new file mode 100644
index 00000000..8fdf6875
--- /dev/null
+++ b/.changeset/shiny-bags-serve.md
@@ -0,0 +1,5 @@
+---
+'@urql/exchange-execute': patch
+---
+
+Support using default values with directives. Previously, using a variables with a default value within a directive would fail the validation if it is empty.
diff --git a/exchanges/execute/src/execute.ts b/exchanges/execute/src/execute.ts
index 688fdf5e..0a42cdd7 100644
--- a/exchanges/execute/src/execute.ts
+++ b/exchanges/execute/src/execute.ts
@@ -141,13 +141,25 @@ export const executeExchange = ({
 
         const contextValue =
           typeof context === 'function' ? context(operation) : context;
+
+        // Filter undefined values from variables before calling execute()
+        // to support default values within directives.
+        const variableValues = Object.create(null);
+        if (operation.variables) {
+          for (const key in operation.variables) {
+            if (operation.variables[key] !== undefined) {
+              variableValues[key] = operation.variables[key];
+            }
+          }
+        }
+
         return pipe(
           makeExecuteSource(operation, {
             schema,
             document: operation.query,
             rootValue,
             contextValue,
-            variableValues: operation.variables,
+            variableValues,
             operationName: getOperationName(operation.query),
             fieldResolver,
             typeResolver,
diff --git a/exchanges/graphcache/default-storage/package.json b/exchanges/graphcache/default-storage/package.json
index f17d0459..e817e5d1 100644
--- a/exchanges/graphcache/default-storage/package.json
+++ b/exchanges/graphcache/default-storage/package.json
@@ -16,7 +16,7 @@
     "./package.json": "./package.json"
   },
   "dependencies": {
-    "@urql/core": ">=2.3.3",
+    "@urql/core": ">=2.3.6",
     "wonka": "^4.0.14"
   }
 }
diff --git a/exchanges/graphcache/src/extras/relayPagination.ts b/exchanges/graphcache/src/extras/relayPagination.ts
index 8e03a2c7..26f5a933 100644
--- a/exchanges/graphcache/src/extras/relayPagination.ts
+++ b/exchanges/graphcache/src/extras/relayPagination.ts
@@ -214,6 +214,9 @@ export const relayPagination = (
       if (page === null) {
         continue;
       }
+      if (page.nodes.length === 0 && page.edges.length === 0 && typename) {
+        continue;
+      }
 
       if (
         mergeMode === 'inwards' &&


Test patch:
diff --git a/exchanges/execute/src/execute.test.ts b/exchanges/execute/src/execute.test.ts
index 19150c59..ff83cd70 100644
--- a/exchanges/execute/src/execute.test.ts
+++ b/exchanges/execute/src/execute.test.ts
@@ -22,7 +22,11 @@ import {
   empty,
   Source,
 } from 'wonka';
-import { queryOperation, subscriptionOperation } from '@urql/core/test-utils';
+import {
+  context,
+  queryOperation,
+  subscriptionOperation,
+} from '@urql/core/test-utils';
 import {
   makeErrorResult,
   makeOperation,
@@ -182,6 +186,45 @@ describe('on operation', () => {
     expect(mocked(execute)).toBeCalledTimes(1);
     expect(fetchMock).toBeCalledTimes(1);
   });
+
+  it('should trim undefined values before calling execute()', async () => {
+    const contextValue = 'USER_ID=123';
+
+    const operation = makeOperation(
+      'query',
+      {
+        ...queryOperation,
+        variables: { ...queryOperation.variables, withLastName: undefined },
+      },
+      context
+    );
+
+    await pipe(
+      fromValue(operation),
+      executeExchange({ schema, context: contextValue })(exchangeArgs),
+      take(1),
+      toPromise
+    );
+
+    expect(mocked(execute)).toBeCalledTimes(1);
+    expect(mocked(execute)).toBeCalledWith({
+      schema,
+      document: queryOperation.query,
+      rootValue: undefined,
+      contextValue: contextValue,
+      variableValues: queryOperation.variables,
+      operationName: expectedQueryOperationName,
+      fieldResolver: undefined,
+      typeResolver: undefined,
+      subscribeFieldResolver: undefined,
+    });
+
+    const variables = mocked(execute).mock.calls[0][0].variableValues;
+
+    for (const key in variables) {
+      expect(variables[key]).not.toBeUndefined();
+    }
+  });
 });
 
 describe('on success response', () => {
diff --git a/exchanges/graphcache/src/extras/relayPagination.test.ts b/exchanges/graphcache/src/extras/relayPagination.test.ts
index 2583ba1e..caac5fd8 100644
--- a/exchanges/graphcache/src/extras/relayPagination.test.ts
+++ b/exchanges/graphcache/src/extras/relayPagination.test.ts
@@ -1319,3 +1319,162 @@ it('handles subsequent queries with larger first values', () => {
   expect(res.partial).toBe(false);
   expect(res.data).toEqual(pageTwo);
 });
+
+it('ignores empty pages when paginating', () => {
+  const PaginationForward = gql`
+    query($first: Int!, $after: String) {
+      __typename
+      items(first: $first, after: $after) {
+        __typename
+        nodes {
+          __typename
+          id
+        }
+        pageInfo {
+          __typename
+          startCursor
+          endCursor
+        }
+      }
+    }
+  `;
+  const PaginationBackward = gql`
+    query($last: Int!, $before: String) {
+      __typename
+      items(last: $last, before: $before) {
+        __typename
+        nodes {
+          __typename
+          id
+        }
+        pageInfo {
+          __typename
+          startCursor
+          endCursor
+        }
+      }
+    }
+  `;
+
+  const store = new Store({
+    resolvers: {
+      Query: {
+        items: relayPagination(),
+      },
+    },
+  });
+
+  const forwardOne = {
+    __typename: 'Query',
+    items: {
+      __typename: 'ItemsConnection',
+      nodes: [itemNode(1), itemNode(2)],
+      pageInfo: {
+        __typename: 'PageInfo',
+        startCursor: '1',
+        endCursor: '2',
+      },
+    },
+  };
+  const forwardAfter = {
+    __typename: 'Query',
+    items: {
+      __typename: 'ItemsConnection',
+      nodes: [],
+      pageInfo: {
+        __typename: 'PageInfo',
+        startCursor: null,
+        endCursor: null,
+      },
+    },
+  };
+  const backwardBefore = {
+    __typename: 'Query',
+    items: {
+      __typename: 'ItemsConnection',
+      nodes: [],
+      pageInfo: {
+        __typename: 'PageInfo',
+        startCursor: null,
+        endCursor: null,
+      },
+    },
+  };
+
+  write(
+    store,
+    { query: PaginationForward, variables: { first: 2 } },
+    forwardOne
+  );
+  write(
+    store,
+    { query: PaginationBackward, variables: { last: 1, before: '1' } },
+    backwardBefore
+  );
+
+  const res = query(store, {
+    query: PaginationForward,
+    variables: { first: 2 },
+  });
+
+  expect(res.partial).toBe(false);
+  expect(res.data).toEqual(forwardOne);
+  write(
+    store,
+    { query: PaginationForward, variables: { first: 1, after: '2' } },
+    forwardAfter
+  );
+
+  expect(res.partial).toBe(false);
+  expect(res.data).toEqual(forwardOne);
+});
+
+it('allows for an empty page when this is the only result', () => {
+  const Pagination = gql`
+    query($first: Int!, $after: String) {
+      __typename
+      items(first: $first, after: $after) {
+        __typename
+        nodes {
+          __typename
+          id
+        }
+        pageInfo {
+          __typename
+          startCursor
+          endCursor
+        }
+      }
+    }
+  `;
+
+  const store = new Store({
+    resolvers: {
+      Query: {
+        items: relayPagination(),
+      },
+    },
+  });
+
+  const pageOne = {
+    __typename: 'Query',
+    items: {
+      __typename: 'ItemsConnection',
+      nodes: [],
+      pageInfo: {
+        __typename: 'PageInfo',
+        startCursor: null,
+        endCursor: null,
+      },
+    },
+  };
+
+  write(store, { query: Pagination, variables: { first: 2 } }, pageOne);
+  const res = query(store, {
+    query: Pagination,
+    variables: { first: 2 },
+  });
+
+  expect(res.partial).toBe(false);
+  expect(res.data).toEqual(pageOne);
+});