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

Add support for variable-types apart from keyword #36

Open
itadventurer opened this issue Aug 21, 2018 · 0 comments · May be fixed by #37
Open

Add support for variable-types apart from keyword #36

itadventurer opened this issue Aug 21, 2018 · 0 comments · May be fixed by #37

Comments

@itadventurer
Copy link

I cross-posted this issue at the seemingly most-active fork https://github.com/district0x/graphql-query as this repository seems to be dead. Issue: district0x/graphql-query#4

Currently the spec describes allows to be a :variable/type to be only a keyword:

(s/def :variable/type keyword?)

This works fine in many cases but is for our use case a serious limitation.

I would propose to add functionality to suport all valid GraphQL input types as described in the GraphQL Spec.

Here are some examples:

I will use following query as an example (and modify it accordingly):

query Foo($Foo:Int){employee{name}}

The corresponding Venia query (as it works now without problems) is:

{:venia/operation {:operation/name "Foo" :operation/type :query}
 :venia/variables
 [{:variable/name "Foo"
   :variable/type :Int}]
 :venia/queries [[:employee [:name]]]}

I will focus now only on the :venia/variables part:

Simple variable

GraphQL:

query Foo($Foo:Int){employee{name}}

Variables:

[{:variable/name "Foo"
   :variable/type :Int}]

Required variable

GraphQL:

query Foo($Foo:Int!){employee{name}}

Variables:

[{:variable/name "Foo"
  :variable/type {:type/type :Int
                  :type/required? true}}]

Another option would be to add a :variable/required? key to the variable map but I personally dislike this option as the required-property is part of the type and not the variable definition.

List

GraphQL:

query Foo($Foo:[Int!]!){employee{name}}

Variables:

[{:variable/name "Foo"
  :variable/type {:type/kind :list ;; :type/kind instead of :type/type!
                  :type.list/items {:type/type :Int
                                    :type/required? true}
                  :type/required? true}}]

I do not particulary like the :type/kind declaration as there is only one valid value (:list) for it but I do not see another nice way to do so. I mislike the idea to add a special :type/type called :List as it is not reserved by the GraphQL specification (as far as I can see) and someone may add a custom :List type to their schema and then this will break.

Final remarks

According to the GraphQL specification this are the only valid options for the variable types.

Implementation

@r0man already implemented a way to add Lists as types here but it misses following patterns (when I read the code correctly): [Int]!, [[Int]] and so on.

I will provide an implemenation (and create a PR) as soon as possible but would be happy for any feedback for this solution approach.

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 a pull request may close this issue.

1 participant