restore v4 healSchema functionality (#1468)

Within v4, schema healing was less agressive and did not attempt to refresh the schemas private variables and correct the root types.

Reversion to the less agressive healSchema makes some sense.
Library users require healSchema because they are modifying the schema in place using visitSchema or visitSchemaDirectives. This lets them do some things that perhaps they shouldn't (e.g. create fields of different types with identical names), but it is unexpected for the more aggressive healing to actually fail where the less agressive healing works fine.

See #1462

This requires a rewrite of the existing Gatsby transforms which rely on the more aggressive healSchema within v5, but this is recommended in any case, as healSchema/visitSchema/visitSchemaDirectives are all now considered legacy in any case.

Instructions for do-it-yourself more aggressive healing are included within comments within the healSchema, which detail the deficiencies of the less aggressive approach rather than releasing the more aggressive healing functionality under an option or new function name. This is because library users are now encourages to not modify existing schemas in place using visitSchema/visitSchemaDirectives, and to instead use mapSchema.

packages/utils/src/heal.ts

@@ -21,57 +21,38 @@ import {
   isNonNullType,
 } from 'graphql';
 
-import { isNamedStub, getBuiltInForStub } from './stub';
 import { TypeMap } from './Interfaces';
 
 // Update any references to named schema types that disagree with the named
 // types found in schema.getTypeMap().
+//
+// healSchema and its callers (visitSchema/visitSchemaDirectives) all modify the schema in place.
+// Therefore, private variables (such as the stored implementation map and the proper root types)
+// are not updated.
+//
+// If this causes issues, the schema could be more aggressively healed as follows:
+//
+// healSchema(schema);
+// const config = schema.toConfig()
+// const healedSchema = new GraphQLSchema({
+//   ...config,
+//   query: schema.getType('<desired new root query type name>'),
+//   mutation: schema.getType('<desired new root mutation type name>'),
+//   subscription: schema.getType('<desired new root subscription type name>'),
+// });
+//
+// One can then also -- if necessary --  assign the correct private variables to the initial schema
+// as follows:
+// Object.assign(schema, healedSchema);
+//
+// These steps are not taken automatically to preserve backwards compatibility with graphql-tools v4.
+// See https://github.com/ardatan/graphql-tools/issues/1462
+//
+// They were briefly taken in v5, but can now be phased out as they were only required when other
+// areas of the codebase were using healSchema and visitSchema more extensively.
+//
 export function healSchema(schema: GraphQLSchema): GraphQLSchema {
-  const typeMap = schema.getTypeMap();
-  const directives = schema.getDirectives();
-
-  const queryType = schema.getQueryType();
-  const mutationType = schema.getMutationType();
-  const subscriptionType = schema.getSubscriptionType();
-
-  const newQueryTypeName =
-    queryType != null ? (typeMap[queryType.name] != null ? typeMap[queryType.name].name : undefined) : undefined;
-  const newMutationTypeName =
-    mutationType != null
-      ? typeMap[mutationType.name] != null
-        ? typeMap[mutationType.name].name
-        : undefined
-      : undefined;
-  const newSubscriptionTypeName =
-    subscriptionType != null
-      ? typeMap[subscriptionType.name] != null
-        ? typeMap[subscriptionType.name].name
-        : undefined
-      : undefined;
-
-  healTypes(typeMap, directives);
-
-  const filteredTypeMap = {};
-
-  Object.keys(typeMap).forEach(typeName => {
-    if (!typeName.startsWith('__')) {
-      filteredTypeMap[typeName] = typeMap[typeName];
-    }
-  });
-
-  const healedSchema = new GraphQLSchema({
-    ...schema.toConfig(),
-    query: newQueryTypeName ? filteredTypeMap[newQueryTypeName] : undefined,
-    mutation: newMutationTypeName ? filteredTypeMap[newMutationTypeName] : undefined,
-    subscription: newSubscriptionTypeName ? filteredTypeMap[newSubscriptionTypeName] : undefined,
-    types: Object.keys(filteredTypeMap).map(typeName => filteredTypeMap[typeName]),
-    directives: directives.slice(),
-  });
-
-  // Reconstruct the schema to reinitialize private variables
-  // e.g. the stored implementation map and the proper root types.
-  Object.assign(schema, healedSchema);
-
+  healTypes(schema.getTypeMap(), schema.getDirectives());
   return schema;
 }
 
@@ -230,15 +211,12 @@ export function healTypes(
       // of truth for all named schema types.
       // Note that new types can still be simply added by adding a field, as
       // the official type will be undefined, not null.
-      let officialType = originalTypeMap[type.name];
-      if (officialType === undefined) {
-        officialType = isNamedStub(type) ? getBuiltInForStub(type) : type;
-        originalTypeMap[officialType.name] = officialType;
+      const officialType = originalTypeMap[type.name];
+      if (officialType && type !== officialType) {
+        return officialType as T;
       }
-      return officialType;
     }
-
-    return null;
+    return type;
   }
 }
 

packages/utils/tests/directives.test.ts

@@ -25,6 +25,7 @@ import {
   isScalarType,
   isListType,
   TypeSystemExtensionNode,
+  GraphQLError,
 } from 'graphql';
 import formatDate from 'dateformat';
 
@@ -1458,4 +1459,102 @@ describe('@directives', () => {
       });
     });
   });
+
+  test('preserves ability to create fields of different types with same name (issue 1462)', () => {
+    function validateStr(value: any, {
+      min = null,
+      message = null,
+    } : {
+      min: number,
+      message: string,
+    }) {
+      console.log(value, min, message);
+      if(min && value.length < min) {
+        throw new GraphQLError(message || `Please ensure the value is at least ${min} characters.`);
+      }
+    }
+
+    class ConstraintType extends GraphQLScalarType {
+      constructor(
+        type: GraphQLScalarType,
+        args: {
+          min: number,
+          message: string,
+        },
+      ) {
+        super({
+          name: 'ConstraintType',
+          serialize: (value) => type.serialize(value),
+          parseValue: (value) => {
+            const trimmed = value.trim();
+            validateStr(trimmed, args);
+            return type.parseValue(trimmed);
+          }
+        });
+      }
+    }
+
+    class ConstraintDirective extends SchemaDirectiveVisitor {
+      visitInputFieldDefinition(field: GraphQLInputField) {
+        if (isNonNullType(field.type) && isScalarType(field.type.ofType)) {
+          field.type = new GraphQLNonNull(
+            new ConstraintType(field.type.ofType, this.args)
+          );
+        } else if (isScalarType(field.type)) {
+          field.type = new ConstraintType(field.type, this.args);
+        } else {
+          throw new Error(`Not a scalar type: ${field.type}`);
+        }
+      }
+    }
+
+    const schema = makeExecutableSchema({
+      typeDefs: `
+        directive @constraint(min: Int, message: String) on INPUT_FIELD_DEFINITION
+
+        input BookInput {
+          name: String! @constraint(min: 10, message: "Book input error!")
+        }
+
+        input AuthorInput {
+          name: String! @constraint(min: 4, message: "Author input error")
+        }
+
+        type Query {
+          getBookById(id: Int): String
+        }
+
+        type Mutation {
+          createBook(input: BookInput!): String
+          createAuthor(input: AuthorInput!): String
+        }
+      `,
+      resolvers: {
+        Mutation: {
+          createBook() {
+            return 'yes';
+          },
+          createAuthor() {
+            return 'no';
+          }
+        }
+      },
+      schemaDirectives: {
+        constraint: ConstraintDirective
+      }
+    });
+
+    return graphql(
+      schema,
+      `
+        mutation {
+          createAuthor(input: {
+            name: "M"
+          })
+        }
+      `,
+    ).then(({ errors }) => {
+      expect(errors[0].message).toEqual('Author input error');
+    });
+  });
 });

packages/wrap/tests/gatsbyTransforms.test.ts

@@ -1,18 +1,20 @@
 import {
-  GraphQLObjectType,
   GraphQLSchema,
   GraphQLFieldResolver,
   GraphQLNonNull,
   graphql,
+  GraphQLObjectType,
+  GraphQLFieldConfigMap,
 } from 'graphql';
 
-import { VisitSchemaKind, cloneType, healSchema, visitSchema } from '@graphql-tools/utils';
+import { mapSchema, MapperKind, removeObjectFields, addTypes, appendObjectFields } from '@graphql-tools/utils';
 import { wrapSchema, RenameTypes } from '../src';
 import { makeExecutableSchema } from '@graphql-tools/schema';
 import { addMocksToSchema } from '@graphql-tools/mock';
 
 // see https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-source-graphql/src/transforms.js
 // and https://github.com/gatsbyjs/gatsby/issues/22128
+// and https://github.com/ardatan/graphql-tools/issues/1462
 
 class NamespaceUnderFieldTransform {
   private readonly typeName: string;
@@ -36,40 +38,42 @@ class NamespaceUnderFieldTransform {
   transformSchema(schema: GraphQLSchema) {
     const query = schema.getQueryType();
 
-    const nestedType = cloneType(query);
-    nestedType.name = this.typeName;
+    let [newSchema, fields] = removeObjectFields(schema, query.name, () => true);
 
-    const typeMap = schema.getTypeMap();
-    typeMap[this.typeName] = nestedType;
+    const nestedType = new GraphQLObjectType({
+        ...query.toConfig(),
+        name: this.typeName,
+        fields,
+    });
 
-    const newQuery = new GraphQLObjectType({
-      name: query.name,
-      fields: {
-        [this.fieldName]: {
-          type: new GraphQLNonNull(nestedType),
-          resolve: (parent, args, context, info) => {
-            if (this.resolver != null) {
-              return this.resolver(parent, args, context, info);
-            }
+    newSchema = addTypes(newSchema, [nestedType]);
 
-            return {};
-          },
+    const newRootFieldConfigMap: GraphQLFieldConfigMap<any, any> = {
+      [this.fieldName]: {
+        type: new GraphQLNonNull(nestedType),
+        resolve: (parent, args, context, info) => {
+          if (this.resolver != null) {
+            return this.resolver(parent, args, context, info);
+          }
+
+          return {};
         },
       },
-    });
-    typeMap[query.name] = newQuery;
+    };
+
+    newSchema = appendObjectFields(newSchema, query.name, newRootFieldConfigMap);
 
-    return healSchema(schema);
+    return newSchema;
   }
 }
 
 class StripNonQueryTransform {
   transformSchema(schema: GraphQLSchema) {
-    return visitSchema(schema, {
-      [VisitSchemaKind.MUTATION]() {
+    return mapSchema(schema, {
+      [MapperKind.MUTATION]() {
         return null;
       },
-      [VisitSchemaKind.SUBSCRIPTION]() {
+      [MapperKind.SUBSCRIPTION]() {
         return null;
       },
     });