Deprecate includeDeprecated, add __schema(includeDeprecated:)#1230
Deprecate includeDeprecated, add __schema(includeDeprecated:)#1230benjie wants to merge 2 commits into
__schema(includeDeprecated:)#1230Conversation
✅ Deploy Preview for graphql-spec-draft ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Could we go further and not add the |
|
That would change the meaning of any existing queries that use introspection and don't specify |
|
Ahh yeah of course. What I really want is for |
|
I really like this! This makes it MUCH more clear that you're accessing one of two schemas: the super set including deprecated elements OR the subset without. That maps very cleanly to the single boolean |
Introspection should describe a consistent schema. The rules of the GraphQL schema validation aim that there are at least two consistent schemas (PR to follow to make this more explicit...): the source schema, and a derived schema that has all deprecated elements removed. Currently, however, an inconsistent schema can be described by mixing different values into
includeDeprecatedarguments throughout introspection. Worse, becauseincludeDeprecateddefaults tofalse, every time a new feature of the schema can be deprecated, existing schema introspection queries can start to represent an invalid schema as newly deprecated entities may be excluded even thoughincludeDeprecated: trueis specified for all previous introspection fields.This PR aims to eliminate this inconsistency by deprecating the old
includeDeprecated:system and replacing it with a__schema-levelincludeDeprecatedargument that applies recursively. This however requires some further changes:includeDeprecatedarguments would conflict with the__schema-levelincludeDeprecated, so they are forbidden when it's specified.includeDeprecatedisfalseby default, specifying__schema(includeDeprecated: true)would still describe the un-deprecated portions of the schema; thus the existingincludeDeprecatedarguments must lose their default value and become optional (which currently means nullable).Were it the case that
includeDeprecatedhad had the default valuetruethroughout history, this change could have been accomplished by simply adding__schema(includeDeprecated: Boolean! = true), since{__schema(includeDeprecated: false) {...}}would represent a schema with all deprecated elements removed independent of any nestedincludeDeprecatedarguments. Alas, we defaulted tofalseand thus to allowincludeDeprecated: trueat the root level we need to put in more effort.Possible expansion: we could make the validation rule stricter: forbid literal null, and require that a variable in that position is defined as non-nullable. I didn't see sufficient value in this to justify adding it, but I'm happy to do so if others think it's worthwhile.