Skip to content

Commit 062572b

Browse files
authored
fix(query-planner): fixed missing referenced variables in the variableUsages field (#3166)
Fixed missing referenced variables in the `variableUsages` field of fetch operations Query variables used in fetch operation should be listed in the `variableUsages` field. However, there was a bug where variables referenced by query-level directives could be missing in the field. The previous PR (#2986) fixed a similar issue in fetch queries, but it didn't fully fix the issue by failing to update the `variableUsages` field. This PR completes the remaining issue. <!-- [FED-387] --> [FED-387]: https://apollographql.atlassian.net/browse/FED-387?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
1 parent 1c99cb0 commit 062572b

File tree

4 files changed

+28
-8
lines changed

4 files changed

+28
-8
lines changed

.changeset/stupid-geese-camp.md

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@apollo/query-planner": patch
3+
"@apollo/query-graphs": patch
4+
"@apollo/federation-internals": patch
5+
---
6+
7+
Fixed missing referenced variables in the `variableUsages` field of fetch operations
8+
9+
Query variables used in fetch operation should be listed in the `variableUsages` field. However, there was a bug where variables referenced by query-level directives could be missing in the field.

internals-js/src/operations.ts

+5
Original file line numberDiff line numberDiff line change
@@ -1193,6 +1193,11 @@ export class NamedFragmentDefinition extends DirectiveTargetElement<NamedFragmen
11931193
}
11941194
}
11951195

1196+
collectVariables(collector: VariableCollector) {
1197+
this.selectionSet.collectVariables(collector);
1198+
this.collectVariablesInAppliedDirectives(collector);
1199+
}
1200+
11961201
toFragmentDefinitionNode() : FragmentDefinitionNode {
11971202
return {
11981203
kind: Kind.FRAGMENT_DEFINITION,

query-planner-js/src/__tests__/buildPlan.test.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -8977,8 +8977,10 @@ describe('handles operations with directives', () => {
89778977
query testQuery__Subgraph1__0($some_var: String!) @withArgs(arg1: $some_var) {
89788978
test
89798979
}
8980-
`); // end of test
8981-
});
8980+
`);
8981+
// Make sure the `variableUsage` also captures the variable as well.
8982+
expect(fetch_nodes[0].variableUsages?.includes('some_var')).toBe(true);
8983+
}); // end of test
89828984
}); // end of `describe`
89838985

89848986
describe('@fromContext impacts on query planning', () => {

query-planner-js/src/buildPlan.ts

+10-6
Original file line numberDiff line numberDiff line change
@@ -1600,22 +1600,26 @@ class FetchGroup {
16001600
);
16011601
}
16021602

1603-
// collect all used variables in the selection and in used Fragments
1604-
const usedVariables = new Set(selection.usedVariables().map(v => v.name));
1603+
// collect all used variables in the selection and the used directives and fragments.
1604+
const collector = new VariableCollector();
1605+
// Note: The operation's selectionSet includes `representations` variable,
1606+
// thus we use `selection` here instead.
1607+
selection.collectVariables(collector);
1608+
operation.collectVariablesInAppliedDirectives(collector);
16051609
if (operation.fragments) {
16061610
for (const namedFragment of operation.fragments.definitions()) {
1607-
namedFragment.selectionSet.usedVariables().forEach(v => {
1608-
usedVariables.add(v.name);
1609-
});
1611+
namedFragment.collectVariables(collector);
16101612
}
16111613
}
1614+
const usedVariables = collector.variables();
1615+
16121616
const operationDocument = operationToDocument(operation);
16131617
const fetchNode: FetchNode = {
16141618
kind: 'Fetch',
16151619
id: this.id,
16161620
serviceName: this.subgraphName,
16171621
requires: inputNodes ? trimSelectionNodes(inputNodes.selections) : undefined,
1618-
variableUsages: Array.from(usedVariables),
1622+
variableUsages: usedVariables.map((v) => v.name),
16191623
operation: stripIgnoredCharacters(print(operationDocument)),
16201624
operationKind: schemaRootKindToOperationKind(operation.rootKind),
16211625
operationName: operation.name,

0 commit comments

Comments
 (0)