Add support for directives on directive definitions#1206
Conversation
✅ Deploy Preview for graphql-spec-draft ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Related graphql-js PR: graphql/graphql-js#4521 |
|
Another use case: |
leebyron
added
📄 Draft (RFC 2)
Allow directives on directive definitions, based on [this spec PR](graphql/graphql-spec#1206) which introduces this syntax: ```graphql directive @onDirective on DIRECTIVE_DEFINITION directive @foo @onDirective on OBJECT directive @baz @deprecated(reason: "...") on OBJECT extend directive @quux @deprecated(reason: "...") ``` Co-authored-by: Martin Bonnin <martin@mbonnin.net> Co-authored-by: Jerel Miller <jerelmiller@gmail.com> Co-authored-by: Yaacov Rydzinski <yaacovCR@gmail.com>
|
|
||
| ### Directive Extensions | ||
|
|
||
| DirectiveExtension : extend directive @ Name Directives[Const] |
There was a problem hiding this comment.
This is interesting: should we be able to extend a directive with new arguments? We can extend a type with new fields that themselves have new arguments.
There is even a special Schema Coordinate for directive arguments.
There was a problem hiding this comment.
Good question!
In my opinion, I think yes it would probably make sense in principle to have that ability, even if for consistency alone.
That being said, I would rather tackle this in a separate change later, and based on demand for it.
Curious to know what others think!
There was a problem hiding this comment.
I don't think it would be controversial, but it does make sense to stack into two PRs to make discussion specifically about argument extension more focused.
There was a problem hiding this comment.
@mjmahone Am I correct that https://gaps.graphql.org/GAP-33/draft/ would also solve this use case?
benjie
left a comment
benjie
left a comment
There was a problem hiding this comment.
Looks good to me, thanks for your work on this! I'd like to discuss includeDeprecated at a WG to determine if we might be able to address this e.g. via __schema(includeDeprecated: Boolean! = false) and then remove the need for this new argument, but otherwise I approve 👍
| mutationType: __Type | ||
| subscriptionType: __Type | ||
| directives: [__Directive!]! | ||
| directives(includeDeprecated: Boolean! = false): [__Directive!]! |
There was a problem hiding this comment.
I'm hesitant to add this, since I think the whole includeDeprecated dance was a mistake. I think I'd rather see the includeDeprecated argument be itself deprecated across the board, but it defaulting to = false makes this challenging.
7 participants
Adds the ability to apply directives to directive definitions and to deprecate directives, with this syntax:
With @IvanGoncharov's blessing, this PR is a continuation and updated version of #907 which I think can be closed. (There's also prior art in #567).
Motivation
Allow applying directive-exclusive features (e.g.
@specifyBy,@deprecated) on directives.For instance, on Apollo Kotlin we'd like to deprecate some client side directives.