RFC: support returning applied directives as part of the introspection

[JoviDeCroock]

The following proposes an addition to the introspection part of the spec. In every place where a directive can be applied we alter the corresponding type to have a list of __AppliedDirective present.

This would support use cases where we need additional information about the field to facilitate functionality like

• @sensitive - to mask sensitive fields from logging information
• @cache - to instruct an external system that a field is cacheable
• in federation folks currently expose their whole SDL under a field so the central router can parse directives

This is already implement in the following implementations

• GraphQL.NET
• GraphQL Java

I tried making this as backwards compatible as possible, an alternative would be to rather than using name referring to the __Directive. Or if wanted we could include type with a reference to __Directive this does not seem present in the Java and .NET implementation.

I am aware of the limitation that String imposes as a type for the .value my main reasoning was that it's very much in line with defaultValue and shouldn't pose that much of an issue.

I am open to implementing this in the JS reference implementation as well.

Related to #300

[netlify]

✅ Deploy Preview for graphql-spec-draft ready!

Name	Link
🔨 Latest commit	9f2e594
🔍 Latest deploy log	https://app.netlify.com/sites/graphql-spec-draft/deploys/65c37ed0b9467f00089e3ad8
😎 Deploy Preview	https://deploy-preview-1075--graphql-spec-draft.netlify.app/draft
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[benjie]

Excellent to see this written up in spec edits; thanks for the work @JoviDeCroock 🙌

Relevant: Lee's essentially equivalent proposal: graphql/graphql-wg#1096

---

Despite the simplicity of the solution and the fact it's already used in the ecosystem, unfortunately I don't see using the defaultValue approach as a workable long-term solution. defaultValue itself is only barely acceptable because tools don't actually consume and extract information from the defaultValue - they typically just care whether or not it exists, or render it as-is. However, when using directives to expose metadata it's highly likely that tools and clients will need to consume this potentially quite complex data, which will require a parser. Introducing the need for a parser causes a few complications:

Bundle size and complexity increases

Clients that wish to consume the directive values will need to bundle a parser, increasing bundle size. It's likely that a simple parser could be built in not much code, so I'm not terribly worried about this, but it is something to keep in mind.

Handling of custom scalars

Custom scalars have their own serialize and parseValue methods which exist only on the server - so given that you serialize on the server and then stringify the result into this String field, how can the client parseValue and use the result given it doesn't have the code to do so? Not all scalars implement @specifiedBy.

It prevents syntax expansion

This, to me, is the biggest show-stopper issue. Because these values need parsing, the parsing becomes a position where breaking changes could occur. If a new syntax was introduced then this new syntax may not be parseable by existing clients, causing unexpected errors. (For example, imagine we added an optional additional argument to an existing directive, and this new argument was of a new type which used a syntax the existing parser did not understand.) Doing this would essentially fix the syntax permanently, which fails the "Preserve option value" guiding principle.

---

I digested the existing solutions proposed for #300, including the approach that you've outlined, in this talk at GraphQL Conf 2022:

https://youtu.be/FC4AFSjh_4k?t=289

Here's another presentation where I talk about the need for granular metadata in GraphQL:

https://docs.google.com/presentation/d/1e6o2kd3fVc_DQH1O8RxJo-idZM0kTOx-q322coNeIIo/present?slide=id.gc6fa3c898_0_0

An alternative solution that I raised a while back is "annotation structs":

https://github.com/graphql/graphql-wg/blob/main/rfcs/AnnotationStructs.md

This solution enables querying schema metadata using regular fields to represent the arguments, so scalars will appear the same in introspection as they would when queried through the non-introspection parts of your schema - there's no need for a parser. Further, the solution allows you to query just parts of the metadata, so that your client does not get overwhelmed - this is useful in similar situations to where you might introspect an enum in your client to find out the list of enumValues to render in a dropdown.

The main issue that the annotation structs proposal faces right now (other than that it relies on the struct proposal*) is hesitation over the auto-generated nature of directive-derived types; for example, directive @source(table: String, column: String, service: ServiceSource) annotation on OBJECT | FIELD_DEFINITION needs to define the type struct __Annotation_source { table: String, column: String, service: ServiceSource } so that it can be queried. Some find these derived types to be distasteful.

* The struct proposal is held up because it solves too many problems 😉

[martinbonnin]

Thanks @benjie for the detailed writeup, as always!

One thing that is still not 100% clear to me after watching the video is the custom scalar part:

Custom scalars have their own serialize and parseValue methods which exist only on the server - so given that you serialize on the server and then stringify the result into this String field, how can the client parseValue and use the result given it doesn't have the code to do so? Not all scalars implement @SpecifiedBy.

I would expect this? Because custom scalars are not specified, how to interpret them has to be passed out of band.

For an example say I have the (completely synthetic) example below:

scalar EmailAddress

"""
The owners of this type. Owners are notified any time a change is requrested and must approve changes
"""
directive @owners(emails: [EmailAddress]) on OBJECT

type Product @owners(emails: ["[email protected]"]) {
  name: String!
  description: String!
  price: Float!
}

• If the client is internal tooling like schema checks, it is required to have knowledge of the server to parse the email addesses. Just like regular clients (web & mobile apps) need to know how to parse custom scalars.
• If the client is a general purpose tool like GraphiQL then all it knows is that owners can be represented as a list of GraphQL StringValue. I'd say this is OK? If a team wants their GraphiQL to be "smarter", they'll have to "augment" it somehow but in the general case, getting the value as GraphQL value seems acceptable to me. What am I missing?

[JoviDeCroock]

I generally agree with the points made by @martinbonnin external providers can declare directives like @sensitive/... which will always be subject to that providers own understanding of this directive, while general purpose directives declared by the graph-owner will most likely have a shared understanding of scalars between server and client. This could also be me having a limited understanding of how directives and the typing of arguments is being used in the wild though.

I don't disagree with having a deeper solution as you are describing @benjie which seems to get us to a wider benefit of these values similar to how this could then be Any as a type 😅 I agree that this spec isn't automatically good because clients are already implementing it but wanted to explore whether folks could get behind this, personally I have been quite eager to get to something like this for a while due to logging/federation/... I agree with the point to not bloat client bundles with parsers, ...

From a backwards/forwards compatibility perspective the change isn't very reversible as we would make it part of the schema, this however made me think if we could somehow make this a point of extension rather than embedding it in the introspection.

[benjie]

I would expect this? Because custom scalars are not specified, how to interpret them has to be passed out of band.

@martinbonnin Right; but you are now giving two separate representations of this scalar - the representation that you'd get from querying it in the output schema, and now this new stringified form which may or may not match that when parsed. The latter may require additional work that the former would not. (Your example would have the same representation both ways, so does not suffer from this complexity; but as a spec we must handle the edge cases.)

For example, if your custom scalar represented a File object (like the Upload type in Jayden's Multipart Request Spec) this might get returned to the GraphQL consumer as an in-memory file object. When stringified as part of a defaultValue-like string, however, it might be base64-encoded or hex-encoded or something else entirely... The client can't know how to interpret that and convert it into what would have been sent had it have just been queried like any other field (i.e. a File object).

This is probably less of an issue when GraphQL is used over a JSON-based transport since GraphQL's language structure and JSON's are very similar (types: object, list, string, number, boolean) but if you use it with something that has more specific typings this could be significant. For example, a GraphQL response over a different transport could encode your scalar UUID4 and scalar GUID efficiently such that the did not need any client parsing; but in GraphQL language they'd have to come out as strings like "ecb9e1dfd9bf491ea487bf8336e3eb61" and the client won't necessarily know how to cast this into the native UUID4/GUID types the consumer expects - special code will have to be added specifically to handle the difference in introspection versus the regular output schema.

[martinbonnin]

@benjie Right 👍 So the issue is that the client has to implement parseLiteral in addition to parseValue, right? parseValue being traditionally implemented but parseLiteral not so much because clients typically do not know anything about GraphQL values.

I think I'd be ok with that, especially since we're saying the client has to bundle a GraphQL value parser anyways (and therefore be "GraphQL value aware").

But yea, I can see how that's definitely a limitation to account for in the final tradeoff.

[benjie]

@martinbonnin Clients don't currently implement parseLiteral or parseValue - these are server-side functions used to interpret the data sent from clients to the server. When clients receive scalars they typically use this data directly without any transformation; we would be introducing the need for transformation, and that transformation would need to be defined for each custom GraphQL scalar type in use.

[martinbonnin]

@benjie True that parseValue/parseLiteral are server-side functions but turns out Apollo Kotlin implements something very similar to parseValue with Adapter.

Adapters adapts from JSON to "runtime object". This way clients can use the type system they are used to. Typical example is java.util.Date that is parsed from strings. I'd expect most codegen clients would have something similar. Without this, users are left with dealing with any or JsonElement types which isn't great.

The Apollo Kotlin client doesn't have anything to transform from GraphQL value to "runtime object" (~= parseLiteral) but in our use case that's probably fine. Only the compiler would need this and the compiler has a lot of code so that adding a additional parseLiteral there is probably acceptable.

Of course, it's not great because it means the client introspection consumer needs to be "GraphQL value aware" now and it's adding complexity and entropy to the universe but on the other hands, would unlock a lot of use cases like getting the @semanticNonNull directive to deal better with nullability.

[benjie]

would unlock a lot of use cases like getting the @semanticNonNull directive to deal better with nullability.

Yes, I'm a huge proponent of metadata directives and have done a lot of research and work on the topic. I'm definitely not arguing against them, just against the specific stringified way of expressing them when we could just use the scalars directly if we figure out how.

[JoviDeCroock]

Closing this in favour of the metadata proposal as highlighted in the above discussion