diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e61680..f364d52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [v3.1.1] - 2024-02-15 + +### Fix + +- The `FroidSchema` class does not include all enum values found across + subgraphs when enum definitions differ. + ## [v3.1.0] - 2023-11-09 - Added a new `FroidSchema` class to test the next version of FROID schema diff --git a/package.json b/package.json index 08d9195..045fe4f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@wayfair/node-froid", - "version": "3.1.0", + "version": "3.1.1", "description": "Federated GQL Relay Object Identification implementation", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/src/schema/FroidSchema.ts b/src/schema/FroidSchema.ts index 0832483..943bdbb 100644 --- a/src/schema/FroidSchema.ts +++ b/src/schema/FroidSchema.ts @@ -5,6 +5,7 @@ import { DefinitionNode, DocumentNode, EnumTypeDefinitionNode, + EnumValueDefinitionNode, FieldDefinitionNode, InterfaceTypeDefinitionNode, Kind, @@ -423,7 +424,7 @@ export class FroidSchema { * * Enum values with @inaccessible tags are stripped in Federation 2. * - * Contract @tag directives are NOt applied when generating non-native scalar + * Contract @tag directives are NOT applied when generating non-native scalar * return types in the Froid subgraph. Contract @tag directives are merged * during supergraph composition so Froid subgraph can rely on @tag directives * defined by the owning subgraph(s), UNLESS an enum value is marked @inaccessible, @@ -450,8 +451,12 @@ export class FroidSchema { }); }); - // De-dupe non-native scalar return types. Any definitions of scalars and enums - // will work since they can be guaranteed to be consistent across subgraphs + // De-dupe non-native scalar return types. + // + // Any definitions of scalars and enums will work since they are + // consistent across subgraphs. + // + // Enums must be combined across all subgraphs since they can deviate. const nonNativeScalarFieldTypes = new Map< string, SupportedFroidReturnTypes @@ -462,30 +467,60 @@ export class FroidSchema { ) as SupportedFroidReturnTypes[] ).forEach((nonNativeScalarType) => { const returnTypeName = nonNativeScalarType.name.value; - if ( - !nonNativeScalarDefinitionNames.has(returnTypeName) || - nonNativeScalarFieldTypes.has(returnTypeName) - ) { + if (!nonNativeScalarDefinitionNames.has(returnTypeName)) { // Don't get types that are not returned in froid schema return; } if (nonNativeScalarType.kind === Kind.ENUM_TYPE_DEFINITION) { + let finalEnumValues: readonly EnumValueDefinitionNode[] = []; + + // Collect the enum values from the enum that is currently being inspected, + // omitting all applied directives. const enumValues = nonNativeScalarType.values?.map((enumValue) => ({ ...enumValue, - directives: enumValue.directives?.filter( - (directive) => directive.name.value === 'inaccessible' - ), + directives: [], })); + + // Get the enum definition we've created so far if one exists + const existingEnum = nonNativeScalarFieldTypes.get( + returnTypeName + ) as EnumTypeDefinitionNode; + + // If there are existing enum values, use them for the final enum + if (existingEnum?.values) { + finalEnumValues = existingEnum.values; + } + + // If there are enum values associated with the enum definition + // we're currently inspecting, include them in the final list of + // enum values + if (enumValues) { + // Deduplicate enum values + const dedupedEnumValues = enumValues.filter( + (value) => + !finalEnumValues.some( + (existingValue) => existingValue.name.value === value.name.value + ) + ); + // Update the final enum value list + finalEnumValues = [...finalEnumValues, ...dedupedEnumValues]; + } + nonNativeScalarFieldTypes.set(returnTypeName, { ...nonNativeScalarType, - values: enumValues, + values: finalEnumValues.length ? finalEnumValues : undefined, directives: [], description: undefined, } as EnumTypeDefinitionNode); return; } + if (nonNativeScalarFieldTypes.has(returnTypeName)) { + // Don't duplicate scalars + return; + } + nonNativeScalarFieldTypes.set(returnTypeName, { ...nonNativeScalarType, description: undefined, diff --git a/src/schema/__tests__/FroidSchema.test.ts b/src/schema/__tests__/FroidSchema.test.ts index 57c5c05..d8842aa 100644 --- a/src/schema/__tests__/FroidSchema.test.ts +++ b/src/schema/__tests__/FroidSchema.test.ts @@ -1739,7 +1739,7 @@ describe('FroidSchema class', () => { enum UsedEnum { VALUE_ONE VALUE_THREE - VALUE_TWO @inaccessible + VALUE_TWO } type User implements Node @key(fields: "customEnum1 customEnum2 customField1 customField2 userId") { @@ -1755,6 +1755,79 @@ describe('FroidSchema class', () => { ); }); + it('includes a combined enum definition when enum values differ across subgraphs', () => { + const userSchema = gql` + enum UsedEnum { + VALUE_ONE + VALUE_TWO + } + `; + const todoSchema = gql` + enum UsedEnum { + VALUE_THREE + VALUE_FOUR + } + + type User @key(fields: "userId enumField") { + userId: String! + enumField: UsedEnum! + } + `; + const bookSchema = gql` + enum UsedEnum { + VALUE_THREE + VALUE_FIVE + } + `; + const subgraphs = new Map(); + subgraphs.set('user-subgraph', userSchema); + subgraphs.set('todo-subgraph', todoSchema); + subgraphs.set('book-subgraph', bookSchema); + + const actual = generateSchema({ + subgraphs, + froidSubgraphName: 'relay-subgraph', + contractTags: ['storefront', 'internal'], + federationVersion: FED2_DEFAULT_VERSION, + }); + + expect(actual).toEqual( + // prettier-ignore + gql` + extend schema @link(url: "https://specs.apollo.dev/federation/v2.0", import: ["@key", "@tag", "@external"]) + + "The global identification interface implemented by all entities." + interface Node @tag(name: "internal") @tag(name: "storefront") { + "The globally unique identifier." + id: ID! + } + + type Query { + "Fetches an entity by its globally unique identifier." + node( + "A globally unique entity identifier." + id: ID! + ): Node @tag(name: "internal") @tag(name: "storefront") + } + + enum UsedEnum { + VALUE_FIVE + VALUE_FOUR + VALUE_ONE + VALUE_THREE + VALUE_TWO + } + + type User implements Node @key(fields: "enumField userId") { + "The globally unique identifier." + id: ID! + enumField: UsedEnum! + userId: String! + } + ` + ); + }); + it('ignores keys that use the `id` field', () => { const productSchema = gql` type Query {