Skip to content

Commit

Permalink
BUG: Ensure building AST schema does not exclude @Skip and @include (#…
Browse files Browse the repository at this point in the history
…380)

`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.
  • Loading branch information
leebyron committed May 6, 2016
1 parent 88cf354 commit 44768a8
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 4 deletions.
60 changes: 60 additions & 0 deletions src/utilities/__tests__/buildASTSchema-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
18 changes: 14 additions & 4 deletions src/utilities/buildASTSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ import { valueFromAST } from './valueFromAST';
import {
LIST_TYPE,
NON_NULL_TYPE,
} from '../language/kinds';

import {
DOCUMENT,
SCHEMA_DEFINITION,
SCALAR_TYPE_DEFINITION,
Expand Down Expand Up @@ -63,7 +60,11 @@ import {
GraphQLNonNull,
} from '../type';

import { GraphQLDirective } from '../type/directives';
import {
GraphQLDirective,
GraphQLSkipDirective,
GraphQLIncludeDirective,
} from '../type/directives';

import {
__Schema,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 44768a8

Please sign in to comment.