fix(stitch): handle isolated abstract types correctly

graphql-hive · Feb 7, 2025 · 552d52f · 552d52f
1 parent 9913cf7
commit 552d52f
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 43 deletions.
diff --git a/.changeset/honest-terms-jump.md b/.changeset/honest-terms-jump.md
@@ -0,0 +1,8 @@
+---
+'@graphql-tools/stitch': patch
+---
+
+Fix a bug while isolating computed abstract type fields
+
+When a field in an interface needs to be isolated,
+it should not remove the field from the base subschema if it is used by other members of the base subschema.
diff --git a/.changeset/tender-geese-perform.md b/.changeset/tender-geese-perform.md
@@ -0,0 +1,5 @@
+---
+'@graphql-tools/stitch': patch
+---
+
+While calculating the best subschema for a selection set, respect fragments
diff --git a/packages/stitch/src/createDelegationPlanBuilder.ts b/packages/stitch/src/createDelegationPlanBuilder.ts
@@ -211,7 +211,7 @@ function calculateDelegationStage(
   }
 
   if (delegationMap.size > 1) {
-    optimizeDelegationMap(delegationMap, mergedTypeInfo.typeName);
+    optimizeDelegationMap(delegationMap, mergedTypeInfo.typeName, fragments);
   }
 
   return {
@@ -452,6 +452,7 @@ export function createDelegationPlanBuilder(
 function optimizeDelegationMap(
   delegationMap: Map<Subschema, SelectionSetNode>,
   typeName: string,
+  fragments: Record<string, FragmentDefinitionNode>,
 ): Map<Subschema, SelectionSetNode> {
   for (const [subschema, selectionSet] of delegationMap) {
     for (const [subschema2, selectionSet2] of delegationMap) {
@@ -464,6 +465,7 @@ function optimizeDelegationMap(
         subschema2.transformedSchema.getType(typeName) as GraphQLObjectType,
         selectionSet,
         () => true,
+        fragments,
       );
       if (!unavailableFields.length) {
         delegationMap.set(subschema2, {

diff --git a/packages/stitch/src/subschemaConfigTransforms/isolateComputedFieldsTransformer.ts b/packages/stitch/src/subschemaConfigTransforms/isolateComputedFieldsTransformer.ts
@@ -82,6 +82,7 @@ export function isolateComputedFieldsTransformer(
           if (mergedTypeConfig.selectionSet) {
             const parsedSelectionSet = parseSelectionSet(
               mergedTypeConfig.selectionSet,
+              { noLocation: true },
             );
             const keyFields = collectFields(
               subschemaConfig.schema,
@@ -96,6 +97,7 @@ export function isolateComputedFieldsTransformer(
             if (entryPoint.selectionSet) {
               const parsedSelectionSet = parseSelectionSet(
                 entryPoint.selectionSet,
+                { noLocation: true },
               );
               const keyFields = collectFields(
                 subschemaConfig.schema,
@@ -168,6 +170,7 @@ export function isolateComputedFieldsTransformer(
                 const keyFieldNames: string[] = [];
                 const parsedSelectionSet = parseSelectionSet(
                   returnTypeSelectionSet,
+                  { noLocation: true },
                 );
                 const keyFields = collectFields(
                   subschemaConfig.schema,
@@ -182,6 +185,7 @@ export function isolateComputedFieldsTransformer(
                   if (entryPoint.selectionSet) {
                     const parsedSelectionSet = parseSelectionSet(
                       entryPoint.selectionSet,
+                      { noLocation: true },
                     );
                     const keyFields = collectFields(
                       subschemaConfig.schema,
@@ -244,10 +248,7 @@ export function isolateComputedFieldsTransformer(
 
   if (Object.keys(isolatedSchemaTypes).length) {
     return [
-      filterIsolatedSubschema({
-        ...subschemaConfig,
-        merge: isolatedSchemaTypes,
-      }),
+      filterIsolatedSubschema(subschemaConfig, isolatedSchemaTypes),
       filterBaseSubschema(
         { ...subschemaConfig, merge: baseSchemaTypes },
         isolatedSchemaTypes,
@@ -322,7 +323,7 @@ function filterBaseSubschema(
         }
       }
       const allTypes = [typeName, ...iFacesForType];
-      const isIsolatedFieldName = allTypes.some((implementingTypeName) =>
+      const isIsolatedFieldName = allTypes.every((implementingTypeName) =>
         isIsolatedField(implementingTypeName, fieldName, isolatedSchemaTypes),
       );
       const isKeyFieldName = allTypes.some((implementingTypeName) =>
@@ -356,7 +357,7 @@ function filterBaseSubschema(
         ...iFacesForType,
         ...typesForInterface[typeName],
       ];
-      const isIsolatedFieldName = allTypes.some((implementingTypeName) =>
+      const isIsolatedFieldName = allTypes.every((implementingTypeName) =>
         isIsolatedField(implementingTypeName, fieldName, isolatedSchemaTypes),
       );
       const isKeyFieldName = allTypes.some((implementingTypeName) =>
@@ -409,14 +410,11 @@ function filterBaseSubschema(
   return filteredSubschema;
 }
 
-type IsolatedSubschemaInput = Exclude<SubschemaConfig, 'merge'> & {
-  merge: Record<string, ComputedTypeConfig>;
-};
-
 function filterIsolatedSubschema(
-  subschemaConfig: IsolatedSubschemaInput,
-  computedFieldTypes: Record<string, boolean> = {}, // contains types of computed fields that have no root field
+  subschemaConfig: SubschemaConfig,
+  isolatedSchemaTypes: Record<string, ComputedTypeConfig>,
 ): SubschemaConfig {
+  const computedFieldTypes: Record<string, boolean> = {};
   const queryRootFields: Record<string, boolean> = {};
   function listReachableTypesToIsolate(
     subschemaConfig: SubschemaConfig,
@@ -427,7 +425,7 @@ function filterIsolatedSubschema(
       return typeNames;
     } else if (
       (isObjectType(type) || isInterfaceType(type)) &&
-      subschemaConfig.merge?.[type.name]?.selectionSet
+      subschemaConfig.merge?.[type.name]
     ) {
       // this is a merged type, no need to descend further
       typeNames.add(type.name);
@@ -562,11 +560,9 @@ function filterIsolatedSubschema(
         return true;
       }
       return (
-        subschemaConfig.merge[typeName] == null ||
+        subschemaConfig.merge?.[typeName] == null ||
         subschemaConfig.merge[typeName]?.fields?.[fieldName] != null ||
-        (subschemaConfig.merge[typeName]?.keyFieldNames ?? []).includes(
-          fieldName,
-        )
+        (isolatedSchemaTypes[typeName]?.keyFieldNames ?? []).includes(fieldName)
       );
     },
     interfaceFieldFilter: (typeName, fieldName, config) => {
@@ -588,12 +584,8 @@ function filterIsolatedSubschema(
       }
       const isIsolatedFieldName =
         typesForInterface[typeName].some((implementingTypeName) =>
-          isIsolatedField(
-            implementingTypeName,
-            fieldName,
-            subschemaConfig.merge,
-          ),
-        ) || subschemaConfig.merge[typeName]?.fields?.[fieldName] != null;
+          isIsolatedField(implementingTypeName, fieldName, isolatedSchemaTypes),
+        ) || subschemaConfig.merge?.[typeName]?.fields?.[fieldName] != null;
       const isComputedFieldType = typesForInterface[typeName].some(
         (implementingTypeName) => {
           if (computedFieldTypes[implementingTypeName]) {
@@ -615,19 +607,16 @@ function filterIsolatedSubschema(
         isComputedFieldType ||
         typesForInterface[typeName].some((implementingTypeName) =>
           (
-            subschemaConfig.merge[implementingTypeName]?.keyFieldNames ?? []
+            isolatedSchemaTypes?.[implementingTypeName]?.keyFieldNames ?? []
           ).includes(fieldName),
         ) ||
-        (subschemaConfig.merge[typeName]?.keyFieldNames ?? []).includes(
-          fieldName,
-        )
+        (isolatedSchemaTypes[typeName]?.keyFieldNames ?? []).includes(fieldName)
       );
     },
   });
-
   const merge = Object.fromEntries(
     // get rid of keyFieldNames again
-    Object.entries(subschemaConfig.merge).map(
+    Object.entries(isolatedSchemaTypes).map(
       ([typeName, { keyFieldNames, ...config }]) => [typeName, config],
     ),
   );

diff --git a/packages/stitch/tests/isolateComputedFieldsTransformer.test.ts b/packages/stitch/tests/isolateComputedFieldsTransformer.test.ts
@@ -465,15 +465,16 @@ describe('isolateComputedFieldsTransformer', () => {
       const baseSubschema = new Subschema(baseConfig);
       const computedSubschema = new Subschema(computedConfig);
 
-      expect(
-        Object.keys(
-          (
-            baseSubschema.transformedSchema.getType(
-              'IProduct',
-            ) as GraphQLInterfaceType
-          ).getFields(),
-        ),
-      ).toEqual(['base']);
+      // /* It doesn't matter what interface has */
+      // expect(
+      //   Object.keys(
+      //     (
+      //       baseSubschema.transformedSchema.getType(
+      //         'IProduct',
+      //       ) as GraphQLInterfaceType
+      //     ).getFields(),
+      //   ),
+      // ).toEqual(['base']);
       expect(
         Object.keys(
           (
@@ -543,10 +544,7 @@ describe('isolateComputedFieldsTransformer', () => {
         throw new Error('Expected both configs to be defined');
       }
       expect(printSchema(new Subschema(baseConfig).transformedSchema).trim())
-        .toBe(`interface IProduct {
-  base: String!
-}
-
+        .toContain(`
 type Product implements IProduct {
   base: String!
 }