Defaulting includeDeprecated to false is an industry wide mistake - it should default to true and make tooling make a choice

[bbakerman]

The spec currently says (and has always said)

Accepts the argument includeDeprecated which defaults to false. If true, deprecated fields are also returned.

What this means is that by default non deprecated fields are not returned in introspection. Sounds sensible but its not.

Lots of tooling including graphql-js (and all the JS based tooling like graphiql and others) and python try to build valid schemas via introspection.

However this valid schema will cause them to choke and fail and interrupt the whole client tooling chain

type Query {
   foo(arg  : Args)  : Foo
}

type Foo {
   lastOfTheFields : String @deprecated(reason : "last field of its kind")
}

input Args {
   lastOfTheInputFields : String @deprecated(reason : "also last field of its kind")
}

The above schema is perfectly valid. Yet if a change was made to deprecated all the fields in a type (which is also a perfectly valid thing to do) then all the front end tooling is likely to break because of this valid schema. This is because the specification AND the tooling that follows it has defaulted to not including deprecated fields.

This is an industry wide mistake.

We have had this case multiple times in Atlassian and when it happens it breaks all our front end code pipelines and client scripts.

What was valid and is STILL valid fails to be processed into schema. Tools like graphiql say

{
  "errors": [
    {
      "message": "Input Object type JiraIssueSearchFieldSets must define one or more fields.",
      "stack": "GraphQLError: Input Object type JiraIssueSearchFieldSets must define one or more fields.\n    at t.reportError (https://dac-static.atlassian.com/_static/graphql-

Some of the other JS tooling also get the same error. Tooling like https://github.com/graphql-python/gql also fail with a similar error.

We ended up having to create schema validation rules to prevent deprecating all the fields in a type because of the many client tooling failures that happen on perfectly valid schema.

We looked into seeing if we could "change the tooling" but some use base libraries that haven't changed in 7 years

For example

https://github.com/prisma-labs/get-graphql-schema/blob/master/src/index.ts
https://github.com/graphql/graphql-js/blob/16.x.x/src/utilities/getIntrospectionQuery.ts#L75
get-graphql-schema is using GraphQL 14.1.1
The source code : https://github.com/graphql/graphql-js/blob/7dd2b2bed841b9fd0c40c328826363ec79cc65c4/src/utilities/introspectionQuery.js#L56
Doesn't specify includeDeprecated
The 14.1.1 tag https://github.com/graphql/graphql-js/releases/tag/v14.1.1

So this is an ongoing edge case where perfectly valid schema is not accepted by tooling and to change all the tooling would require boiling the ocean.

But if we change the spec to include deprecated fields by default then the older tooling would start to work and any new tooling would have to choose not to see deprecated fields via their new code.

[benjie]

We ended up having to create schema validation rules to prevent deprecating all the fields in a type because of the many client tooling failures that happen on perfectly valid schema.

IMO, we should have an overarching schema validation rule that states that the schema derived by removing all deprecated elements should itself be a valid schema; this is something I proposed again recently and hope to enable by moving the includeDeprecated to the position from which you read the schema (perhaps __service.schema(includeDeprecated:…)) such that there are literally two schemas, and deprecate (haha) the existing includeDeprecated arguments.

In the mean time this is definitely a missing schema validation rule, would you been interested in adding it to the spec?

[martinbonnin]

the schema derived by removing all deprecated elements should itself be a valid schema

Been thinking about this and I haven't found a use case for the "non-deprecated" schema.

Apollo Kotlin always retrieves deprecated fields because they are valid fields.

I would expect GraphiQL and most other tools to do the same. Sure using a deprecated field isn't great but the tooling must be aware of this and display a warning accordingly (and therefore needs to retrieve those fields).

We looked into seeing if we could "change the tooling" [...] would require boiling the ocean.

For better or worse, I think this is the way forward here. Making the default false was probably an early mistake but changing the default at this point would probably be another mistake IMO (breaking behaviour change, same query now returns different results, etc...).

That being said, I'm hopeful about changing the tooling!

@bbakerman do you want to open an issue on get-graphql-schema? We could also raise this in community-wg and/or use grant for this.

If some libs haven't been updated in 7 years, this sounds like the symptom of a much wider problem. We need maintained libraries for a sane ecosystem. Capabilities, fragments arguments and more are coming. If we can't fix includeDeprecated then we have a lot of issues. Let's make sure this isn't the case.