From cc4573471696ef78d04fa00c4cf8e5c50314ba9f Mon Sep 17 00:00:00 2001 From: Dariusz Kuc <9501705+dariuszkuc@users.noreply.github.com> Date: Mon, 7 Oct 2024 14:02:38 -0500 Subject: [PATCH] fix: normalize field set selection (#3162) `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. --- .changeset/slow-dots-explode.md | 8 ++ internals-js/src/federation.ts | 2 +- .../src/__tests__/buildPlan.test.ts | 88 +++++++++++++++++++ 3 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 .changeset/slow-dots-explode.md diff --git a/.changeset/slow-dots-explode.md b/.changeset/slow-dots-explode.md new file mode 100644 index 000000000..8028f255b --- /dev/null +++ b/.changeset/slow-dots-explode.md @@ -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. \ No newline at end of file diff --git a/internals-js/src/federation.ts b/internals-js/src/federation.ts index 54efc200e..5cefe8be0 100644 --- a/internals-js/src/federation.ts +++ b/internals-js/src/federation.ts @@ -2302,7 +2302,7 @@ export function parseFieldSetArgument({ } }); } - return selectionSet; + return selectionSet.normalize({ parentType, recursive: true }); } catch (e) { if (!(e instanceof GraphQLError) || !decorateValidationErrors) { throw e; diff --git a/query-planner-js/src/__tests__/buildPlan.test.ts b/query-planner-js/src/__tests__/buildPlan.test.ts index 9822c0ccc..5283ffe3f 100644 --- a/query-planner-js/src/__tests__/buildPlan.test.ts +++ b/query-planner-js/src/__tests__/buildPlan.test.ts @@ -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', () => {