From cab37c724ac90878635237fdc35557d27ad9890a Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Wed, 3 Oct 2018 00:34:25 +0200 Subject: [PATCH 01/13] [RFC] Added support for repeatable directives --- .../__tests__/schema-kitchen-sink.graphql | 4 + src/language/__tests__/schema-parser-test.js | 100 ++++++++++++++++++ src/language/__tests__/schema-printer-test.js | 2 + src/language/ast.js | 1 + src/language/parser.js | 19 +++- src/language/printer.js | 3 +- src/type/directives.js | 6 ++ src/type/introspection.js | 6 ++ .../__tests__/buildASTSchema-test.js | 16 +++ .../__tests__/buildClientSchema-test.js | 6 ++ src/utilities/__tests__/schemaPrinter-test.js | 6 ++ src/utilities/buildASTSchema.js | 1 + src/utilities/buildClientSchema.js | 1 + src/utilities/introspectionQuery.js | 2 + src/utilities/schemaPrinter.js | 1 + .../UniqueDirectivesPerLocation-test.js | 26 +++++ src/validation/__tests__/harness.js | 11 ++ src/validation/rules/KnownDirectives.js | 6 +- .../rules/UniqueDirectivesPerLocation.js | 6 +- src/validation/validate.js | 16 +++ 20 files changed, 231 insertions(+), 8 deletions(-) diff --git a/src/language/__tests__/schema-kitchen-sink.graphql b/src/language/__tests__/schema-kitchen-sink.graphql index f94f47c8e5..f7864a186b 100644 --- a/src/language/__tests__/schema-kitchen-sink.graphql +++ b/src/language/__tests__/schema-kitchen-sink.graphql @@ -115,3 +115,7 @@ directive @include2(if: Boolean!) on | FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT + +directive @myRepeatableDir(name: String!) repeatable on + | OBJECT + | INTERFACE diff --git a/src/language/__tests__/schema-parser-test.js b/src/language/__tests__/schema-parser-test.js index 1c9fe0f144..a026e4a61a 100644 --- a/src/language/__tests__/schema-parser-test.js +++ b/src/language/__tests__/schema-parser-test.js @@ -761,6 +761,106 @@ input Hello { ); }); + it('Directive definition', () => { + const body = 'directive @foo on OBJECT | INTERFACE'; + const doc = parse(body); + const expected = { + kind: 'Document', + definitions: [ + { + kind: 'DirectiveDefinition', + name: { + kind: 'Name', + value: 'foo', + loc: { + start: 11, + end: 14, + }, + }, + arguments: [], + repeatable: false, + locations: [ + { + kind: 'Name', + value: 'OBJECT', + loc: { + start: 18, + end: 24, + }, + }, + { + kind: 'Name', + value: 'INTERFACE', + loc: { + start: 27, + end: 36, + }, + }, + ], + loc: { + start: 0, + end: 36, + }, + }, + ], + loc: { + start: 0, + end: 36, + }, + }; + expect(printJson(doc)).to.equal(printJson(expected)); + }); + + it('Repeatable directive definition', () => { + const body = 'directive @foo repeatable on OBJECT | INTERFACE'; + const doc = parse(body); + const expected = { + kind: 'Document', + definitions: [ + { + kind: 'DirectiveDefinition', + name: { + kind: 'Name', + value: 'foo', + loc: { + start: 11, + end: 14, + }, + }, + arguments: [], + repeatable: true, + locations: [ + { + kind: 'Name', + value: 'OBJECT', + loc: { + start: 29, + end: 35, + }, + }, + { + kind: 'Name', + value: 'INTERFACE', + loc: { + start: 38, + end: 47, + }, + }, + ], + loc: { + start: 0, + end: 47, + }, + }, + ], + loc: { + start: 0, + end: 47, + }, + }; + expect(printJson(doc)).to.equal(printJson(expected)); + }); + it('Directive with incorrect locations', () => { expectSyntaxError( ` diff --git a/src/language/__tests__/schema-printer-test.js b/src/language/__tests__/schema-printer-test.js index cb3a3cd4c6..4728c3c01c 100644 --- a/src/language/__tests__/schema-printer-test.js +++ b/src/language/__tests__/schema-printer-test.js @@ -153,6 +153,8 @@ describe('Printer: SDL document', () => { directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT directive @include2(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT + + directive @myRepeatableDir(name: String!) repeatable on OBJECT | INTERFACE `); }); }); diff --git a/src/language/ast.js b/src/language/ast.js index 6b6f5fbcd2..b5acf42650 100644 --- a/src/language/ast.js +++ b/src/language/ast.js @@ -563,5 +563,6 @@ export type DirectiveDefinitionNode = { +description?: StringValueNode, +name: NameNode, +arguments?: $ReadOnlyArray, + +repeatable: boolean, +locations: $ReadOnlyArray, }; diff --git a/src/language/parser.js b/src/language/parser.js index 20120eeb67..50775eedf5 100644 --- a/src/language/parser.js +++ b/src/language/parser.js @@ -1326,7 +1326,7 @@ function parseInputObjectTypeExtension( /** * DirectiveDefinition : - * - Description? directive @ Name ArgumentsDefinition? on DirectiveLocations + * - Description? directive @ Name ArgumentsDefinition? `repeatable`? on DirectiveLocations */ function parseDirectiveDefinition(lexer: Lexer<*>): DirectiveDefinitionNode { const start = lexer.token; @@ -1335,6 +1335,7 @@ function parseDirectiveDefinition(lexer: Lexer<*>): DirectiveDefinitionNode { expect(lexer, TokenKind.AT); const name = parseName(lexer); const args = parseArgumentDefs(lexer); + const repeatable = expectOptionalKeyword(lexer, 'repeatable'); expectKeyword(lexer, 'on'); const locations = parseDirectiveLocations(lexer); return { @@ -1342,6 +1343,7 @@ function parseDirectiveDefinition(lexer: Lexer<*>): DirectiveDefinitionNode { description, name, arguments: args, + repeatable, locations, loc: loc(lexer, start), }; @@ -1477,6 +1479,21 @@ function expectKeyword(lexer: Lexer<*>, value: string): Token { ); } +/** + * If the next token is a keyword with the given value, return true after advancing + * the lexer. Otherwise, do not change the parser state and return false. + */ +function expectOptionalKeyword(lexer: Lexer<*>, value: string): boolean { + const token = lexer.token; + const match = token.kind === TokenKind.NAME && token.value === value; + + if (match) { + lexer.advance(); + } + + return match; +} + /** * Helper function for creating an error when an unexpected lexed token * is encountered. diff --git a/src/language/printer.js b/src/language/printer.js index daf984d7e8..52d19056eb 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -209,10 +209,11 @@ const printDocASTReducer = { join(['extend input', name, join(directives, ' '), block(fields)], ' '), DirectiveDefinition: addDescription( - ({ name, arguments: args, locations }) => + ({ name, arguments: args, locations, repeatable }) => 'directive @' + name + wrap('(', join(args, ', '), ')') + + (repeatable ? ' repeatable' : '') + ' on ' + join(locations, ' | '), ), diff --git a/src/type/directives.js b/src/type/directives.js index aa8a734cce..19955184d3 100644 --- a/src/type/directives.js +++ b/src/type/directives.js @@ -42,12 +42,17 @@ export class GraphQLDirective { locations: Array; args: Array; astNode: ?DirectiveDefinitionNode; + repeatable: boolean; constructor(config: GraphQLDirectiveConfig): void { this.name = config.name; this.description = config.description; this.locations = config.locations; this.astNode = config.astNode; + this.repeatable = + config.repeatable === undefined || config.repeatable === null + ? false + : config.repeatable; invariant(config.name, 'Directive must be named.'); invariant( Array.isArray(config.locations), @@ -82,6 +87,7 @@ export type GraphQLDirectiveConfig = { locations: Array, args?: ?GraphQLFieldConfigArgumentMap, astNode?: ?DirectiveDefinitionNode, + repeatable?: ?boolean, }; /** diff --git a/src/type/introspection.js b/src/type/introspection.js index 41bbbb53fd..eaa66cfe1a 100644 --- a/src/type/introspection.js +++ b/src/type/introspection.js @@ -92,6 +92,12 @@ export const __Directive = new GraphQLObjectType({ type: GraphQLNonNull(GraphQLList(GraphQLNonNull(__InputValue))), resolve: directive => directive.args || [], }, + repeatable: { + type: GraphQLNonNull(GraphQLBoolean), + description: + 'Permits using the directive multiple times at the same location.', + resolve: directive => directive.repeatable, + }, // NOTE: the following three fields are deprecated and are no longer part // of the GraphQL specification. onOperation: { diff --git a/src/utilities/__tests__/buildASTSchema-test.js b/src/utilities/__tests__/buildASTSchema-test.js index b4b0545df3..a70829dd10 100644 --- a/src/utilities/__tests__/buildASTSchema-test.js +++ b/src/utilities/__tests__/buildASTSchema-test.js @@ -89,6 +89,22 @@ describe('Schema Builder', () => { expect(output).to.equal(body); }); + it('With repeatable directives', () => { + const body = dedent` + directive @foo(arg: Int) repeatable on FIELD + + type Query { + str: String + } + `; + + const output = cycleOutput(body); + expect(output).to.equal(body); + + const schema = buildASTSchema(parse(body)); + expect(schema.getDirective('foo').repeatable).to.equal(true); + }); + it('Supports descriptions', () => { const body = dedent` """This is a directive""" diff --git a/src/utilities/__tests__/buildClientSchema-test.js b/src/utilities/__tests__/buildClientSchema-test.js index 6a93a96646..b99fbef525 100644 --- a/src/utilities/__tests__/buildClientSchema-test.js +++ b/src/utilities/__tests__/buildClientSchema-test.js @@ -587,6 +587,12 @@ describe('Type System: build schema from introspection', () => { description: 'This is a custom directive', locations: ['FIELD'], }), + new GraphQLDirective({ + name: 'customRepeatableDirective', + description: 'This is a custom repeatable directive', + repeatable: true, + locations: ['FIELD'], + }), ], }); diff --git a/src/utilities/__tests__/schemaPrinter-test.js b/src/utilities/__tests__/schemaPrinter-test.js index 66c8baabf5..5adf10ee65 100644 --- a/src/utilities/__tests__/schemaPrinter-test.js +++ b/src/utilities/__tests__/schemaPrinter-test.js @@ -648,6 +648,9 @@ describe('Type System Printer', () => { description: String locations: [__DirectiveLocation!]! args: [__InputValue!]! + + """Permits using the directive multiple times at the same location.""" + repeatable: Boolean! onOperation: Boolean! @deprecated(reason: "Use \`locations\`.") onFragment: Boolean! @deprecated(reason: "Use \`locations\`.") onField: Boolean! @deprecated(reason: "Use \`locations\`.") @@ -881,6 +884,9 @@ describe('Type System Printer', () => { description: String locations: [__DirectiveLocation!]! args: [__InputValue!]! + + # Permits using the directive multiple times at the same location. + repeatable: Boolean! onOperation: Boolean! @deprecated(reason: "Use \`locations\`.") onFragment: Boolean! @deprecated(reason: "Use \`locations\`.") onField: Boolean! @deprecated(reason: "Use \`locations\`.") diff --git a/src/utilities/buildASTSchema.js b/src/utilities/buildASTSchema.js index 21dc318eb4..87a24d1672 100644 --- a/src/utilities/buildASTSchema.js +++ b/src/utilities/buildASTSchema.js @@ -296,6 +296,7 @@ export class ASTDefinitionBuilder { args: directiveNode.arguments && this._makeInputValues(directiveNode.arguments), + repeatable: directiveNode.repeatable, astNode: directiveNode, }); } diff --git a/src/utilities/buildClientSchema.js b/src/utilities/buildClientSchema.js index 3dddb07d50..de80ab944c 100644 --- a/src/utilities/buildClientSchema.js +++ b/src/utilities/buildClientSchema.js @@ -373,6 +373,7 @@ export function buildClientSchema( description: directiveIntrospection.description, locations, args: buildInputValueDefMap(directiveIntrospection.args), + repeatable: directiveIntrospection.repeatable, }); } diff --git a/src/utilities/introspectionQuery.js b/src/utilities/introspectionQuery.js index 9c67dbd0e4..ad64dae7d4 100644 --- a/src/utilities/introspectionQuery.js +++ b/src/utilities/introspectionQuery.js @@ -33,6 +33,7 @@ export function getIntrospectionQuery(options?: IntrospectionOptions): string { args { ...InputValue } + repeatable } } } @@ -268,4 +269,5 @@ export type IntrospectionDirective = {| +description?: ?string, +locations: $ReadOnlyArray, +args: $ReadOnlyArray, + +repeatable: boolean, |}; diff --git a/src/utilities/schemaPrinter.js b/src/utilities/schemaPrinter.js index a630f27d23..86802bba32 100644 --- a/src/utilities/schemaPrinter.js +++ b/src/utilities/schemaPrinter.js @@ -299,6 +299,7 @@ function printDirective(directive, options) { 'directive @' + directive.name + printArgs(options, directive.args) + + (directive.repeatable ? ' repeatable' : '') + ' on ' + directive.locations.join(' | ') ); diff --git a/src/validation/__tests__/UniqueDirectivesPerLocation-test.js b/src/validation/__tests__/UniqueDirectivesPerLocation-test.js index d4e86fbcf7..d31e57c4b1 100644 --- a/src/validation/__tests__/UniqueDirectivesPerLocation-test.js +++ b/src/validation/__tests__/UniqueDirectivesPerLocation-test.js @@ -77,6 +77,32 @@ describe('Validate: Directives Are Unique Per Location', () => { ); }); + it('repeatable directives in same location', () => { + expectPassesRule( + UniqueDirectivesPerLocation, + ` + type Test @repeatableDirective(id: 1) @repeatableDirective(id: 2) { + field: String! + } + `, + ); + }); + + it('repeatable directives in similar locations', () => { + expectPassesRule( + UniqueDirectivesPerLocation, + ` + type Test @repeatableDirective(id: 1) @repeatableDirective(id: 2) { + field: String! + } + + extend type Test @repeatableDirective(id: 3) { + anotherField: String! + } + `, + ); + }); + it('duplicate directives in one location', () => { expectFailsRule( UniqueDirectivesPerLocation, diff --git a/src/validation/__tests__/harness.js b/src/validation/__tests__/harness.js index 6ef9725db3..d0309ced96 100644 --- a/src/validation/__tests__/harness.js +++ b/src/validation/__tests__/harness.js @@ -404,6 +404,17 @@ export const testSchema = new GraphQLSchema({ name: 'onInputFieldDefinition', locations: ['INPUT_FIELD_DEFINITION'], }), + new GraphQLDirective({ + name: 'repeatableDirective', + args: { + id: { + type: GraphQLNonNull(GraphQLInt), + description: 'Some generic ID.', + }, + }, + repeatable: true, + locations: ['OBJECT'], + }), ], }); diff --git a/src/validation/rules/KnownDirectives.js b/src/validation/rules/KnownDirectives.js index 61ce36e48a..97ec75af8e 100644 --- a/src/validation/rules/KnownDirectives.js +++ b/src/validation/rules/KnownDirectives.js @@ -9,7 +9,6 @@ import type { ValidationContext } from '../index'; import { GraphQLError } from '../../error'; -import find from '../../jsutils/find'; import { Kind } from '../../language/kinds'; import { DirectiveLocation } from '../../language/directiveLocation'; import type { ASTVisitor } from '../../language/visitor'; @@ -34,10 +33,7 @@ export function misplacedDirectiveMessage( export function KnownDirectives(context: ValidationContext): ASTVisitor { return { Directive(node, key, parent, path, ancestors) { - const directiveDef = find( - context.getSchema().getDirectives(), - def => def.name === node.name.value, - ); + const directiveDef = context.getDirectiveByName(node.name.value); if (!directiveDef) { context.reportError( new GraphQLError(unknownDirectiveMessage(node.name.value), [node]), diff --git a/src/validation/rules/UniqueDirectivesPerLocation.js b/src/validation/rules/UniqueDirectivesPerLocation.js index 3bc6fe18f4..17d73c7dcc 100644 --- a/src/validation/rules/UniqueDirectivesPerLocation.js +++ b/src/validation/rules/UniqueDirectivesPerLocation.js @@ -48,7 +48,11 @@ export function UniqueDirectivesPerLocation( ]), ); } else { - knownDirectives[directiveName] = directive; + const directiveDef = context.getDirectiveByName(directiveName); + + if (!directiveDef || !directiveDef.repeatable) { + knownDirectives[directiveName] = directive; + } } }); } diff --git a/src/validation/validate.js b/src/validation/validate.js index 09a7494c8d..e57dc4d80a 100644 --- a/src/validation/validate.js +++ b/src/validation/validate.js @@ -110,6 +110,7 @@ export class ValidationContext { OperationDefinitionNode, $ReadOnlyArray, >; + _directivesByName: ?Map; constructor( schema: GraphQLSchema, @@ -124,6 +125,7 @@ export class ValidationContext { this._recursivelyReferencedFragments = new Map(); this._variableUsages = new Map(); this._recursiveVariableUsages = new Map(); + this._directivesByName = null; } reportError(error: GraphQLError): void { @@ -271,6 +273,20 @@ export class ValidationContext { return this._typeInfo.getDirective(); } + getDirectiveByName(name: string): ?GraphQLDirective { + if (this._directivesByName === null) { + this._directivesByName = new Map(); + + this._schema.getDirectives().forEach(directive => { + if (this._directivesByName) { + this._directivesByName.set(directive.name, directive); + } + }); + } + + return this._directivesByName ? this._directivesByName.get(name) : null; + } + getArgument(): ?GraphQLArgument { return this._typeInfo.getArgument(); } From 7eb1fd7e7ebfa756681d4ec6f7dfb3f36736468c Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Wed, 3 Oct 2018 01:51:20 +0200 Subject: [PATCH 02/13] Updated with the most recent changes --- src/language/__tests__/schema-parser-test.js | 14 +++++---- src/language/printer.js | 2 +- src/type/__tests__/introspection-test.js | 18 +++++++++++ src/utilities/__tests__/schemaPrinter-test.js | 2 +- .../UniqueDirectivesPerLocation-test.js | 4 +-- .../rules/UniqueDirectivesPerLocation.js | 30 ++++++++++++++++--- 6 files changed, 56 insertions(+), 14 deletions(-) diff --git a/src/language/__tests__/schema-parser-test.js b/src/language/__tests__/schema-parser-test.js index 57440c4306..96beac4114 100644 --- a/src/language/__tests__/schema-parser-test.js +++ b/src/language/__tests__/schema-parser-test.js @@ -807,11 +807,13 @@ input Hello { it('Directive definition', () => { const body = 'directive @foo on OBJECT | INTERFACE'; const doc = parse(body); - const expected = { + + expect(toJSONDeep(doc)).to.deep.equal({ kind: 'Document', definitions: [ { kind: 'DirectiveDefinition', + description: undefined, name: { kind: 'Name', value: 'foo', @@ -850,18 +852,19 @@ input Hello { start: 0, end: 36, }, - }; - expect(printJson(doc)).to.equal(printJson(expected)); + }); }); it('Repeatable directive definition', () => { const body = 'directive @foo repeatable on OBJECT | INTERFACE'; const doc = parse(body); - const expected = { + + expect(toJSONDeep(doc)).to.deep.equal({ kind: 'Document', definitions: [ { kind: 'DirectiveDefinition', + description: undefined, name: { kind: 'Name', value: 'foo', @@ -900,8 +903,7 @@ input Hello { start: 0, end: 47, }, - }; - expect(printJson(doc)).to.equal(printJson(expected)); + }); }); it('Directive with incorrect locations', () => { diff --git a/src/language/printer.js b/src/language/printer.js index 8f99fae4e6..f9d7cadd26 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -181,7 +181,7 @@ const printDocASTReducer = { ), DirectiveDefinition: addDescription( - ({ name, arguments: args, locations }) => + ({ name, arguments: args, locations, repeatable }) => 'directive @' + name + (args.every(arg => arg.indexOf('\n') === -1) diff --git a/src/type/__tests__/introspection-test.js b/src/type/__tests__/introspection-test.js index 532a19db3e..cca8111e0c 100644 --- a/src/type/__tests__/introspection-test.js +++ b/src/type/__tests__/introspection-test.js @@ -700,6 +700,21 @@ describe('Introspection', () => { isDeprecated: false, deprecationReason: null, }, + { + "args": [], + "deprecationReason": null, + "isDeprecated": false, + "name": "repeatable", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null, + }, + }, + }, ], inputFields: null, interfaces: [], @@ -815,6 +830,7 @@ describe('Introspection', () => { directives: [ { name: 'include', + repeatable: false, locations: ['FIELD', 'FRAGMENT_SPREAD', 'INLINE_FRAGMENT'], args: [ { @@ -834,6 +850,7 @@ describe('Introspection', () => { }, { name: 'skip', + repeatable: false, locations: ['FIELD', 'FRAGMENT_SPREAD', 'INLINE_FRAGMENT'], args: [ { @@ -853,6 +870,7 @@ describe('Introspection', () => { }, { name: 'deprecated', + repeatable: false, locations: ['FIELD_DEFINITION', 'ENUM_VALUE'], args: [ { diff --git a/src/utilities/__tests__/schemaPrinter-test.js b/src/utilities/__tests__/schemaPrinter-test.js index e30a95c5e7..08fc91f940 100644 --- a/src/utilities/__tests__/schemaPrinter-test.js +++ b/src/utilities/__tests__/schemaPrinter-test.js @@ -650,7 +650,7 @@ describe('Type System Printer', () => { description: String locations: [__DirectiveLocation!]! args: [__InputValue!]! - + """Permits using the directive multiple times at the same location.""" repeatable: Boolean! } diff --git a/src/validation/__tests__/UniqueDirectivesPerLocation-test.js b/src/validation/__tests__/UniqueDirectivesPerLocation-test.js index fdbe404741..aea3b4ba2a 100644 --- a/src/validation/__tests__/UniqueDirectivesPerLocation-test.js +++ b/src/validation/__tests__/UniqueDirectivesPerLocation-test.js @@ -103,11 +103,11 @@ describe('Validate: Directives Are Unique Per Location', () => { expectPassesRule( UniqueDirectivesPerLocation, ` - type Test @repeatableDirective(id: 1) @repeatableDirective(id: 2) { + type Test @repeatableDirective(id: 1) { field: String! } - extend type Test @repeatableDirective(id: 3) { + extend type Test @repeatableDirective(id: 2) { anotherField: String! } `, diff --git a/src/validation/rules/UniqueDirectivesPerLocation.js b/src/validation/rules/UniqueDirectivesPerLocation.js index b74b810210..1a7932cb9c 100644 --- a/src/validation/rules/UniqueDirectivesPerLocation.js +++ b/src/validation/rules/UniqueDirectivesPerLocation.js @@ -7,10 +7,15 @@ * @flow strict */ -import type { ASTValidationContext } from '../ValidationContext'; +import type { + SDLValidationContext, + ValidationContext, +} from '../ValidationContext'; import { GraphQLError } from '../../error/GraphQLError'; import type { DirectiveNode } from '../../language/ast'; import type { ASTVisitor } from '../../language/visitor'; +import { Kind } from '../../language/kinds'; +import { specifiedDirectives } from '../../type/directives'; export function duplicateDirectiveMessage(directiveName: string): string { return ( @@ -26,8 +31,25 @@ export function duplicateDirectiveMessage(directiveName: string): string { * are uniquely named. */ export function UniqueDirectivesPerLocation( - context: ASTValidationContext, + context: ValidationContext | SDLValidationContext, ): ASTVisitor { + const repeatableMap = Object.create(null); + + const schema = context.getSchema(); + const definedDirectives = schema + ? schema.getDirectives() + : specifiedDirectives; + for (const directive of definedDirectives) { + repeatableMap[directive.name] = directive.repeatable; + } + + const astDefinitions = context.getDocument().definitions; + for (const def of astDefinitions) { + if (def.kind === Kind.DIRECTIVE_DEFINITION) { + repeatableMap[def.name.value] = def.repeatable; + } + } + return { // Many different AST nodes may contain directives. Rather than listing // them all, just listen for entering any node, and check to see if it @@ -48,9 +70,9 @@ export function UniqueDirectivesPerLocation( ]), ); } else { - const directiveDef = context.getDirectiveByName(directiveName); + const repeatable = repeatableMap[directiveName]; - if (!directiveDef || !directiveDef.repeatable) { + if (repeatable === undefined || !repeatable) { knownDirectives[directiveName] = directive; } } From 8cd0e52f31ac9713513bc275d8c44ffff1b6acf5 Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Wed, 3 Oct 2018 01:53:49 +0200 Subject: [PATCH 03/13] Adjusted json formatting --- src/type/__tests__/introspection-test.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/type/__tests__/introspection-test.js b/src/type/__tests__/introspection-test.js index cca8111e0c..0d352741ee 100644 --- a/src/type/__tests__/introspection-test.js +++ b/src/type/__tests__/introspection-test.js @@ -701,17 +701,17 @@ describe('Introspection', () => { deprecationReason: null, }, { - "args": [], - "deprecationReason": null, - "isDeprecated": false, - "name": "repeatable", - "type": { - "kind": "NON_NULL", - "name": null, - "ofType": { - "kind": "SCALAR", - "name": "Boolean", - "ofType": null, + args: [], + deprecationReason: null, + isDeprecated: false, + name: 'repeatable', + type: { + kind: 'NON_NULL', + name: null, + ofType: { + kind: 'SCALAR', + name: 'Boolean', + ofType: null, }, }, }, From 77f85b390a250c34ee703057cb7ee766ad51e6e4 Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Thu, 4 Oct 2018 13:51:32 +0200 Subject: [PATCH 04/13] Skip unknown directives + other improvements based on the review comments --- src/type/directives.js | 6 +- .../UniqueDirectivesPerLocation-test.js | 72 ++++++++++--------- src/validation/__tests__/harness.js | 8 +++ .../rules/UniqueDirectivesPerLocation.js | 12 ++-- 4 files changed, 57 insertions(+), 41 deletions(-) diff --git a/src/type/directives.js b/src/type/directives.js index 3d908c4ccc..f24603fda3 100644 --- a/src/type/directives.js +++ b/src/type/directives.js @@ -51,10 +51,8 @@ export class GraphQLDirective { this.description = config.description; this.locations = config.locations; this.astNode = config.astNode; - this.repeatable = - config.repeatable === undefined || config.repeatable === null - ? false - : config.repeatable; + this.repeatable = config.repeatable == null ? false : config.repeatable; + invariant(config.name, 'Directive must be named.'); invariant( Array.isArray(config.locations), diff --git a/src/validation/__tests__/UniqueDirectivesPerLocation-test.js b/src/validation/__tests__/UniqueDirectivesPerLocation-test.js index aea3b4ba2a..50f9205e35 100644 --- a/src/validation/__tests__/UniqueDirectivesPerLocation-test.js +++ b/src/validation/__tests__/UniqueDirectivesPerLocation-test.js @@ -47,8 +47,8 @@ describe('Validate: Directives Are Unique Per Location', () => { expectPassesRule( UniqueDirectivesPerLocation, ` - fragment Test on Type @directiveA { - field @directiveB + fragment Test on Type @genericDirectiveA { + field @genericDirectiveB } `, ); @@ -58,8 +58,8 @@ describe('Validate: Directives Are Unique Per Location', () => { expectPassesRule( UniqueDirectivesPerLocation, ` - fragment Test on Type @directiveA @directiveB { - field @directiveA @directiveB + fragment Test on Type @genericDirectiveA @genericDirectiveB { + field @genericDirectiveA @genericDirectiveB } `, ); @@ -69,8 +69,8 @@ describe('Validate: Directives Are Unique Per Location', () => { expectPassesRule( UniqueDirectivesPerLocation, ` - fragment Test on Type @directiveA { - field @directiveA + fragment Test on Type @genericDirectiveA { + field @genericDirectiveA } `, ); @@ -81,8 +81,8 @@ describe('Validate: Directives Are Unique Per Location', () => { UniqueDirectivesPerLocation, ` fragment Test on Type { - field @directive - field @directive + field @genericDirectiveA + field @genericDirectiveA } `, ); @@ -119,10 +119,10 @@ describe('Validate: Directives Are Unique Per Location', () => { UniqueDirectivesPerLocation, ` fragment Test on Type { - field @directive @directive + field @genericDirectiveA @genericDirectiveA } `, - [duplicateDirective('directive', 3, 15, 3, 26)], + [duplicateDirective('genericDirectiveA', 3, 15, 3, 34)], ); }); @@ -131,12 +131,12 @@ describe('Validate: Directives Are Unique Per Location', () => { UniqueDirectivesPerLocation, ` fragment Test on Type { - field @directive @directive @directive + field @genericDirectiveA @genericDirectiveA @genericDirectiveA } `, [ - duplicateDirective('directive', 3, 15, 3, 26), - duplicateDirective('directive', 3, 15, 3, 37), + duplicateDirective('genericDirectiveA', 3, 15, 3, 34), + duplicateDirective('genericDirectiveA', 3, 15, 3, 53), ], ); }); @@ -146,12 +146,12 @@ describe('Validate: Directives Are Unique Per Location', () => { UniqueDirectivesPerLocation, ` fragment Test on Type { - field @directiveA @directiveB @directiveA @directiveB + field @genericDirectiveA @genericDirectiveB @genericDirectiveA @genericDirectiveB } `, [ - duplicateDirective('directiveA', 3, 15, 3, 39), - duplicateDirective('directiveB', 3, 27, 3, 51), + duplicateDirective('genericDirectiveA', 3, 15, 3, 53), + duplicateDirective('genericDirectiveB', 3, 34, 3, 72), ], ); }); @@ -160,19 +160,27 @@ describe('Validate: Directives Are Unique Per Location', () => { expectFailsRule( UniqueDirectivesPerLocation, ` - fragment Test on Type @directive @directive { - field @directive @directive + fragment Test on Type @genericDirectiveA @genericDirectiveA { + field @genericDirectiveA @genericDirectiveA } `, [ - duplicateDirective('directive', 2, 29, 2, 40), - duplicateDirective('directive', 3, 15, 3, 26), + duplicateDirective('genericDirectiveA', 2, 29, 2, 48), + duplicateDirective('genericDirectiveA', 3, 15, 3, 34), ], ); }); it('duplicate directives on SDL definitions', () => { expectSDLErrors(` + directive @directive on + | SCHEMA + | SCALAR + | OBJECT + | INTERFACE + | UNION + | INPUT_OBJECT + schema @directive @directive { query: Dummy } extend schema @directive @directive @@ -191,18 +199,18 @@ describe('Validate: Directives Are Unique Per Location', () => { input TestInput @directive @directive extend input TestInput @directive @directive `).to.deep.equal([ - duplicateDirective('directive', 2, 14, 2, 25), - duplicateDirective('directive', 3, 21, 3, 32), - duplicateDirective('directive', 5, 25, 5, 36), - duplicateDirective('directive', 6, 32, 6, 43), - duplicateDirective('directive', 8, 23, 8, 34), - duplicateDirective('directive', 9, 30, 9, 41), - duplicateDirective('directive', 11, 31, 11, 42), - duplicateDirective('directive', 12, 38, 12, 49), - duplicateDirective('directive', 14, 23, 14, 34), - duplicateDirective('directive', 15, 30, 15, 41), - duplicateDirective('directive', 17, 23, 17, 34), - duplicateDirective('directive', 18, 30, 18, 41), + duplicateDirective('directive', 10, 14, 10, 25), + duplicateDirective('directive', 11, 21, 11, 32), + duplicateDirective('directive', 13, 25, 13, 36), + duplicateDirective('directive', 14, 32, 14, 43), + duplicateDirective('directive', 16, 23, 16, 34), + duplicateDirective('directive', 17, 30, 17, 41), + duplicateDirective('directive', 19, 31, 19, 42), + duplicateDirective('directive', 20, 38, 20, 49), + duplicateDirective('directive', 22, 23, 22, 34), + duplicateDirective('directive', 23, 30, 23, 41), + duplicateDirective('directive', 25, 23, 25, 34), + duplicateDirective('directive', 26, 30, 26, 41), ]); }); }); diff --git a/src/validation/__tests__/harness.js b/src/validation/__tests__/harness.js index d28afdafcd..2b4c4a6f14 100644 --- a/src/validation/__tests__/harness.js +++ b/src/validation/__tests__/harness.js @@ -380,6 +380,14 @@ export const testSchema = new GraphQLSchema({ name: 'onVariableDefinition', locations: ['VARIABLE_DEFINITION'], }), + new GraphQLDirective({ + name: 'genericDirectiveA', + locations: ['FIELD', 'FRAGMENT_DEFINITION'], + }), + new GraphQLDirective({ + name: 'genericDirectiveB', + locations: ['FIELD', 'FRAGMENT_DEFINITION'], + }), new GraphQLDirective({ name: 'repeatableDirective', args: { diff --git a/src/validation/rules/UniqueDirectivesPerLocation.js b/src/validation/rules/UniqueDirectivesPerLocation.js index 1a7932cb9c..103bb9d10b 100644 --- a/src/validation/rules/UniqueDirectivesPerLocation.js +++ b/src/validation/rules/UniqueDirectivesPerLocation.js @@ -62,6 +62,12 @@ export function UniqueDirectivesPerLocation( const knownDirectives = Object.create(null); for (const directive of directives) { const directiveName = directive.name.value; + const repeatable = repeatableMap[directiveName]; + + if (repeatable === undefined || repeatable) { + continue; + } + if (knownDirectives[directiveName]) { context.reportError( new GraphQLError(duplicateDirectiveMessage(directiveName), [ @@ -70,11 +76,7 @@ export function UniqueDirectivesPerLocation( ]), ); } else { - const repeatable = repeatableMap[directiveName]; - - if (repeatable === undefined || !repeatable) { - knownDirectives[directiveName] = directive; - } + knownDirectives[directiveName] = directive; } } } From 20deb26d975001370ea80db1ae23b0bd039bad53 Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Thu, 4 Oct 2018 13:57:37 +0200 Subject: [PATCH 05/13] Test for unknown directives --- .../__tests__/UniqueDirectivesPerLocation-test.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/validation/__tests__/UniqueDirectivesPerLocation-test.js b/src/validation/__tests__/UniqueDirectivesPerLocation-test.js index 50f9205e35..d1b8cd00cc 100644 --- a/src/validation/__tests__/UniqueDirectivesPerLocation-test.js +++ b/src/validation/__tests__/UniqueDirectivesPerLocation-test.js @@ -114,6 +114,21 @@ describe('Validate: Directives Are Unique Per Location', () => { ); }); + it('unknown directives must be ignored', () => { + expectPassesRule( + UniqueDirectivesPerLocation, + ` + type Test @unknownDirective @unknownDirective { + field: String! + } + + extend type Test @unknownDirective { + anotherField: String! + } + `, + ); + }); + it('duplicate directives in one location', () => { expectFailsRule( UniqueDirectivesPerLocation, From 1180d929919f71deb91fb97a4ce3629f8c579b6b Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Thu, 4 Oct 2018 14:06:40 +0200 Subject: [PATCH 06/13] Added support for repeatable directives in `extendSchema` --- src/utilities/__tests__/extendSchema-test.js | 11 ++++++++++- src/utilities/extendSchema.js | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/utilities/__tests__/extendSchema-test.js b/src/utilities/__tests__/extendSchema-test.js index e9bfa43fe7..eb13748b7a 100644 --- a/src/utilities/__tests__/extendSchema-test.js +++ b/src/utilities/__tests__/extendSchema-test.js @@ -115,6 +115,15 @@ const FooDirective = new GraphQLDirective({ ], }); +const RepeatableDirective = new GraphQLDirective({ + name: 'repeatableDirective', + args: { + input: { type: SomeInputType }, + }, + repeatable: true, + locations: [DirectiveLocation.OBJECT, DirectiveLocation.INTERFACE], +}); + const testSchema = new GraphQLSchema({ query: new GraphQLObjectType({ name: 'Query', @@ -134,7 +143,7 @@ const testSchema = new GraphQLSchema({ }), }), types: [FooType, BarType], - directives: specifiedDirectives.concat([FooDirective]), + directives: specifiedDirectives.concat([FooDirective, RepeatableDirective]), }); function extendTestSchema(sdl, options) { diff --git a/src/utilities/extendSchema.js b/src/utilities/extendSchema.js index 62b184a5be..21ce29973c 100644 --- a/src/utilities/extendSchema.js +++ b/src/utilities/extendSchema.js @@ -320,6 +320,7 @@ export function extendSchema( locations: directive.locations, args: extendArgs(directive.args), astNode: directive.astNode, + repeatable: directive.repeatable, }); } From f8378d0a6f75b69447e251ad305bc006640d091e Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Thu, 4 Oct 2018 14:28:32 +0200 Subject: [PATCH 07/13] Added `DIRECTIVE_REPEATABLE_REMOVED` breaking change --- .../__tests__/findBreakingChanges-test.js | 18 +++++++++++++ src/utilities/findBreakingChanges.js | 27 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/utilities/__tests__/findBreakingChanges-test.js b/src/utilities/__tests__/findBreakingChanges-test.js index 32d969a193..066a3f4344 100644 --- a/src/utilities/__tests__/findBreakingChanges-test.js +++ b/src/utilities/__tests__/findBreakingChanges-test.js @@ -35,6 +35,7 @@ import { findAddedNonNullDirectiveArgs, findRemovedLocationsForDirective, findRemovedDirectiveLocations, + findRemovedDirectiveRepeatable, } from '../findBreakingChanges'; import { @@ -1007,6 +1008,23 @@ describe('findBreakingChanges', () => { }, ]); }); + + it('should detect removal of repeatable flag', () => { + const oldSchema = buildSchema(` + directive @foo repeatable on OBJECT + `); + + const newSchema = buildSchema(` + directive @foo on OBJECT + `); + + expect(findRemovedDirectiveRepeatable(oldSchema, newSchema)).to.eql([ + { + type: BreakingChangeType.DIRECTIVE_REPEATABLE_REMOVED, + description: 'Repeatable flag was removed from foo', + }, + ]); + }); }); describe('findDangerousChanges', () => { diff --git a/src/utilities/findBreakingChanges.js b/src/utilities/findBreakingChanges.js index 9f7962138f..6a77981a4b 100644 --- a/src/utilities/findBreakingChanges.js +++ b/src/utilities/findBreakingChanges.js @@ -50,6 +50,7 @@ export const BreakingChangeType = { DIRECTIVE_REMOVED: 'DIRECTIVE_REMOVED', DIRECTIVE_ARG_REMOVED: 'DIRECTIVE_ARG_REMOVED', DIRECTIVE_LOCATION_REMOVED: 'DIRECTIVE_LOCATION_REMOVED', + DIRECTIVE_REPEATABLE_REMOVED: 'DIRECTIVE_REPEATABLE_REMOVED', REQUIRED_DIRECTIVE_ARG_ADDED: 'REQUIRED_DIRECTIVE_ARG_ADDED', }; @@ -94,6 +95,7 @@ export function findBreakingChanges( ...findRemovedDirectiveArgs(oldSchema, newSchema), ...findAddedNonNullDirectiveArgs(oldSchema, newSchema), ...findRemovedDirectiveLocations(oldSchema, newSchema), + ...findRemovedDirectiveRepeatable(oldSchema, newSchema), ]; } @@ -840,6 +842,31 @@ export function findRemovedDirectiveLocations( return removedLocations; } +export function findRemovedDirectiveRepeatable( + oldSchema: GraphQLSchema, + newSchema: GraphQLSchema, +): Array { + const removedRepeatable = []; + const oldSchemaDirectiveMap = getDirectiveMapForSchema(oldSchema); + + for (const newDirective of newSchema.getDirectives()) { + const oldDirective = oldSchemaDirectiveMap[newDirective.name]; + + if (!oldDirective) { + continue; + } + + if (oldDirective.repeatable && !newDirective.repeatable) { + removedRepeatable.push({ + type: BreakingChangeType.DIRECTIVE_REPEATABLE_REMOVED, + description: `Repeatable flag was removed from ${newDirective.name}`, + }); + } + } + + return removedRepeatable; +} + function getDirectiveMapForSchema( schema: GraphQLSchema, ): ObjMap { From 3b5bf03609b2e92184f511fe7230365628a53182 Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Thu, 4 Oct 2018 17:50:04 +0200 Subject: [PATCH 08/13] Skip `repeatable` flag by default in the introspection. --- src/type/__tests__/introspection-test.js | 78 +++++++++++++++++++++++- src/utilities/introspectionQuery.js | 7 ++- 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/type/__tests__/introspection-test.js b/src/type/__tests__/introspection-test.js index 0d352741ee..41e0c81417 100644 --- a/src/type/__tests__/introspection-test.js +++ b/src/type/__tests__/introspection-test.js @@ -830,7 +830,6 @@ describe('Introspection', () => { directives: [ { name: 'include', - repeatable: false, locations: ['FIELD', 'FRAGMENT_SPREAD', 'INLINE_FRAGMENT'], args: [ { @@ -850,7 +849,6 @@ describe('Introspection', () => { }, { name: 'skip', - repeatable: false, locations: ['FIELD', 'FRAGMENT_SPREAD', 'INLINE_FRAGMENT'], args: [ { @@ -870,7 +868,6 @@ describe('Introspection', () => { }, { name: 'deprecated', - repeatable: false, locations: ['FIELD_DEFINITION', 'ENUM_VALUE'], args: [ { @@ -890,6 +887,81 @@ describe('Introspection', () => { }); }); + it('includes repeatable flag on directives', () => { + const EmptySchema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'QueryRoot', + fields: { + onlyField: { type: GraphQLString }, + }, + }), + }); + const query = getIntrospectionQuery({ + descriptions: false, + directiveRepeatableFlag: true, + }); + const result = graphqlSync(EmptySchema, query); + + expect((result.data: any).__schema.directives).to.deep.equal([ + { + name: 'include', + locations: ['FIELD', 'FRAGMENT_SPREAD', 'INLINE_FRAGMENT'], + args: [ + { + name: 'if', + type: { + kind: 'NON_NULL', + name: null, + ofType: { + kind: 'SCALAR', + name: 'Boolean', + ofType: null, + }, + }, + defaultValue: null, + }, + ], + repeatable: false, + }, + { + name: 'skip', + locations: ['FIELD', 'FRAGMENT_SPREAD', 'INLINE_FRAGMENT'], + args: [ + { + name: 'if', + type: { + kind: 'NON_NULL', + name: null, + ofType: { + kind: 'SCALAR', + name: 'Boolean', + ofType: null, + }, + }, + defaultValue: null, + }, + ], + repeatable: false, + }, + { + name: 'deprecated', + locations: ['FIELD_DEFINITION', 'ENUM_VALUE'], + args: [ + { + name: 'reason', + type: { + kind: 'SCALAR', + name: 'String', + ofType: null, + }, + defaultValue: '"No longer supported"', + }, + ], + repeatable: false, + }, + ]); + }); + it('introspects on input object', () => { const TestInputObject = new GraphQLInputObjectType({ name: 'TestInputObject', diff --git a/src/utilities/introspectionQuery.js b/src/utilities/introspectionQuery.js index 3cc2ab3aeb..6b7e67d5f9 100644 --- a/src/utilities/introspectionQuery.js +++ b/src/utilities/introspectionQuery.js @@ -13,10 +13,15 @@ export type IntrospectionOptions = {| // Whether to include descriptions in the introspection result. // Default: true descriptions: boolean, + // Whether to include `repeatable` flag on directives. + // Default: false + directiveRepeatableFlag?: ?boolean, |}; export function getIntrospectionQuery(options?: IntrospectionOptions): string { const descriptions = !(options && options.descriptions === false); + const directiveRepeatableFlag = + options && options.directiveRepeatableFlag === true; return ` query IntrospectionQuery { __schema { @@ -33,7 +38,7 @@ export function getIntrospectionQuery(options?: IntrospectionOptions): string { args { ...InputValue } - repeatable + ${directiveRepeatableFlag ? 'repeatable' : ''} } } } From 3a7fafac79e35384c0c40f888c3abc0a6d9d9679 Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Thu, 4 Oct 2018 18:07:02 +0200 Subject: [PATCH 09/13] `repeatable` flag in the introspection results is now optional --- src/utilities/__tests__/buildClientSchema-test.js | 14 ++++++++++++-- src/utilities/buildClientSchema.js | 5 ++++- src/utilities/introspectionQuery.js | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/utilities/__tests__/buildClientSchema-test.js b/src/utilities/__tests__/buildClientSchema-test.js index 0572b7dca8..5429d5f8d7 100644 --- a/src/utilities/__tests__/buildClientSchema-test.js +++ b/src/utilities/__tests__/buildClientSchema-test.js @@ -37,9 +37,19 @@ import { // query against the client-side schema, it should get a result identical to // what was returned by the server. function testSchema(serverSchema) { - const initialIntrospection = introspectionFromSchema(serverSchema); + const introspectionOptions = { + descriptions: true, + directiveRepeatableFlag: true, + }; + const initialIntrospection = introspectionFromSchema( + serverSchema, + introspectionOptions, + ); const clientSchema = buildClientSchema(initialIntrospection); - const secondIntrospection = introspectionFromSchema(clientSchema); + const secondIntrospection = introspectionFromSchema( + clientSchema, + introspectionOptions, + ); expect(secondIntrospection).to.deep.equal(initialIntrospection); } diff --git a/src/utilities/buildClientSchema.js b/src/utilities/buildClientSchema.js index ea3ddae3ee..d1da2eb013 100644 --- a/src/utilities/buildClientSchema.js +++ b/src/utilities/buildClientSchema.js @@ -355,7 +355,10 @@ export function buildClientSchema( description: directiveIntrospection.description, locations: directiveIntrospection.locations.slice(), args: buildInputValueDefMap(directiveIntrospection.args), - repeatable: directiveIntrospection.repeatable, + repeatable: + directiveIntrospection.repeatable === undefined + ? false + : directiveIntrospection.repeatable, }); } diff --git a/src/utilities/introspectionQuery.js b/src/utilities/introspectionQuery.js index 6b7e67d5f9..330f241947 100644 --- a/src/utilities/introspectionQuery.js +++ b/src/utilities/introspectionQuery.js @@ -279,5 +279,5 @@ export type IntrospectionDirective = {| +description?: ?string, +locations: $ReadOnlyArray, +args: $ReadOnlyArray, - +repeatable: boolean, + +repeatable?: boolean, |}; From ae9e7f98f73a069096523575de6869d85707c09f Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Thu, 4 Oct 2018 22:37:39 +0200 Subject: [PATCH 10/13] Use `uniqueDirectiveMap` instead of `repeatableMap` --- .../rules/UniqueDirectivesPerLocation.js | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/validation/rules/UniqueDirectivesPerLocation.js b/src/validation/rules/UniqueDirectivesPerLocation.js index 103bb9d10b..fb2782bc7a 100644 --- a/src/validation/rules/UniqueDirectivesPerLocation.js +++ b/src/validation/rules/UniqueDirectivesPerLocation.js @@ -33,20 +33,20 @@ export function duplicateDirectiveMessage(directiveName: string): string { export function UniqueDirectivesPerLocation( context: ValidationContext | SDLValidationContext, ): ASTVisitor { - const repeatableMap = Object.create(null); + const uniqueDirectiveMap = Object.create(null); const schema = context.getSchema(); const definedDirectives = schema ? schema.getDirectives() : specifiedDirectives; for (const directive of definedDirectives) { - repeatableMap[directive.name] = directive.repeatable; + uniqueDirectiveMap[directive.name] = !directive.repeatable; } const astDefinitions = context.getDocument().definitions; for (const def of astDefinitions) { if (def.kind === Kind.DIRECTIVE_DEFINITION) { - repeatableMap[def.name.value] = def.repeatable; + uniqueDirectiveMap[def.name.value] = !def.repeatable; } } @@ -62,21 +62,18 @@ export function UniqueDirectivesPerLocation( const knownDirectives = Object.create(null); for (const directive of directives) { const directiveName = directive.name.value; - const repeatable = repeatableMap[directiveName]; - if (repeatable === undefined || repeatable) { - continue; - } - - if (knownDirectives[directiveName]) { - context.reportError( - new GraphQLError(duplicateDirectiveMessage(directiveName), [ - knownDirectives[directiveName], - directive, - ]), - ); - } else { - knownDirectives[directiveName] = directive; + if (uniqueDirectiveMap[directiveName]) { + if (knownDirectives[directiveName]) { + context.reportError( + new GraphQLError(duplicateDirectiveMessage(directiveName), [ + knownDirectives[directiveName], + directive, + ]), + ); + } else { + knownDirectives[directiveName] = directive; + } } } } From 56fee7d28b68d44665007e175129839af11ae8d2 Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Thu, 4 Oct 2018 22:56:46 +0200 Subject: [PATCH 11/13] =?UTF-8?q?Rename=20`repeatable`=20=E2=86=92=20`isRe?= =?UTF-8?q?peatable`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/language/__tests__/schema-parser-test.js | 4 ++-- src/language/ast.js | 2 +- src/language/parser.js | 2 +- src/language/printer.js | 4 ++-- src/type/__tests__/introspection-test.js | 8 ++++---- src/type/directives.js | 7 ++++--- src/type/introspection.js | 4 ++-- src/utilities/__tests__/buildASTSchema-test.js | 2 +- src/utilities/__tests__/buildClientSchema-test.js | 2 +- src/utilities/__tests__/extendSchema-test.js | 2 +- src/utilities/__tests__/schemaPrinter-test.js | 4 ++-- src/utilities/buildASTSchema.js | 2 +- src/utilities/buildClientSchema.js | 6 +++--- src/utilities/extendSchema.js | 2 +- src/utilities/findBreakingChanges.js | 2 +- src/utilities/introspectionQuery.js | 6 +++--- src/utilities/schemaPrinter.js | 2 +- src/validation/__tests__/harness.js | 2 +- src/validation/rules/UniqueDirectivesPerLocation.js | 4 ++-- 19 files changed, 34 insertions(+), 33 deletions(-) diff --git a/src/language/__tests__/schema-parser-test.js b/src/language/__tests__/schema-parser-test.js index 96beac4114..029b3411d2 100644 --- a/src/language/__tests__/schema-parser-test.js +++ b/src/language/__tests__/schema-parser-test.js @@ -823,7 +823,7 @@ input Hello { }, }, arguments: [], - repeatable: false, + isRepeatable: false, locations: [ { kind: 'Name', @@ -874,7 +874,7 @@ input Hello { }, }, arguments: [], - repeatable: true, + isRepeatable: true, locations: [ { kind: 'Name', diff --git a/src/language/ast.js b/src/language/ast.js index 8dee33c8a3..4b9906bb9a 100644 --- a/src/language/ast.js +++ b/src/language/ast.js @@ -508,7 +508,7 @@ export type DirectiveDefinitionNode = { +description?: StringValueNode, +name: NameNode, +arguments?: $ReadOnlyArray, - +repeatable: boolean, + +isRepeatable: boolean, +locations: $ReadOnlyArray, }; diff --git a/src/language/parser.js b/src/language/parser.js index ddc728b941..7b6d621bf4 100644 --- a/src/language/parser.js +++ b/src/language/parser.js @@ -1395,7 +1395,7 @@ function parseDirectiveDefinition(lexer: Lexer<*>): DirectiveDefinitionNode { description, name, arguments: args, - repeatable, + isRepeatable: repeatable, locations, loc: loc(lexer, start), }; diff --git a/src/language/printer.js b/src/language/printer.js index f9d7cadd26..e72a9d67f7 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -181,13 +181,13 @@ const printDocASTReducer = { ), DirectiveDefinition: addDescription( - ({ name, arguments: args, locations, repeatable }) => + ({ name, arguments: args, locations, isRepeatable }) => 'directive @' + name + (args.every(arg => arg.indexOf('\n') === -1) ? wrap('(', join(args, ', '), ')') : wrap('(\n', indent(join(args, '\n')), '\n)')) + - (repeatable ? ' repeatable' : '') + + (isRepeatable ? ' repeatable' : '') + ' on ' + join(locations, ' | '), ), diff --git a/src/type/__tests__/introspection-test.js b/src/type/__tests__/introspection-test.js index 41e0c81417..2dc82784f4 100644 --- a/src/type/__tests__/introspection-test.js +++ b/src/type/__tests__/introspection-test.js @@ -704,7 +704,7 @@ describe('Introspection', () => { args: [], deprecationReason: null, isDeprecated: false, - name: 'repeatable', + name: 'isRepeatable', type: { kind: 'NON_NULL', name: null, @@ -921,7 +921,7 @@ describe('Introspection', () => { defaultValue: null, }, ], - repeatable: false, + isRepeatable: false, }, { name: 'skip', @@ -941,7 +941,7 @@ describe('Introspection', () => { defaultValue: null, }, ], - repeatable: false, + isRepeatable: false, }, { name: 'deprecated', @@ -957,7 +957,7 @@ describe('Introspection', () => { defaultValue: '"No longer supported"', }, ], - repeatable: false, + isRepeatable: false, }, ]); }); diff --git a/src/type/directives.js b/src/type/directives.js index f24603fda3..4c9c5d68fd 100644 --- a/src/type/directives.js +++ b/src/type/directives.js @@ -44,14 +44,15 @@ export class GraphQLDirective { locations: Array; args: Array; astNode: ?DirectiveDefinitionNode; - repeatable: boolean; + isRepeatable: boolean; constructor(config: GraphQLDirectiveConfig): void { this.name = config.name; this.description = config.description; this.locations = config.locations; this.astNode = config.astNode; - this.repeatable = config.repeatable == null ? false : config.repeatable; + this.isRepeatable = + config.isRepeatable == null ? false : config.isRepeatable; invariant(config.name, 'Directive must be named.'); invariant( @@ -95,7 +96,7 @@ export type GraphQLDirectiveConfig = {| locations: Array, args?: ?GraphQLFieldConfigArgumentMap, astNode?: ?DirectiveDefinitionNode, - repeatable?: ?boolean, + isRepeatable?: ?boolean, |}; /** diff --git a/src/type/introspection.js b/src/type/introspection.js index f8cf49f075..8493470992 100644 --- a/src/type/introspection.js +++ b/src/type/introspection.js @@ -98,11 +98,11 @@ export const __Directive = new GraphQLObjectType({ type: GraphQLNonNull(GraphQLList(GraphQLNonNull(__InputValue))), resolve: directive => directive.args || [], }, - repeatable: { + isRepeatable: { type: GraphQLNonNull(GraphQLBoolean), description: 'Permits using the directive multiple times at the same location.', - resolve: directive => directive.repeatable, + resolve: directive => directive.isRepeatable, }, }), }); diff --git a/src/utilities/__tests__/buildASTSchema-test.js b/src/utilities/__tests__/buildASTSchema-test.js index 7b5b43809f..3f526228d2 100644 --- a/src/utilities/__tests__/buildASTSchema-test.js +++ b/src/utilities/__tests__/buildASTSchema-test.js @@ -104,7 +104,7 @@ describe('Schema Builder', () => { expect(output).to.equal(body); const schema = buildASTSchema(parse(body)); - expect(schema.getDirective('foo').repeatable).to.equal(true); + expect(schema.getDirective('foo').isRepeatable).to.equal(true); }); it('Supports descriptions', () => { diff --git a/src/utilities/__tests__/buildClientSchema-test.js b/src/utilities/__tests__/buildClientSchema-test.js index 5429d5f8d7..91db2c8767 100644 --- a/src/utilities/__tests__/buildClientSchema-test.js +++ b/src/utilities/__tests__/buildClientSchema-test.js @@ -596,7 +596,7 @@ describe('Type System: build schema from introspection', () => { new GraphQLDirective({ name: 'customRepeatableDirective', description: 'This is a custom repeatable directive', - repeatable: true, + isRepeatable: true, locations: ['FIELD'], }), ], diff --git a/src/utilities/__tests__/extendSchema-test.js b/src/utilities/__tests__/extendSchema-test.js index eb13748b7a..97b9021467 100644 --- a/src/utilities/__tests__/extendSchema-test.js +++ b/src/utilities/__tests__/extendSchema-test.js @@ -120,7 +120,7 @@ const RepeatableDirective = new GraphQLDirective({ args: { input: { type: SomeInputType }, }, - repeatable: true, + isRepeatable: true, locations: [DirectiveLocation.OBJECT, DirectiveLocation.INTERFACE], }); diff --git a/src/utilities/__tests__/schemaPrinter-test.js b/src/utilities/__tests__/schemaPrinter-test.js index 08fc91f940..419f990972 100644 --- a/src/utilities/__tests__/schemaPrinter-test.js +++ b/src/utilities/__tests__/schemaPrinter-test.js @@ -652,7 +652,7 @@ describe('Type System Printer', () => { args: [__InputValue!]! """Permits using the directive multiple times at the same location.""" - repeatable: Boolean! + isRepeatable: Boolean! } """ @@ -888,7 +888,7 @@ describe('Type System Printer', () => { args: [__InputValue!]! # Permits using the directive multiple times at the same location. - repeatable: Boolean! + isRepeatable: Boolean! } # A Directive can be adjacent to many parts of the GraphQL language, a diff --git a/src/utilities/buildASTSchema.js b/src/utilities/buildASTSchema.js index 3354198db6..f3dfcc57fb 100644 --- a/src/utilities/buildASTSchema.js +++ b/src/utilities/buildASTSchema.js @@ -282,7 +282,7 @@ export class ASTDefinitionBuilder { args: directiveNode.arguments && this._makeInputValues(directiveNode.arguments), - repeatable: directiveNode.repeatable, + isRepeatable: directiveNode.isRepeatable, astNode: directiveNode, }); } diff --git a/src/utilities/buildClientSchema.js b/src/utilities/buildClientSchema.js index d1da2eb013..3f267a185f 100644 --- a/src/utilities/buildClientSchema.js +++ b/src/utilities/buildClientSchema.js @@ -355,10 +355,10 @@ export function buildClientSchema( description: directiveIntrospection.description, locations: directiveIntrospection.locations.slice(), args: buildInputValueDefMap(directiveIntrospection.args), - repeatable: - directiveIntrospection.repeatable === undefined + isRepeatable: + directiveIntrospection.isRepeatable === undefined ? false - : directiveIntrospection.repeatable, + : directiveIntrospection.isRepeatable, }); } diff --git a/src/utilities/extendSchema.js b/src/utilities/extendSchema.js index 21ce29973c..ca6338332b 100644 --- a/src/utilities/extendSchema.js +++ b/src/utilities/extendSchema.js @@ -320,7 +320,7 @@ export function extendSchema( locations: directive.locations, args: extendArgs(directive.args), astNode: directive.astNode, - repeatable: directive.repeatable, + isRepeatable: directive.isRepeatable, }); } diff --git a/src/utilities/findBreakingChanges.js b/src/utilities/findBreakingChanges.js index 6a77981a4b..36669cd224 100644 --- a/src/utilities/findBreakingChanges.js +++ b/src/utilities/findBreakingChanges.js @@ -856,7 +856,7 @@ export function findRemovedDirectiveRepeatable( continue; } - if (oldDirective.repeatable && !newDirective.repeatable) { + if (oldDirective.isRepeatable && !newDirective.isRepeatable) { removedRepeatable.push({ type: BreakingChangeType.DIRECTIVE_REPEATABLE_REMOVED, description: `Repeatable flag was removed from ${newDirective.name}`, diff --git a/src/utilities/introspectionQuery.js b/src/utilities/introspectionQuery.js index 330f241947..f3ed594d1a 100644 --- a/src/utilities/introspectionQuery.js +++ b/src/utilities/introspectionQuery.js @@ -13,7 +13,7 @@ export type IntrospectionOptions = {| // Whether to include descriptions in the introspection result. // Default: true descriptions: boolean, - // Whether to include `repeatable` flag on directives. + // Whether to include `isRepeatable` flag on directives. // Default: false directiveRepeatableFlag?: ?boolean, |}; @@ -38,7 +38,7 @@ export function getIntrospectionQuery(options?: IntrospectionOptions): string { args { ...InputValue } - ${directiveRepeatableFlag ? 'repeatable' : ''} + ${directiveRepeatableFlag ? 'isRepeatable' : ''} } } } @@ -279,5 +279,5 @@ export type IntrospectionDirective = {| +description?: ?string, +locations: $ReadOnlyArray, +args: $ReadOnlyArray, - +repeatable?: boolean, + +isRepeatable?: boolean, |}; diff --git a/src/utilities/schemaPrinter.js b/src/utilities/schemaPrinter.js index f6572a7283..15ee31fe65 100644 --- a/src/utilities/schemaPrinter.js +++ b/src/utilities/schemaPrinter.js @@ -309,7 +309,7 @@ function printDirective(directive, options) { 'directive @' + directive.name + printArgs(options, directive.args) + - (directive.repeatable ? ' repeatable' : '') + + (directive.isRepeatable ? ' repeatable' : '') + ' on ' + directive.locations.join(' | ') ); diff --git a/src/validation/__tests__/harness.js b/src/validation/__tests__/harness.js index 2b4c4a6f14..c5ce1b5221 100644 --- a/src/validation/__tests__/harness.js +++ b/src/validation/__tests__/harness.js @@ -396,7 +396,7 @@ export const testSchema = new GraphQLSchema({ description: 'Some generic ID.', }, }, - repeatable: true, + isRepeatable: true, locations: ['OBJECT'], }), ], diff --git a/src/validation/rules/UniqueDirectivesPerLocation.js b/src/validation/rules/UniqueDirectivesPerLocation.js index fb2782bc7a..a4eef87bc6 100644 --- a/src/validation/rules/UniqueDirectivesPerLocation.js +++ b/src/validation/rules/UniqueDirectivesPerLocation.js @@ -40,13 +40,13 @@ export function UniqueDirectivesPerLocation( ? schema.getDirectives() : specifiedDirectives; for (const directive of definedDirectives) { - uniqueDirectiveMap[directive.name] = !directive.repeatable; + uniqueDirectiveMap[directive.name] = !directive.isRepeatable; } const astDefinitions = context.getDocument().definitions; for (const def of astDefinitions) { if (def.kind === Kind.DIRECTIVE_DEFINITION) { - uniqueDirectiveMap[def.name.value] = !def.repeatable; + uniqueDirectiveMap[def.name.value] = !def.isRepeatable; } } From 0dff21fb3508191cc7efbaeff1d2e72135dd9809 Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Thu, 4 Oct 2018 23:18:41 +0200 Subject: [PATCH 12/13] Updated `UniqueDirectivesPerLocation` validation docs --- src/validation/rules/UniqueDirectivesPerLocation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validation/rules/UniqueDirectivesPerLocation.js b/src/validation/rules/UniqueDirectivesPerLocation.js index a4eef87bc6..6d7cb59608 100644 --- a/src/validation/rules/UniqueDirectivesPerLocation.js +++ b/src/validation/rules/UniqueDirectivesPerLocation.js @@ -27,7 +27,7 @@ export function duplicateDirectiveMessage(directiveName: string): string { /** * Unique directive names per location * - * A GraphQL document is only valid if all directives at a given location + * A GraphQL document is only valid if directives, that are not marked as `isRepeatable`, at a given location * are uniquely named. */ export function UniqueDirectivesPerLocation( From 89f5780fa4d5f55ab4028d2136ba7fa14b63bd2c Mon Sep 17 00:00:00 2001 From: Oleg Ilyenko Date: Wed, 7 Nov 2018 14:04:28 +0100 Subject: [PATCH 13/13] Use name `repeatable` instead of `isRepeatable` in all places except introspection --- src/language/__tests__/schema-parser-test.js | 4 ++-- src/language/ast.js | 2 +- src/language/parser.js | 2 +- src/language/printer.js | 4 ++-- src/type/directives.js | 7 +++---- src/type/introspection.js | 2 +- src/utilities/__tests__/buildASTSchema-test.js | 2 +- src/utilities/__tests__/buildClientSchema-test.js | 2 +- src/utilities/__tests__/extendSchema-test.js | 2 +- src/utilities/buildASTSchema.js | 2 +- src/utilities/buildClientSchema.js | 2 +- src/utilities/extendSchema.js | 2 +- src/utilities/findBreakingChanges.js | 2 +- src/utilities/schemaPrinter.js | 2 +- src/validation/__tests__/harness.js | 2 +- src/validation/rules/UniqueDirectivesPerLocation.js | 4 ++-- 16 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/language/__tests__/schema-parser-test.js b/src/language/__tests__/schema-parser-test.js index 488815c6f5..65abe8bc1c 100644 --- a/src/language/__tests__/schema-parser-test.js +++ b/src/language/__tests__/schema-parser-test.js @@ -833,7 +833,7 @@ input Hello { }, }, arguments: [], - isRepeatable: false, + repeatable: false, locations: [ { kind: 'Name', @@ -884,7 +884,7 @@ input Hello { }, }, arguments: [], - isRepeatable: true, + repeatable: true, locations: [ { kind: 'Name', diff --git a/src/language/ast.js b/src/language/ast.js index 4b9906bb9a..8dee33c8a3 100644 --- a/src/language/ast.js +++ b/src/language/ast.js @@ -508,7 +508,7 @@ export type DirectiveDefinitionNode = { +description?: StringValueNode, +name: NameNode, +arguments?: $ReadOnlyArray, - +isRepeatable: boolean, + +repeatable: boolean, +locations: $ReadOnlyArray, }; diff --git a/src/language/parser.js b/src/language/parser.js index 2ccce53767..c37043be57 100644 --- a/src/language/parser.js +++ b/src/language/parser.js @@ -1368,7 +1368,7 @@ function parseDirectiveDefinition(lexer: Lexer<*>): DirectiveDefinitionNode { description, name, arguments: args, - isRepeatable: repeatable, + repeatable, locations, loc: loc(lexer, start), }; diff --git a/src/language/printer.js b/src/language/printer.js index a570eb7644..2f6390ffeb 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -181,13 +181,13 @@ const printDocASTReducer = { ), DirectiveDefinition: addDescription( - ({ name, arguments: args, locations, isRepeatable }) => + ({ name, arguments: args, locations, repeatable }) => 'directive @' + name + (hasMultilineItems(args) ? wrap('(\n', indent(join(args, '\n')), '\n)') : wrap('(', join(args, ', '), ')')) + - (isRepeatable ? ' repeatable' : '') + + (repeatable ? ' repeatable' : '') + ' on ' + join(locations, ' | '), ), diff --git a/src/type/directives.js b/src/type/directives.js index 4c9c5d68fd..f24603fda3 100644 --- a/src/type/directives.js +++ b/src/type/directives.js @@ -44,15 +44,14 @@ export class GraphQLDirective { locations: Array; args: Array; astNode: ?DirectiveDefinitionNode; - isRepeatable: boolean; + repeatable: boolean; constructor(config: GraphQLDirectiveConfig): void { this.name = config.name; this.description = config.description; this.locations = config.locations; this.astNode = config.astNode; - this.isRepeatable = - config.isRepeatable == null ? false : config.isRepeatable; + this.repeatable = config.repeatable == null ? false : config.repeatable; invariant(config.name, 'Directive must be named.'); invariant( @@ -96,7 +95,7 @@ export type GraphQLDirectiveConfig = {| locations: Array, args?: ?GraphQLFieldConfigArgumentMap, astNode?: ?DirectiveDefinitionNode, - isRepeatable?: ?boolean, + repeatable?: ?boolean, |}; /** diff --git a/src/type/introspection.js b/src/type/introspection.js index 8493470992..6c60295f14 100644 --- a/src/type/introspection.js +++ b/src/type/introspection.js @@ -102,7 +102,7 @@ export const __Directive = new GraphQLObjectType({ type: GraphQLNonNull(GraphQLBoolean), description: 'Permits using the directive multiple times at the same location.', - resolve: directive => directive.isRepeatable, + resolve: directive => directive.repeatable, }, }), }); diff --git a/src/utilities/__tests__/buildASTSchema-test.js b/src/utilities/__tests__/buildASTSchema-test.js index 3f526228d2..7b5b43809f 100644 --- a/src/utilities/__tests__/buildASTSchema-test.js +++ b/src/utilities/__tests__/buildASTSchema-test.js @@ -104,7 +104,7 @@ describe('Schema Builder', () => { expect(output).to.equal(body); const schema = buildASTSchema(parse(body)); - expect(schema.getDirective('foo').isRepeatable).to.equal(true); + expect(schema.getDirective('foo').repeatable).to.equal(true); }); it('Supports descriptions', () => { diff --git a/src/utilities/__tests__/buildClientSchema-test.js b/src/utilities/__tests__/buildClientSchema-test.js index 91db2c8767..5429d5f8d7 100644 --- a/src/utilities/__tests__/buildClientSchema-test.js +++ b/src/utilities/__tests__/buildClientSchema-test.js @@ -596,7 +596,7 @@ describe('Type System: build schema from introspection', () => { new GraphQLDirective({ name: 'customRepeatableDirective', description: 'This is a custom repeatable directive', - isRepeatable: true, + repeatable: true, locations: ['FIELD'], }), ], diff --git a/src/utilities/__tests__/extendSchema-test.js b/src/utilities/__tests__/extendSchema-test.js index 97b9021467..eb13748b7a 100644 --- a/src/utilities/__tests__/extendSchema-test.js +++ b/src/utilities/__tests__/extendSchema-test.js @@ -120,7 +120,7 @@ const RepeatableDirective = new GraphQLDirective({ args: { input: { type: SomeInputType }, }, - isRepeatable: true, + repeatable: true, locations: [DirectiveLocation.OBJECT, DirectiveLocation.INTERFACE], }); diff --git a/src/utilities/buildASTSchema.js b/src/utilities/buildASTSchema.js index f3dfcc57fb..3354198db6 100644 --- a/src/utilities/buildASTSchema.js +++ b/src/utilities/buildASTSchema.js @@ -282,7 +282,7 @@ export class ASTDefinitionBuilder { args: directiveNode.arguments && this._makeInputValues(directiveNode.arguments), - isRepeatable: directiveNode.isRepeatable, + repeatable: directiveNode.repeatable, astNode: directiveNode, }); } diff --git a/src/utilities/buildClientSchema.js b/src/utilities/buildClientSchema.js index 3f267a185f..791c26518d 100644 --- a/src/utilities/buildClientSchema.js +++ b/src/utilities/buildClientSchema.js @@ -355,7 +355,7 @@ export function buildClientSchema( description: directiveIntrospection.description, locations: directiveIntrospection.locations.slice(), args: buildInputValueDefMap(directiveIntrospection.args), - isRepeatable: + repeatable: directiveIntrospection.isRepeatable === undefined ? false : directiveIntrospection.isRepeatable, diff --git a/src/utilities/extendSchema.js b/src/utilities/extendSchema.js index ca6338332b..21ce29973c 100644 --- a/src/utilities/extendSchema.js +++ b/src/utilities/extendSchema.js @@ -320,7 +320,7 @@ export function extendSchema( locations: directive.locations, args: extendArgs(directive.args), astNode: directive.astNode, - isRepeatable: directive.isRepeatable, + repeatable: directive.repeatable, }); } diff --git a/src/utilities/findBreakingChanges.js b/src/utilities/findBreakingChanges.js index 36669cd224..6a77981a4b 100644 --- a/src/utilities/findBreakingChanges.js +++ b/src/utilities/findBreakingChanges.js @@ -856,7 +856,7 @@ export function findRemovedDirectiveRepeatable( continue; } - if (oldDirective.isRepeatable && !newDirective.isRepeatable) { + if (oldDirective.repeatable && !newDirective.repeatable) { removedRepeatable.push({ type: BreakingChangeType.DIRECTIVE_REPEATABLE_REMOVED, description: `Repeatable flag was removed from ${newDirective.name}`, diff --git a/src/utilities/schemaPrinter.js b/src/utilities/schemaPrinter.js index 15ee31fe65..f6572a7283 100644 --- a/src/utilities/schemaPrinter.js +++ b/src/utilities/schemaPrinter.js @@ -309,7 +309,7 @@ function printDirective(directive, options) { 'directive @' + directive.name + printArgs(options, directive.args) + - (directive.isRepeatable ? ' repeatable' : '') + + (directive.repeatable ? ' repeatable' : '') + ' on ' + directive.locations.join(' | ') ); diff --git a/src/validation/__tests__/harness.js b/src/validation/__tests__/harness.js index c5ce1b5221..2b4c4a6f14 100644 --- a/src/validation/__tests__/harness.js +++ b/src/validation/__tests__/harness.js @@ -396,7 +396,7 @@ export const testSchema = new GraphQLSchema({ description: 'Some generic ID.', }, }, - isRepeatable: true, + repeatable: true, locations: ['OBJECT'], }), ], diff --git a/src/validation/rules/UniqueDirectivesPerLocation.js b/src/validation/rules/UniqueDirectivesPerLocation.js index 6d7cb59608..d3c85cb6e5 100644 --- a/src/validation/rules/UniqueDirectivesPerLocation.js +++ b/src/validation/rules/UniqueDirectivesPerLocation.js @@ -40,13 +40,13 @@ export function UniqueDirectivesPerLocation( ? schema.getDirectives() : specifiedDirectives; for (const directive of definedDirectives) { - uniqueDirectiveMap[directive.name] = !directive.isRepeatable; + uniqueDirectiveMap[directive.name] = !directive.repeatable; } const astDefinitions = context.getDocument().definitions; for (const def of astDefinitions) { if (def.kind === Kind.DIRECTIVE_DEFINITION) { - uniqueDirectiveMap[def.name.value] = !def.isRepeatable; + uniqueDirectiveMap[def.name.value] = !def.repeatable; } }