diff --git a/docs/source/schema-directives.md b/docs/source/schema-directives.md index 954ab058bd3..e945bfd2a5d 100644 --- a/docs/source/schema-directives.md +++ b/docs/source/schema-directives.md @@ -116,26 +116,67 @@ Enforcing the requirements of the declaration is something a `SchemaDirectiveVis However, if you're attempting to implement a reusable `SchemaDirectiveVisitor`, you may not be the one writing the SDL syntax, so you may not have control over which directives the schema author decides to declare, and how. That's why a well-implemented, reusable `SchemaDirectiveVisitor` should consider overriding the `getDirectiveDeclaration` method: ```typescript +import { + DirectiveLocation, + GraphQLDirective, + GraphQLEnumType, +} from "graphql"; + class AuthDirectiveVisitor extends SchemaDirectiveVisitor { public visitObject(object: GraphQLObjectType) {...} public visitFieldDefinition(field: GraphQLField) {...} public static getDirectiveDeclaration( directiveName: string, - previousDirective?: GraphQLDirective, + schema: GraphQLSchema, ): GraphQLDirective { - previousDirective.args.forEach(arg => { - if (arg.name === 'requires') { - // Lower the default minimum Role from ADMIN to REVIEWER. - arg.defaultValue = 'REVIEWER'; - } + const previousDirective = schema.getDirective(directiveName); + if (previousDirective) { + // If a previous directive declaration exists in the schema, it may be + // better to modify it than to return a new GraphQLDirective object. + previousDirective.args.forEach(arg => { + if (arg.name === 'requires') { + // Lower the default minimum Role from ADMIN to REVIEWER. + arg.defaultValue = 'REVIEWER'; + } + }); + + return previousDirective; + } + + // If a previous directive with this name was not found in the schema, + // there are several options: + // + // 1. Construct a new GraphQLDirective (see below). + // 2. Throw an exception to force the client to declare the directive. + // 3. Return null, and forget about declaring this directive. + // + // All three are valid options, since the visitor will still work without + // any declared directives. In fact, unless you're publishing a directive + // implementation for public consumption, you can probably just ignore + // getDirectiveDeclaration altogether. + + return new GraphQLDirective({ + name: directiveName, + locations: [ + DirectiveLocation.OBJECT, + DirectiveLocation.FIELD_DEFINITION, + ], + args: { + requires: { + // Having the schema available here is important for obtaining + // references to existing type objects, such as the Role enum. + type: (schema.getType('Role') as GraphQLEnumType), + // Set the default minimum Role to REVIEWER. + defaultValue: 'REVIEWER', + } + }] }); - return previousDirective; } } ``` -Since the `getDirectiveDeclaration` method receives not only the name of the directive but also any previous declaration found in the schema, it can either return a totally new `GraphQLDirective` object, or simply modify the `previousDirective` and return it. Either way, if the visitor returns a non-null `GraphQLDirective` from `getDirectiveDeclaration`, that declaration will be used to check arguments and permissible locations. +Since the `getDirectiveDeclaration` method receives not only the name of the directive but also the `GraphQLSchema` object, it can modify and/or reuse previous declarations found in the schema, as an alternative to returning a totally new `GraphQLDirective` object. Either way, if the visitor returns a non-null `GraphQLDirective` from `getDirectiveDeclaration`, that declaration will be used to check arguments and permissible locations. ## Examples diff --git a/src/schemaVisitor.ts b/src/schemaVisitor.ts index 883cb300491..7c41869a70d 100644 --- a/src/schemaVisitor.ts +++ b/src/schemaVisitor.ts @@ -298,9 +298,9 @@ export class SchemaDirectiveVisitor extends SchemaVisitor { // appear. By default, any declaration found in the schema will be returned. public static getDirectiveDeclaration( directiveName: string, - previousDeclaration?: GraphQLDirective, + schema: GraphQLSchema, ): GraphQLDirective { - return previousDeclaration; + return schema.getDirective(directiveName); } // Call SchemaDirectiveVisitor.visitSchemaDirectives to visit every @@ -433,15 +433,7 @@ export class SchemaDirectiveVisitor extends SchemaVisitor { // goes to the trouble of implementing getDirectiveDeclaration, it should // be able to rely on that implementation. each(directiveVisitors, (visitorClass, directiveName) => { - const decl = visitorClass.getDirectiveDeclaration( - directiveName, - // Give the getDirectiveDeclaration method a chance to look at the - // existing declaration, in case it only needs to make minor tweaks. - // Ironically, this grants the SchemaDirectiveVisitor subclass the - // ability to visit/inspect/modify directive declarations found in - // the schema, even though they can't be decorated with @directives! - declaredDirectives[directiveName] || null, - ); + const decl = visitorClass.getDirectiveDeclaration(directiveName, schema); if (decl) { declaredDirectives[directiveName] = decl; } diff --git a/src/test/testDirectives.ts b/src/test/testDirectives.ts index 87ce7cb2f72..210590c3f33 100644 --- a/src/test/testDirectives.ts +++ b/src/test/testDirectives.ts @@ -18,7 +18,6 @@ import { GraphQLInputObjectType, GraphQLObjectType, GraphQLSchema, - GraphQLDirective, GraphQLString, GraphQLID, GraphQLScalarType, @@ -454,7 +453,10 @@ describe('@directives', () => { it('can also handle declared arguments', () => { const schemaText = ` - directive @oyez(times: Int = 5) on OBJECT | FIELD_DEFINITION + directive @oyez( + times: Int = 5, + party: Party = IMPARTIAL, + ) on OBJECT | FIELD_DEFINITION schema { query: Courtroom @@ -463,16 +465,12 @@ describe('@directives', () => { type Courtroom @oyez { judge: String @oyez(times: 0) marshall: String @oyez - lawyers( - # Should @oyez be disallowed here, since it wasn't declared with - # the ARGUMENT_DEFINITION location, or simply ignored? - party: Party @oyez(times: 0) - ): [String] } enum Party { DEFENSE PROSECUTION + IMPARTIAL }`; const schema = makeExecutableSchema({ typeDefs: schemaText }); @@ -485,8 +483,10 @@ describe('@directives', () => { oyez: class extends SchemaDirectiveVisitor { public static getDirectiveDeclaration( name: string, - prev: GraphQLDirective, + theSchema: GraphQLSchema, ) { + assert.strictEqual(theSchema, schema); + const prev = schema.getDirective(name); prev.args.some(arg => { if (arg.name === 'times') { // Override the default value of the times argument to be 3 @@ -510,6 +510,7 @@ describe('@directives', () => { } else if (field.name === 'marshall') { assert.strictEqual(this.args.times, 3); } + assert.strictEqual(this.args.party, 'IMPARTIAL'); } } }, context);