fix includeUnusedVariables not working with BatchHttpLink (#10754)

Co-authored-by: Alessia Bellisario <alessia@apollographql.com>
Co-authored-by: Jerel Miller <jerelmiller@gmail.com>

.changeset/old-moles-run.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Fix `includeUnusedVariables` option not working with `BatchHttpLink`

src/__tests__/__snapshots__/exports.ts.snap

@@ -225,6 +225,7 @@ Array [
 exports[`exports of public entry points @apollo/client/link/utils 1`] = `
 Array [
   "createOperation",
+  "filterOperationVariables",
   "fromError",
   "fromPromise",
   "throwServerError",

src/link/batch-http/__tests__/batchHttpLink.ts

@@ -371,6 +371,7 @@ describe('SharedHttpTest', () => {
     link: ApolloLink,
     after: () => void,
     includeExtensions: boolean,
+    includeUnusedVariables: boolean,
     reject: (e: Error) => void,
   ) => {
     const next = jest.fn();
@@ -389,7 +390,7 @@ describe('SharedHttpTest', () => {
         try {
           let body = convertBatchedBody(fetchMock.lastCall()![1]!.body);
           expect(body.query).toBe(print(sampleMutation));
-          expect(body.variables).toEqual(variables);
+          expect(body.variables).toEqual(includeUnusedVariables ? variables : {});
           expect(body.context).not.toBeDefined();
           if (includeExtensions) {
             expect(body.extensions).toBeDefined();
@@ -410,8 +411,9 @@ describe('SharedHttpTest', () => {
     const link = createHttpLink({ uri: '/data', includeExtensions: true });
     verifyRequest(
       link,
-      () => verifyRequest(link, resolve, true, reject),
+      () => verifyRequest(link, resolve, true, false, reject),
       true,
+      false,
       reject,
     );
   });
@@ -420,7 +422,8 @@ describe('SharedHttpTest', () => {
     const link = createHttpLink({ uri: '/data' });
     verifyRequest(
       link,
-      () => verifyRequest(link, resolve, false, reject),
+      () => verifyRequest(link, resolve, false, false, reject),
+      false,
       false,
       reject,
     );

src/link/batch-http/batchHttpLink.ts

@@ -17,6 +17,7 @@ import {
   createSignalIfSupported,
 } from '../http';
 import { BatchLink } from '../batch';
+import { filterOperationVariables } from "../utils/filterOperationVariables";
 
 export namespace BatchHttpLink {
   export type Options = Pick<
@@ -49,6 +50,7 @@ export class BatchHttpLink extends ApolloLink {
       batchDebounce,
       batchMax,
       batchKey,
+      includeUnusedVariables = false,
       ...requestOptions
     } = fetchParams || ({} as BatchHttpLink.Options);
 
@@ -118,15 +120,21 @@ export class BatchHttpLink extends ApolloLink {
       }
 
       //uses fallback, link, and then context to build options
-      const optsAndBody = operations.map((operation, index) =>
-        selectHttpOptionsAndBodyInternal(
+      const optsAndBody = operations.map((operation, index) => {
+        const result = selectHttpOptionsAndBodyInternal(
           { ...operation, query: queries[index]! },
           print,
           fallbackHttpConfig,
           linkConfig,
-          contextConfig,
-        ),
-      );
+          contextConfig
+        );
+
+        if (result.body.variables && !includeUnusedVariables) {
+          result.body.variables = filterOperationVariables(result.body, operation);
+        }
+
+        return result;
+      });
 
       const loadedBody = optsAndBody.map(({ body }) => body);
       const options = optsAndBody[0].options;

src/link/http/createHttpLink.ts

@@ -1,6 +1,6 @@
 import { __DEV__, invariant } from '../../utilities/globals';
 
-import { visit, DefinitionNode, VariableDefinitionNode } from 'graphql';
+import { DefinitionNode } from 'graphql';
 
 import { ApolloLink } from '../core';
 import { Observable, hasDirectives } from '../../utilities';
@@ -20,7 +20,7 @@ import {
 } from './selectHttpOptionsAndBody';
 import { createSignalIfSupported } from './createSignalIfSupported';
 import { rewriteURIForGET } from './rewriteURIForGET';
-import { fromError } from '../utils';
+import { fromError, filterOperationVariables } from '../utils';
 import {
   maybe,
   getMainDefinition,
@@ -114,26 +114,7 @@ export const createHttpLink = (linkOptions: HttpOptions = {}) => {
     );
 
     if (body.variables && !includeUnusedVariables) {
-      const unusedNames = new Set(Object.keys(body.variables));
-      visit(operation.query, {
-        Variable(node, _key, parent) {
-          // A variable type definition at the top level of a query is not
-          // enough to silence server-side errors about the variable being
-          // unused, so variable definitions do not count as usage.
-          // https://spec.graphql.org/draft/#sec-All-Variables-Used
-          if (parent && (parent as VariableDefinitionNode).kind !== 'VariableDefinition') {
-            unusedNames.delete(node.name.value);
-          }
-        },
-      });
-      if (unusedNames.size) {
-        // Make a shallow copy of body.variables (with keys in the same
-        // order) and then delete unused variables from the copy.
-        body.variables = { ...body.variables };
-        unusedNames.forEach(name => {
-          delete body.variables![name];
-        });
-      }
+      body.variables = filterOperationVariables(body.variables, operation);
     }
 
     let controller: any;

src/link/utils/__tests__/filterOperationVariables.ts

@@ -0,0 +1,39 @@
+import gql from 'graphql-tag';
+import { filterOperationVariables } from '../filterOperationVariables';
+import { createOperation } from '../createOperation';
+
+const sampleQueryWithVariables = gql`
+  query MyQuery($a: Int!) {
+    stub(a: $a) {
+      id
+    }
+  }
+`;
+
+const sampleQueryWithoutVariables = gql`
+  query MyQuery {
+    stub {
+      id
+    }
+  }
+`;
+
+describe('filterOperationVariables', () => {
+  it('filters unused variables', () => {
+    const variables = { a: 1, b: 2, c: 3 };
+    const result = filterOperationVariables(
+      variables,
+      createOperation({}, { query: sampleQueryWithoutVariables, variables })
+    );
+    expect(result).toEqual({});
+  });
+
+  it('does not filter used variables', () => {
+    const variables = { a: 1, b: 2, c: 3 };
+    const result = filterOperationVariables(
+      variables,
+      createOperation({}, { query: sampleQueryWithVariables, variables })
+    );
+    expect(result).toEqual({ a: 1 });
+  });
+});

src/link/utils/filterOperationVariables.ts

@@ -0,0 +1,23 @@
+import { VariableDefinitionNode, visit } from "graphql";
+
+import { Operation } from "../core";
+
+export function filterOperationVariables(variables: Record<string, any>, operation: Operation) {
+  const result = { ...variables };
+  const unusedNames = new Set(Object.keys(variables));
+  visit(operation.query, {
+    Variable(node, _key, parent) {
+      // A variable type definition at the top level of a query is not
+      // enough to silence server-side errors about the variable being
+      // unused, so variable definitions do not count as usage.
+      // https://spec.graphql.org/draft/#sec-All-Variables-Used
+      if (parent && (parent as VariableDefinitionNode).kind !== 'VariableDefinition') {
+        unusedNames.delete(node.name.value);
+      }
+    },
+  });
+  unusedNames.forEach(name => {
+    delete result![name];
+  });
+  return result;
+}

src/link/utils/index.ts

@@ -7,3 +7,4 @@ export { ServerError, throwServerError } from './throwServerError';
 export { validateOperation } from './validateOperation';
 export { createOperation } from './createOperation';
 export { transformOperation } from './transformOperation';
+export { filterOperationVariables } from './filterOperationVariables';