Handle extensions

The "design" of extensions tries to optimize for extensions being
supported, but not getting in the way. The assumption is that the
most code are likely to not want to care if something is an extension or
a proper definition. So a type is still just one object, regardless of
whether it's components comes from both a definition, an extension or
a mix of those. However, each type lists how many extensions it's
been built from, and each direct component of the type lists if it
comes from an extension.

In other wordds, it's easy to ignore whether something comes from
extensions or not, but it's easy to know if a particular type element
was originally defined in an extension or not. Rebuilding a particular
extension however do require to iterate on the elements of the type
and filter those that belong to the extension we care about.

Alternative designs have been considered:
- having extensions be completely distinct objects from definitions.
  However, it feels painful that for a given type name you'll always
  have to potentially deal with multiple objects (a definition and a
  list of potential extensions).
- Have a hierachy where we have an object for each type, but also
  separate object for defintiion and extensions. The "type" object
  would point to it's constituent (definition and/or extensions)
  but would also have methods to conveniently access all the
  elements of the type. In other world, we'd have `ObjectTypeDefinition`
  and `ObjecTypeExtensions`, both of which my have `fields`, and
  then we have `ObjectType` that points to (at most) an
  `ObjectTypeDefinition` and some `ObjectTypeExtensions` and also
  expose `fields` which is just a merged iterator over the fields
  in the definition and extensions.
  In term of exposing extension, this would be fairly clean.
  However, this require a lot of reorganisation of the library
  and quite a bit of new absctractions. Concept like `SchemaElement`
  gets a bit more complicated (you don't want `schema.allSchemaElement`).
  to return both `ObjectType` _and_ `ObjectTypeDefinition`/`ObjectTypeExtension`
  because that would duplicate things, but you still want to think of
  all of those as "schema elements" in other context, and it's unclear
  how to not make all of this overly complex. The 'parent' of elements
  like fields also become a more complex thing, etc...
- A mix of previous points where you only have `ObjectTypeDefinition`
  and `ObjectTypeExtension`, but `ObjectTypeDefinition` has methods
  like `ownFields` and `fields` (the later return both `ownFields` and
  the extensions fields).
  While this simplify some of the question of the previous point
  but not all of them (this still complicate the hiearchy quite a bit)
  and this introduces other awkwardness (for instance, having
  `directives` on an `ObjectTypeDefinition` also including the
  directives of the extensions can be a tad suprising at first, but if
  you call it `allDirectives` to avoid the confusion, now you get some
  inconsistencies with other elements so it's not perfect either.

Tl;dr, while the current design has minor downside, the alternative have
other ones and feels more complex overall.

Author: Sylvain Lebresne

core-js/src/__tests__/definitions.test.ts

@@ -5,7 +5,8 @@ import {
   DirectiveDefinition,
   InterfaceType,
   EnumType,
-  SchemaElement
+  SchemaElement,
+  UnionType
 } from '../../dist/definitions';
 import { printSchema } from '../../dist/print';
 import { buildSchema } from '../../dist/buildSchema';
@@ -21,6 +22,11 @@ function expectInterfaceType(type?: Type): asserts type is InterfaceType {
   expect(type!.kind).toBe('InterfaceType');
 }
 
+function expectUnionType(type?: Type): asserts type is UnionType {
+  expect(type).toBeDefined();
+  expect(type!.kind).toBe('UnionType');
+}
+
 function expectEnumType(type?: Type): asserts type is EnumType {
   expect(type).toBeDefined();
   expect(type!.kind).toBe('EnumType');
@@ -37,7 +43,12 @@ declare global {
 }
 
 function deIndent(str: string): string {
+  // Strip leading \n
   str = str.slice(str.search(/[^\n]/));
+  // Strip trailing \n or space
+  while (str.charAt(str.length - 1) === '\n' || str.charAt(str.length - 1) === ' ') {
+    str = str.slice(0, str.length - 1);
+  }
   const indent = str.search(/[^ ]/);
   return str
     .split('\n')
@@ -72,7 +83,7 @@ expect.extend({
     }
   },
 
-  toHaveDirective(element: SchemaElement<any, any>, definition: DirectiveDefinition, args?: Record<string, any>) {
+  toHaveDirective(element: SchemaElement<any>, definition: DirectiveDefinition, args?: Record<string, any>) {
     const directives = element.appliedDirectivesOf(definition);
     if (directives.length == 0) {
       return {
@@ -103,7 +114,8 @@ expect.extend({
   },
 
   toMatchString(expected: string, received: string) {
-    const pass = this.equals(expected, deIndent(received));
+    received = deIndent(received);
+    const pass = this.equals(expected, received);
     const message = pass
       ? () => this.utils.matcherHint('toMatchString', undefined, undefined)
           + '\n\n'
@@ -120,7 +132,7 @@ expect.extend({
 
 test('building a simple schema programatically', () => {
   const schema = new Schema(federationBuiltIns);
-  const queryType = schema.schemaDefinition.setRoot('query', schema.addType(new ObjectType('Query')));
+  const queryType = schema.schemaDefinition.setRoot('query', schema.addType(new ObjectType('Query'))).type;
   const typeA = schema.addType(new ObjectType('A'));
   const inaccessible = federationBuiltIns.inaccessibleDirective(schema);
   const key = federationBuiltIns.keyDirective(schema);
@@ -130,7 +142,7 @@ test('building a simple schema programatically', () => {
   typeA.applyDirective(inaccessible);
   typeA.applyDirective(key, { fields: 'a'});
 
-  expect(queryType).toBe(schema.schemaDefinition.root('query'));
+  expect(queryType).toBe(schema.schemaDefinition.root('query')!.type);
   expect(queryType).toHaveField('a', typeA);
   expect(typeA).toHaveField('q', queryType);
   expect(typeA).toHaveDirective(inaccessible);
@@ -159,7 +171,7 @@ test('parse schema and modify', () => {
   const typeA = schema.type('A')!;
   expectObjectType(queryType);
   expectObjectType(typeA);
-  expect(schema.schemaDefinition.root('query')).toBe(queryType);
+  expect(schema.schemaDefinition.root('query')!.type).toBe(queryType);
   expect(queryType).toHaveField('a', typeA);
   const f2 = typeA.field('f2');
   expect(f2).toHaveDirective(federationBuiltIns.inaccessibleDirective(schema));
@@ -248,7 +260,7 @@ test('removal of all inacessible elements of a schema', () => {
     directive @bar on ARGUMENT_DEFINITION
   `, federationBuiltIns);
 
-  for (const element of schema.allSchemaElement()) {
+  for (const element of schema.allNamedSchemaElement()) {
     if (element.appliedDirectivesOf(federationBuiltIns.inaccessibleDirective(schema)).length > 0) {
       element.remove();
     }
@@ -452,3 +464,51 @@ test('handling of descriptions', () => {
 
   expect(printSchema(schema)).toMatchString(sdl);
 });
+
+test('handling of extensions', () => {
+  const sdl = `
+    extend schema {
+      query: AType
+    }
+
+    interface AInterface {
+      i1: Int
+    }
+
+    extend interface AInterface @deprecated
+
+    scalar AScalar
+
+    extend scalar AScalar @deprecated
+
+    extend type AType {
+      t1: Int
+      t2: String
+    }
+
+    type AType2 {
+      t1: String
+    }
+
+    type AType3 {
+      t2: Int
+    }
+
+    union AUnion = AType | AType2
+
+    extend union AUnion = AType3
+  `;
+
+  const schema = buildSchema(sdl);
+  expect(printSchema(schema)).toMatchString(sdl);
+
+  const atype = schema.type('AType');
+  expectObjectType(atype);
+  expect(atype).toHaveField('t1', schema.intType());
+  expect(atype).toHaveField('t2', schema.stringType());
+
+  const aunion = schema.type('AUnion');
+  expectUnionType(aunion);
+  expect([...aunion.types()].map(t => t.name)).toEqual(['AType', 'AType2', 'AType3']);
+
+});

core-js/src/buildSchema.ts

@@ -16,7 +16,8 @@ import {
   NamedTypeNode,
   ArgumentNode,
   StringValueNode,
-  ASTNode
+  ASTNode,
+  SchemaExtensionNode
 } from "graphql";
 import { Maybe } from "graphql/jsutils/Maybe";
 import {
@@ -43,7 +44,8 @@ import {
   DirectiveDefinition,
   UnionType,
   InputObjectType,
-  EnumType
+  EnumType,
+  Extension
 } from "./definitions";
 
 function buildValue(value?: ValueNode): any {
@@ -75,6 +77,12 @@ export function buildSchemaFromAST(documentNode: DocumentNode, builtIns: BuiltIn
       case 'SchemaDefinition':
         buildSchemaDefinitionInner(definitionNode, schema.schemaDefinition);
         break;
+      case 'SchemaExtension':
+        buildSchemaDefinitionInner(
+          definitionNode,
+          schema.schemaDefinition, 
+          schema.schemaDefinition.newExtension());
+        break;
       case 'ScalarTypeDefinition':
       case 'ObjectTypeDefinition':
       case 'InterfaceTypeDefinition':
@@ -83,17 +91,20 @@ export function buildSchemaFromAST(documentNode: DocumentNode, builtIns: BuiltIn
       case 'InputObjectTypeDefinition':
         buildNamedTypeInner(definitionNode, schema.type(definitionNode.name.value)!);
         break;
-      case 'DirectiveDefinition':
-        buildDirectiveDefinitionInner(definitionNode, schema.directive(definitionNode.name.value)!);
-        break;
-      case 'SchemaExtension':
       case 'ScalarTypeExtension':
       case 'ObjectTypeExtension':
       case 'InterfaceTypeExtension':
       case 'UnionTypeExtension':
       case 'EnumTypeExtension':
       case 'InputObjectTypeExtension':
-        throw new Error("Extensions are a TODO");
+        const toExtend = schema.type(definitionNode.name.value)!;
+        const extension = toExtend.newExtension();
+        extension.sourceAST = definitionNode;
+        buildNamedTypeInner(definitionNode, toExtend, extension);
+        break;
+      case 'DirectiveDefinition':
+        buildDirectiveDefinitionInner(definitionNode, schema.directive(definitionNode.name.value)!);
+        break;
     }
   }
   return schema;
@@ -108,7 +119,16 @@ function buildNamedTypeAndDirectivesShallow(documentNode: DocumentNode, schema:
       case 'UnionTypeDefinition':
       case 'EnumTypeDefinition':
       case 'InputObjectTypeDefinition':
-        schema.addType(newNamedType(withoutTrailingDefinition(definitionNode.kind), definitionNode.name.value));
+      case 'ScalarTypeExtension':
+      case 'ObjectTypeExtension':
+      case 'InterfaceTypeExtension':
+      case 'UnionTypeExtension':
+      case 'EnumTypeExtension':
+      case 'InputObjectTypeExtension':
+        // Note that because of extensions, this may be called multiple times for the same type.
+        if (!schema.type(definitionNode.name.value)) {
+          schema.addType(newNamedType(withoutTrailingDefinition(definitionNode.kind), definitionNode.name.value));
+        }
         break;
       case 'DirectiveDefinition':
         schema.addDirectiveDefinition(definitionNode.name.value);
@@ -122,7 +142,8 @@ type NodeWithDescription = {description?: Maybe<StringValueNode>};
 type NodeWithArguments = {arguments?: ReadonlyArray<ArgumentNode>};
 
 function withoutTrailingDefinition(str: string): NamedTypeKind {
-  return str.slice(0, str.length - 'Definition'.length) as NamedTypeKind;
+  const endString = str.endsWith('Definition') ? 'Definition' : 'Extension';
+  return str.slice(0, str.length - endString.length) as NamedTypeKind;
 }
 
 function getReferencedType(node: NamedTypeNode, schema: Schema): NamedType {
@@ -145,7 +166,7 @@ function withNodeAttachedToError(operation: () => void, node: ASTNode) {
         e.source,
         e.positions,
         e.path,
-        e.originalError,
+        e,
         e.extensions
       );
     } else {
@@ -154,19 +175,30 @@ function withNodeAttachedToError(operation: () => void, node: ASTNode) {
   }
 }
 
-function buildSchemaDefinitionInner(schemaNode: SchemaDefinitionNode, schemaDefinition: SchemaDefinition) {
-  for (const opTypeNode of schemaNode.operationTypes) {
-    withNodeAttachedToError(() => schemaDefinition.setRoot(opTypeNode.operation, opTypeNode.type.name.value), opTypeNode);
+function buildSchemaDefinitionInner(
+  schemaNode: SchemaDefinitionNode | SchemaExtensionNode,
+  schemaDefinition: SchemaDefinition,
+  extension?: Extension<SchemaDefinition>
+) {
+  for (const opTypeNode of schemaNode.operationTypes ?? []) {
+    withNodeAttachedToError(
+      () => schemaDefinition.setRoot(opTypeNode.operation, opTypeNode.type.name.value).setOfExtension(extension),
+      opTypeNode);
   }
   schemaDefinition.sourceAST = schemaNode;
-  schemaDefinition.description = schemaNode.description?.value;
-  buildAppliedDirectives(schemaNode, schemaDefinition);
+  schemaDefinition.description = 'description' in schemaNode ? schemaNode.description?.value : undefined;
+  buildAppliedDirectives(schemaNode, schemaDefinition, extension);
 }
 
-function buildAppliedDirectives(elementNode: NodeWithDirectives, element: SchemaElement<any>) {
+function buildAppliedDirectives(
+  elementNode: NodeWithDirectives,
+  element: SchemaElement<any>,
+  extension?: Extension<any>
+) {
   for (const directive of elementNode.directives ?? []) {
     withNodeAttachedToError(() => {
       const d = element.applyDirective(directive.name.value, buildArgs(directive));
+      d.setOfExtension(extension);
       d.sourceAST = directive;
     }, directive);
   }
@@ -180,38 +212,55 @@ function buildArgs(argumentsNode: NodeWithArguments): Record<string, any> {
   return args;
 }
 
-function buildNamedTypeInner(definitionNode: DefinitionNode & NodeWithDirectives & NodeWithDescription, type: NamedType) {
+function buildNamedTypeInner(
+  definitionNode: DefinitionNode & NodeWithDirectives & NodeWithDescription,
+  type: NamedType,
+  extension?: Extension<any>
+) {
   switch (definitionNode.kind) {
     case 'ObjectTypeDefinition':
+    case 'ObjectTypeExtension':
     case 'InterfaceTypeDefinition':
+    case 'InterfaceTypeExtension':
       const fieldBasedType = type as ObjectType | InterfaceType;
       for (const fieldNode of definitionNode.fields ?? []) {
-        buildFieldDefinitionInner(fieldNode, fieldBasedType.addField(fieldNode.name.value));
+        const field = fieldBasedType.addField(fieldNode.name.value);
+        field.setOfExtension(extension);
+        buildFieldDefinitionInner(fieldNode, field);
       }
       for (const itfNode of definitionNode.interfaces ?? []) {
-        withNodeAttachedToError(() => fieldBasedType.addImplementedInterface(itfNode.name.value), itfNode);
+        withNodeAttachedToError(
+          () => fieldBasedType.addImplementedInterface(itfNode.name.value).setOfExtension(extension),
+          itfNode);
       }
       break;
     case 'UnionTypeDefinition':
+    case 'UnionTypeExtension':
       const unionType = type as UnionType;
       for (const namedType of definitionNode.types ?? []) {
-        withNodeAttachedToError(() => unionType.addType(namedType.name.value), namedType);
+        withNodeAttachedToError(
+          () => unionType.addType(namedType.name.value).setOfExtension(extension),
+          namedType);
       }
       break;
     case 'EnumTypeDefinition':
+    case 'EnumTypeExtension':
       const enumType = type as EnumType;
       for (const enumVal of definitionNode.values ?? []) {
-        enumType.addValue(enumVal.name.value);
+        enumType.addValue(enumVal.name.value).setOfExtension(extension);
       }
       break;
     case 'InputObjectTypeDefinition':
+    case 'InputObjectTypeExtension':
       const inputObjectType = type as InputObjectType;
       for (const fieldNode of definitionNode.fields ?? []) {
-        buildInputFieldDefinitionInner(fieldNode, inputObjectType.addField(fieldNode.name.value));
+        const field = inputObjectType.addField(fieldNode.name.value);
+        field.setOfExtension(extension);
+        buildInputFieldDefinitionInner(fieldNode, field);
       }
       break;
   }
-  buildAppliedDirectives(definitionNode, type);
+  buildAppliedDirectives(definitionNode, type, extension);
   type.description = definitionNode.description?.value;
   type.sourceAST = definitionNode;
 }