Merge pull request #26 from wayfair-incubator/gwardwell_fix_enum_support

fix: include all enum values in FROID schema

Author: gwardwell

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

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",

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,

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 {