Add support for directives on directives

[IvanGoncharov]

I suggest allowing to specify directives on directives with the following syntax:

directive @foo on DIRECTIVE_DEFINITION
directive @bar on FIELD @foo

Motivation

Allow applying directive-exclusive features (e.g. @specifyBy, @deprecate) on directives.

Validation

We already have a rule in place that prevent cycles in directives

A directive definition must not contain the use of a directive that references itself indirectly by referencing a Type or Directive which transitively includes a reference to this directive.

https://spec.graphql.org/draft/#sel-HAHnBTDDABAB_F_kC

So validation is already in place and already paid "complexity cost".

[netlify]

✔️ Deploy Preview for graphql-spec-draft ready!

🔨 Explore the source changes: 93eb694

🔍 Inspect the deploy log: https://app.netlify.com/sites/graphql-spec-draft/deploys/61a8ffa89a70010007283c1d

😎 Browse the preview: https://deploy-preview-907--graphql-spec-draft.netlify.app

[benjie]

A directive definition must not contain the use of a directive that references itself indirectly by referencing a Type or Directive which transitively includes a reference to this directive.

This text needs to be modified to cover direct reference to itself (directive @foo on DIRECTIVE_DEFINITION @foo). Maybe something like:

A directive definition must not contain the use of a directive that references itself directly, or indirectly by referencing a Type or Directive which transitively includes a reference to this directive.

[rivantsov]

why are we so scared of these recursive definitions? things like that happen in real progr languages, no trouble. Why all this word soup to prohibit smth like that - if there's potential trouble, it would be a choice of the server-side dev, he wants it for some reason - he would handle the consequences if any

[mjmahone]

why are we so scared of these recursive definitions? things like that happen in real progr languages, no trouble. Why all this word soup to prohibit smth like that - if there's potential trouble, it would be a choice of the server-side dev, he wants it for some reason - he would handle the consequences if any

GraphQL is supposed to be non-Turing Complete, to provide a guarantee to the server that, outside of the code used by Resolvers, the program itself will halt. The requirement for directives on directives to not form a cycle may or may not be required to keep GraphQL from becoming Turing Complete, but whenever we are adding new constructs to the language, by default we usually try to avoid recursion in the first pass to reduce Turing Completeness considerations from the RFC.

[rivantsov]

Turing?! complete?! what are you talking about?! it is not a programming language. Guarantee to the server?!!! I am a server-side programmer, and I can tell you nobody other than myself, no spec can guarantee anything about my server code, will it complete, crash or loop forever. Spec can't guarantee anything, especially about server implementation code, period. What Spec can do is introduce unnecessary, hard to understand restrictions that leave with one question - why is this word soup there at all
If server can implement recursive directives - let it do it. If it can't - it wont.

[yaacovCR]

GraphQL is supposed to be non-Turing Complete, to provide a guarantee to the server that, outside of the code used by Resolvers, the program itself will halt.

This is really interesting? Is there a reference in the spec or other originating material documenting this?

In the spec, I just see a statement I read as noting that GraphQL is not a programming language capable of arbitrary computation, but nothing per se that we have to avoid recursion. I see the point you are raising in terms of guarantees of completion when resolvers are properly implemented, but perhaps not every recursion application instance would prohibit this.

Maybe it would? I guess I am wondering if the specifiedBy directive could recursively apply to itself, for example, without non-halting concerns…

[yaacovCR]

I guess the question is: considering some recursive directive references could be nonparsable, nonsensical, etc, etc, should all such references be disallowed?

Considering that even some non recursive directive references have the same problem and require separate validation, I am not sure that recursion itself has to be forbidden.

maybe it should for now for simplicity, of course! That would be my preference, until a compelling use case presents itself

[leebyron]

@benjie

This text needs to be modified to cover direct reference to itself

That seems to already exist, see https://spec.graphql.org/draft/#sel-EAHnBTEBAAAEDDA_FtwX

@IvanGoncharov

I think putting directives at the end of the list of locations isn't consistent with similar syntax of Union types

union Example @directive =
  | TypeA
  | TypeB
  | TypeC

compare to:

directive @directive(arg: Type) on
  | Field
  | Fragment @deprecated

It also reads like the deprecation is applying to the Fragment location instead of the directive itself.

Putting it after the argument list would be more consistent with directives on FieldDefinitions:

directive @directive(arg: Type) @deprecated on
  | Field
  | Fragment

In the case where there are no args and repeatable:

directive @directive @deprecated repeatable on
  | Field
  | Fragment

Either way, I still don't love this syntax. The repeated @name is hard to read, but I dislike other alternatives more

[fotoetienne]

re syntax: Would it be crazy to generally allow directives to be used within docstrings? For directive definitions, this could then be the only way to apply a directive in order to keep the syntax clean.

[n1ru4l]

What are potential use-cases for directives that transform the actual behavior of an existing directive?

In the WG we mentioned @deprecated and @specifiedBy. Both of those mainly enrich the parts on which they are specified with additional information that is available via introspection.

While discussing this I noticed a lot of similarities to JSDOC @deprecated and @link.

What if those "metadata enrichment directives" could instead be part of the field/argument/directive description string?

directive @deprecated(reason: String) on TYPE_DESCRIPTION, FIELD_DESCRIPTION, ARGUMENT_DESCRIPTION

type User {
  """
  This is the id
  """
  id: ID!
  """
  This is the name
  @deprecated(reason: "This is deprecated.")
  """
  name: String!
}

"""
@deprecated(reason: "This is deprecated.")
"""
directive @bar on FIELD

Edit: @fotoetienne was faster than me

[queerviolet]

could we consider allowing d-o-ds to apply to individual locations?

a use case is when a directive is @deprecated in one location, but not in another:

directive @explode(arg: Type) @specifiedBy(url: X) on
  | FIELD 
  | FRAGMENT @deprecated

the @explode directive here:

• is @specifiedBy(url: X) in all locations
• its use on FRAGMENTs is @deprecated

[leoloso]

What are potential use-cases for directives that transform the actual behavior of an existing directive?

I have a couple of directives that modify the behavior of the contiguous directive(s) in the query/SDL:

• @forEach: It iterates the items in the field value (whenever it is a List), and passes each of them to the next directive
• @advancePointerInArrayOrObject: Given an associative array or object in the field value, it places the pointer into a certain path, and the next directive is applied on that element

These are useful to apply a directive that does not expect a List. For instance, let's say that @titleCase only handles an input of type String:

directive @titleCase on FIELD | FIELD_DEFINITION

type Query {
  siteName: String
    @titleCase
}

Then it can receive [String] by placing @forEach before it:

type Query {
  categoryNames: [String] 
    @forEach 
      @titleCase
}

In the query:

query {
  categoryNames
    @forEach 
      @titleCase
}

These directives can also nest each other, as many times as needed. For instance, if the input is [[String]], it can do this:

query {
  customerAllNames
    @forEach 
      @forEach 
        @titleCase
}

I use @advancePointerInArrayOrObject mainly to modify the response from an external source that I got via REST. For instance, I have field getJSON that returns a JSONObject. Let's say some REST endpoint returns this:

[
  {
    "name": "Leo",
    "email": "leo@ponga.com"
  },
  {
    "name": "Manuel",
    "email": "manuel@ponga.com"
  }
]

I can then apply @titleCase only on the "name" entry of each result, like this:

query {
  userData: getJSON(url: "https://...")
    @forEach 
      @advancePointerInArrayOrObject(path: "name") 
        @titleCase
}

These directives have an affect param to indicate which are the directives whose behavior must be modified, then they can modify more than one. This param is an [Int] indicating the relative position(s) to the modified directive(s):

type Query {
  categoryNames: [String] 
    @forEach(affect: [1, 2])
      @titleCase
      @translate(from: "en", to: "fr")
}

By default affect has value [1], that's why in the previous examples, where a directive modifies the directive to its right, affect needs not be provided.

Btw, I just published a more detailed article explaining all this, here.

[n1ru4l]

Every time I see an example such as this:

type Post {
  categories: [String] @forEach @titleCase
}

directive @forEach on FIELD_DEFINITION
directive @titleCaseon FIELD_DEFINITION

I immediately think that the following might be the better solution:

enum StringFormat {
  titleCase
  upperCase
  snakeCase
}

type Post {
  categories(format: StringFormat): [String]
}

My main reasons:

@titleCase could be applied on any field as directives don't have an "on applied field type" constraint.

The following cannot be caught by GraphQL validation:

type Post {
  a: Int @titleCase
}

[leoloso]

@n1ru4l I acknowledge that not everybody likes this solution, but I don't think it makes the use case invalid.

There are examples of things we can do in different ways in GraphQL, which can work better for different applications, and they are all valid. For instance:

• pagination via cursor connections or not
• modeling errors via unions of types or simple descriptions

In what context would @forEach @titleClase work better? As to simplify the resolvers (whether field resolver or directive resolver), so that they don't need to apply different logic if the input is [[String]], [String] or String; they only handle String.

My concern is to not have to update the resolvers whenever the client has a novel type input, such as [[[String]]], and not have to think all possibilities in advance (and have to code all of them, bloating the code). Because of this, for my own API, I find @forEach @titleClase more suitable.

Even if you prefer an alternative solution, don't you think that @forEach @titleClase is valid? Shouldn't it be considered an actual (not only potential, since I'm already using it in my server) use case for directives that transform the actual behavior of an existing directive?

[rivantsov]

I think we got two threads mixed up here. The original post suggests allowing directives on directive definitions. That seems logical and unambiguous to me.

The later post by @leoloso (with @foreach and @titleCase) suggests directives on directive instances in queries. This is quite different I think. My biggest problem with this: having

query {
  customerAllNames
      @forEach 
        @titleCase
}

the idea is that in this case titleCase is applied to forEach? and why not both directives apply to customerAllNames field?! what's the rule to distinguish the case that the second directive is applied to previous directive, not the field?

[leoloso]

@rivantsov You have a point! The question was about directives modifying the behavior of other directives, which happens with @forEach, but it works by being applied on the same field, so it must be defined in FIELD | FIELD_DEFINITION. Now that you mention, I agree we are talking about two separate issues (sorry for the mix-up).

Here is an idea to implement a similar concept using DIRECTIVE_DEFINITION (I'm not really sure this makes a lot of sense, though, but I still bring it out to explore ideas for this issue):

A directive @composeDirectives could create a new directive from combining the behavior of other directives. The magic would be that the SDL would be enough, so the new directive (@forEachTitleCase) would be created without any resolver code:

directive @titleCase on FIELD | FIELD_DEFINITION

# It converts an input of type `[String]` into an array of `String`, passes each downstream to the composed directive, and finally recreates the original array
directive @forEach on FIELD | FIELD_DEFINITION

# Create a new directive from composing 2 or more directives
directive @composeDirectives($directiveNames: [String!]!) on DIRECTIVE_DEFINITION

# Create a new directive combining the behavior of other directives, just through SDL
directive @forEachTitleCase on FIELD | FIELD_DEFINITION
    @composeDirectives(directiveNames: ["forEach", "titleCase"])

(This solution will not work for my server, though, since it's code-first, so I need to stick to using @forEach in the query)