From c49b13bb7b439674603c00120174a057d66de2e3 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Fri, 6 May 2016 11:48:42 -0700 Subject: [PATCH] BUG: Ensure building AST schema does not exclude @skip and @include `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. --- .../__tests__/buildASTSchema-test.js | 60 +++++++++++++++++++ src/utilities/buildASTSchema.js | 18 ++++-- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/src/utilities/__tests__/buildASTSchema-test.js b/src/utilities/__tests__/buildASTSchema-test.js index ed8e259dc8..4c5db4474e 100644 --- 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 index 5a1d04a5c5..14d8f5f245 100644 --- 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,