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

[RFC] GraphQL Schema Definition Language (SDL) #90

Merged
merged 28 commits into from
Feb 8, 2018
Merged

[RFC] GraphQL Schema Definition Language (SDL) #90

merged 28 commits into from
Feb 8, 2018

Conversation

leebyron
Copy link
Collaborator

@leebyron leebyron commented Sep 2, 2015

This adds the type definition syntax to the GraphQL specification.

There are some remaining issues to solve:

  • Include reference in Execution chapter about non-executable GraphQL
  • Description is not yet represented
  • Deprecation is not yet represented
  • Directives are not yet represented
  • Top level Schema is not yet represented
  • Investigate impact on validation rules
  • Decide on any or many rules for type fields / enum values
  • Combine into type system chapter
  • Consider separating into two top level grammars
  • Solve ambiguity for extending interfaces (Add test case for failing parsing graphql-js#1166)

@leebyron
Copy link
Collaborator Author

leebyron commented Sep 2, 2015

Proposal for schema:

schema {
  query: QueryType
  mutation: MutationType
}
SchemaDefinition : schema { OperationTypeDefinition+ }
OperationTypeDefinition : OperationType : NamedType


InterfaceTypeDefinition : interface Name { FieldDefinition+ }

UnionTypeDefinition : union Name = UnionMembers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax for UnionMembers is missing in appendix

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed!

@nuance
Copy link

nuance commented Nov 2, 2015

Should this also include directives? Something like:

DirectiveDefinition : directive Name ArgumentsDefinition?

This doesn't capture scope / validation rules, so perhaps it should include an 'on' clause?

DirectiveDefinition : directive Name ArgumentsDefinition? DirectiveScopes?
DirectiveScopes : on DirectiveScope+
DirectiveScope : field | fragment | operation

So include would be defined like:

directive include(if: Boolean!) on field, fragment

@joshprice
Copy link

Is there any movement on this? This is a pretty compelling piece of the GraphQL puzzle and opens up the possibility of tooling around schema design and also allows the creation of a cross language test suite.

@nuance
Copy link

nuance commented Jan 21, 2016

I ended up implementing most of this grammar in a project I'm working on. Here's the relevant lines from a PEG grammar (just the type parsing stuff):

TypeDefinition <- ObjectTypeDefinition / InterfaceTypeDefinition / UnionTypeDefinition / ScalarTypeDefinition / EnumTypeDefinition / InputObjectTypeDefinition / TypeExtensionDefinition / DirectiveDefinition
ObjectTypeDefinition <- "type" _ Name _ ImplementsInterfaces? _ "{" __ (FieldDefinition __)* "}"
ImplementsInterfaces <- "implements" _ (Name _)+
FieldDefinition <- Name _ ArgumentsDefinition? _ ':' _ InputType
ArgumentsDefinition <- '(' _ ( InputValueDefinition _ )* ')'
InputValueDefinition <- Name _ ':' _ InputType _ DefaultValue?
InterfaceTypeDefinition <- "interface" _ Name _ '{' __ (FieldDefinition __)+ '}'
UnionTypeDefinition <- ("union" / "Union") _ Name _ "=" _ UnionMembers
UnionMembers <- NamedType _ ("|" _ NamedType _)*
ScalarTypeDefinition <- "scalar" _ Name
EnumTypeDefinition <- "enum" _ Name _ "{" __ (EnumValue __)+ __ "}"
InputTypeDefinition <- Name _ ':' _ InputType _ DefaultValue?
InputObjectTypeDefinition <- "input" _ Name _ "{" __ (InputTypeDefinition __)+ "}"
DirectiveDefinition <- "directive" _ Name _ "on" _ (Name _)+ ArgumentsDefinition?
TypeExtensionDefinition <- "extend" _ ObjectTypeDefinition

I think this is basically compatible with the language defined in this PR, with a few additions (directives).

Answering the questions @leebyron asked:

  • description and deprecation are both expressed via directives
  • not sure what top level schema means - we define a query root object like:
type QueryRoot{}

then extend it as needed (I'm assuming that's what you're referencing?)

extend type QueryRoot {
    __schema : __Schema!
    __type(name: String!) : __Type
}
  • It's not clear what validation would be required. We're doing a little to catch compliance with the relay extensions and handle some of our own extensions, but nothing that passes the grammar has brought up anything interesting.

@joshprice
Copy link

  • Got a concrete example for implementing metadata (description, isDeprecated, etc) using directives @nuance ?
  • Top level schema I think refers to being able to parse the schema itself which is fairly trivial, just needs to be spec'd fully, but it looks like your example could use existing primitives
schema {
  query: QueryType
  mutation: MutationType
}
  • You can't mix type defs and queries which is described in the spec already, but presumably not in the execution section yet
  • Schema validation would be somewhat different to query validation, but as you point out if it is parsable then it's probably right, not sure what semantics could be validated? Guidance on orphan/unreferenced types, circular references, for a start

@joshprice
Copy link

For reference there are some concrete examples here: https://github.com/matthewmueller/graph.ql

Descriptions are assumed to be in comments immediately preceding the thing it's describing, assuming deprecation etc could be handled in a similar fashion with a specially formatted comment.

Another possibility for metadata might be type annotations starting with @:

@description Describes a person
type Person {
  @isDeprecated true
  field: Int
}

@joshprice
Copy link

Thinking more about how this is done with directives, you could make something quite readable:

@metadata(description: "Describes a person")
type Person {
  @metadata(isDeprecated true)
  field: Int
}

@joshprice
Copy link

Enums are also an issue, since values are not able to be supplied using the current syntax

@leebyron
Copy link
Collaborator Author

Updated to rebase atop recent changes and includes syntax for defining directives based on graphql/graphql-js#318

@leebyron leebyron force-pushed the rfc-idl branch 2 times, most recently from df904d2 to ace9871 Compare March 22, 2016 22:14
@leebyron
Copy link
Collaborator Author

Updated to rebase, fixed some outdated language and added SchemaDefinition

leebyron added a commit to graphql/graphql-js that referenced this pull request Mar 22, 2016
This implements the schema definition in the spec proposal in graphql/graphql-spec#90

Adjusts AST, parser, printer, visitor, but also changes the API of buildASTSchema to require a schema definition instead of passing the type names into the function.

#### Object

ObjectTypeDefinition : type Name ImplementsInterfaces? { FieldDefinition+ }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leebyron I think I found a mismatch with the reference implementation. It uses any to parse the list of fields, but according to spec it should use many (one or more field definitions):

https://github.com/graphql/graphql-js/blob/master/src/language/parser.js#L765

Should ObjectTypeDefinition be defined like this?

ObjectTypeDefinition : type Name ImplementsInterfaces? { FieldDefinition* }

(it's the same with the InputObjectTypeDefinition)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the catch and close read. I'll take a closer look and figure out which is more appropriate, but I think you're probably right.

sogko added a commit to sogko/graphql that referenced this pull request May 30, 2016
graphql/graphql-spec#90

Commit:
b0885a038ec0e654962d69fb910ac86659279579 [b0885a0]
Parents:
fdafe32724
Author:
Lee Byron <[email protected]>
Date:
23 March 2016 at 6:35:37 AM SGT
Commit Date:
23 March 2016 at 6:35:39 AM SGT
sogko added a commit to sogko/graphql that referenced this pull request May 30, 2016
This implements the schema definition in the spec proposal in graphql/graphql-spec#90

Adjusts AST, parser, printer, visitor, but also changes the API of buildASTSchema to require a schema definition instead of passing the type names into the function.

Commit:
8379e71f7011fe044574f4bdef2a761d18d6cf2c [8379e71]
Parents:
176076c8a6
Author:
Lee Byron <[email protected]>
Date:
23 March 2016 at 7:38:15 AM SGT
Labels:
HEAD
@leebyron leebyron merged commit 593dd2c into master Feb 8, 2018
@leebyron leebyron deleted the rfc-idl branch February 8, 2018 21:43
tonyghita added a commit to graph-gophers/graphql-go that referenced this pull request Mar 7, 2018
This replaces:

```graphql
type Foo implements Bar, Baz { field: Type }
```

With:

```graphql
type Foo implements Bar & Baz { field: Type }
```

This is more consistent with other trailing lists of values which
either have an explicit separator (union members) or are prefixed
with a sigil (directives). This avoids parse ambiguity in the
case of an omitted field set, illustrated by
[github.com/graphql/graphql-js#1166](graphql/graphql-js#1166).

For now, the old method of declaration remains valid.

References:
- graphql/graphql-js#1169
- graphql/graphql-spec#90
- graphql/graphql-spec@32b55ed
@ermik
Copy link

ermik commented May 11, 2018

@leebyron why is the spec still dated October 2016 at http://facebook.github.io/graphql/?

@IvanGoncharov
Copy link
Member

@ermik Because it's the last official release.
You can find the draft of the next release (including SDL) here:
http://facebook.github.io/graphql/draft/

tinnywang pushed a commit to tinnywang/graphql-go that referenced this pull request Dec 13, 2018
This replaces:

```graphql
type Foo implements Bar, Baz { field: Type }
```

With:

```graphql
type Foo implements Bar & Baz { field: Type }
```

This is more consistent with other trailing lists of values which
either have an explicit separator (union members) or are prefixed
with a sigil (directives). This avoids parse ambiguity in the
case of an omitted field set, illustrated by
[github.com/graphql/graphql-js#1166](graphql/graphql-js#1166).

For now, the old method of declaration remains valid.

References:
- graphql/graphql-js#1169
- graphql/graphql-spec#90
- graphql/graphql-spec@32b55ed
mattstern31 pushed a commit to mattstern31/graphql-gqllero-repository that referenced this pull request Nov 10, 2022
graphql/graphql-spec#90

Commit:
b0885a038ec0e654962d69fb910ac86659279579 [b0885a0]
Parents:
fdafe32724
Author:
Lee Byron <[email protected]>
Date:
23 March 2016 at 6:35:37 AM SGT
Commit Date:
23 March 2016 at 6:35:39 AM SGT
mattstern31 pushed a commit to mattstern31/graphql-gqllero-repository that referenced this pull request Nov 10, 2022
This implements the schema definition in the spec proposal in graphql/graphql-spec#90

Adjusts AST, parser, printer, visitor, but also changes the API of buildASTSchema to require a schema definition instead of passing the type names into the function.

Commit:
8379e71f7011fe044574f4bdef2a761d18d6cf2c [8379e71]
Parents:
176076c8a6
Author:
Lee Byron <[email protected]>
Date:
23 March 2016 at 7:38:15 AM SGT
Labels:
HEAD
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

Successfully merging this pull request may close these issues.