Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Non-existent request field operator #1014

Open
SuibianP opened this issue Feb 9, 2023 · 8 comments
Open

[RFC] Non-existent request field operator #1014

SuibianP opened this issue Feb 9, 2023 · 8 comments
Assignees

Comments

@SuibianP
Copy link

SuibianP commented Feb 9, 2023

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.

@benjie
Copy link
Member

benjie commented Feb 9, 2023

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

@martinbonnin
Copy link
Contributor

+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

@glasser
Copy link
Contributor

glasser commented Feb 9, 2023

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
Copy link
Contributor

glasser commented Feb 9, 2023

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".)

@SuibianP
Copy link
Author

SuibianP commented Feb 13, 2023

@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.

@benjie
Copy link
Member

benjie commented Feb 13, 2023

@SuibianP No rush!

@SuibianP
Copy link
Author

SuibianP commented Mar 8, 2023

@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?

@benjie
Copy link
Member

benjie commented Mar 8, 2023

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
      }
    }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants