diff --git a/composition-js/src/__tests__/connectors.test.ts b/composition-js/src/__tests__/connectors.test.ts index 03dea1d96..fb26fe89c 100644 --- a/composition-js/src/__tests__/connectors.test.ts +++ b/composition-js/src/__tests__/connectors.test.ts @@ -39,7 +39,7 @@ describe("connect spec and join__directive", () => { "schema @link(url: \\"https://specs.apollo.dev/link/v1.0\\") @link(url: \\"https://specs.apollo.dev/join/v0.5\\", for: EXECUTION) - @link(url: \\"https://specs.apollo.dev/connect/v0.1\\", for: EXECUTION) + @link(url: \\"https://specs.apollo.dev/connect/v0.2\\", for: EXECUTION) @join__directive(graphs: [WITH_CONNECTORS], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.1\\", import: [\\"@connect\\", \\"@source\\"]}) @join__directive(graphs: [WITH_CONNECTORS], name: \\"source\\", args: {name: \\"v1\\", http: {baseURL: \\"http://v1\\"}}) { @@ -322,7 +322,7 @@ describe("connect spec and join__directive", () => { "schema @link(url: \\"https://specs.apollo.dev/link/v1.0\\") @link(url: \\"https://specs.apollo.dev/join/v0.5\\", for: EXECUTION) - @link(url: \\"https://specs.apollo.dev/connect/v0.1\\", for: EXECUTION) + @link(url: \\"https://specs.apollo.dev/connect/v0.2\\", for: EXECUTION) @join__directive(graphs: [WITH_CONNECTORS], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.1\\", as: \\"http\\", import: [{name: \\"@connect\\", as: \\"@http\\"}, {name: \\"@source\\", as: \\"@api\\"}]}) @join__directive(graphs: [WITH_CONNECTORS], name: \\"api\\", args: {name: \\"v1\\", http: {baseURL: \\"http://v1\\"}}) { diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts index ef5abfff5..10e1fefc0 100644 --- a/composition-js/src/merging/merge.ts +++ b/composition-js/src/merging/merge.ts @@ -84,6 +84,8 @@ import { DirectiveCompositionSpecification, CoreImport, inaccessibleIdentity, + FeatureDefinitions, + CONNECT_VERSIONS, } from "@apollo/federation-internals"; import { ASTNode, GraphQLError, DirectiveLocation } from "graphql"; import { @@ -376,7 +378,7 @@ class Merger { private linkSpec: CoreSpecDefinition; private inaccessibleDirectiveInSupergraph?: DirectiveDefinition; private latestFedVersionUsed: FeatureVersion; - private joinDirectiveIdentityURLs = new Set(); + private joinDirectiveFeatureDefinitionsByIdentity = new Map(); private schemaToImportNameToFeatureUrl = new Map>(); private fieldsWithFromContext: Set; private fieldsWithOverride: Set; @@ -413,10 +415,9 @@ class Merger { this.subgraphNamesToJoinSpecName = this.prepareSupergraph(); this.appliedDirectivesToMerge = []; - [ // Represent any applications of directives imported from these spec URLs - // using @join__directive in the merged supergraph. - connectIdentity, - ].forEach(url => this.joinDirectiveIdentityURLs.add(url)); + // Represent any applications of directives imported from these spec URLs + // using @join__directive in the merged supergraph. + this.joinDirectiveFeatureDefinitionsByIdentity.set(CONNECT_VERSIONS.identity, CONNECT_VERSIONS); } private getLatestFederationVersionUsed(): FeatureVersion { @@ -3105,10 +3106,7 @@ class Merger { } private shouldUseJoinDirectiveForURL(url: FeatureUrl | undefined): boolean { - return Boolean( - url && - this.joinDirectiveIdentityURLs.has(url.identity) - ); + return Boolean(url && this.joinDirectiveFeatureDefinitionsByIdentity.has(url.identity)); } private computeMapFromImportNameToIdentityUrl( @@ -3209,21 +3207,29 @@ class Merger { const linkDirective = this.linkSpec.coreDirective(this.merged); - // When persisting features as @link directives in the supergraph, we can't - // repeat features that have the same identity, but different versions. This - // chooses the highest version of each feature to persist. + // When persisting features as @link directives in the supergraph, we have + // to pick a single version. For these features, we've decided to always + // pick the latest known version, regardless of what version is use in + // subgraphs. This means that a composition version change will change the + // output, even if the subgraphs don't change, requiring a newer version of + // the router. We made this decision because these features are pre-1.0 and + // change more frequently than federation features. // // (The original feature version is still recorded in a @join__directive // so we're not losing any information.) - const highestLinkByIdentity = [...linksToPersist].reduce((map, link) => { - const existing = map.get(link.identity); - if (!existing || existing.version.lt(link.version)) { - map.set(link.identity, link); + const latestOrHighestLinkByIdentity = [...linksToPersist].reduce((map, link) => { + let latest = this.joinDirectiveFeatureDefinitionsByIdentity.get(link.identity)?.latest(); + + const existing = map.get(link.identity) ?? link; + if (!latest || existing?.version.gt(latest.version)) { + latest = existing; } + + map.set(link.identity, latest); return map; }, new Map()); - for (const [_, link] of highestLinkByIdentity) { + for (const [_, link] of latestOrHighestLinkByIdentity) { dest.applyDirective(linkDirective, { url: link.toString(), for: link.defaultCorePurpose,