Schema Definition Language Parser #74
Conversation
| @@ -242,3 +242,52 @@ export type NonNullType = { | |||
| loc?: ?Location; | |||
| type: NamedType | ListType; | |||
| } | |||
|
|
|||
| export type TypeDefinition = { | |||
There was a problem hiding this comment.
These should probably go into another file, clientAST.js or something like that.
|
This all seems pretty solid to me. I think there's just some relatively minor code isolation issues to fix up, perhaps moving most of this stuff into I also think you can terse up the parser a bit by putting the parse steps inline in the object definition - that pattern is pretty pervasive through the current parser. |
|
One higher-level question is how this relates to client parsers (read: FB's graphql-data) which interleave GraphQL query and GraphQL schema for the purposes of code-generation, and isolate fragments into separate files and need to refer to them in some way. It seems like either this schema should be merged with that use case, or there is a 3rd variety of parser we will want to eventually support. |
|
re: your last comment definitely a third type with interleaving IMO |
| unexpected, | ||
| expectKeyword, | ||
| advance, | ||
| } from './parserCore'; |
There was a problem hiding this comment.
The role of this is a little muddy. I wonder if there's a better place for parseName and the parseType group to exist? They're definitely specific parser rules not parser core functionality.
For the short term, and for the sake of maintaining the clarity of this document relative to the spec, I would suggest (deep breath) copying these few functions directly in schema/parser.js. I think DRY is working against us here.
| @@ -597,13 +425,24 @@ function parseDirective(parser): Directive { | |||
| }; | |||
| } | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
remain in current position
| }; | ||
| } | ||
|
|
||
| function parseSchemaDocument(parser): SchemaDocument { |
There was a problem hiding this comment.
just in terms of ease of reading this file, I'd suggest moving this fn between parseSchema and parseNextSchemaDefinition (which I may also suggest naming just parseSchemaDefinition).
I'd also suggest organizing the functions to be in the same order as they're listed within the switch statement in parseNextSchemaDefinition
schrockn-zz
force-pushed
the
schema-dsl
branch
2 times, most recently
from
f6902c2
to
47a9168
Compare
| NON_NULL_TYPE, | ||
| LIST_TYPE, |
schrockn-zz
force-pushed
the
schema-dsl
branch
3 times, most recently
from
89eba2f
to
51b593a
Compare
This is a parser for the ad-hoc type definition language included in the spec. This should also serve as a de-facto spec for the language. Clearly once this is stable we should include it in the actual spec as well. This should end up being useful in a few contexts, including client-side types and also a DSL for producing the JS type definitions which can be tedious to write, and a way to specify acceptance tests. (This would be a clear next step for this project)
Schema Definition Language Parser This is a parser for the ad-hoc type definition language included in the spec. This should also serve as a de-facto spec for the language. Clearly once this is stable we should include it in the actual spec as well. This should end up being useful in a few contexts, including client-side types and also a DSL for producing the JS type definitions which can be tedious to write, and a way to specify acceptance tests. (This would be a clear next step for this project)
This is a parser for the ad-hoc type definition language included in the
spec. This should also serve as a de-facto spec for the language.
Clearly once this is stable we should include it in the actual spec as
well.
This should end up being useful in a few contexts, including
client-side types and also a DSL for producing the JS type definitions
which can be tedious to write, and a way to specify acceptance tests.
(This would be a clear next step for this project)