Skip to content

Add missing query graph edge for interfaces implementing interfaces #3122

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.

Sign up for GitHub

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

Merged
dariuszkuc merged 3 commits into from
Aug 21, 2024
Merged

Add missing query graph edge for interfaces implementing interfaces #3122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8a6bfec
Fix bug where abstract type edge addition mistakenly believed edges w…
sachindshinde Aug 9, 2024
496e5a3
The previous change fixes an outstanding TODO in an interfaces-implem…
sachindshinde Aug 9, 2024
7040fba
Add changeset
sachindshinde Aug 21, 2024
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
6 changes: 6 additions & 0 deletions .changeset/cuddly-badgers-sit.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@apollo/gateway": patch
"@apollo/query-graphs": patch
---

Avoid type explosion for inline fragments where the type condition is an interface that implements the parent type.
12 changes: 2 additions & 10 deletions gateway-js/src/__tests__/executeQueryPlan.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2300,22 +2300,14 @@ describe('executeQueryPlan', () => {

queryPlan = buildPlan(operation, queryPlanner);

// TODO: we're actually type-exploding in this case because currently, as soon as we need to type-explode, we do
// so into all the runtime types, while here it could make sense to only type-explode into the direct sub-types=
// (the sub-interfaces). We should fix this (but it's only sub-optimal, not incorrect).
expect(queryPlan).toMatchInlineSnapshot(`
QueryPlan {
Fetch(service: "S1") {
{
allValues {
__typename
... on T1 {
a
}
... on T2 {
a
}
... on T4 {
... on SubInterface1 {
__typename
a
}
}
Expand Down
6 changes: 0 additions & 6 deletions query-graphs-js/src/querygraph.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1611,12 +1611,6 @@ class GraphBuilderFromSchema extends GraphBuilder {
for (let j = i; j < abstractTypesWithTheirRuntimeTypes.length; j++) {
const t2 = abstractTypesWithTheirRuntimeTypes[j];

// We ignore the pair if both are interfaces and one implements the other. We'll already have appropriate
// edges if that's the case.
if (isInterfaceType(t1.type) && isInterfaceType(t2.type) && (t1.type.implementsInterface(t2.type) || t2.type.implementsInterface(t1.type))) {
continue;
}

let addT1ToT2 = false;
let addT2ToT1 = false;
if (t1.type === t2.type) {
Expand Down