[RFC] Add support for directives for an object field name

[francisu]

Custom directives have been widely used to increase the power of GraphQL, however they are missing from object field names.

NOTE: Such directives are also missing from argument names as well, but we don't have a pressing use case for that. However, I think for uniformity and extensibility, they should be allowed in both object fields and arguments, but that's a topic for another PR (though I'm happy to included here if people want it).

Back to object fields.

It would be useful to indicate a different mutation behavior. For example, in a mutation whose purpose is to update an object, we can can consider the default behavior to update only the objects and fields specified in the mutation.

There is a pattern in some GraphQL implementations of automatically generating generic CRUD queries and mutations in the schema, where the actual definitions are declared outside of the GraphQL schema.

In this example, we have a generic updateCustomer mutation (automatically generated in the schema) which allows arbitrary changes to the customer.

input CustomerInput {
  id: String
  address: AddressInput
}

input AddressInput {
  street1: String
  apartment: String
  city: String
  state: String
  zipCode: String
}

If we wanted to change the zip code, without changing anything else:

mutation { 
  updateCustomer( input: { id: "custId"
    address: { 
      zipCode: "94610-2233" 
    } 
  } ) { id }
}

However, if we wanted to replace the entire address, we could use a custom directive @replace which would replace the entire contents of the object with what was specified. So in this case, if the customer previously had an apartment, that would not be present after this mutation:

mutation { 
  updateCustomer( input: { id: "custId"
    @replace address: { 
      street1: "123 Main St"
      city: "Oakland"
      state: "CA"
      zipCode: "94610"
    } 
  } ) { id }
}

Of course you could argue that simply providing null values for all of the other fields would accomplish the same thing, but this is tedious and error-prone. And in fact the client may not know all of the possible values in the object.

Note also, contrary to the other locations directives are used, the directive would appear before the name rather than after. This is to make the syntax with the use of the ":" separating the name and value more clean.

If this is accepted, I would be happy to do the work of championing the process to implementation, including any required work in the reference implementations.

[benjie]

And in fact the client may not know all of the possible values in the object.

This, particularly for static queries, is the really important point here. If a new nullable input field is added later the client won't know it also needs to reset this, and previously valid operations may now have unexpected consequences due to not resetting the new fields.

That said, I think I'd model this differently in GraphQL. To patch the address I'd use something like patchAddress:

mutation { 
  updateCustomer( input: { id: "custId"
    patchAddress: { 
      zipCode: "94610"
    } 
  } ) { id }
}

to replace the address, something like replaceAddress:

mutation { 
  updateCustomer( input: { id: "custId"
    replaceAddress: { 
      street1: "123 Main St"
      city: "Oakland"
      state: "CA"
      zipCode: "94610"
    } 
  } ) { id }
}

This is much more explicit, it doesn't require spec edits, it's discoverable and documentable using the introspection features already baked into GraphQL/GraphiQL, and it doesn't require any arcane directive usage.

(In my APIs these would likely be separate mutations because I generally prefer small focussed mutations to autogenerated CRUD, but that's really a matter of taste.)

Please expand on the reasons why you think a directive would be the right approach here as opposed to using different input fields.

[francisu]

@benjie thanks for your thoughtful and helpful response. tl;dr, however, I still stand by my proposal.

(In my APIs these would likely be separate mutations because I generally prefer small focussed mutations to autogenerated CRUD, but that's really a matter of taste.)

This is the interesting thing. There appear to be two major philosophies about the use of GraphQL: one is that the queries and mutations are generated manually to take more focused actions (yours, which is I think the majority view), and the other where everything is automatically generated from some source (in our case, a separate metadata system). My lack of practice in the former lead me to not consider different methods of autogeneration to solve the problem, I just went right away to a directive, and I still think the reasons for it are good.

From a hand-coding perspective, your approach makes a lot of sense.

Part of our difference in philosophies is that all of our business logic (front end or back end) is built on top of GraphQL, which serves only as a simple data layer and has no other special processing. We would compare this to the use of SQL. This makes the model easy for developers, as they don't have to guess the semantics of arbitrarily constructed mutations; they understand how everything impacts our data.

This is much more explicit, it doesn't require spec edits, it's discoverable and documentable using the introspection features already baked into GraphQL/GraphiQL, and it doesn't require any arcane directive usage.

With your proposal, we would generate two variants for each object, one as we do now, and one with a special prefix of "replace", which has the semantics of the proposed directive.

This violates DRY principles in having this two versions of all objects in the generated schema, where an orthogonal means (like a directive) wouldn't.

Since this generation of "replace" variant would be a convention, we don't want this to be discoverable and documentable through the introspection features as we are polluting that namespace unnecessarily. This would make something like GraphiQL more unmanageable for even a modest sized schema.

We have a (small) standard set of directives we use to help with GraphQL operations (much like what is done in SQL), and it's easy for our clients to understand the meaning of a separate directive. And since it's a directive, it will work nicely with all standard tooling.

Regarding the spec edit: I think this is a clean, backwards compatible, consistent, and simple addition, so I don't see that as an issue (other than going through the process). Honestly, I don't understand why directives were not just provided for all user-named objects in the execution (arguments and object field names were missed). This would make the specification more consistent and flexible without harming it's integrity.

[benjie]

This violates DRY principles in having this two versions of all objects in the generated schema, where an orthogonal means (like a directive) wouldn't.

To be clear the two fields would use the exact same input object type in my proposal; where you had address: AddressInput before, I propose you have patchAddress: AddressInput, replaceAddress: AddressInput. There's no repetition since the two fields have different semantic meanings, and are implemented differently.

This would make something like GraphiQL more unmanageable for even a modest sized schema.

I'm not convinced the addition of a single extra field in each of these cases would make things unmanageable. Perhaps you were thinking we'd need an entire new input object type also?

Regarding the spec edit: I think this is a clean, backwards compatible, consistent, and simple addition, so I don't see that as an issue (other than going through the process).

I'll admit that I've not looked at your proposed spec edits yet, it's standard practice when reviewing GraphQL spec changes to understand the motivation behind a change before concerning ourselves with specific implementation details. I'm not trying to put you off of making these changes, I'm trying to help you to find a convincing argument for the change. After all, one of the guiding principles is "favor no change" - so there has to be a good reason to make a change.

[francisu]

This violates DRY principles in having this two versions of all objects in the generated schema, where an orthogonal means (like a directive) wouldn't.

To be clear the two fields would use the exact same input object type in my proposal; where you had address: AddressInput before, I propose you have patchAddress: AddressInput, replaceAddress: AddressInput. There's no repetition since the two fields have different semantic meanings, and are implemented differently.

Yes, I always considered they would use the same input type.

This would make something like GraphiQL more unmanageable for even a modest sized schema.

I'm not convinced the addition of a single extra field in each of these cases would make things unmanageable. Perhaps you were thinking we'd need an entire new input object type also?

But the object field names would be repeated for any object. And if you have an object with a lot of subobjects, this is significant. But even one unnecessary repetition of the pure field names for the purpose of how the data is changed is too much. And when we have additional similar use cases, following this pattern will quickly get out of control. This is the sort of thing that directives were meant to handle naturally, not overloading the meaning of field names (again, this is with the GraphQL as a data layer concept).

Regarding the spec edit: I think this is a clean, backwards compatible, consistent, and simple addition, so I don't see that as an issue (other than going through the process).

I'll admit that I've not looked at your proposed spec edits yet, it's standard practice when reviewing GraphQL spec changes to understand the motivation behind a change before concerning ourselves with specific implementation details. I'm not trying to put you off of making these changes, I'm trying to help you to find a convincing argument for the change. After all, one of the guiding principles is "favor no change" - so there has to be a good reason to make a change.

Sure, no problem; the edits are very small, and I understand the argument must be convincing.

I still think it's important to do it this way, as for those of us who use the specification as a generic data layer, generating from large metadata, this is mixing the concerns in an arbitrary way (sometimes a name is a object field name, other times it must have a special prefix to have the mutation so something different). A directive in this case is cleaner, and I think there might be other use cases for directives on the object field name.

GraphQL without directives would have been unusable for many use cases (including ours); I think the directive capability in general should be more complete and that would improve the overall usability of the specification, particularly for generic data access.

It's reasonable to wonder why directives were omitted from arguments and object names when they were consistently applied to all other names in the specification.

[sungam3r]

So that changes the meaning of FIELD location. Does new location make sense?

[francisu]

I don't see how it changes anything about the FIELD location. FIELD and OBJECT_FIELD are unrelated. But I have a feeling I'm missing your point. Can you say more?

[sungam3r]

What is the difference between FIELD and OBJECT_FIELD ?

[francisu]

FIELD refers to a GraphQL field (see section 2.5 of the spec). OBJECT_FIELD refers to a GraphQL ObjectField (2.9.8). They have nothing to do with each other. If you look at the proposed grammar change, the directive is supposed before the Name element of the ObjectField.

[francisu]

To avoid this confusion and make the spec easier to understand, it would be helpful to refer to the section in the spec where each of these items refer to. I will make a separate PR for that.

[sungam3r]

OK. Now I understand. At first I thought about something completely different.

[benjie]

It’s not clear how you would add directives when an input object is specified via a variable rather than statically in the document.

[timscharfenort]

Alles perfekt

[francisu]

It’s not clear how you would add directives when an input object is specified via a variable rather than statically in the document.

Forgive the delay on this, I actually implemented this (because we need it, and it also validates the approach).

For our use case, the @replace directive is only meaningful on an object name when that refers to another object (specifying @replace on a scalar value would have no meaning).

mutation { 
  updateCustomer( input: { id: "custId"
    @replace address: { 
      street1: "123 Main St"
      city: "Oakland"
      state: "CA"
      zipCode: "94610"
    } 
  } ) { id }
}

If we are using a variable to represent the address, we add a _replace in the address object. Of course this is a local matter, one could argue beyond the scope of the specification.

However, the specification has reserved the use of any field beginning with "--" (section 4.1), so it would not be unreasonable to extent this PR with the addition of a __directive field which could have a text directive specification.