fix(compose/hoist-field): create new field if the target does not exi…

…st (#7676) * fix(compose/hoist-field): create new field if the target does not exist * Update changeset * Update snapshots
ardatan · Sep 12, 2024 · adc3293 · adc3293
1 parent 37fefea
commit adc3293
Show file tree

Hide file tree

Showing 16 changed files with 238 additions and 102 deletions.
diff --git a/.changeset/polite-mangos-beg.md b/.changeset/polite-mangos-beg.md
@@ -0,0 +1,37 @@
+---
+'@graphql-mesh/fusion-composition': patch
+'@graphql-mesh/fusion-runtime': patch
+---
+
+If the target hoisted field is a new field name that doesn't exist, create that field and keep the
+existing one;
+
+```ts
+createHoistFieldTransform({
+  mapping: [
+    {
+      typeName: 'Query',
+      pathConfig: ['users', 'results'],
+      newFieldName: 'usersResults',
+    },
+  ],
+})
+```
+
+```diff
+type Query {
+  users(limit: Int!, page: Int): UserSearchResult # Keep this
++ usersResults(limit: Int!, page: Int): [User!]! # Add a new one
+}
+
+scalar _HoistConfig
+
+type UserSearchResult {
+  page: Int!
+}
+
+type User {
+  id: ID!
+  name: String!
+}
+```
diff --git a/e2e/auto-type-merging/__snapshots__/auto-type-merging.test.ts.snap b/e2e/auto-type-merging/__snapshots__/auto-type-merging.test.ts.snap
@@ -121,7 +121,7 @@ directive @merge(
   additionalArgs: String
 ) repeatable on FIELD_DEFINITION
 
-directive @source(name: String!, type: String, subgraph: String!, hoist: _HoistConfig)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
+directive @source(name: String!, type: String, subgraph: String!)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
 
 directive @extraSchemaDefinitionDirective(directives: _DirectiveExtensions)  repeatable on OBJECT
 
@@ -154,8 +154,6 @@ scalar File @join__type(graph: PETSTORE)
 
 scalar ObjMap @join__type(graph: PETSTORE) 
 
-scalar _HoistConfig @join__type(graph: PETSTORE)  @join__type(graph: VACCINATION) 
-
 scalar _DirectiveExtensions @join__type(graph: PETSTORE)  @join__type(graph: VACCINATION) 
 
 scalar TransportOptions @join__type(graph: VACCINATION) 

diff --git a/e2e/federation-mixed/__snapshots__/federation-mixed.test.ts.snap b/e2e/federation-mixed/__snapshots__/federation-mixed.test.ts.snap
@@ -119,14 +119,12 @@ directive @transport(
   options: TransportOptions
 ) repeatable on SCHEMA
 
-directive @source(name: String!, type: String, subgraph: String!, hoist: _HoistConfig)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
+directive @source(name: String!, type: String, subgraph: String!)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
 
 directive @extraSchemaDefinitionDirective(directives: _DirectiveExtensions)  repeatable on OBJECT
 
 scalar ObjMap @join__type(graph: ACCOUNTS) 
 
-scalar _HoistConfig @join__type(graph: ACCOUNTS) 
-
 scalar _DirectiveExtensions @join__type(graph: ACCOUNTS)  @join__type(graph: INVENTORY)  @join__type(graph: PRODUCTS)  @join__type(graph: REVIEWS) 
 
 scalar TransportOptions @join__type(graph: INVENTORY)  @join__type(graph: PRODUCTS)  @join__type(graph: REVIEWS) 

diff --git a/e2e/openapi-arg-rename/__snapshots__/openapi-arg-rename.test.ts.snap b/e2e/openapi-arg-rename/__snapshots__/openapi-arg-rename.test.ts.snap
@@ -106,14 +106,12 @@ directive @transport(
   queryParams: [[String]]
 ) repeatable on SCHEMA
 
-directive @source(name: String!, type: String, subgraph: String!, hoist: _HoistConfig)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
+directive @source(name: String!, type: String, subgraph: String!)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
 
 directive @extraSchemaDefinitionDirective(directives: _DirectiveExtensions)  repeatable on OBJECT
 
 scalar ObjMap @join__type(graph: WIKI) 
 
-scalar _HoistConfig @join__type(graph: WIKI) 
-
 scalar _DirectiveExtensions @join__type(graph: WIKI) 
 
 type Query @extraSchemaDefinitionDirective(

diff --git a/e2e/openapi-naming-convention/__snapshots__/openapi-naming-convention.test.ts.snap b/e2e/openapi-naming-convention/__snapshots__/openapi-naming-convention.test.ts.snap
@@ -108,16 +108,14 @@ directive @transport(
   queryParams: [[String]]
 ) repeatable on SCHEMA
 
-directive @source(name: String!, type: String, subgraph: String!, hoist: _HoistConfig)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
+directive @source(name: String!, type: String, subgraph: String!)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
 
 directive @extraSchemaDefinitionDirective(directives: _DirectiveExtensions)  repeatable on OBJECT
 
 directive @extraEnumValueDirective(name: String!, value: String!, directives: _DirectiveExtensions)  repeatable on OBJECT
 
 scalar ObjMap @join__type(graph: WIKI) 
 
-scalar _HoistConfig @join__type(graph: WIKI) 
-
 scalar _DirectiveExtensions @join__type(graph: WIKI) 
 
 type Query @extraSchemaDefinitionDirective(

diff --git a/e2e/openapi-prune/__snapshots__/openapi-prune.test.ts.snap b/e2e/openapi-prune/__snapshots__/openapi-prune.test.ts.snap
@@ -106,14 +106,12 @@ directive @transport(
   queryParams: [[String]]
 ) repeatable on SCHEMA
 
-directive @source(name: String!, type: String, subgraph: String!, hoist: _HoistConfig)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
+directive @source(name: String!, type: String, subgraph: String!)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
 
 directive @extraSchemaDefinitionDirective(directives: _DirectiveExtensions)  repeatable on OBJECT
 
 scalar ObjMap @join__type(graph: WIKI) 
 
-scalar _HoistConfig @join__type(graph: WIKI) 
-
 scalar _DirectiveExtensions @join__type(graph: WIKI) 
 
 type Query @extraSchemaDefinitionDirective(

diff --git a/e2e/type-merging-batching/__snapshots__/type-merging-batching.test.ts.snap b/e2e/type-merging-batching/__snapshots__/type-merging-batching.test.ts.snap
@@ -25,7 +25,7 @@ directive @merge(subgraph: String, argsExpr: String, keyArg: String, keyField: S
 
 directive @extraSchemaDefinitionDirective(directives: _DirectiveExtensions) repeatable on OBJECT
 
-directive @source(name: String!, type: String, subgraph: String!, hoist: _HoistConfig) repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
+directive @source(name: String!, type: String, subgraph: String!) repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
 
 directive @additionalField on FIELD_DEFINITION
 
@@ -53,8 +53,6 @@ scalar TransportOptions @join__type(graph: AUTHORS) @join__type(graph: BOOKS)
 
 scalar _DirectiveExtensions @join__type(graph: AUTHORS) @join__type(graph: BOOKS)
 
-scalar _HoistConfig @join__type(graph: BOOKS)
-
 type Query @extraSchemaDefinitionDirective(directives: {transport: [{kind: "http", subgraph: "authors", location: "http://localhost:<authors_port>/graphql", options: {}}]}) @extraSchemaDefinitionDirective(directives: {transport: [{kind: "http", subgraph: "books", location: "http://localhost:<books_port>/graphql", options: {}}]}) @join__type(graph: AUTHORS) @join__type(graph: BOOKS) {
   author(id: ID!): Author @merge(subgraph: "authors", keyField: "id", keyArg: "id") @merge(subgraph: "books", keyField: "id", keyArg: "id") @source(name: "authorWithBooks", type: "AuthorWithBooks", subgraph: "books")
   authors(ids: [ID]): [Author] @merge(subgraph: "authors", keyField: "id", keyArg: "ids") @join__field(graph: AUTHORS)

diff --git a/packages/fusion/composition/src/compose.ts b/packages/fusion/composition/src/compose.ts
@@ -506,15 +506,11 @@ export function getAnnotatedSubgraphs(
       }
     }
     if (sourceDirectiveUsed) {
-      importedDirectivesAST.add(/* GraphQL */ `
-        scalar _HoistConfig
-      `);
       importedDirectivesAST.add(/* GraphQL */ `
         directive @source(
           name: String!
           type: String
           subgraph: String!
-          hoist: _HoistConfig
         ) repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
       `);
     }

diff --git a/packages/fusion/composition/src/transforms/hoist-field.ts b/packages/fusion/composition/src/transforms/hoist-field.ts
@@ -1,12 +1,18 @@
-import type {
-  GraphQLField,
-  GraphQLFieldConfigArgumentMap,
-  GraphQLInterfaceType,
-  GraphQLNamedType,
-  GraphQLObjectType,
+import {
+  DirectiveLocation,
+  getNamedType,
+  GraphQLDirective,
+  GraphQLScalarType,
   GraphQLSchema,
+  GraphQLString,
+  isInterfaceType,
+  isObjectType,
+  type GraphQLField,
+  type GraphQLFieldConfigArgumentMap,
+  type GraphQLInterfaceType,
+  type GraphQLNamedType,
+  type GraphQLObjectType,
 } from 'graphql';
-import { getNamedType, isInterfaceType, isObjectType } from 'graphql';
 import { getDirectiveExtensions, MapperKind, mapSchema } from '@graphql-tools/utils';
 import type { SubgraphConfig, SubgraphTransform } from '../compose.js';
 import { TransformValidationError } from './utils.js';
@@ -57,9 +63,24 @@ function checkTypeWithFields(
   return isObjectType(type) || isInterfaceType(type);
 }
 
+export const hoistDirective = new GraphQLDirective({
+  name: 'hoist',
+  locations: [DirectiveLocation.FIELD_DEFINITION],
+  args: {
+    subgraph: {
+      type: GraphQLString,
+    },
+    pathConfig: {
+      type: new GraphQLScalarType({
+        name: '_HoistConfig',
+      }),
+    },
+  },
+});
+
 export function createHoistFieldTransform(opts: CreateHoistFieldTransformOpts): SubgraphTransform {
   return function hoistFieldTransform(schema: GraphQLSchema, subgraphConfig: SubgraphConfig) {
-    return mapSchema(schema, {
+    const mappedSchema = mapSchema(schema, {
       [MapperKind.TYPE](type) {
         if (opts.mapping) {
           if (checkTypeWithFields(type)) {
@@ -102,25 +123,20 @@ export function createHoistFieldTransform(opts: CreateHoistFieldTransformOpts):
                 }
                 changed = true;
                 const existingFieldConfig = newFieldConfigMap[mapping.newFieldName];
-                const directives = getDirectiveExtensions(existingFieldConfig);
-                const newSourceDirectives = directives?.source?.map(directive => {
-                  if (directive.subgraph === subgraphConfig.name) {
-                    return {
-                      ...directive,
-                      hoist: mapping.pathConfig,
-                    };
-                  }
-                  return directive;
-                });
                 newFieldConfigMap[mapping.newFieldName] = {
                   ...(existingFieldConfig || {}),
                   args: argsConfig,
                   type: field.type,
                   extensions: {
                     ...existingFieldConfig?.extensions,
                     directives: {
-                      ...directives,
-                      source: newSourceDirectives,
+                      ...(existingFieldConfig?.extensions?.directives as Record<string, any>),
+                      hoist: [
+                        {
+                          subgraph: subgraphConfig.name,
+                          pathConfig: mapping.pathConfig,
+                        },
+                      ],
                     },
                   },
                 };
@@ -137,5 +153,12 @@ export function createHoistFieldTransform(opts: CreateHoistFieldTransformOpts):
         return type;
       },
     });
+    if (mappedSchema.getDirective('hoist') == null) {
+      return new GraphQLSchema({
+        ...mappedSchema.toConfig(),
+        directives: [...mappedSchema.toConfig().directives, hoistDirective],
+      });
+    }
+    return mappedSchema;
   };
 }
diff --git a/packages/fusion/composition/tests/__snapshots__/composition.spec.ts.snap b/packages/fusion/composition/tests/__snapshots__/composition.spec.ts.snap
@@ -195,7 +195,7 @@ enum join__Graph {
   B @join__graph(name: "B", url: "") 
 }
 
-directive @source(name: String!, type: String, subgraph: String!, hoist: _HoistConfig)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
+directive @source(name: String!, type: String, subgraph: String!)  repeatable on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION
 
 directive @merge(
   subgraph: String
@@ -206,8 +206,6 @@ directive @merge(
   additionalArgs: String
 ) repeatable on FIELD_DEFINITION
 
-scalar _HoistConfig @join__type(graph: A)  @join__type(graph: B) 
-
 type Query @source(name: "Query", subgraph: "A")  @source(name: "Query", subgraph: "B")  @join__type(graph: A)  @join__type(graph: B)  {
   A_myFoo: A_Foo! @source(name: "myFoo", type: "Foo!", subgraph: "A")  @join__field(graph: A) 
   B_foo(id: ID!) : B_Foo! @source(name: "foo", type: "Foo!", subgraph: "B")  @merge(subgraph: "B", keyField: "id", keyArg: "id")  @join__field(graph: B) 

diff --git a/packages/fusion/runtime/src/federation/subgraph.ts b/packages/fusion/runtime/src/federation/subgraph.ts
@@ -170,28 +170,33 @@ export function handleFederationSubschema({
         }
         renameFieldByTypeNamesReversed[realTypeName][fieldName] = realName;
       }
-      if (sourceDirective?.hoist) {
-        const pathConfig: (
-          | string
-          | {
-              fieldName: string;
-              argFilter?: (arg: GraphQLArgument) => boolean;
-            }
-        )[] = sourceDirective.hoist.map(annotation => {
-          if (typeof annotation === 'string') {
-            return {
-              fieldName: annotation,
-              argFilter: () => true,
-            };
+      const hoistDirectives = fieldDirectives.hoist;
+      if (hoistDirectives?.length > 0) {
+        for (const hoistDirective of hoistDirectives) {
+          if (hoistDirective.subgraph === subgraphName) {
+            const pathConfig: (
+              | string
+              | {
+                  fieldName: string;
+                  argFilter?: (arg: GraphQLArgument) => boolean;
+                }
+            )[] = hoistDirective.pathConfig.map(annotation => {
+              if (typeof annotation === 'string') {
+                return {
+                  fieldName: annotation,
+                  argFilter: () => true,
+                };
+              }
+              return {
+                fieldName: annotation.fieldName,
+                argFilter: annotation.filterArgs
+                  ? arg => !annotation.filterArgs.includes(arg.name)
+                  : () => true,
+              };
+            });
+            transforms.push(new HoistField(typeName, pathConfig, fieldName));
           }
-          return {
-            fieldName: annotation.fieldName,
-            argFilter: annotation.filterArgs
-              ? arg => !annotation.filterArgs.includes(arg.name)
-              : () => true,
-          };
-        });
-        transforms.push(new HoistField(typeName, pathConfig, fieldName));
+        }
       }
       const newArgs: GraphQLFieldConfigArgumentMap = {};
       if (fieldConfig.args) {