Make 'ASTDefinitionBuilder' responsible only for build types from AST (#1230)

* Make 'ASTDefinitionBuilder' responsible only for build types from AST

* Address review comments.

* Simplify extendType()

* Fix flow errors

Author: IvanGoncharov

src/utilities/buildASTSchema.js

@@ -170,22 +170,20 @@ export function buildASTSchema(
   const operationTypes = schemaDef
     ? getOperationTypes(schemaDef)
     : {
-        query: nodeMap.Query ? 'Query' : null,
-        mutation: nodeMap.Mutation ? 'Mutation' : null,
-        subscription: nodeMap.Subscription ? 'Subscription' : null,
+        query: nodeMap.Query,
+        mutation: nodeMap.Mutation,
+        subscription: nodeMap.Subscription,
       };
 
   const definitionBuilder = new ASTDefinitionBuilder(
     nodeMap,
     options,
-    typeName => {
-      throw new Error(`Type "${typeName}" not found in document.`);
+    typeRef => {
+      throw new Error(`Type "${typeRef.name.value}" not found in document.`);
     },
   );
 
-  const types = typeDefs.map(def =>
-    definitionBuilder.buildType(def.name.value),
-  );
+  const types = typeDefs.map(def => definitionBuilder.buildType(def));
 
   const directives = directiveDefs.map(def =>
     definitionBuilder.buildDirective(def),
@@ -237,17 +235,14 @@ export function buildASTSchema(
           `Specified ${operation} type "${typeName}" not found in document.`,
         );
       }
-      opTypes[operation] = typeName;
+      opTypes[operation] = operationType.type;
     });
     return opTypes;
   }
 }
 
 type TypeDefinitionsMap = ObjMap<TypeDefinitionNode>;
-type TypeResolver = (
-  typeName: string,
-  node?: ?NamedTypeNode,
-) => GraphQLNamedType;
+type TypeResolver = (typeRef: NamedTypeNode) => GraphQLNamedType;
 
 export class ASTDefinitionBuilder {
   _typeDefinitionsMap: TypeDefinitionsMap;
@@ -270,25 +265,21 @@ export class ASTDefinitionBuilder {
     );
   }
 
-  _buildType(typeName: string, typeNode?: ?NamedTypeNode): GraphQLNamedType {
+  buildType(node: NamedTypeNode | TypeDefinitionNode): GraphQLNamedType {
+    const typeName = node.name.value;
     if (!this._cache[typeName]) {
-      const defNode = this._typeDefinitionsMap[typeName];
-      if (defNode) {
-        this._cache[typeName] = this._makeSchemaDef(defNode);
+      if (node.kind === Kind.NAMED_TYPE) {
+        const defNode = this._typeDefinitionsMap[typeName];
+        this._cache[typeName] = defNode
+          ? this._makeSchemaDef(defNode)
+          : this._resolveType(node);
       } else {
-        this._cache[typeName] = this._resolveType(typeName, typeNode);
+        this._cache[typeName] = this._makeSchemaDef(node);
       }
     }
     return this._cache[typeName];
   }
 
-  buildType(ref: string | NamedTypeNode): GraphQLNamedType {
-    if (typeof ref === 'string') {
-      return this._buildType(ref);
-    }
-    return this._buildType(ref.name.value, ref);
-  }
-
   _buildWrappedType(typeNode: TypeNode): GraphQLType {
     const typeDef = this.buildType(getNamedTypeNode(typeNode));
     return buildWrappedType(typeDef, typeNode);

src/utilities/extendSchema.js

@@ -9,9 +9,11 @@
 
 import invariant from '../jsutils/invariant';
 import keyMap from '../jsutils/keyMap';
+import objectValues from '../jsutils/objectValues';
 import { ASTDefinitionBuilder } from './buildASTSchema';
 import { GraphQLError } from '../error/GraphQLError';
 import { isSchema, GraphQLSchema } from '../type/schema';
+import { isIntrospectionType } from '../type/introspection';
 
 import type { GraphQLSchemaValidationOptions } from '../type/schema';
 
@@ -189,56 +191,52 @@ export function extendSchema(
     return schema;
   }
 
-  const definitionBuilder = new ASTDefinitionBuilder(
+  const astBuilder = new ASTDefinitionBuilder(
     typeDefinitionMap,
     options,
-    (typeName, node) => {
+    typeRef => {
+      const typeName = typeRef.name.value;
       const existingType = schema.getType(typeName);
       if (existingType) {
-        return extendType(existingType);
+        return getExtendedType(existingType);
       }
 
-      if (node) {
-        throw new GraphQLError(
-          `Unknown type: "${typeName}". Ensure that this type exists ` +
-            'either in the original schema, or is added in a type definition.',
-          [node],
-        );
-      }
-      throw GraphQLError('Missing type from schema');
+      throw new GraphQLError(
+        `Unknown type: "${typeName}". Ensure that this type exists ` +
+          'either in the original schema, or is added in a type definition.',
+        [typeRef],
+      );
     },
   );
 
+  const extendTypeCache = Object.create(null);
+
   // Get the root Query, Mutation, and Subscription object types.
   // Note: While this could make early assertions to get the correctly
   // typed values below, that would throw immediately while type system
   // validation with validateSchema() will produce more actionable results.
   const existingQueryType = schema.getQueryType();
   const queryType = existingQueryType
-    ? (definitionBuilder.buildType(existingQueryType.name): any)
+    ? getExtendedType(existingQueryType)
     : null;
 
   const existingMutationType = schema.getMutationType();
   const mutationType = existingMutationType
-    ? (definitionBuilder.buildType(existingMutationType.name): any)
+    ? getExtendedType(existingMutationType)
     : null;
 
   const existingSubscriptionType = schema.getSubscriptionType();
   const subscriptionType = existingSubscriptionType
-    ? (definitionBuilder.buildType(existingSubscriptionType.name): any)
+    ? getExtendedType(existingSubscriptionType)
     : null;
 
-  // Iterate through all types, getting the type definition for each, ensuring
-  // that any type not directly referenced by a field will get created.
-  const typeMap = schema.getTypeMap();
-  const types = Object.keys(typeMap).map(typeName =>
-    definitionBuilder.buildType(typeName),
-  );
-
-  // Do the same with new types, appending to the list of defined types.
-  Object.keys(typeDefinitionMap).forEach(typeName => {
-    types.push(definitionBuilder.buildType(typeName));
-  });
+  const types = [
+    // Iterate through all types, getting the type definition for each, ensuring
+    // that any type not directly referenced by a field will get created.
+    ...objectValues(schema.getTypeMap()).map(type => getExtendedType(type)),
+    // Do the same with new types.
+    ...objectValues(typeDefinitionMap).map(type => astBuilder.buildType(type)),
+  ];
 
   // Support both original legacy names and extended legacy names.
   const schemaAllowedLegacyNames = schema.__allowedLegacyNames;
@@ -277,20 +275,24 @@ export function extendSchema(
     const existingDirectives = schema.getDirectives();
     invariant(existingDirectives, 'schema must have default directives');
 
-    const newDirectives = directiveDefinitions.map(directiveNode =>
-      definitionBuilder.buildDirective(directiveNode),
+    return existingDirectives.concat(
+      directiveDefinitions.map(node => astBuilder.buildDirective(node)),
     );
-    return existingDirectives.concat(newDirectives);
   }
 
-  function getTypeFromDef<T: GraphQLNamedType>(typeDef: T): T {
-    const type = definitionBuilder.buildType(typeDef.name);
-    return (type: any);
+  function getExtendedType<T: GraphQLNamedType>(type: T): T {
+    if (!extendTypeCache[type.name]) {
+      extendTypeCache[type.name] = extendType(type);
+    }
+    return (extendTypeCache[type.name]: any);
   }
 
-  // Given a type's introspection result, construct the correct
-  // GraphQLType instance.
-  function extendType(type: GraphQLNamedType): GraphQLNamedType {
+  // To be called at most once per type. Only getExtendedType should call this.
+  function extendType(type) {
+    if (isIntrospectionType(type)) {
+      // Introspection types are not extended.
+      return type;
+    }
     if (isObjectType(type)) {
       return extendObjectType(type);
     }
@@ -300,6 +302,7 @@ export function extendSchema(
     if (isUnionType(type)) {
       return extendUnionType(type);
     }
+    // This type is not yet extendable.
     return type;
   }
 
@@ -344,7 +347,7 @@ export function extendSchema(
     return new GraphQLUnionType({
       name: type.name,
       description: type.description,
-      types: type.getTypes().map(getTypeFromDef),
+      types: type.getTypes().map(getExtendedType),
       astNode: type.astNode,
       resolveType: type.resolveType,
     });
@@ -353,7 +356,7 @@ export function extendSchema(
   function extendImplementedInterfaces(
     type: GraphQLObjectType,
   ): Array<GraphQLInterfaceType> {
-    const interfaces = type.getInterfaces().map(getTypeFromDef);
+    const interfaces = type.getInterfaces().map(getExtendedType);
 
     // If there are any extensions to the interfaces, apply those here.
     const extensions = typeExtensionsMap[type.name];
@@ -363,7 +366,7 @@ export function extendSchema(
           // Note: While this could make early assertions to get the correctly
           // typed values, that would throw immediately while type system
           // validation with validateSchema() will produce more actionable results.
-          interfaces.push((definitionBuilder.buildType(namedType): any));
+          interfaces.push((astBuilder.buildType(namedType): any));
         });
       });
     }
@@ -399,7 +402,7 @@ export function extendSchema(
               [field],
             );
           }
-          newFieldMap[fieldName] = definitionBuilder.buildField(field);
+          newFieldMap[fieldName] = astBuilder.buildField(field);
         });
       });
     }
@@ -414,6 +417,6 @@ export function extendSchema(
     if (isNonNullType(typeDef)) {
       return (GraphQLNonNull(extendFieldType(typeDef.ofType)): any);
     }
-    return getTypeFromDef(typeDef);
+    return getExtendedType(typeDef);
   }
 }