graphql-spec icon indicating copy to clipboard operation
graphql-spec copied to clipboard

[RFC] Non-existent request field operator

Open SuibianP opened this issue 2 years ago • 8 comments

As it currently stands, the “breaking change” GraphQL best practice is largely based upon the assumption that server schemas are never older than client schemas, which is not the case for self-hosted instances of server software that are not centrally managed.

For example, GitHub client applications may desire to query the newly added hasVulnerabilityAlertsEnabled field on the Repository type to provide additional information to the user if available. However, adding this field to the query is sure to break the compatibility with servers on earlier schema versions, such as GitHub Enterprise users.

This effectively leaves the same problem as traditional REST APIs — every change can be seen as breaking. To ensure both backward and forward compatibility, the client has to resort to either separate queries or API versioning. Both of the workarounds are against the design rationale of GraphQL and quickly become unmanageable with the rolling schema iteration encouraged by GraphQL design.

There should be a way to specify that a field in the query may be non-existent, or “I hope your server have some clue about this field but do not panic if not and just give me whatever you have now“. The need is not satisfied with nullable fields or error policies because it is a validation error instead of a resolver one.

If null is to be used as the sentinel response value of non-existent field, this operator shall imply non-null.

SuibianP avatar Feb 09 '23 02:02 SuibianP

This is a really interesting idea, and may help with adding things to the introspection schema too. Currently we have to do two or more introspection queries, the first determining what the server supports and the second then being built based on this. I imagine a @ifExists directive that acts a little like @skip in that it simply drops the selection if it isn’t supported would be a straightforward way of solving this.

Are you looking to be champion of this proposal? If so, you should add yourself and the topic to an upcoming WG agenda:

https://github.com/graphql/graphql-wg

benjie avatar Feb 09 '23 08:02 benjie

+1. I've bumped into this issue recently as well and was considering modifying the document at runtime based on some directives:

type Query {
  v1Field: String @since(version: 1)
  v2Field: String @since(version: 2)
}

If talking to a v1 server, a client could decide to strip the v2Field at runtime. It's nice that it clearly documents in the schema what fields are available at what version but has several drawbacks:

  • requires knowledge of the schema in the client
  • requires a GraphQL parser in the client
  • more generally, it's not trivial to implement

So having a simple @ifExists that could be added to queries and "do the right thing" would be a convenient tradeoff IMO

martinbonnin avatar Feb 09 '23 10:02 martinbonnin

Not to open a can of worms here, but is this need specific to fields, or to other things as well (type names in inline fragments, argument names, directives...)?

glasser avatar Feb 09 '23 16:02 glasser

Would the semantics here be that, if the field does not exist, validation (and execution) treat the field as if it were absent from the AST? Does that include ignoring its arguments and everything inside its selection set recursively (even, eg, directives applied inside its selection set that may or may not be defined) for the sake of validation? (Though not for all validation rules: eg, if a variable is only used in an argument to an @ifExists field or inside its selection set, the operation should not fail "all variables must be used".)

glasser avatar Feb 09 '23 16:02 glasser

@benjie I can be the champion after the end of my current affiliation weeks later, otherwise there is an obligatory, usually lengthy process to get through in order to sign the CLA. Please do not mark this rejected due to the lack of a champion for now.

SuibianP avatar Feb 13 '23 08:02 SuibianP

@SuibianP No rush!

benjie avatar Feb 13 '23 09:02 benjie

@glasser

is this need specific to fields, or to other things as well (type names in inline fragments, argument names, directives...)?

I believe it should be applicable for anything that can result in a component in the response -- which inline fragments as a whole and the fields within it definitely are. For argument names and directives, I am not so sure, partly because one cannot tell the existence of such token from the response in the lack of a sentinel value.

Does that include ignoring its arguments and everything inside its selection set recursively (even, eg, directives applied inside its selection set that may or may not be defined) for the sake of validation?

Yes, anything marked so is essentially purged from the AST, except for the validation rules you mentioned.

Also, what should happen if all fields are non-existent, resulting in an empty object?

SuibianP avatar Mar 08 '23 03:03 SuibianP

Also, what should happen if all fields are non-existent, resulting in an empty object?

We already allow empty objects; both of the following queries could return {"me":{}}. I don't think that's particularly problematic.

query EmptyObject1 {
  me {
    __typename @skip(if: true)
  }
}

query EmptyObject2 {
  me { # `type User implements Node`
    ... on Node {
      ... on Post {
        id
      }
    }
  }
}

benjie avatar Mar 08 '23 11:03 benjie