search

Shopify / graphql-js-schema

Transforms the JSON representation of a GraphQL schema into a set of ES6 type modules
MIT License
35 stars 12 forks source link

Implemented parsing type information for args #9

Closed mikkoh closed 7 years ago

mikkoh commented 8 years ago

I'm putting this up now so you @minasmart @jamesmacaulay can review.

I still need actual tests for checking if a args which are Enums or are Interfaces are parsed properly. But in light of our discussions on validation I'd like you to let me know if this is doing too much.

jamesmacaulay commented 8 years ago

In order to do proper argument validation we would actually need the full schema of those INPUT_OBJECT types, unlike for validating the rest of the query where we can get away with only knowing the base types of object fields.

The current implementation in this PR doesn't provide enough of the schema for this, because it collapses LIST and NON_NULL types into their base type plus boolean flags indicating the presence of these type modifiers. This is insufficient for argument validation, because it doesn't distinguish between e.g. [String!]! and [String!] and [[String!]!]! and every other valid combination.

IMO query validation isn't something we should be working on right now. I think we should strip the schema down to only the information that allows us to customize deserialization with user-supplied type constructors and take advantage of Relay features like pagination. Even without any validation code in graphql-js-client, a client project can validate queries as part of their automated tests by including graphql-js as a dev dependency and validating based on the full schema and graphql-js-client's query.toString() output. If we actually encounter real pain with that approach when we dogfood it, that's when I suggest we should design our own solution.