fix(stitch): handle isolated root fields in correct order (#6107)

ardatan · Apr 29, 2024 · b281dd6 · b281dd6
1 parent 7ef2ad7
commit b281dd6
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 69 deletions.
diff --git a/.changeset/itchy-peas-admire.md b/.changeset/itchy-peas-admire.md
@@ -0,0 +1,5 @@
+---
+"@graphql-tools/stitch": patch
+---
+
+Fix the priority of isolated fields
diff --git a/packages/federation/test/supergraphs.test.ts b/packages/federation/test/supergraphs.test.ts
@@ -35,7 +35,9 @@ describe('Supergraphs', () => {
               !getDirective(sortedInputSchema, enumValueConfig, 'inaccessible')?.length,
           }),
         );
-        expect(printSchema(sortedSchema).trim()).toBe(printSchema(filteredInputSchema).trim());
+        expect(printSchema(pruneSchema(sortedSchema)).trim()).toBe(
+          printSchema(filteredInputSchema).trim(),
+        );
       });
     });
   });

diff --git a/packages/merge/tests/merge-nodes.spec.ts b/packages/merge/tests/merge-nodes.spec.ts
@@ -492,23 +492,5 @@ describe('Merge Nodes', () => {
       assertNamedTypeNode(type.fields[1].type);
       expect(type.fields[1].type.name.value).toBe('String');
     });
-
-    it.skip('should remove schema definition', () => {
-      const type1 = parse(/* GraphQL */ `
-        schema {
-          query: Query
-        }
-        type Query {
-          f1: String
-        }
-      `);
-      const type2 = parse(/* GraphQL */ `
-        type Query {
-          f2: String
-        }
-      `);
-      const merged = mergeGraphQLNodes([...type1.definitions, ...type2.definitions]);
-      expect(Object.values(merged).length).toBe(1);
-    });
   });
 });
diff --git a/packages/stitch/src/subschemaConfigTransforms/isolateComputedFieldsTransformer.ts b/packages/stitch/src/subschemaConfigTransforms/isolateComputedFieldsTransformer.ts
@@ -92,20 +92,31 @@ export function isolateComputedFieldsTransformer(
             const returnTypeMergeConfig = subschemaConfig.merge[type.name];
 
             if (isObjectType(type)) {
-              if (returnTypeMergeConfig?.selectionSet) {
+              const returnTypeSelectionSet = returnTypeMergeConfig?.selectionSet;
+              if (returnTypeSelectionSet) {
                 // this is a merged type, include the selection set
-                // TODO: how to handle entryPoints
                 const keyFieldNames: string[] = [];
-                if (isObjectType(type)) {
-                  const parsedSelectionSet = parseSelectionSet(returnTypeMergeConfig.selectionSet!);
-                  const keyFields = collectFields(
-                    subschemaConfig.schema,
-                    {},
-                    {},
-                    type,
-                    parsedSelectionSet,
-                  );
-                  keyFieldNames.push(...Array.from(keyFields.fields.keys()));
+                const parsedSelectionSet = parseSelectionSet(returnTypeSelectionSet);
+                const keyFields = collectFields(
+                  subschemaConfig.schema,
+                  {},
+                  {},
+                  type,
+                  parsedSelectionSet,
+                );
+                keyFieldNames.push(...Array.from(keyFields.fields.keys()));
+                for (const entryPoint of returnTypeMergeConfig.entryPoints ?? []) {
+                  if (entryPoint.selectionSet) {
+                    const parsedSelectionSet = parseSelectionSet(entryPoint.selectionSet);
+                    const keyFields = collectFields(
+                      subschemaConfig.schema,
+                      {},
+                      {},
+                      type,
+                      parsedSelectionSet,
+                    );
+                    keyFieldNames.push(...Array.from(keyFields.fields.keys()));
+                  }
                 }
 
                 isolatedSchemaTypes[type.name] = {
@@ -128,10 +139,12 @@ export function isolateComputedFieldsTransformer(
                     const implementingTypeFields = isolatedSchemaTypes[implementingType]?.fields;
                     if (implementingTypeFields) {
                       for (const fieldName in implementingTypeFields) {
-                        fields[fieldName] = {
-                          ...implementingTypeFields[fieldName],
-                          ...fields[fieldName],
-                        };
+                        if (implementingTypeFields[fieldName]) {
+                          fields[fieldName] = {
+                            ...implementingTypeFields[fieldName],
+                            ...fields[fieldName],
+                          };
+                        }
                       }
                     }
                   }
@@ -191,7 +204,7 @@ function isIsolatedField(
   isolatedSchemaTypes: Record<string, ComputedTypeConfig>,
 ): boolean {
   const fieldConfig = isolatedSchemaTypes[typeName]?.fields?.[fieldName];
-  if (fieldConfig && Object.keys(fieldConfig).length > 0) {
+  if (fieldConfig) {
     return true;
   }
   return false;
@@ -345,42 +358,37 @@ function filterIsolatedSubschema(subschemaConfig: IsolatedSubschemaInput): Subsc
     }
   }
 
-  const interfaceFields: Record<string, Record<string, boolean>> = {};
-  for (const typeName in subschemaConfig.merge) {
-    const type = subschemaConfig.schema.getType(typeName);
-    if (!type || isObjectType(type)) {
-      if (!type || !('getInterfaces' in type)) {
-        throw new Error(`${typeName} expected to have 'getInterfaces' method`);
-      }
-      for (const int of type.getInterfaces()) {
-        const intType = subschemaConfig.schema.getType(int.name);
-        if (!intType || !('getFields' in intType)) {
-          throw new Error(`${int.name} expected to have 'getFields' method`);
-        }
-        for (const intFieldName in intType.getFields()) {
-          if (
-            subschemaConfig.merge[typeName].fields?.[intFieldName] ||
-            subschemaConfig.merge[typeName].keyFieldNames.includes(intFieldName)
-          ) {
-            interfaceFields[int.name] = interfaceFields[int.name] || {};
-            interfaceFields[int.name][intFieldName] = true;
-          }
-        }
-      }
-    }
-  }
-
+  const typesForInterface: Record<string, string[]> = {};
   const filteredSchema = pruneSchema(
     filterSchema({
       schema: subschemaConfig.schema,
-      rootFieldFilter: (operation, fieldName, config) =>
-        operation === 'Query' &&
-        (rootFields[fieldName] != null || computedFieldTypes[getNamedType(config.type).name]),
+      rootFieldFilter: (_, fieldName, config) => {
+        if (rootFields[fieldName]) {
+          return true;
+        }
+        const returnType = getNamedType(config.type);
+        if (isAbstractType(returnType)) {
+          const typesForInterface = getImplementingTypes(returnType.name, subschemaConfig.schema);
+          return typesForInterface.some(t => computedFieldTypes[t] != null);
+        }
+        return computedFieldTypes[returnType.name] != null;
+      },
       objectFieldFilter: (typeName, fieldName) =>
         subschemaConfig.merge[typeName] == null ||
         subschemaConfig.merge[typeName]?.fields?.[fieldName] != null ||
         (subschemaConfig.merge[typeName]?.keyFieldNames ?? []).includes(fieldName),
-      interfaceFieldFilter: (typeName, fieldName) => interfaceFields[typeName]?.[fieldName] != null,
+      interfaceFieldFilter: (typeName, fieldName) => {
+        if (!typesForInterface[typeName]) {
+          typesForInterface[typeName] = getImplementingTypes(typeName, subschemaConfig.schema);
+        }
+        const isIsolatedFieldName = typesForInterface[typeName].some(implementingTypeName =>
+          isIsolatedField(implementingTypeName, fieldName, subschemaConfig.merge),
+        );
+        return (
+          isIsolatedFieldName ||
+          (subschemaConfig.merge[typeName]?.keyFieldNames ?? []).includes(fieldName)
+        );
+      },
     }),
     { skipPruning: typ => computedFieldTypes[typ.name] != null },
   );

diff --git a/packages/stitch/tests/isolateComputedFieldsTransformer.test.ts b/packages/stitch/tests/isolateComputedFieldsTransformer.test.ts
@@ -465,18 +465,14 @@ describe('isolateComputedFieldsTransformer', () => {
       expect(computedQueryTypeFields['_item']).toBeDefined();
       expect(computedQueryTypeFields['_giftOptions']).toBeDefined();
 
-      /* TODO: Federation missing field issue
       const baseGiftOptionsType = baseSchema.transformedSchema.getType(
         'GiftOptions',
       ) as GraphQLObjectType;
       expect(baseGiftOptionsType).toBeUndefined();
-*/
       const baseQueryType = baseSchema.transformedSchema.getType('Query') as GraphQLObjectType;
       const baseQueryTypeFields = baseQueryType.getFields();
       expect(baseQueryTypeFields['_item']).toBeDefined();
-      /* TODO: Same with above
       expect(baseQueryTypeFields['_giftOptions']).toBeUndefined();
-      */
     });
 
     it('return type is merged type', () => {