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

Grammar should have single root node #1031

Open
vwkd opened this issue Jun 22, 2023 · 2 comments
Open

Grammar should have single root node #1031

vwkd opened this issue Jun 22, 2023 · 2 comments

Comments

@vwkd
Copy link

vwkd commented Jun 22, 2023

Currently, the GraphQL grammar defines four root nodes that don't have a parent node: Document, an ExecutableDocument, a TypeSystemDocument and a TypeSystemExtensionDocument.

Lacking a unique root node makes the spec ambiguous, as it could refer to any of the four grammars defined by these four root nodes.

The grammar should be explicitly defined by introducing a single wrapping root node like

GraphQLDocument
  Document
  ExecutableDocument
  TypeSystemDocument
  TypeSystemExtensionDocument

Allowing services to support some documents and not others doesn't need to come at the cost of an ambiguous grammar. It can instead be defined in accompanying prose.

@benjie
Copy link
Member

benjie commented Jun 22, 2023

Unfortunately this suggestion would make the grammar ambiguous. Since every TypeSystemDocument is also a valid TypeSystemExtensionDocument and a valid Document, the document:

type Query {
  i: Int
}

could be parsed according to your grammar as GraphQLDocument > Document > ..., GraphQLDocument > TypeSystemDocument > ... and GraphQLDocument > TypeSystemExtensionDocument > ...


Rather than thinking of these as 4 alternative "root nodes", you should note that there is exactly one root node: Document. ExecutableDocument, TypeSystemDocument and TypeSystemExtensionDocument are constrained forms of Document (and not necessarily mutually exclusive) - they do not change that every GraphQL document is a Document. They each cover (non-exhaustive and overlapping) subsets of Document.

Some examples:

Example 1

type Query {
  i: Int
}

query Q {
  __typename
}

This document is a Document. It does not conform to ExecutableDocument (since it contains type definitions) and it does not conform to TypeSystemDocument or TypeSystemExtensionDocument (since it contains executable definitions).

Example 2

query Q {
  __typename
}

This document is a Document. It also conforms to ExecutableDocument.

Example 3

type Query {
  i: Int
}

This document is a Document. It also conforms to both TypeSystemDocument and TypeSystemExtensionDocument.

Example 4

type Query {
  i: Int
}
extend type Query {
  f: Float
}

This document is a Document. It also conforms to TypeSystemExtensionDocument.


I hope this helps :)

@vwkd
Copy link
Author

vwkd commented Jun 22, 2023

Thanks for taking the time to answer.

Since every TypeSystemDocument is also a valid TypeSystemExtensionDocument and a valid Document

According to the grammar, TypeSystemDocument and TypeSystemExtensionDocument aren't in the tree of Document, hence they are not a Document.

the document [..] could be parsed according to your grammar as GraphQLDocument > Document > ..., GraphQLDocument > TypeSystemDocument > ... and GraphQLDocument > TypeSystemExtensionDocument > ...

Right, it could be parsed going down either of these trees, hence it could be either of these documents.

you should note that there is exactly one root node: Document.

According to the grammar, there are four nodes which don't have a parent node, hence there are four root nodes.

ExecutableDocument, TypeSystemDocument and TypeSystemExtensionDocument are constrained forms of Document (and not necessarily mutually exclusive)

This may be the desire, which is also implied in the naming, but the grammar doesn't reflect that.

According to the grammar, ExecutableDocument, TypeSystemDocument, and TypeSystemExtensionDocument each are a list of a child node of Document but they are not children of Document.

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

2 participants