fix: handle overlapping fields correctly across subschemas

.changeset/fresh-kangaroos-exist.md

@@ -0,0 +1,34 @@
+---
+"@graphql-tools/stitch": minor
+---
+
+New option `useNonNullableFieldOnConflict` in `typeMergingOptions` of `stitchSchemas`
+
+When you have two schemas like below, you will get a warning about the conflicting fields because `name` field is defined as non-null in one schema and nullable in the other schema, and non-nullable field can exist in the stitched schema because of the order or any other reasons, and this might actually cause an unexpected behavior when you fetch `User.name` from the one who has it as non-nullable.
+This option supresses the warning, and takes the field from the schema that has it as non-nullable.
+
+```graphql
+  type Query {
+    user: User
+  }
+
+  type User {
+    id: ID!
+    name: String
+    email: String
+  }
+```
+And;
+
+```graphql
+  type Query {
+    user: User
+  }
+
+  type User {
+    id: ID!
+    name: String!
+  }
+```
+
+

.changeset/many-ads-run.md

@@ -0,0 +1,97 @@
+---
+"@graphql-tools/federation": patch
+"@graphql-tools/delegate": patch
+"@graphql-tools/stitch": patch
+---
+
+If the gateway receives a query with an overlapping fields for the subschema, it uses aliases to resolve it correctly.
+
+Let's say subschema A has the following schema;
+
+```graphql
+  type Query {
+    user: User
+  }
+
+  interface User {
+    id: ID!
+    name: String!
+  }
+
+  type Admin implements User {
+    id: ID!
+    name: String!
+    role: String!
+  }
+
+  type Customer implements User {
+    id: ID!
+    name: String
+    email: String
+  }
+```
+
+And let's say the gateway has the following schema instead;
+
+```graphql
+  type Query {
+    user: User
+  }
+
+  interface User {
+    id: ID!
+    name: String!
+  }
+
+  type Admin implements User {
+    id: ID!
+    name: String!
+    role: String!
+  }
+
+  type Customer implements User {
+    id: ID!
+    name: String!
+    email: String!
+  }
+```
+
+In this case, the following query is fine for the gateway but for the subschema, it's not;
+
+```graphql
+  query {
+    user {
+      ... on Admin {
+        id
+        name # This is nullable in the subschema
+        role
+      }
+      ... on Customer {
+        id
+        name # This is non-nullable in the subschema
+        email
+      }
+    }
+  }
+```
+
+So the subgraph will throw based on this rule [OverlappingFieldsCanBeMerged](https://github.com/graphql/graphql-js/blob/main/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts)
+
+To avoid this, the gateway will use aliases to resolve the query correctly. The query will be transformed to the following;
+
+```graphql
+  query {
+    user {
+      ... on Admin {
+        id
+        name # This is nullable in the subschema
+        role
+      }
+      ... on Customer {
+        id
+        name: _nullable_name # This is non-nullable in the subschema
+        email
+      }
+    }
+  }
+```

packages/delegate/src/OverlappingAliasesTransform.ts

@@ -0,0 +1,114 @@
+import { isNullableType, Kind, visit } from 'graphql';
+import { ExecutionRequest, ExecutionResult } from '@graphql-tools/utils';
+import { DelegationContext, Transform } from './types.js';
+
+const OverlappingAliases = Symbol('OverlappingAliases');
+
+interface OverlappingAliasesContext {
+  [OverlappingAliases]: boolean;
+}
+
+export class OverlappingAliasesTransform<TContext>
+  implements Transform<OverlappingAliasesContext, TContext>
+{
+  transformRequest(
+    request: ExecutionRequest,
+    delegationContext: DelegationContext<TContext>,
+    transformationContext: OverlappingAliasesContext,
+  ) {
+    const newDocument = visit(request.document, {
+      [Kind.SELECTION_SET]: node => {
+        const seenNonNullable = new Set<string>();
+        const seenNullable = new Set<string>();
+        return {
+          ...node,
+          selections: node.selections.map(selection => {
+            if (selection.kind === Kind.INLINE_FRAGMENT) {
+              const selectionTypeName = selection.typeCondition?.name.value;
+              if (selectionTypeName) {
+                const selectionType =
+                  delegationContext.transformedSchema.getType(selectionTypeName);
+                if (selectionType && 'getFields' in selectionType) {
+                  const selectionTypeFields = selectionType.getFields();
+                  return {
+                    ...selection,
+                    selectionSet: {
+                      ...selection.selectionSet,
+                      selections: selection.selectionSet.selections.map(subSelection => {
+                        if (subSelection.kind === Kind.FIELD) {
+                          const fieldName = subSelection.name.value;
+                          if (!subSelection.alias) {
+                            const field = selectionTypeFields[fieldName];
+                            if (field) {
+                              let currentNullable: boolean;
+                              if (isNullableType(field.type)) {
+                                seenNullable.add(fieldName);
+                                currentNullable = true;
+                              } else {
+                                seenNonNullable.add(fieldName);
+                                currentNullable = false;
+                              }
+                              if (seenNullable.size && seenNonNullable.size) {
+                                transformationContext[OverlappingAliases] = true;
+                                return {
+                                  ...subSelection,
+                                  alias: {
+                                    kind: Kind.NAME,
+                                    value: currentNullable
+                                      ? `_nullable_${fieldName}`
+                                      : `_nonNullable_${fieldName}`,
+                                  },
+                                };
+                              }
+                            }
+                          }
+                        }
+                        return subSelection;
+                      }),
+                    },
+                  };
+                }
+              }
+            }
+            return selection;
+          }),
+        };
+      },
+    });
+    return {
+      ...request,
+      document: newDocument,
+    };
+  }
+
+  transformResult(
+    result: ExecutionResult,
+    _delegationContext: DelegationContext<TContext>,
+    transformationContext: OverlappingAliasesContext,
+  ) {
+    if (transformationContext[OverlappingAliases]) {
+      return removeOverlappingAliases(result);
+    }
+    return result;
+  }
+}
+
+function removeOverlappingAliases(result: any): any {
+  if (result != null) {
+    if (Array.isArray(result)) {
+      return result.map(removeOverlappingAliases);
+    } else if (typeof result === 'object') {
+      const newResult: Record<string, any> = {};
+      for (const key in result) {
+        if (key.startsWith('_nullable_') || key.startsWith('_nonNullable_')) {
+          const newKey = key.replace(/^_nullable_/, '').replace(/^_nonNullable_/, '');
+          newResult[newKey] = removeOverlappingAliases(result[key]);
+        } else {
+          newResult[key] = removeOverlappingAliases(result[key]);
+        }
+      }
+      return newResult;
+    }
+  }
+  return result;
+}

packages/delegate/src/Transformer.ts

@@ -1,6 +1,7 @@
 import { ExecutionRequest, ExecutionResult } from '@graphql-tools/utils';
 import { checkResultAndHandleErrors } from './checkResultAndHandleErrors.js';
 import { finalizeGatewayRequest } from './finalizeGatewayRequest.js';
+import { OverlappingAliasesTransform } from './OverlappingAliasesTransform.js';
 import { prepareGatewayDocument } from './prepareGatewayDocument.js';
 import { DelegationContext, Transform } from './types.js';
 
@@ -18,8 +19,10 @@ export class Transformer<TContext extends Record<string, any> = Record<string, a
     const transforms = context.transforms;
     const delegationTransforms = transforms.slice().reverse();
     for (const transform of delegationTransforms) {
-      this.addTransform(transform, {});
+      this.addTransform(transform);
     }
+    // TODO: Move this to the core, later
+    this.addTransform(new OverlappingAliasesTransform());
   }
 
   private addTransform(transform: Transform<any, TContext>, context = {}) {
@@ -52,7 +55,6 @@ export class Transformer<TContext extends Record<string, any> = Record<string, a
 
   public transformResult(originalResult: ExecutionResult) {
     let result = originalResult;
-
     // from right to left
     for (let i = this.transformations.length - 1; i >= 0; i--) {
       const transformation = this.transformations[i];
@@ -64,7 +66,6 @@ export class Transformer<TContext extends Record<string, any> = Record<string, a
         );
       }
     }
-
     return checkResultAndHandleErrors(result, this.delegationContext);
   }
 }

packages/federation/src/supergraph.ts

@@ -13,6 +13,7 @@ import {
   ObjectTypeDefinitionNode,
   ObjectTypeExtensionNode,
   parse,
+  parseType,
   print,
   ScalarTypeDefinitionNode,
   TypeDefinitionNode,
@@ -159,8 +160,16 @@ export function getSubschemasFromSupergraphSdl({
                     argumentNode.value.value === true,
                 );
                 if (!isExternal) {
+                  const typeArg = joinFieldDirectiveNode.arguments?.find(
+                    argumentNode => argumentNode.name.value === 'type',
+                  );
+                  const typeNode =
+                    typeArg?.value.kind === Kind.STRING
+                      ? parseType(typeArg.value.value)
+                      : fieldNode.type;
                   fieldDefinitionNodesOfSubgraph.push({
                     ...fieldNode,
+                    type: typeNode,
                     directives: fieldNode.directives?.filter(
                       directiveNode => directiveNode.name.value !== 'join__field',
                     ),
@@ -708,6 +717,9 @@ export function getStitchedSchemaFromSupergraphSdl(opts: GetSubschemasFromSuperg
     subschemas: [...subschemaMap.values()],
     assumeValid: true,
     assumeValidSDL: true,
+    typeMergingOptions: {
+      useNonNullableFieldOnConflict: true,
+    },
   });
   return filterInternalFieldsAndTypes(supergraphSchema);
 }

packages/federation/test/federation-compatibility.test.ts

@@ -43,7 +43,7 @@ describe('Federation Compatibility', () => {
           } else {
             if ('errors' in result && result.errors) {
               for (const error of result.errors) {
-                console.error(error.message);
+                console.error(error.stack);
               }
             }
             expect(result).toMatchObject({