Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document Level Directives #1041

Open
tusharmath opened this issue Aug 29, 2023 · 10 comments
Open

Document Level Directives #1041

tusharmath opened this issue Aug 29, 2023 · 10 comments

Comments

@tusharmath
Copy link

GraphQL Document-Level Directives Proposal

Introduction

As GraphQL's popularity continues to surge, its potential extends beyond just efficient data fetching. Solutions like Apollo Federation, Hasura, Prisma and Tailcall have already showcased the power and flexibility of the GraphQL DSL. Recognizing this, our proposal seeks to further harness this potential by allowing directives at the document level, opening doors to more intuitive ORMs, CMS configurations, and backend tooling.

Problem Statement

While the current GraphQL specification permits directives on the schema, it does not support them at a broader document level. This limitation restricts the configuration options for applications that use the GraphQL DSL for more than just data-fetching.

Proposed Solution

Allow directives to be attached directly to the document. Instead of the current:

schema
  @disableHttpCache
  @idleConnectionPool(size: 300, timeout: 300)
  @namespace(name: "global") {
    query: Query
    mutation: Mutation
}

We propose:

@disableHttpCache
@idleConnectionPool(size: 300, timeout: 300)
@namespace(name: "global")

schema {
  query: Query
  mutation: Mutation
}

This change provides a cleaner syntax, especially when directives don't semantically fit at the schema level.

Test On Guiding Principles

  • Backwards Compatibility: Existing APIs remain unaffected. This proposal introduces a new type for directive definitions: DOCUMENT.

    directive @disableHttpCache on DOCUMENT
  • Performance: No performance implications.

  • Favour No Change: Implementing this proposal will require changes in existing GraphQL parsers.

  • Real Use Cases: With GraphQL DSL powering ORMs, proxies (e.g., [Tailcall]), and orchestrations through Federation, this change can make directive definitions even more impactful.

  • Simplicity & Consistency: The proposal aligns with existing syntax and does not introduce new complexities.

  • Preserve Option: No implications on this principle.

  • Understandability: The proposed change aims to be intuitive, but we welcome community feedback.

Conclusion

By embracing document-level directives, GraphQL can further solidify its position as a versatile tool for diverse applications. We look forward to the community's feedback on this proposal.

@benjie
Copy link
Member

benjie commented Aug 29, 2023

Please could you expand on the use case for this. Further, could you explain what it would mean for executable documents (client-side) vs type system documents (server-side)? Presumably directives in these two places would mean different things? Further, could you clarify if you're using these directives to indicate meta-data (like #300) or if you are using these directives as part of building the final GraphQL schema, and they will evaporate (like custom schema directives currently do) once the schema is built?

I'm currently struggling with why directives would be useful on a document rather than on an operation or the schema.

Note the following is a GraphQL document:

type Query {
  n: Int
}

query {
  n
}

What would a directive mean in this document?

@tusharmath
Copy link
Author

Hey @benjie ! Thanks for having a look at the issue.

Please could you expand on the use case for this.

The usecase is both — to provide additional information while generating the final schema and while implementing the resolvers. For example I want to specify a database connection url and import some other graphql file, I could do this —

@import(src: "./foo.graphql")
@db(host: "localhost", port: 8080, type: "mysql")

Schema Usecase: The import directive could change the final outputted schema by stitching multiple schemas (depending on the server implementation of import and merge).

Resolver Usecase: The db directive would be used to implement a resolver that connects to a mysql database on port 8080. This is used by a "Schema First" graphql server to figure out the graphql resolvers.

Further, could you explain what it would mean for executable documents (client-side) vs type system documents (server-side)? Presumably directives in these two places would mean different things?

The directives defined on the server side are used only by the server and aren't exposed to the client. However this capability should exist on the clients as well. For example I could do something like —

@server(url: "http://localhost:8080/graphql")
query {
  n
}

The client SDK could extract the server information for this query and the actual request will just contain the query.

Further, could you clarify if you're using these directives to indicate meta-data (like #300) or if you are using these directives as part of building the final GraphQL schema, and they will evaporate (like custom schema directives currently do) once the schema is built?

The server side directives should get evaporated once consumed by the server and should not be exposed via introspection. Similarly the client side directives should be consumed by the client sdk to provide meta-data and need not be sent over the wire to the server. To summarize — Yes, it's evaporated after consumption and the client and server don't share this information with each other.

@benjie
Copy link
Member

benjie commented Aug 29, 2023

Schema Usecase: The import directive could change the final outputted schema by stitching multiple schemas (depending on the server implementation of import and merge).

It feels to me like a SchemaExtension:

extend schema @import(src: "./foo.graphql")

would be suitable for this use case. No need for an un-tethered directive - you are stating that you want the schema to be extended by a particular import.

Resolver Usecase: The db directive would be used to implement a resolver that connects to a mysql database on port 8080. This is used by a "Schema First" graphql server to figure out the graphql resolvers.

Again, this feels like it's a behavior of the schema, not of the document, and thus extend schema @db(host: "localhost", port: 8080, type: "mysql") would make sense to me. These don't feel like document-level concerns to me.


@server(url: "http://localhost:8080/graphql")
query {
  n
}

This feels like it's a behavior for the operation, not for the document, and thus should be:

query @server(url: "http://localhost:8080/graphql") {
  n
}

Do you have any use cases that are specific to the document, rather than the schema or the operation?

@tusharmath
Copy link
Author

tusharmath commented Aug 29, 2023

Hope the following makes sense —

# omit.graphql
@actual 
schema {
  query: Query
}

type Query {
  user: User
}

type User {
  name: String
  age: Int @omit
}

@expect 
schema {
  query: Query
}

type Query {
  user: User
}

type User {
  name: String
}

Over here our graphql engine understands the omit directive that is on theage field. We use that directive to remove the field from the final directive. The key directives to think about is @actual and @expect. They are defined at a document level, and separate the actual inputted schema from the expected outputted schema.

Current Implementation

# @actual 
schema {
  query: Query
}

type Query {
  user: User
}

type User {
  name: String
  age: Int @omit
}

# @expect 
schema {
  query: Query
}

type Query {
  user: User
}

type User {
  name: String
}

As you can see, we hide these annotations under comments and write a custom parser to extract information.

@benjie
Copy link
Member

benjie commented Aug 29, 2023

That's an interesting example, however GraphQL works on a global scope basis (you can define types in any order), so all of the document-level directives would be hoisted (i.e. it would become @actual @expect schema { ... }) which doesn't make sense, similarly the types themselves would clash (there are two type User, which would make the document invalid). Instead I'd expect to find these as separate documents, e.g. actual.graphql and expected.graphql, and then they either do not need to be marked up at all (it's implicit by where the documents are loaded from), or they could be marked up via a schema directive since one is the @actual schema, and one is the @expect schema.

If you wanted to do this in a single GraphQL file, then what I would expect you to do currently would be to use a comment to divide the document into two halves, and then you'd have code that split the document into two documents before parsing them. I don't think it would make sense to use a directive to do this.


One other concern with these document-level directives using the syntax currently proposed is that unless they're only introduced at the very start of the document (which would be before they are even defined in a directive statement!) they would introduce ambiguity. For example:

extend type Foo @deprecated(reason: "Use Bar instead")

@server(url: "http://localhost:8080/graphql")

would actually be parsed as:

extend type Foo @deprecated(reason: "Use Bar instead") @server(url: "http://localhost:8080/graphql")

I.e. both directives would apply to the Foo type, not the document.

@tusharmath
Copy link
Author

tusharmath commented Aug 30, 2023

I see where you are coming from. Leaving my proposal aside for a bit, I have a couple of questions —

Why does a document need to be bound to a file? Ideally a file should only be responsible for understanding how you store data and nothing semantically should be inferred from your file structure. It would be great if we could have some sort of modularization that could allow us to write multiple graphql documents in the same file.

document Actual {
  schema {
    query: Query
  }
}

document Expected {
  schema {
    query: Query
  }
}

This is a bit of a deviation from what I am proposing, however I think it's still relevant here. The semantics of how a document is implemented should not be coupled with a file IMHO. Having such a structure will allow us to have a lot more modular configurations.

Splitting a large file into smaller files doesn't seem very scalable, because now you would have many tiny files with one or two lines of graphql code. The other problem is how do you name those files? In our case, I would end up creating a ton of actual.graphql and expected.graphql files. That's difficult to navigate in an IDE and is quite error prone. Ultimately using files to create modules means eventually we need to resort to folders to group multiple modules together.

I think Rust, Scala and a few other languages have gone in this direction. Would love to know your thoughts.

@benjie
Copy link
Member

benjie commented Aug 30, 2023

Documents are not necessarily related to files, the files were just one example. Another example would be a JSON object like {”before”:”schema {…} type …”, “after”: “schema {…} type …”} which contains two documents. Another example would be a single string with a \n""""""""""\n split in the middle that separates two documents. Another example could be a multipart/mixed MIME encoded request body. Or any number of other ways. Essentially a document is the string that contains all the definitions (this is not intended to be a strict definition, for the actual definition see the spec). If you were to create a document construct as you suggest then you would need to introduce a new concept for the thing that contains those two documents, and this process could go on ad infinitum. First you need to show making it so a single string value you would pass to a GraphQL tool (currently called a document) containing two what we call documents currently would have sufficient value.

@benjie
Copy link
Member

benjie commented Jul 19, 2024

Reopening this as we have a solid use-case for document-level directives out of last night's WG meeting (replay; agenda; notes): semantic nullability. The Semantic Nullability Schema proposal introduces the @semanticNullability directive that changes the interpretation of the ! symbol to mean semantically non-null (as opposed to it's traditional strictly non-null interpretation); for more details please see the proposal.

There's still much to be determined, many of the questions above are still relevant (e.g. do we need to differentiate directives on ExecutableDocument vs TypeSystemDocument vs Document), so input is welcome!

@benjie benjie reopened this Jul 19, 2024
@calvincestari
Copy link

Lee raised a valid concern in the WG meeting about concatenating documents together if the document level directives are required to be top-level of a document. I'm not sure if that concern has been raised before but it would definitely have an impact to how we do code generation in Apollo iOS.

@benjie
Copy link
Member

benjie commented Jul 22, 2024

Indeed; it would require tooling to concatenate documents. This would be needed if we used the pragma solution (the previously proposed solution) to semantic nullability schema too; I think essentially we're going to have to require tooling rather than simple concatenation going forward - that or use syntax that no-one can agree on such as Int*, Int!? or Int^.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants