Unify extending of type system (#1261)

graphql · Feb 28, 2018 · a62eea8 · a62eea8
1 parent 261b99b
commit a62eea8
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 69 deletions.
diff --git a/src/utilities/__tests__/extendSchema-test.js b/src/utilities/__tests__/extendSchema-test.js
@@ -794,7 +794,7 @@ describe('extendSchema', () => {
       }
     `);
     expect(() => extendSchema(testSchema, ast)).to.throw(
-      'Cannot extend interface "UnknownInterfaceType" because it does not ' +
+      'Cannot extend type "UnknownInterfaceType" because it does not ' +
         'exist in the existing schema.',
     );
   });

diff --git a/src/utilities/buildASTSchema.js b/src/utilities/buildASTSchema.js
@@ -184,8 +184,7 @@ export function buildASTSchema(
     },
   );
 
-  const types = typeDefs.map(def => definitionBuilder.buildType(def));
-
+  const types = definitionBuilder.buildTypes(typeDefs);
   const directives = directiveDefs.map(def =>
     definitionBuilder.buildDirective(def),
   );
@@ -266,6 +265,12 @@ export class ASTDefinitionBuilder {
     );
   }
 
+  buildTypes(
+    nodes: $ReadOnlyArray<NamedTypeNode | TypeDefinitionNode>,
+  ): Array<GraphQLNamedType> {
+    return nodes.map(node => this.buildType(node));
+  }
+
   buildType(node: NamedTypeNode | TypeDefinitionNode): GraphQLNamedType {
     const typeName = node.name.value;
     if (!this._cache[typeName]) {
@@ -334,11 +339,15 @@ export class ASTDefinitionBuilder {
 
   _makeTypeDef(def: ObjectTypeDefinitionNode) {
     const typeName = def.name.value;
+    const interfaces = def.interfaces;
     return new GraphQLObjectType({
       name: typeName,
       description: getDescription(def, this._options),
       fields: () => this._makeFieldDefMap(def),
-      interfaces: () => this._makeImplementedInterfaces(def),
+      // 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: interfaces ? () => (this.buildTypes(interfaces): any) : [],
       astNode: def,
     });
   }
@@ -355,16 +364,6 @@ export class ASTDefinitionBuilder {
       : {};
   }
 
-  _makeImplementedInterfaces(def: ObjectTypeDefinitionNode) {
-    return (
-      def.interfaces &&
-      // 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.
-      def.interfaces.map(iface => (this.buildType(iface): any))
-    );
-  }
-
   _makeInputValues(values: $ReadOnlyArray<InputValueDefinitionNode>) {
     return keyValMap(
       values,
@@ -419,7 +418,7 @@ export class ASTDefinitionBuilder {
       // Note: While this could make assertions to get the correctly typed
       // values below, that would throw immediately while type system
       // validation with validateSchema() will produce more actionable results.
-      types: def.types ? def.types.map(t => (this.buildType(t): any)) : [],
+      types: def.types ? (this.buildTypes(def.types): any) : [],
       astNode: def,
     });
   }

diff --git a/src/utilities/extendSchema.js b/src/utilities/extendSchema.js
@@ -34,12 +34,7 @@ import { GraphQLDirective } from '../type/directives';
 import { Kind } from '../language/kinds';
 
 import type { GraphQLType, GraphQLNamedType } from '../type/definition';
-
-import type {
-  DocumentNode,
-  DirectiveDefinitionNode,
-  TypeExtensionNode,
-} from '../language/ast';
+import type { DocumentNode, DirectiveDefinitionNode } from '../language/ast';
 
 type Options = {|
   ...GraphQLSchemaValidationOptions,
@@ -114,6 +109,7 @@ export function extendSchema(
         typeDefinitionMap[typeName] = def;
         break;
       case Kind.OBJECT_TYPE_EXTENSION:
+      case Kind.INTERFACE_TYPE_EXTENSION:
         // Sanity check that this type extension exists within the
         // schema's existing types.
         const extendedTypeName = def.name.value;
@@ -125,39 +121,12 @@ export function extendSchema(
             [def],
           );
         }
-        if (!isObjectType(existingType)) {
-          throw new GraphQLError(
-            `Cannot extend non-object type "${extendedTypeName}".`,
-            [def],
-          );
-        }
-        typeExtensionsMap[extendedTypeName] = appendExtensionToTypeExtensions(
-          def,
-          typeExtensionsMap[extendedTypeName],
-        );
-        break;
-      case Kind.INTERFACE_TYPE_EXTENSION:
-        const extendedInterfaceTypeName = def.name.value;
-        const existingInterfaceType = schema.getType(extendedInterfaceTypeName);
-        if (!existingInterfaceType) {
-          throw new GraphQLError(
-            `Cannot extend interface "${extendedInterfaceTypeName}" because ` +
-              'it does not exist in the existing schema.',
-            [def],
-          );
-        }
-        if (!isInterfaceType(existingInterfaceType)) {
-          throw new GraphQLError(
-            `Cannot extend non-interface type "${extendedInterfaceTypeName}".`,
-            [def],
-          );
-        }
-        typeExtensionsMap[
-          extendedInterfaceTypeName
-        ] = appendExtensionToTypeExtensions(
-          def,
-          typeExtensionsMap[extendedInterfaceTypeName],
-        );
+        checkExtensionNode(existingType, def);
+
+        const existingTypeExtensions = typeExtensionsMap[extendedTypeName];
+        typeExtensionsMap[extendedTypeName] = existingTypeExtensions
+          ? existingTypeExtensions.concat([def])
+          : [def];
         break;
       case Kind.DIRECTIVE_DEFINITION:
         const directiveName = def.name.value;
@@ -212,9 +181,6 @@ export function extendSchema(
   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
     ? getExtendedType(existingQueryType)
@@ -235,7 +201,7 @@ export function extendSchema(
     // 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)),
+    ...astBuilder.buildTypes(objectValues(typeDefinitionMap)),
   ];
 
   // Support both original legacy names and extended legacy names.
@@ -257,17 +223,6 @@ export function extendSchema(
     allowedLegacyNames,
   });
 
-  function appendExtensionToTypeExtensions(
-    extension: TypeExtensionNode,
-    existingTypeExtensions: ?Array<TypeExtensionNode>,
-  ): Array<TypeExtensionNode> {
-    if (!existingTypeExtensions) {
-      return [extension];
-    }
-    existingTypeExtensions.push(extension);
-    return existingTypeExtensions;
-  }
-
   // Below are functions used for producing this schema that have closed over
   // this scope and have access to the schema, cache, and newly defined types.
 
@@ -420,3 +375,24 @@ export function extendSchema(
     return getExtendedType(typeDef);
   }
 }
+
+function checkExtensionNode(type, node) {
+  switch (node.kind) {
+    case Kind.OBJECT_TYPE_EXTENSION:
+      if (!isObjectType(type)) {
+        throw new GraphQLError(
+          `Cannot extend non-object type "${type.name}".`,
+          [node],
+        );
+      }
+      break;
+    case Kind.INTERFACE_TYPE_EXTENSION:
+      if (!isInterfaceType(type)) {
+        throw new GraphQLError(
+          `Cannot extend non-interface type "${type.name}".`,
+          [node],
+        );
+      }
+      break;
+  }
+}