Does not return Object type arguments

[gienec]

Does not return Object type arguments

[dhurlburtusa]

@robrichard I need this feature for a project I am working on. So, I have forked the repo and started working on the implementation. However, I ran into a decision issue with the structure of the __arguments field.

Given the following query

query {
  foo {
    bars(isQuux: true, limit: 10, status: "active", sort: ["name", "asc"]) {
      name
      status
    }
  }
}

We would get something like

{
  bars: {
    __arguments: [
      {
        isQuux: {
          kind: 'BooleanValue',
          value: true,
        },
      },
      {
        limit: {
          kind: 'IntValue',
          value: 10,
        },
      },
      {
        status: {
          kind: 'StringValue',
          value: 'active',
        },
      },
      {
        sort: {
          kind: 'ListValue',
          value: ['name', 'asc'],
        },
      },
    ],
    name: {},
    status: {},
  }
}

Now, consider a query like the following where the predicates argument is an object.

query {
  foo {
    bars(predicates: { name: { regex: "^Quu?x" }, status: { eq: "active" } }, limit: 10, sort: ["name", "asc"]) {
      name
      status
    }
  }
}

There are several possible structures that could be returned.

1. Nested kinds, backwards convention-compatible

{
  bar: {
    __arguments: [
      {
        predicates: {
          kind: 'ObjectValue',
          value: {
            name: {
              kind: 'ObjectValue',
              value: {
                regex: {
                  kind: 'StringValue',
                  value: '^Quu?x',
                }
              },
            },
            status: {
              kind: 'ObjectValue',
              value: {
                eq: {
                  kind: 'StringValue',
                  value: 'active',
                }
              },
            },
          },
        },
      },
      {
        limit: {
          kind: 'IntValue',
          value: 10,
        },
      },
      {
        sort: {
          kind: 'ListValue',
          value: ['name', 'asc'],
        },
      },
    ],
    name: {},
    status: {},
  }
}

2. Top-level kinds, backwards convention-compatible

{
  bar: {
    __arguments: [
      {
        predicates: {
          kind: 'ObjectValue',
          value: {
            name: {
              regex: '^Quu?x',
            },
            status: {
              eq: 'active',
            },
          },
        },
      },
      {
        limit: {
          kind: 'IntValue',
          value: 10,
        },
      },
      {
        sort: {
          kind: 'ListValue',
          value: ['name', 'asc'],
        },
      },
    ],
    name: {},
    status: {},
  }
}

3. Simple, not-backwards compatible (good for version 3x). This is the same structure that the args argument (second parameter) has for the resolve function.

{
  bar: {
    __arguments: {
      predicates: {
        name: {
          regex: '^Quu?x',
        },
        status: {
          eq: 'active',
        },
      },
      limit: 10,
      sort: ['name', 'asc'],
    },
    name: {},
    status: {},
  }
}

Which structure would you prefer that I return for version 2? Option 1 or 2?

Should we start version 3 now so that we can use the simple structure?

[robrichard]

@dhurlburtusa I'd prefer to go with No. 1 to keep consistency. I don't think the improvement in No. 3 is worth the churn of breaking changes

[dhurlburtusa]

@robrichard Thanks for the timely reply.

BTW, did you notice that the items in a ListValue are flattened in version 2 (at least for primitives)? We can't change this for version 2 because that would be a breaking change. (Well, maybe we can by saying it is fixing an unintentional bug.) But Lists with Strings really should be like the following if we want to include kind with each value:

{
  bar: {
    __arguments: [
      # Same as in previous examples
      ...
      {
        sort: {
          kind: 'ListValue',
          value: [
            {
              kind: 'StringValue',
              value: 'name',
            },
            {
              kind: 'StringValue',
              value: 'asc',
            },
          ],
        },
      },
    ],
    ...
  }
}

But the current code simply takes the value.value of the item AST. It does not examine the kind of the items of the List. So, a List of Lists will have undefined for each item since a List AST doesn't have a value property of the value property. It has a values property instead (notice the s at the end), i.e., value.values. Similarly, Object AST doesn't have a value property of the value property. It has a fields property instead, i.e., value.fields.

This is an unreported bug. Would you like me to open an issue for it? How should we handle the fix? Currently, primitive values get flattened and non-primitives become undefined. For backwards compatibility, I guess we can keep primitives as flat instead of an object like { kind: ..., value: ... } but keep them expanded for Lists and Objects. Or we can flatten them when they are items of a list. Which way would you prefer to go?

BTW, wish I could see your talk at JSKongress - 2019 on "Streaming HTTP and GraphQL".

[gienec]

This works for ObjectValue type arguments.
I'd say this issue could be closed.