Implementations may not deprecate a field that the interface hasn't deprecated

[benjie]

This PR addresses a spec validation omission; if an interface field is not deprecated then any implementation of that interface field should also not be deprecated. I.e. the following schema should be invalid, but before this PR it is valid:

interface Node {
  id: ID!
}

type Foo implements Node {
  id: ID! @deprecated(reason: "...")
}

type Query {
  foo: Foo
}

To solve this, either the deprecation should be removed, or the id field on Node should also be deprecated:

interface Node {
  id: ID! @deprecated(reason: "...")
}

Relevant action item: graphql/graphql-wg#1331

GraphQL.js Implementation: graphql/graphql-js#3986

[netlify]

✅ Deploy Preview for graphql-spec-draft ready!

Name	Link
🔨 Latest commit	9a5f88b
🔍 Latest deploy log	https://app.netlify.com/sites/graphql-spec-draft/deploys/6752d017a7c434000842749a
😎 Deploy Preview	https://deploy-preview-1053--graphql-spec-draft.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[fotoetienne]

Should the graphql-js impl be part of a major release?

[mjmahone]

One potential wrinkle to this: we don't really have deprecated interface implementations.

Like, maybe I'm deprecating a field because I will not be implementing an interface in the future. What I'm really trying to say is:

interface Node {
  id: ID!
}

type Foo implements Node @deprecated(reason: "Node is terrible for this purpose") {
  id: ID! 
}

type Query {
  foo: Foo
}

I guess the answer here is that deprecating implements is bad and you should migrate to a new type in that case.

[leebyron]

suggest rewriting to:

• avoid directly referencing directives, but just the deprecated status directly.
• if / then to avoid the negative

[fotoetienne]

maybe I'm deprecating a field because I will not be implementing an interface in the future.

I think @mjmahone has a valid point. Perhaps we don't want clients to do this anymore:

... on Foo {
    id
}

but this is still ok

... on Node {
    id
}

Marking id on foo as deprecated would indicate this.

[fotoetienne]

On further reflection, I'm thinking that this validation isn't needed. It doesn't bring enough benefit to warrant removing some deprecation capabilities.

[benjie]

@fotoetienne I think the general intent of deprecation (and specifically the includeDeprecated feature in introspection) is that you should be able to represent a schema with all deprecated entities removed and that schema should still be valid. Without this rule, this is not the case - if you remove the deprecated fields then Foo no longer implements Node, and the "not deprecated" schema is now invalid.

If and when we support deprecation on interface implementation as Matt suggest, this rule can be changed such that either the interface field is deprecated or the implementation of this interface is deprecated.

[benjie]

Discussed at tonights WG; we boiled it down to this:

If you remove everything deprecated from a GraphQL schema, should the resulting schema be valid?

Consensus was yes, @michaelstaib expressed it as something along the lines of: if you're building a new client to a GraphQL schema, you don't want to use any of the deprecated fields so you should be able to just use a schema with everything deprecated removed.

Do you agree with this guiding principle to deprecation, @fotoetienne?

If so, but you feel this is removing the deprecation capability of deprecating a field where you plan to remove the type from an interface, then perhaps we need to pair this with a spec change that enables deprecation on object type interface implementation?