-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
make includeDeprecated
non nullable
#1142
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for graphql-spec-draft ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is technically a breaking change if anyone did explicitly pass null
to this field (unlikely):
{
__schema {
queryType {
fields(includeDeprecated: null) {
name
}
}
}
}
but it's not a breaking change for existing queries that use nullable variables:
query I($includeDeprecated: Boolean) {
__schema {
queryType {
fields(includeDeprecated: $includeDeprecated) {
name
}
}
}
}
unless of course you explicitly pass the variable set to null
(unlikely).
One hesitation: if there are applications that use a query like the one above and use a system where null
and not set
are not differentiated (such as Facebook's system), then it's possible that they are passing null
explicitly as the variable value. We should try and find out if such use-cases exist before accepting this.
Confirmed Meta is okay with this change |
We have this now implemented in Hot Chocolate 15 and had no negative feedback so far. |
includeDeprecated
non-null graphql-js#4354In the same spirit as #1040, make
includeDeprecated
non-nullable in the introspection schema.I don't think there is ever a reason to passe
includeDeprecated: null
?