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

Expand Deprecation Use Cases For Better Client Support and Service Support #1119

Open
gwardwell opened this issue Oct 17, 2024 · 4 comments
Open

Comments

@gwardwell
Copy link

gwardwell commented Oct 17, 2024

Current State

GraphQL schema updates should be as self-service as possible to reduce bottlenecks for change. In addition to adding new schema, schema must be replaced/removed over time.

The typical way this is achieved is through deprecation via the @deprecated directive:

type Book {
  title: String!
  oldTitle: String! @deprecated(reason: "The the title field")
}

input BookInput {
  id: ID!
  bookId: String! @deprecated(reason: "use the id argument")
}

enum Genre {
  FICTION
  NO_GENRE @deprecated(reason: "use an explicit genre")
}

The existing @deprecated directive provides deprecation information as part of introspection queries. This information is also often surfaced in IDE and platform tooling and used to communicate pending removal of schema to clients when they perform operations.

Existing efforts

There is currently an open PR for supporting the deprecation of objects, but there has been little movement in recent days. This PR is also insufficient to cover all the use cases that need to be addressed.

Problem Statement

The problem is that the @deprecated directive is solely focused on the clients performing operations and only valid on a very limited subset of locations:

  • FIELD_DEFINITION
  • ARGUMENT_DEFINITION
  • INPUT_FIELD_DEFINITION
  • ENUM_VALUE

Not only are these locations insufficient for the client use case, leaving gaps that prove challenging to overcome when attempting to make schema changes, they also completely miss the use case for coordinating work in a distributed system.

Additionally, the @deprecated directive does not require a reason, which does not add clarity as to why something is being removed. This can also be addressed.

Client Side Deprecation Gaps

GraphQL schema needs the ability to communicate when object-level schema is intended to be removed. This is true for use cases like:

  • Removing an Interface Implementation from an Object
  • Removing a Union Member

In both cases, deprecating the field that returns the union or interface may not be an option if the goal is only to remove an interface implementer or member of the union. A separate solution is necessary.

This is a fairly unique use case to other languages because of the public API that GraphQL represents. Other languages do not expose union members or interface implementations in the same way as GraphQL — which appear in fragments within client operations.

Let's dig into the problem a bit deeper.

Removing an Interface Implementation

Interface implementers can appear as inline or named fragments within an operation. For example:

type Query {
  printedMedia: PrintedMedia
}

interface PrintedMedia {
  title: String!
}

# What if I don't want EReader to implement this interface anymore?
type EReader implements PrintedMedia {
  title: String!
}
query GetPrintedMedia {
  printedMedia {
    # How do we communicate to clients that EReader will stop implementing the interface?
    ... on EReader {
     ...EReaderFields
   }
  }
}

# How do we communicate to clients that EReader will stop implementing the interface?
fragment EReaderFields on EReader {
  title
}

Removing a Union Member

Similar to interfaces, Union members can appear as inline or named fragments within an operation. For example:

type Query {
  printedMedia: PrintedMedia
}

# What if I don't want EReader to be a member of the union anymore?
union PrintedMedia = EReader | Book

type EReader implements PrintedMedia {
  title: String!
}
query GetPrintedMedia {
  printedMedia {
    # How do we communicate to clients that EReader will be removed from the union?
    ... on EReader {
     ...EReaderFields
   }
  }
}

# How do we communicate to clients that EReader will be removed from the union
fragment EReaderFields on EReader {
  title
}

Service Deprecation Use Case

The service developer use case is currently completely ignored. With the creation of the Composite Schema Working Group, focus is starting to shift to enabling the distributed management of GraphQL. To manage schema at scale, we need a way to coordinate change at scale. This requires evolution of our tooling and provides an opportunity for deprecation to step beyond service-and-client communication to service-and-service communication.

This will require adding support for deprecation to additional locations:

  • SCALAR — to communicate when a scalar should no longer be extended, returned by fields, or used for input arguments
  • OBJECT — to communicate when an Object should no longer be extended, references, or returned by fields
  • INTERFACE — to communicate when an interface should no longer be extended or implemented on objects or returned by fields
  • INPUT — to communicate when an input object should no longer be extended or used by input arguments
  • UNION — to communicate when unions should no longer be extended, receive new members, or be returned by fields
  • ENUM — to communicate when enums should no longer be extended, receive new values, be returned by fields, or used as input arguments

This use case is covered in many programming languages which allow things like interfaces, classes, etc. to be annotated as deprecated.

Scalar deprecation

# I want to get rid of the BigInt scalar. I can deprecate existing fields that return BigInt,
# but how do I stop new fields from using BigInt while I work to remove it?
scalar BigInt

type Book {
  # How do I stop this?
  pageCount: BigInt
}

Object deprecation

# I want to get rid of the BadBook type. I can deprecate existing fields that return BadBook,
# but how do I stop extensions of the Book type or new fields from returning BadBook
# while I work to remove it?
type BadBook {
  # ...
}

# How do I stop this?
extend type BadBook {
  # How do I stop this?
  bookRef: BadBook
}

Interface deprecation

# I want to get rid of the Media interface. I can deprecate existing fields that return Media,
# but how do I stop extensions of the Media interface, new fields from returning Media,
# or new objects from implementing Media while I work to remove it?
interface Media {
  # ...
}

# How do I stop this?
type Book implements Media {
  # How do I stop this?
  media: Media
}

Input deprecation

# I want to get rid of the BookInput. I can deprecate existing fields that return BookInput,
# but how do I stop extensions of BookInput or new uses of BookInput for arguments
# while I work to remove it?
input BookInput {
  # ...
}

type Query {
  book(
    # How do I stop this?
    bookInput: BookInput
  ): Book
}

Union deprecation

# I want to get rid of the Vehicle Union. I can deprecate existing fields that return Vehicle,
# but how do I stop extensions of Vehicle, new members from being added to Vehicle,
# or new fields from returning Vehicle while I work to remove it?
union Vehicle = Car | Truck

type Query {
  # How do I prevent this?
  vehicle: Vehicle
}

Enum deprecation

# I want to get rid of the BadGenre Enum. I can deprecate existing fields that return BadGenre,
# but how do I stop extensions of BadGenre, new values being added to BadGenre, new fields
# from returning BadGenre, or new arguments that use BadGenre while I work to remove it?
enum BadGenre {
  # ...
}

type Query {
  bookByGenre(
    # How do I prevent this?
    genre: BadGenre
  ): Book
}

type Book {
  # How do I prevent this?
  genre: BadGenre
}

Lack of Deprecation Clarity

The current @deprecated directive definition defines the reason argument as optional:

directive @deprecated(
  reason: String
) ...

This means that schema can be deprecated with no additional clarity as to why:

type Book {
  title: String @deprecated
}

Worse yet, the reason argument can accept a null value, which is a different type of lack of clarity:

type Book {
  title: String @deprecated(reason: null)
}

Proposal

Make the @deprecated Directive's reason Argument Required

The @deprecated directive's reason argument should be requires. Providing a default value will allow it to be backwards compatible with existing schemas (see #53 (comment) for additional context).

directive @deprecated(
  reason: String! = "No longer supported"
) ...

Allow the Deprecation of Union Members and Interface Implementations

Option 1: Expand Directive Locations To Union Members and Interface Implementations

The directive locations could be expanded to allow applying directives to union members and interface implementations (credit). This would allow the @deprecated directive to be applied to the union member and interface implementation directly. The resulting API would look something like this:

enum __DirectiveLocation {
   # ...
   UNION_MEMBER
   IMPLEMENTED_INTERFACE
}

directive @deprecated(
  reason: String! = "No longer supported"
) on 
 # ...
 UNION_MEMBER | IMPLEMENTED_INTERFACE

union PrintedMedia  = Book | Magazine | @deprecated(reason: "EReaders aren't printed") EReader 
type Dentist implements @deprecated(reason: "Going to the dentist isn't fun") FunStuff

This would avoid built-in directive bloat while still providing the intended value.

Option 2: Add @deprecatedMember and @deprecatedImplementation Directives

If expanding directive locations becomes a non-option, new directives could be added to the spec to allow the deprecation of union members and interface implementations, which would be applied to the host (the union or the object type): @deprecatedMember and @deprecatedImplementation

@deprecatedMember Directive

The @deprecatedMember directive would define a member of a union that will be removed at some point in the future. The proposed definition would look like this:

directive @deprecatedMember(member: String!, reason: String! = "No longer supported") repeatable on UNION

The directive would be applied to a union and define the member that will be removed along with an optional reason. For example:

union PrintedMedia @deprecatedMember(member: "EReader", reason: "EReaders aren't printed.") = Book | Magazine | EReader

When a client performs an operation against the union and mentions the deprecated member by name, client tooling would understand the deprecated use in much the same way as it understands deprecation on a field:

query GetPrintedMedia {
  printedMedia {
    ... on EReader {
      # This is still a valid named fragment, but client tooling could warn that
      # this member of the union is deprecated and will go away. This would allow
      # clients to move away from the deprecated union member.
      # ...
    }
  }
}

@deprecatedImplementation Directive

Similar to @deprecatedMember, the @deprecatedImplementation directive would define an interface implemented on an object that will be removed at some point in the future. The proposed definition would look like this:

directive @deprecatedImplementation(interface: String!, reason: String! = "No longer supported") repeatable on Object

The directive would be applied to an object and define an implemented interface that will be removed along with an optional reason. For example:

interface FunStuff {
  name: String!
}

type Dentist implements FunStuff @deprecatedImplementation(interface: "FunStuff", reason: "Going to the dentist isn't fun") {
  name: String!
}

When a client performs an operation against the interface and mentions the deprecated implementing object by name, client tooling would understand the deprecated use in much the same way as it understands deprecation on a field:

query GetPrintedMedia {
  funStuff {
    ... on Dentist {
      # This is still a valid named fragment, but client tooling could warn that
      # this implementer of the interface is deprecated and will go away.
      # This would allow clients to move away from the deprecated interface
      # implementer.
      # ...
    }
  }
}

Expand @deprecated Locations for Service Use Cases

To cover the outlined service use cases, the @deprecated directive's locations can be expanded to object-level schema like Object Types, Interfaces, Scalars, etc. This is consistent with other languages that allow classes, interfaces, etc. to be annotated as deprecated.

directive @deprecated(
  reason: String! = "No longer supported"
) on FIELD_DEFINITION | ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION | ENUM_VALUE | SCALAR | OBJECT | INTERFACE | INPUT | UNION | ENUM

This would allow deprecation of all schema locations that can currently accept a directive, which will give the most flexibility for communicating change within a Graph.

union ReadingMaterial @deprecated(reason: "No longer supported") = Book

scalar BigInt @deprecated(reason: "No longer supported")

type Book @deprecated(reason: "No longer supported") {
  # ...
}

interface PrintedMedia @deprecated(reason: "No longer supported") {
  # ...
}

enum BadGenre @deprecated(reason: "No longer supported") {
  # ...
}

input BadBookInput @deprecated(reason: "No longer supported") {
  # ...
}

Next Steps

I would love to get this work moving to add full support for deprecation to the GraphQL spec. I'd love comments/feedback and any guidance for introducing this to the spec that folks can provide.

@gwardwell gwardwell changed the title Expand deprecation to all schema types Expand Covered Deprecation Use Cases Oct 17, 2024
@gwardwell gwardwell changed the title Expand Covered Deprecation Use Cases Expand Deprecation Use Cases For Better Client Support and Service Support Oct 17, 2024
@gwardwell
Copy link
Author

Edited to:

  • Denote that union member and interface implementation deprecation is a unique problem to GraphQL
  • Denote that deprecation for service development use cases is standard for many other languages
  • Add the proposal of new directive locations for union members and interface implementations as the preferred solution for deprecating those attributes

@dariuszkuc
Copy link

NIT: If we are adding new @deprecatedX directives I'd personally vote to make reason non-nullable. I understand that proposal was made to make it close to existing @deprecated directive.... but current behavior is IMHO very confusing as it is possible for folks to specify @deprecated(reason: null). See #53 (comment) for more details.

@gwardwell
Copy link
Author

gwardwell commented Oct 18, 2024

@dariuszkuc I love it. Historically I've solved that with a custom linting rule to enforce a deprecation reason. I love the idea of non-null with a default value for backwards compatibility, too. I'll update the issue to reflect this.

Edit: I have updated the issue description to add info on making the deprecation reason required.

@benjie
Copy link
Member

benjie commented Oct 24, 2024

Thanks for your work on this Greg! The best way forward is to break this up into smaller chunks and get each of them through the RFC process - you can do that under the umbrella of this issue, but in general we prefer smaller isolated changes that can be discussed separately; the needs of union member deprecation are likely to have different issues to that of unions themselves, for example. Something like @deprecatedImplementation should be discussed in isolation without slowing down progress on other clearer/cleaner changes.

The first thing I'd recommend you do is to bring this to the next GraphQL Working Group:

https://github.com/graphql/graphql-wg/blob/main/agendas/2024/11-Nov/07-wg-primary.md

Discussion there will help give you guidance on how to proceed (and surface any immediate roadblocks).

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

3 participants