Adding default condition doesn't work as expected (requires change to GraphQL Spec)

[benjie]

Adding a default value for a condition (e.g. fields.includeArchived = { type: GraphQLBoolean, defaultValue: false}) doesn't apply unless you pass an empty condition object (e.g. {myCollection { ... } } doesn't respect it, but {myCollection(condition: {}) { ... } } does). However this cannot be solved by setting the defaultValue of the condition argument to be {}; though this gets correctly exposed in GraphiQL:

The results of running a query show that this condition wasn't actually applied, since the two queries above would produce different results when they should not.

I don't know why this is, but at the moment I'm leaning towards it being a bug in the way that Graphile Engine processes arguments (perhaps not applying defaults recursively?). It might actually be a GraphQL thing, but that has yet to be determined.

Interestingly, if you set the default value for the condition argument to defaultValue: {includeArchived: false} then it works as it should, but it should not be necessary to do this additional lookup.

Warrants further investigation. Also, once solved, we should give condition a default value of {} so these default conditions can apply automatically.

[benjie]

This is, indeed, an issue in GraphQL itself (arguably in the spec)

To reproduce:

const {
  graphqlSync,
  GraphQLSchema,
  GraphQLObjectType,
  GraphQLInputObjectType,
  GraphQLInt,
} = require("graphql");

const ExampleInputObject = new GraphQLInputObjectType({
  name: "ExampleInputObject",
  fields: {
    number: {
      type: GraphQLInt,
      defaultValue: 3,
    },
  },
});

const Query = new GraphQLObjectType({
  name: "Query",
  fields: {
    example: {
      args: {
        inputObject: {
          type: ExampleInputObject,
          default: {},
        },
      },
      type: GraphQLInt,
      resolve(source, { inputObject }) {
        return inputObject && inputObject.number;
      },
    },
  },
});

const schema = new GraphQLSchema({
  query: Query,
});

// All three of these should be equivalent?
const result1 = graphqlSync(schema, `{example}`);
const result2 = graphqlSync(schema, `{example(inputObject:{})}`);
const result3 = graphqlSync(schema, `{example(inputObject:{number: 3})}`);

console.log(JSON.stringify(result1.data));
console.log(JSON.stringify(result2.data));
console.log(JSON.stringify(result3.data));

You'd expect all these results to be the same, but this isn't the case:

node default-value-disparity.js 
{"example":null}
{"example":3}
{"example":3}

We can (and should) work around this in Graphile Engine.

[benjie]

It seems obvious to me that the defaultValue should be coerced to match the type in the same way that a supplied argument would at runtime (otherwise it breaks the type safety guarantees of GraphQL); I have opened an RFC here:

graphql/graphql-spec#793

And added it to the next GraphQL WG:

https://github.com/graphql/graphql-wg/blob/master/agendas/2020-12-03.md#agenda

[benjie]

Having had an in-depth discussion with Ivan (maintainer of GraphQL.js), I'm going to hold off on fixing this. Implementation is here: #692 and you can do it yourself with a plugin if you need it in the interrim.

[benjie]

Work on this is progressing as part of the GraphQL Spec Working Group; see recent working group agendas and notes (and linked PRs) for info.

[benjie]

This is unlikely to make it into V4 because it's still awaiting fixes to the GraphQL spec