Spec and implementation inconsistent about circular references in input types

[kaqqao]

The Type System spec, under Input objects, states (emphasis mine):

The Object type defined above is inappropriate for re-use here, because Objects can contain fields that express circular references or references to interfaces and unions, neither of which is appropriate for use as an input argument. For this reason, input objects have a separate type in the system.

The reference implementation indeed disallows interfaces and unions in inputs, implying the above sentence should be understood as a rule forbidding them and not just a recommendation against using them.
On the other hand, the implementation allows circular references in GraphQLInputObjectTypes.
E.g. the following is a legal declaration (ArticleInputType referring to ArticleInputType):

const ArticleInputType = new GraphQLInputObjectType({
  name: 'ArticleInput',
  fields: () => ({
    id:          { type: GraphQLInt },
    title:       { type: new GraphQLNonNull(GraphQLString) },
    body:        { type: GraphQLString },
    relatedArticles: { type: new GraphQLList(ArticleInputType) }
  })
});

This implies the sentence from the spec should be understood as a recommendation.

I suggest the wording in the corresponding spec section is changed to be consistent with the reference implementation or vice versa.
The current state leads to inconsistencies between the reference and other implementations, depending on their interpretation of the spec.

[leebyron]

Great feedback, thanks! I'll work on clarifying this.

[leebyron]

I think that the existing phrasing is being unduly strict. Circular types should probably be allowed, but circular non-null types should not.

For example, this input type seems like it should probably be valid:

input Something {
  name: String
  somethingElse: Something
}
# example { name: "Foo", somethingElse: { name: "Bar" } }

But this one is not valid since you cannot possibly provide a legal value:

input Something {
  name: String
  somethingElse: Something!
}
# example ... uhhhh

[dschafer]

Circular types should probably be allowed, but circular non-null types should not.

Yeah, agreed. As it stands, the explanatory text is right (objects can have circular references or references to interfaces and unions, neither of which is appropriate for use as an input argument), but then we don't correctly disallow that in input objects. I would probably update the explanatory text to say (emphasis mine):

objects can have non-null circular references or references to interfaces and unions, neither of which is appropriate for use as an input argument

and then add a 4th item to "Input Object type validation" that clarifies that input objects cannot contain circular references that are not broken up by a nullable field.

---

I think that the existing phrasing is being unduly strict

Hm, which existing phrasing is that?

[leebyron]

Hm, which existing phrasing is that?

I take it back, it's just easy to misread, not strict. I think it's easy to misinterpret as "circular references" are not allowed based on the following "neither of which is appropriate for use as an input argument" - even though that's referring to Interfaces and Unions.

Your analysis is dead on. I'll work out the suggested clarification and include some additional validation - that may also require some changes to implementations to enforce the validation.

[OlegIlyenko]

and then add a 4th item to "Input Object type validation" that clarifies that input objects cannot contain circular references that are not broken up by a nullable field.

I would also consider the fact that an empty list can break a circular reference as well:

input Something {
  name: String
  somethingElse: [Something!]!
}

[spawnia]

What is the status on this? I have written up this paragraph, which could be added to the specification for Input Object - Type Validation:

4. If an Input Object references itself either directly or through subordinated
   Input Objects, one of the fields in the chain of references must be either nullable
   or a list. Input Objects with non-nullable circular references are invalid, because there
   is no way to provide a legal value for them.

Is there a plan to implement such validation in the reference implementation yet? Should we open an issue for it?

[IvanGoncharov]

I have written up this paragraph, which could be added to the specification for Input Object - Type Validation:

@spawnia Would be great if you open PR for this change.

Is there a plan to implement such validation in the reference implementation yet? Should we open an issue for it?

Would be great if you open an issue 👍

[spawnia]

I am working on it, planning on submitting a PR in the reference implementation.

I also came across the following case:

input Hello {
  world: Hello
}

This Input definition does not really make much sense, as the only use might be somehow conveying a certain nesting depth? Do you think this should be allowed or also caught by schema validation?

[kaqqao]

input Hello {
  world: Hello
}

This example looks correct to me. Maybe unusual, but certainly not wrong, so I don't think it should be disallowed.

[spawnia]

Good point @kaqqao, this was already discussed in the PR i made.

Follow #445 for the status on this and for further discussion.

[IvanGoncharov]

Closing, please use #445 to discuss this feature and track its progress.