From 7620efacc52855177015ce8fee56f814a9bbd7c0 Mon Sep 17 00:00:00 2001 From: Ben Date: Fri, 5 Aug 2022 18:11:30 -0500 Subject: [PATCH 1/2] Don't apply `@shareable` if it's already `@shareable` In the course of updating to fed2, it's feasible that subgraphs may want to add `@shareable` applications without `@link`ing to federation/2.0 and importing it from there. Previously, this would throw an error because the schema upgrader will try to add a `@shareable` application that has already been manually added. Now we'll check first. --- .../__tests__/composeFed1Subgraphs.test.ts | 37 +++++++++++++++++++ internals-js/src/schemaUpgrader.ts | 4 +- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/composition-js/src/__tests__/composeFed1Subgraphs.test.ts b/composition-js/src/__tests__/composeFed1Subgraphs.test.ts index 60a8254fb..6ad5c1fcf 100644 --- a/composition-js/src/__tests__/composeFed1Subgraphs.test.ts +++ b/composition-js/src/__tests__/composeFed1Subgraphs.test.ts @@ -583,6 +583,43 @@ describe('shareable', () => { } `); }); + + it('supports fed1 subgraphs that define @shareable', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + type Queryf { + friendlyFruit: Fruit! + } + + directive @shareable on OBJECT | FIELD_DEFINITION + + type Fruit @shareable { + id: ID! + name: String! + } + ` + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + type Query { + forbiddenFruit: Fruit! + } + + directive @shareable on OBJECT | FIELD_DEFINITION + + type Fruit @shareable { + id: ID! + name: String! + } + ` + }; + + const result = composeServices([subgraphA, subgraphB]); + assertCompositionSuccess(result); + }); }); describe('override', () => { diff --git a/internals-js/src/schemaUpgrader.ts b/internals-js/src/schemaUpgrader.ts index 879c98bbf..f01afdb20 100644 --- a/internals-js/src/schemaUpgrader.ts +++ b/internals-js/src/schemaUpgrader.ts @@ -676,14 +676,14 @@ class SchemaUpgrader { continue; } const otherResolvingSubgraphs = this.otherSubgraphs.filter((s) => resolvesField(s, field)); - if (otherResolvingSubgraphs.length > 0) { + if (otherResolvingSubgraphs.length > 0 && !field.hasAppliedDirective(shareableDirective)) { field.applyDirective(shareableDirective); this.addChange(new ShareableFieldAddition(field.coordinate, otherResolvingSubgraphs.map((s) => s.name))); } } } else { const otherDeclaringSubgraphs = this.otherSubgraphs.filter((s) => s.schema.type(type.name)); - if (otherDeclaringSubgraphs.length > 0) { + if (otherDeclaringSubgraphs.length > 0 && !type.hasAppliedDirective(shareableDirective)) { type.applyDirective(shareableDirective); this.addChange(new ShareableTypeAddition(type.coordinate, otherDeclaringSubgraphs.map((s) => s.name))); } From 85d4c31271e2f562e889880d6f84d0ec0d1b0612 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 8 Aug 2022 16:21:38 -0500 Subject: [PATCH 2/2] Add changelog --- composition-js/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/composition-js/CHANGELOG.md b/composition-js/CHANGELOG.md index 116c30f30..3d22141e3 100644 --- a/composition-js/CHANGELOG.md +++ b/composition-js/CHANGELOG.md @@ -2,6 +2,8 @@ This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The Federation v0.x equivalent for this package can be found [here](https://github.com/apollographql/federation/blob/version-0.x/federation-js/CHANGELOG.md) on the `version-0.x` branch of this repo. +- Don't apply @shareable when upgrading fed1 supergraphs if it's already @shareable [PR #2043](https://github.com/apollographql/federation/pull/2043) + ## 2.1.0-alpha.2 - Add `@composeDirective` directive to specify directives that should be merged to the supergraph during composition [PR #1996](https://github.com/apollographql/federation/pull/1996).