Attempt at providing optional typechecking for directives

Author: Sylvain Lebresne

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

@@ -30,7 +30,7 @@ declare global {
   namespace jest {
     interface Matchers<R> {
       toHaveField(name: string, type?: Type): R;
-      toHaveDirective(directive: DirectiveDefinition, args?: Record<string, any>): R;
+      toHaveDirective<TArgs extends {[key: string]: any}>(directive: DirectiveDefinition<TArgs>, args?: TArgs): R;
     }
   }
 }
@@ -97,8 +97,8 @@ test('building a simple schema programatically', () => {
   const schema = new Schema(federationBuiltIns);
   const queryType = schema.schemaDefinition.setRoot('query', schema.addType(new ObjectType('Query')));
   const typeA = schema.addType(new ObjectType('A'));
-  const inaccessible = schema.directive('inaccessible')!;
-  const key = schema.directive('key')!;
+  const inaccessible = federationBuiltIns.inaccessibleDirective(schema);
+  const key = federationBuiltIns.keyDirective(schema);
 
   queryType.addField('a', typeA);
   typeA.addField('q', queryType);
@@ -137,7 +137,7 @@ type MyQuery {
   expect(schema.schemaDefinition.root('query')).toBe(queryType);
   expect(queryType).toHaveField('a', typeA);
   const f2 = typeA.field('f2');
-  expect(f2).toHaveDirective(schema.directive('inaccessible')!);
+  expect(f2).toHaveDirective(federationBuiltIns.inaccessibleDirective(schema));
   expect(printSchema(schema)).toBe(sdl);
 
   expect(typeA).toHaveField('f1');
@@ -224,7 +224,7 @@ test('removal of all inacessible elements of a schema', () => {
   `, federationBuiltIns);
 
   for (const element of schema.allSchemaElement()) {
-    if (element.appliedDirectivesOf(schema.directive('inaccessible')!).length > 0) {
+    if (element.appliedDirectivesOf(federationBuiltIns.inaccessibleDirective(schema)).length > 0) {
       element.remove();
     }
   }
@@ -425,9 +425,5 @@ interface Product {
   expectInterfaceType(product);
   expect(product.field('description')!.description).toBe(longComment);
 
-  const directive = schema.directive('Important')!;
-  if (directive.repeatable === false)
-    product.appliedDirectivesOf(directive);
-
   expect(printSchema(schema)).toBe(sdl);
 });

core-js/src/definitions.ts

@@ -125,7 +125,7 @@ export abstract class SchemaElement<TParent extends SchemaElement<any, any> | Sc
   }
 
   appliedDirectivesOf(name: string): Directive[];
-  appliedDirectivesOf(definition: DirectiveDefinition): Directive[];
+  appliedDirectivesOf<TApplicationArgs extends {[key: string]: any} = {[key: string]: any}>(definition: DirectiveDefinition<TApplicationArgs>): Directive<TApplicationArgs>[];
   appliedDirectivesOf(nameOrDefinition: string | DirectiveDefinition): Directive[] {
     const directiveName = typeof nameOrDefinition === 'string' ? nameOrDefinition : nameOrDefinition.name;
     return this._appliedDirectives.filter(d => d.name == directiveName);
@@ -156,6 +156,7 @@ export abstract class SchemaElement<TParent extends SchemaElement<any, any> | Sc
       Directive.prototype['setParent'].call(toAdd, this);
       toAdd.source = source;
     }
+    // TODO: we should typecheck arguments or our TApplicationArgs business is just a lie.
     this._appliedDirectives.push(toAdd);
     DirectiveDefinition.prototype['addReferencer'].call(toAdd.definition!, toAdd);
     return toAdd;
@@ -351,6 +352,33 @@ export class BuiltIns {
   protected addBuiltInDirective(schema: Schema, name: string): DirectiveDefinition {
     return schema.addDirectiveDefinition(new DirectiveDefinition(name, true));
   }
+
+  protected getTypedDirective<TApplicationArgs extends {[key: string]: any}>(
+    schema: Schema,
+    name: string
+  ): DirectiveDefinition<TApplicationArgs> {
+    const directive = schema.directive(name);
+    if (!directive) {
+      throw new Error(`The provided schema has not be built with the ${name} directive built-in`);
+    }
+    return directive as DirectiveDefinition<TApplicationArgs>;
+  }
+
+  includeDirective(schema: Schema): DirectiveDefinition<{if: boolean}> {
+    return this.getTypedDirective(schema, 'include');
+  }
+
+  skipDirective(schema: Schema): DirectiveDefinition<{if: boolean}> {
+    return this.getTypedDirective(schema, 'skip');
+  }
+
+  deprecatedDirective(schema: Schema): DirectiveDefinition<{reason?: string}> {
+    return this.getTypedDirective(schema, 'deprecated');
+  }
+
+  specifiedByDirective(schema: Schema): DirectiveDefinition<{url: string}> {
+    return this.getTypedDirective(schema, 'specifiedBy');
+  }
 }
 
 export class Schema {
@@ -1054,13 +1082,13 @@ export class EnumValue extends BaseNamedElement<EnumType, never> {
   }
 }
 
-export class DirectiveDefinition extends BaseNamedElement<Schema, Directive> {
+export class DirectiveDefinition<TApplicationArgs extends {[key: string]: any} = {[key: string]: any}> extends BaseNamedElement<Schema, Directive> {
   readonly kind: 'DirectiveDefinition' = 'DirectiveDefinition';
 
   private readonly _args: Map<string, ArgumentDefinition<DirectiveDefinition>> = new Map();
   repeatable: boolean = false;
   private readonly _locations: DirectiveLocationEnum[] = [];
-  private readonly _referencers: Set<Directive> = new Set();
+  private readonly _referencers: Set<Directive<TApplicationArgs>> = new Set();
 
   constructor(name: string, readonly isBuiltIn: boolean = false) {
     super(name);
@@ -1132,7 +1160,7 @@ export class DirectiveDefinition extends BaseNamedElement<Schema, Directive> {
     return this;
   }
 
-  private addReferencer(referencer: Directive) {
+  private addReferencer(referencer: Directive<TApplicationArgs>) {
     assert(referencer, 'Referencer should exists');
     this._referencers.add(referencer);
   }
@@ -1170,11 +1198,11 @@ export class DirectiveDefinition extends BaseNamedElement<Schema, Directive> {
 // value if `x` has one and wasn't explicitly set in the application. This would make code usage more pleasant. Should
 // `arguments()` also return those though? Maybe have an option to both method to say if it should include them or not.
 // (The question stands for matchArguments() as well though).
-export class Directive implements Named {
+export class Directive<TArgs extends {[key: string]: any} = {[key: string]: any}> implements Named {
   private _parent?: SchemaElement<any, any>;
   source?: ASTNode;
 
-  constructor(readonly name: string, private _args: Record<string, any>) {}
+  constructor(readonly name: string, private _args: TArgs) {}
 
   schema(): Schema | undefined {
     return this._parent?.schema();
@@ -1194,14 +1222,10 @@ export class Directive implements Named {
     return doc?.directive(this.name);
   }
 
-  get arguments() : Record<string, any> {
+  get arguments() : TArgs {
     return this._args;
   }
 
-  argument(name: string): any {
-    return this._args.get(name);
-  }
-
   matchArguments(expectedArgs: Record<string, any>): boolean {
     const entries = Object.entries(this._args);
     if (entries.length !== Object.keys(expectedArgs).length) {

core-js/src/federation.ts

@@ -1,4 +1,4 @@
-import { BuiltIns, Schema } from "./definitions";
+import { BuiltIns, Schema, DirectiveDefinition, NonNullType } from "./definitions";
 
 // TODO: Need a way to deal with the fact that the _Entity type is built after validation.
 export class FederationBuiltIns extends BuiltIns {
@@ -15,7 +15,7 @@ export class FederationBuiltIns extends BuiltIns {
 
     this.addBuiltInDirective(schema, 'key')
       .addLocations('OBJECT', 'INTERFACE')
-      .addArgument('fields', schema.stringType());
+      .addArgument('fields', new NonNullType(schema.stringType()));
 
     this.addBuiltInDirective(schema, 'extends')
       .addLocations('OBJECT', 'INTERFACE');
@@ -26,12 +26,36 @@ export class FederationBuiltIns extends BuiltIns {
     for (const name of ['requires', 'provides']) {
       this.addBuiltInDirective(schema, name)
         .addLocations('FIELD_DEFINITION')
-        .addArgument('fields', schema.stringType());
+        .addArgument('fields', new NonNullType(schema.stringType()));
     }
 
     this.addBuiltInDirective(schema, 'inaccessible')
       .addAllLocations();
   }
+
+  keyDirective(schema: Schema): DirectiveDefinition<{fields: string}> {
+    return this.getTypedDirective(schema, 'key');
+  }
+
+  extendsDirective(schema: Schema): DirectiveDefinition<{}> {
+    return this.getTypedDirective(schema, 'extends');
+  }
+
+  externalDirective(schema: Schema): DirectiveDefinition<{}> {
+    return this.getTypedDirective(schema, 'external');
+  }
+
+  requiresDirective(schema: Schema): DirectiveDefinition<{fields: string}> {
+    return this.getTypedDirective(schema, 'requires');
+  }
+
+  providesDirective(schema: Schema): DirectiveDefinition<{fields: string}> {
+    return this.getTypedDirective(schema, 'provides');
+  }
+
+  inaccessibleDirective(schema: Schema): DirectiveDefinition<{}> {
+    return this.getTypedDirective(schema, 'inaccessible');
+  }
 }
 
 export const federationBuiltIns = new FederationBuiltIns();