Skip to content

Commit

Permalink
fix: normalize field set selection (#3162)
Browse files Browse the repository at this point in the history
`FieldSet` scalar represents a selection set without outer braces. This
means that users could potentially specify some selections that could be
normalized (i.e. eliminate duplicate field selections, hoist/collapse
unnecessary inline fragments, etc). Previously we were using `@requires`
field set selection AS-IS for edge conditions. With this change we will
now normalize the `FieldSet` selections before using them as fetch node
conditions.

<!-- FED-390 -->
  • Loading branch information
dariuszkuc authored Oct 7, 2024
1 parent df5eb3c commit cc45734
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 1 deletion.
8 changes: 8 additions & 0 deletions .changeset/slow-dots-explode.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@apollo/query-planner": patch
"@apollo/federation-internals": patch
---

fix: normalize field set selection sets

`FieldSet` scalar represents a selection set without outer braces. This means that users could potentially specify some selections that could be normalized (i.e. eliminate duplicate field selections, hoist/collapse unnecessary inline fragments, etc). Previously we were using `@requires` field set selection AS-IS for edge conditions. With this change we will now normalize the `FieldSet` selections before using them as fetch node conditions.
2 changes: 1 addition & 1 deletion internals-js/src/federation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2302,7 +2302,7 @@ export function parseFieldSetArgument({
}
});
}
return selectionSet;
return selectionSet.normalize({ parentType, recursive: true });
} catch (e) {
if (!(e instanceof GraphQLError) || !decorateValidationErrors) {
throw e;
Expand Down
88 changes: 88 additions & 0 deletions query-planner-js/src/__tests__/buildPlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2986,6 +2986,94 @@ describe('@requires', () => {
}
`);
});

it(`normalizes requires field set selection`, () => {
const subgraph1 = {
name: 'Subgraph1',
typeDefs: gql`
type Query {
t: T
}
type T @key(fields: "id") {
id: ID!
a: A
}
type A {
a1: Int
a2: Int
}
`,
};

const subgraph2 = {
name: 'Subgraph2',
typeDefs: gql`
type T @key(fields: "id") {
id: ID!
a: A @external
b: Int @requires(fields: "a { ... on A { a1 a2 } a1 }")
}
type A @external {
a1: Int
a2: Int
}
`,
};

const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2);
const operation = operationFromDocument(
api,
gql`
{
t {
b
}
}
`,
);

const plan = queryPlanner.buildQueryPlan(operation);
expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Sequence {
Fetch(service: "Subgraph1") {
{
t {
__typename
id
a {
a1
a2
}
}
}
},
Flatten(path: "t") {
Fetch(service: "Subgraph2") {
{
... on T {
__typename
id
a {
a1
a2
}
}
} =>
{
... on T {
b
}
}
},
},
},
}
`);
});
});

describe('fetch operation names', () => {
Expand Down

0 comments on commit cc45734

Please sign in to comment.