allow directives on directive definitions

[benhead]

This is the minimum viable edit to enable this feature. I imagine we'd also want to allow directive extensions if this feature is approved.

#566

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[benhead]

Original feature request:

We'd like to be able to declare directives on directive definitions.

We have tooling that uses type system directives to perform special processing on a schema, and we'd like users of that tooling to be able to define their own directives, with metadata indicating to the tooling that this directive is for its use. For example, if we could define directive @Handler(class:String!) on DIRECTIVE_DEFINITION then our users could define directive @CustomDirective(...) on FIELD_DEFINITION @Handler(class:"Foo") telling our tooling that their directive is meant to be handled by it, passing arguments we declare to our tooling, and still providing whatever arguments they declare to their own hooks.

That directives can't be declared on directive definitions seems like an oversight.

Please note that I'm a Googler and we're reviewing that we can sign the CLA. I don't anticipate a problem, but bear it in mind until we confirm.

[spawnia]

Looks good to me, clean and easy.

We already have the counter example that disallows self-referencing directive definitions, so we should be clear there.

[spawnia]

I would like to another use case that requires this change that might be a bit more common: Deprecation of directives.

directive @foo on OBJECT @deprecated

[benjie]

One thing to consider is that this position would prevent future deprecation of one of the positions:

directive @foo on
| QUERY
| MUTATION @deprecated(reason: "Doesn't work well with side effects")
| SUBSCRIPTION

(I don't think this is particularly important, but it's worth noting.)

[martinbonnin]

You mean because the syntax would become ambiguous, right? I think I'm okay with this (famous last words 😅 )

[dariuszkuc]

As mentioned in the composite schemas WG, directives on directives could also simplify the current usage of Apollo Federation's @composeDirective that is used to indicate that certain directives should be preserved in the composed schemas, i.e.

# with directives on directives we could use simplified syntax
# naming of the compose directive is TBD
directive @custom on FIELD_DEFINITION @composeDirective 

#### vs
# current compose directive logic requires explicit linking/importings on the schema element
extend schema @link(url: "http://fake.url/customSpec", import: ["@custom"])
@composeDirective(name: "@custom")

directive @custom on FIELD_DEFINITION

[BoD]

We would love this in Apollo Kotlin because we'd like to deprecate certain client-side directives.

@benhead I know this PR is almost 7 years old, but are you still interested in championing this change? I think this would be useful and shouldn’t be too controversial (famous last words 😅). From what I can tell, the PR also needs:

• Make @deprecated applicable to directive definitions
• Introspection: add isDeprecated/deprecationReason to __Directive, includeDepracated: Boolean = false to __Schema.directives, and DIRECTIVE_DEFINITION to __DirectiveLocation.
• Validation: probably some clarification in “5.7.1 Directives Are Defined” that a directive can be applied to its own definition
• Type system: add directive extensions (e.g. extend directive @foo @bar)

Then the next step is to implement it on graphql-js.

[BoD]

Woops I didn't notice there was actually a more recent PR (2021) from @IvanGoncharov with the same proposal and with more conversation. I think it's fair to say this PR should be abandoned and any follow-up conversation should happen at #907.

[martinbonnin]

Closing in favor of #1206, which reached RFC1 during yesterday's working group. Thanks @BoD for presenting!