BUG: Ensure building AST schema does not exclude @Skip and @include

leebyron · leebyron · commit c49b13bb7b43 · 2016-05-06T11:57:30.000-07:00
`buildASTSchema` used to not regard directives in 0.4.x, just always including only `@skip` and `@include`. Since 0.5.0 included the ability to use directives in the experimental schema language, existing use of this tool found no defined directives and therefore excluded these two built-ins.

This fixes the issue by implicitly adding these built-in directives if they were not explicitly defined.
diff --git a/src/utilities/__tests__/buildASTSchema-test.js b/src/utilities/__tests__/buildASTSchema-test.js
@@ -12,6 +12,10 @@ import { describe, it } from 'mocha';
 import { parse } from '../../language';
 import { printSchema } from '../schemaPrinter';
 import { buildASTSchema } from '../buildASTSchema';
+import {
+  GraphQLSkipDirective,
+  GraphQLIncludeDirective,
+} from '../../type/directives';
 
 /**
  * This function does a full cycle of going from a
@@ -61,6 +65,62 @@ type Hello {
     expect(output).to.equal(body);
   });
 
+
+  it('Maintains @skip & @include', () => {
+    const body = `
+schema {
+  query: Hello
+}
+
+type Hello {
+  str: String
+}
+`;
+    const schema = buildASTSchema(parse(body));
+    expect(schema.getDirectives().length).to.equal(2);
+    expect(schema.getDirective('skip')).to.equal(GraphQLSkipDirective);
+    expect(schema.getDirective('include')).to.equal(GraphQLIncludeDirective);
+  });
+
+  it('Overriding directives excludes built-ins', () => {
+    const body = `
+schema {
+  query: Hello
+}
+
+directive @skip on FIELD
+directive @include on FIELD
+
+type Hello {
+  str: String
+}
+`;
+    const schema = buildASTSchema(parse(body));
+    expect(schema.getDirectives().length).to.equal(2);
+    expect(schema.getDirective('skip')).to.not.equal(GraphQLSkipDirective);
+    expect(
+      schema.getDirective('include')
+    ).to.not.equal(GraphQLIncludeDirective);
+  });
+
+  it('Adding directives maintains @skip & @include', () => {
+    const body = `
+schema {
+  query: Hello
+}
+
+directive @foo(arg: Int) on FIELD
+
+type Hello {
+  str: String
+}
+`;
+    const schema = buildASTSchema(parse(body));
+    expect(schema.getDirectives().length).to.equal(3);
+    expect(schema.getDirective('skip')).to.not.equal(undefined);
+    expect(schema.getDirective('include')).to.not.equal(undefined);
+  });
+
   it('Type modifiers', () => {
     const body = `
 schema {
diff --git a/src/utilities/buildASTSchema.js b/src/utilities/buildASTSchema.js
@@ -16,9 +16,6 @@ import { valueFromAST } from './valueFromAST';
 import {
   LIST_TYPE,
   NON_NULL_TYPE,
-} from '../language/kinds';
-
-import {
   DOCUMENT,
   SCHEMA_DEFINITION,
   SCALAR_TYPE_DEFINITION,
@@ -63,7 +60,11 @@ import {
   GraphQLNonNull,
 } from '../type';
 
-import { GraphQLDirective } from '../type/directives';
+import {
+  GraphQLDirective,
+  GraphQLSkipDirective,
+  GraphQLIncludeDirective,
+} from '../type/directives';
 
 import {
   __Schema,
@@ -223,6 +224,15 @@ export function buildASTSchema(ast: Document): GraphQLSchema {
 
   const directives = directiveDefs.map(getDirective);
 
+  // If skip and include were not explicitly declared, add them.
+  if (!directives.some(directive => directive.name === 'skip')) {
+    directives.push(GraphQLSkipDirective);
+  }
+
+  if (!directives.some(directive => directive.name === 'include')) {
+    directives.push(GraphQLIncludeDirective);
+  }
+
   return new GraphQLSchema({
     query: getObjectType(astMap[queryTypeName]),
     mutation: mutationTypeName ? getObjectType(astMap[mutationTypeName]) : null,
