Skip to content

Commit

Permalink
fix: variables incorrectly removed in BatchHttpLink (#10964)
Browse files Browse the repository at this point in the history
  • Loading branch information
alessbell authored Jun 9, 2023
1 parent 2e833b2 commit f331715
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/odd-pumpkins-care.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@apollo/client': patch
---

Fixes a bug in `BatchHttpLink` that removed variables from all requests by default.
56 changes: 56 additions & 0 deletions src/link/batch-http/__tests__/batchHttpLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,62 @@ describe('SharedHttpTest', () => {
);
});

itAsync('strips unused variables, respecting nested fragments', (resolve, reject) => {
const link = createHttpLink({ uri: '/data' });

const query = gql`
query PEOPLE (
$declaredAndUsed: String,
$declaredButUnused: Int,
) {
people(
surprise: $undeclared,
noSurprise: $declaredAndUsed,
) {
... on Doctor {
specialty(var: $usedByInlineFragment)
}
...LawyerFragment
}
}
fragment LawyerFragment on Lawyer {
caseCount(var: $usedByNamedFragment)
}
`;

const variables = {
unused: 'strip',
declaredButUnused: 'strip',
declaredAndUsed: 'keep',
undeclared: 'keep',
usedByInlineFragment: 'keep',
usedByNamedFragment: 'keep',
};

execute(link, {
query,
variables,
}).subscribe({
next: makeCallback(resolve, reject, () => {
const [uri, options] = fetchMock.lastCall()!;
const { method, body } = options!;
expect(JSON.parse(body as string)).toEqual([{
operationName: "PEOPLE",
query: print(query),
variables: {
declaredAndUsed: 'keep',
undeclared: 'keep',
usedByInlineFragment: 'keep',
usedByNamedFragment: 'keep',
},
}]);
expect(method).toBe('POST');
expect(uri).toBe('/data');
}),
error: error => reject(error),
});
});

itAsync('unsubscribes without calling subscriber', (resolve, reject) => {
const link = createHttpLink({ uri: '/data' });
const observable = execute(link, {
Expand Down
5 changes: 4 additions & 1 deletion src/link/batch-http/batchHttpLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ export class BatchHttpLink extends ApolloLink {
);

if (result.body.variables && !includeUnusedVariables) {
result.body.variables = filterOperationVariables(result.body, operation);
result.body.variables = filterOperationVariables(
result.body.variables,
operation.query
);
}

return result;
Expand Down
2 changes: 1 addition & 1 deletion src/link/http/createHttpLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export const createHttpLink = (linkOptions: HttpOptions = {}) => {
);

if (body.variables && !includeUnusedVariables) {
body.variables = filterOperationVariables(body.variables, operation);
body.variables = filterOperationVariables(body.variables, operation.query);
}

let controller: any;
Expand Down
5 changes: 2 additions & 3 deletions src/link/utils/__tests__/filterOperationVariables.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import gql from 'graphql-tag';
import { filterOperationVariables } from '../filterOperationVariables';
import { createOperation } from '../createOperation';

const sampleQueryWithVariables = gql`
query MyQuery($a: Int!) {
Expand All @@ -23,7 +22,7 @@ describe('filterOperationVariables', () => {
const variables = { a: 1, b: 2, c: 3 };
const result = filterOperationVariables(
variables,
createOperation({}, { query: sampleQueryWithoutVariables, variables })
sampleQueryWithoutVariables
);
expect(result).toEqual({});
});
Expand All @@ -32,7 +31,7 @@ describe('filterOperationVariables', () => {
const variables = { a: 1, b: 2, c: 3 };
const result = filterOperationVariables(
variables,
createOperation({}, { query: sampleQueryWithVariables, variables })
sampleQueryWithVariables
);
expect(result).toEqual({ a: 1 });
});
Expand Down
20 changes: 12 additions & 8 deletions src/link/utils/filterOperationVariables.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,27 @@
import type { VariableDefinitionNode} from "graphql";
import { visit } from "graphql";
import type { VariableDefinitionNode, DocumentNode } from 'graphql';
import { visit } from 'graphql';

import type { Operation } from "../core";

export function filterOperationVariables(variables: Record<string, any>, operation: Operation) {
export function filterOperationVariables(
variables: Record<string, any>,
query: DocumentNode
) {
const result = { ...variables };
const unusedNames = new Set(Object.keys(variables));
visit(operation.query, {
visit(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') {
if (
parent &&
(parent as VariableDefinitionNode).kind !== 'VariableDefinition'
) {
unusedNames.delete(node.name.value);
}
},
});
unusedNames.forEach(name => {
unusedNames.forEach((name) => {
delete result![name];
});
return result;
Expand Down

0 comments on commit f331715

Please sign in to comment.