Aliases

[thomassuckow]

When there is a conflict between fields this library resolves the arguments differently. The tree will claim the argument is one thing (2) but the resolve method for foo will claim another (1).

query {
  a {
    foo(arg: 1) {
      id
    }
    foo(arg: 2) {
      id
    }
  }
}

This is a bad case and I would argue you're in trouble anyway, but this leads us to another thing: aliases

This library has no handling for:

query {
  a {
    foo(arg: 1) {
      id
    }
    bar: foo(arg: 2) {
      id
    }
  }
}

I would argue for aliases resulting in an array in the tree with a $alias field.

Now, I think for what I am working on I will ignore using aliases and conflicts so this is a low priority to me. But I wanted to post this as a shortcoming because I think an alternate implementation of graphql-server centered around a tree like this has a lot of potential.

[alekbarszczewski]

@thomassuckow In my opinion aliases are something that backend/server should not depend on. They are just a feature for client side. Anyway graphql.js takes care of them for you and you don't have to return for example objects with aliased fields.

query {
  postsAlias1: posts { idAlias: id }
  postsAlias2: posts { id }
  posts { id } 
}

For all those cases GraphqlQueryTree will be (and it should be) identical.

There was one bug though, that I've just fixed: tree.getParentField() was returning alias instead of original field name. This is fixed in latest version.

[thomassuckow]

So that should work if we called this library in the resolver of the thing being aliased. In our setup we have an ElasticSearch database that accepts complex queries returning multiple data sets in a single pass.

We are utilizing this library to build a representation of the graphql query which we then use to construct an elasticsearch query. We do this all at the top resolver in graphql despite having nested and recursive structures in GraphQL. So far this is working amazingly, though it is difficult to use fragments.

[alekbarszczewski]

Hmm, I am not sure if I understand what is the issue? How you would like it to behave?
Could you provide exact GraphQL query and generated query tree that you think is invalid / incomplete?

[thomassuckow]

Query:

query getSearch($query: JSON!) {
  search(query: $query) {
    things(limit: 1000, type: "dog") {
      id
    }
    
    cat: things(limit: 500, type: "cat") {
      id
    }
  }
}

Generated Tree (note the lack of the 1000/dog arguments):

{
  "$args":{
    "query":{"foo":"bar"}
  },
  "$type":"searchResult",
  "things":{
    "$args":{
      "limit":500,
      "type":"cat"
    },
    "$type":"thing",
    "id":{"$args":{},"$type":"ID"}
  }
}

What I might expect:

{
  "$args":{
    "query":{"foo":"bar"}
  },
  "$type":"searchResult",
  "things":[
    {
      "$args":{
        "limit":1000,
        "type":"dog"
      },
      "$type":"thing",
      "id":{"$args":{},"$type":"ID"}
    },
    {
      "$args":{
        "limit":500,
        "type":"cat"
      },
      "$type":"thing",
      "$alias":"cat",
      "id":{"$args":{},"$type":"ID"}
    }
  ]
}

[alekbarszczewski]

You are right. Damn those aliases make everything really complicated.
I will have to change the whole API around GraphqlQueryTree... I guess it will look like this:

  tree.isSelected('things'); // true
  tree.getChildFields(); // return array of all sub trees
  tree.getChildFields('things'); // return array of sub trees for each selection of field 'things'.
  tree.getAlias(); // return alias of tree / sub tree
  tree.getArguments(); // return arguments of tree / sub tree
  tree.getType('things'); // 'Thing'

[thomassuckow]

To be honest, I reach in and take the _tree and don't use your selectors. I use lodash/fp/get to access things.

if( get('things.$args.type')(tree._tree) ) { ... }

Though with aliases we would have to do something like:

const things = tree._tree.things;
if( isArray(things) ) {
 map(handleThings)(things)
} else if(things) {
  handleThings(things)
}

[alekbarszczewski]

Hmm, I think to keep it simple and consistent I will change every field to be an array instead of object as you suggested:

{
  $args: {},
  $type: '',
  id: [{ $args, $type, $alias, ... }],
  things:  [{ $args, $type, $alias, ... },  { $args, $type, $alias, ... }, ...],
  ...
}

or even all fields in one array:

{
  $args: {},
  $type: '',
  $fields: [{ $field: 'id' $args, $type, $alias, ... }, { $field: 'things', $args, $type, $alias, ... },  { $field: 'things', $args, $type, $alias, ... }],
  ...
}

[thomassuckow]

I would not do the latter, it makes it difficult to check things and you end up making a lot of temporary arrays when traversing.

The former is slightly annoying when traversing the simple case (no args), but if you treat everything like arrays it makes handling aliases easier since you can always map/foreach. It is probably worth the extra [0]'s on simple cases.