Add Appendix D - Specified Type System Definitions#1037
Add Appendix D - Specified Type System Definitions#1037leebyron merged 10 commits intographql:mainfrom
Conversation
✅ Deploy Preview for graphql-spec-draft ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Would that qualify as an The validation/execution behaviour is essentially unchanged. The main thing that could be considered a behaviour change is that the definitions proposed here contain descriptions. We could either:
|
benjie
left a comment
There was a problem hiding this comment.
Let's keep this to just adding the appendix, it does not need to be referenced in the spec text (just as Appendix B is not referenced when grammar is mentioned).
|
I guess there's a distinction to be made between:
Edit: I settled on "Type System Definitions" |
BoD
left a comment
There was a problem hiding this comment.
Added a few dots and capital letters.
benjie
left a comment
There was a problem hiding this comment.
I reproduced your work by starting from a clean slate and then extracting the relevant definitions from the relevant sections; doing so revealed a few discrepancies that have crept in since you authored this:
| isRepeatable: Boolean! | ||
| locations: [__DirectiveLocation!]! | ||
| args(includeDeprecated: Boolean = false): [__InputValue!]! |
There was a problem hiding this comment.
Let's maintain the order of fields in the Introspection chapter
| isRepeatable: Boolean! | |
| locations: [__DirectiveLocation!]! | |
| args(includeDeprecated: Boolean = false): [__InputValue!]! | |
| locations: [__DirectiveLocation!]! | |
| args(includeDeprecated: Boolean = false): [__InputValue!]! | |
| isRepeatable: Boolean! |
There was a problem hiding this comment.
Either we should re-order this in the spec, or in GraphQL-js. We should make it consistent.
There was a problem hiding this comment.
There was a problem hiding this comment.
Changed the order to match graphql-js semantic ordering instead e274c76
See graphql/graphql-js#4428 (comment) for data about why semantic ordering is probably the safe bet.
benjie
left a comment
There was a problem hiding this comment.
Let's discuss whether or not we need this, or whether it should be re-incorporated back into the other chapters of the spec... But if agreed that a new appendix is warranted then I confirm the current state of it as accurate 👍
|
Feedback from May wg:
|
benjie
left a comment
There was a problem hiding this comment.
LGTM; couple minor suggestions.
| isRepeatable: Boolean! | ||
| locations: [__DirectiveLocation!]! | ||
| args(includeDeprecated: Boolean = false): [__InputValue!]! |
There was a problem hiding this comment.
Either we should re-order this in the spec, or in GraphQL-js. We should make it consistent.
Co-authored-by: Benjie <benjie@jemjie.com>
scripts/update-appendix-c.mjs
Outdated
| const suffix = ` | ||
| \`\`\` | ||
| ` | ||
| await writeFile(FILE, prefix + printSpecifiedScalars() + '\n\n' + introspectionSchema + suffix); No newline at end of file |
There was a problem hiding this comment.
add a line break at the end
line break
As we dive into full schemas, introspection and more (see #1036), I'd find it useful to have an authoritative source for built-in definitions that come with a GraphQL implementation (scalar, directives & introspection types).
It shows the type system definitions at a glance and makes it easy to diff, capture a change by looking at the git history. It's also nice for library authors because it uniformizes the descriptions.
The definitions are taken from graphql-js
printIntrospectionSchema()with two small changes:__Type.fields,__Type.interface, etc... descriptions from the current spec commentsI was doing this for my own needs so figured out I might as well contribute it to the spec but I'm happy to move it somewhere else if you think this doesn't belong in the spec.