Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure query variables used in the directives applied at the operation level are retained in subgraph queries #2986

Merged
merged 3 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/two-dryers-deny.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/query-planner": patch
---

Ensure query variables used in the directives applied at the operation level are retained in subgraph queries (#2986)
38 changes: 38 additions & 0 deletions query-planner-js/src/__tests__/buildPlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8719,4 +8719,42 @@ describe('handles operations with directives', () => {
}
`);
});

test('subgraph query retains the query variables used in the directives applied to the query', () => {
const subgraph1 = {
name: 'Subgraph1',
typeDefs: gql`
directive @withArgs(arg1: String) on QUERY

type Query {
test: String!
}
`,
};

const subgraph2 = {
name: 'Subgraph2',
typeDefs: gql`
directive @withArgs(arg1: String) on QUERY
`,
};

const query = gql`
query testQuery($some_var: String!) @withArgs(arg1: $some_var) {
test
}
`;

const [api, qp] = composeAndCreatePlanner(subgraph1, subgraph2);
const op = operationFromDocument(api, query);
const queryPlan = qp.buildQueryPlan(op);
const fetch_nodes = findFetchNodes(subgraph1.name, queryPlan.node);
expect(fetch_nodes).toHaveLength(1);
// Note: `($some_var: String!)` used to be missing.
expect(parse(fetch_nodes[0].operation)).toMatchInlineSnapshot(`
query testQuery__Subgraph1__0($some_var: String!) @withArgs(arg1: $some_var) {
test
}
`); // end of test
});
}); // end of `describe`
19 changes: 17 additions & 2 deletions query-planner-js/src/buildPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
Variable,
VariableDefinition,
VariableDefinitions,
VariableCollector,
newDebugLogger,
selectionOfElement,
selectionSetOfElement,
Expand Down Expand Up @@ -4753,6 +4754,20 @@
return new VariableDefinition(schema, representationsVariable, representationsType);
}

// Collects all variables used in the operation to be created.
// - It's computed based on its selection set and directives.
function collectUsedVariables(selectionSet: SelectionSet, operationDirectives?: readonly Directive<any>[]) {
const collector = new VariableCollector();
selectionSet.collectVariables(collector);

if (operationDirectives) {
for (const applied of operationDirectives) {
collector.collectInArguments(applied.arguments());

Check warning on line 4765 in query-planner-js/src/buildPlan.ts

View check run for this annotation

Codecov / codecov/patch

query-planner-js/src/buildPlan.ts#L4765

Added line #L4765 was not covered by tests
}
}
return collector.variables();
}

function operationForEntitiesFetch(
subgraphSchema: Schema,
selectionSet: SelectionSet,
Expand All @@ -4763,7 +4778,7 @@
const variableDefinitions = new VariableDefinitions();
variableDefinitions.add(representationsVariableDefinition(subgraphSchema));
variableDefinitions.addAll(
allVariableDefinitions.filter(selectionSet.usedVariables()),
allVariableDefinitions.filter(collectUsedVariables(selectionSet, directives)),
);

const queryType = subgraphSchema.schemaDefinition.rootType('query');
Expand Down Expand Up @@ -4799,6 +4814,6 @@
// Note that this is called _before_ named fragments reuse is attempted, so there is not spread in
// the selection, hence the `undefined` for fragments.
return new Operation(subgraphSchema, rootKind, selectionSet,
allVariableDefinitions.filter(selectionSet.usedVariables()),
allVariableDefinitions.filter(collectUsedVariables(selectionSet, directives)),
/*fragments*/undefined, operationName, directives);
}